new port following the WantedPorts wiki page.
Details
Details
Diff Detail
Diff Detail
- Repository
- rP FreeBSD ports repository
- Lint
No Lint Coverage - Unit
No Test Coverage - Build Status
Buildable 32435 Build 29912: arc lint + arc unit
Event Timeline
Comment Actions
A quick review, I found a couple of things, most of them related to the handling of init scripts in FreeBSD.
Can you also include the patch for www/Makefile?
Don't hesitate to reach out if anything is unclear!
| www/phpdaemon/Makefile | ||
|---|---|---|
| 25 | You probably want SHEBANG_FILES rather than SHEBANG_GLOB since it's two specific files. | |
| 36 | Use USE_RC_SUBR infrastructure, see comments for the init script file. | |
| www/phpdaemon/files/patch-bin_phpd | ||
| 5–6 | Shouldn't this be /usr/bin/env php? | |
| www/phpdaemon/files/patch-bin_sampleapp | ||
| 1–8 ↗ | (On Diff #74684) | Can this be handled by SHEBANG_FILES? |
| www/phpdaemon/files/patch-init-scripts_phpd.freebsd | ||
| 1 ↗ | (On Diff #74684) | In general, FreeBSD init scripts are usually handled by putting a file named application.in in files/. In this case that would be phpdaemon.in. This file is then processed, changing things like %%PREFIX%% to the prefix things are installed in and so on. You then use USE_RC_SUBR in the Makefile to handle the file. You an use for instance net-mgmt/lldpd as an example of this. |
| 21 ↗ | (On Diff #74684) | Hard coded path. |
| www/phpdaemon/pkg-plist | ||
| 15 | use USE_RC_SUBR instead (see above) | |
| www/phpdaemon/files/phpdaemon.in | ||
|---|---|---|
| 18 | Still hardcoded, you must use %%PREFIX%%/bin/phpdaemon, in case the application is installed somewhere not /usr/local. | |
Comment Actions
It looks like you are emulating moving PHPDaemon/Config/Object.php to PHPDaemon/Config/_Object by a rm and a patch adding the file.
Unless both files are completely different, there should probably be a ${MV} in post-extract, and a smaller patch changing only what needs to be.
| www/phpdaemon/Makefile | ||
|---|---|---|
| 16 | No. If you need the php command, you need to USES=php:cli, a runtime dependency is always present. | |
| 32–33 | This looks like cleanup, it should happen in post-extract. | |
| 38 | ${PREFIX}/etc/${PORTNAME} is also called ${ETCDIR}. | |
| 39–40 | ${PREFIX}/share/${PORTNAME} is also called ${DATADIR}. | |
| www/phpdaemon/files/patch-bin_phpd | ||
| 17 | /usr/local/ should not be hardcoded. | |
| www/phpdaemon/files/patch-bin_phpd | ||
|---|---|---|
| 17 | You can replace /usr/localwith %%PREFIX%% using the REINPLACE_CMD in post-patch section. See 4.4.3. Simple Automatic Replacements in https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/book.html#slow-patch | |
Comment Actions
Yeah, all the files look good now and build is fine. I am somehow a bit hesitated to add this because I'm curious if there is any party using this, or there is any software depends on this. The upstream development seems stopped for 4 years. I am happy to add this port even the possible user is very few, but I want to know more about the potential users of this port first.
Comment Actions
Last commit was Jun 30 of this year https://github.com/kakserpom/phpdaemon/commit/9362e586d21343de7fb511f6b3cbfc5ae13a4364
Comment Actions
Indeed, sorry that I always got confused by github's new interface. From the commit log, there seems to have lots of fixes after the 1.0-beta release. Would it be a better choice to use a snapshot instead of using a beta release from 4 years ago?
Comment Actions
I was thinking that, the latest commit fixed things like _Object like I did for my patches so the phpdaemon can work with PHP 7+ and some other changes, but I wasnt sure if that is a good way of doing the packages on FreeBSD, I could use the latest commit on the Makefile and remove some unnecessary patches I made if so.
Comment Actions
If the upstream already fixed this (and other issues), I feel it's better to directly use them. Please check if the USE_GITHUB related sutff at `/Mk/bsd.sites.mk are useful to you.
Comment Actions
There should be some place where you document why you rename the Object.php file to _Object.php. Because I cannot figure out why myself.
Also, all the patch files should include a header explaining why the patch is doing what it is doing. For example of what to write, see ports/dns/bind911/files/patch-configure.
| www/phpdaemon/pkg-plist | ||
|---|---|---|
| 5–11 | If these files are examples, they should go in EXAMPLESDIR, if they can be modified by the user, they should be using @sample. | |
| 14 | This should probably be using @sample. | |