Page MenuHomeFreeBSD

www/phpdaemon
Needs ReviewPublic

Authored by miguel_gocobachi.dev on Jul 20 2020, 10:15 AM.

Details

Reviewers
None
Group Reviewers
Contributor Reviewers (ports)
Summary

new port following the WantedPorts wiki page.

Test Plan

poudriere

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 32523
Build 29995: arc lint + arc unit

Event Timeline

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
24

You probably want SHEBANG_FILES rather than SHEBANG_GLOB since it's two specific files.

35

Use USE_RC_SUBR infrastructure, see comments for the init script file.

www/phpdaemon/files/patch-bin_phpd
4–5 ↗(On Diff #74684)

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
14

use USE_RC_SUBR instead (see above)

Can you also include the patch for www/Makefile?

honestly I do not know how to add the www/Makefile with this requests using arc.

www/phpdaemon/files/phpdaemon.in
18

Still hardcoded, you must use %%PREFIX%%/bin/phpdaemon, in case the application is installed somewhere not /usr/local.

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.
PHP_FLAVOR is the name of the current php flavor, it is absolutely not a port directory name, or a port name.

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 ↗(On Diff #74688)

/usr/local/ should not be hardcoded.

www/phpdaemon/files/patch-bin_phpd
17 ↗(On Diff #74688)

@mat how can I fix to use it with %%PREFIX%% instead? thanks in advance for the help.

lbartoletti added inline comments.
www/phpdaemon/files/patch-bin_phpd
17 ↗(On Diff #74688)

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

I think I got all the comments covered.

I think I got all the comments covered.

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.

I think I got all the comments covered.

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.

Last commit was Jun 30 of this year https://github.com/kakserpom/phpdaemon/commit/9362e586d21343de7fb511f6b3cbfc5ae13a4364

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?

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?

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.

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.

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.

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
4–10

If these files are examples, they should go in EXAMPLESDIR, if they can be modified by the user, they should be using @sample.

13

This should probably be using @sample.