Created attachment 188055 [details] Adds ftp/php72-fastdfs Provided patch adds ftp/php72-fastdfs Passes poudriere testport + portlint -AC
I see, you use "svn add". I think, its better to use "svn cp" from ftp/php72-fastdfs. I see this, when i use portlint -AC: ftp/php72-fastdfs: portlint -AC [21:22:46] WARN: Makefile: "IGNORE_WITH_PHP" has to appear earlier. WARN: /usr/home/joneum/dev/ports/ftp/php72-fastdfs/pkg-message: possible use of absolute pathname "/var/tmp". WARN: /usr/home/joneum/dev/ports/ftp/php72-fastdfs/pkg-message: possible use of absolute pathname "/usr/local/etc/fdfs/...". WARN: /usr/home/joneum/dev/ports/ftp/php72-fastdfs/pkg-message: possible direct use of "fastdfs_client.tracker_group0 = /usr/local" found. if so, use ${PREFIX} or ${LOCALBASE}, as appropriate. 0 fatal errors and 4 warnings found. We use "DISTVERSION" instead of "PORTVERSION": PORTNAME= fastdfs DISTVERSION= 5.0.11 CATEGORIES= ftp PKGNAMEPREFIX= php72- about the patch: instead of doing this, why not pass ROOT=$LOCALBASE to the configure script? It only falls by to ROOT=/usr if $ROOT is not yet set. Patching in a hard-coded /usr/local is also not ideal (you should use use the value of $LOCALBASE). or you can use CONFIGURE_ENV= ROOT=${LOCALBASE} instead of the patch. As it is used in the following way PHP_ADD_INCLUDE($ROOT/include/fastcommon) PHP_ADD_INCLUDE($ROOT/include/fastdfs) setting it to $LOCALBASE seems like the correct move
Thanks for the input! I've split the changes into 2 patches so you can apply them in a reasonable way. I wasn't using the latest version of portlint it seems so I did not get the IGNORE_WITH_PHP warning, I've now upgraded my portlint and sorted that too in the first patch. Apply fastdfs-patch-1.patch This changes to DISTVERSION and removes the patch as requested across the board, this should not require a portrevision bump as it doesn't change anything in the final package. Commit that, then do svn cp php71-fastdfs php72-fastdfs Apply fastdfs-patch-2.patch And we should be good to go. All 4 ports passes poudriere testport after the changes.
Created attachment 188068 [details] First part which removes patch + changes to DISTVERSION
Created attachment 188069 [details] Second part which patches the new port after svn cp
A commit references this bug: Author: joneum Date: Sat Nov 25 07:51:20 UTC 2017 New revision: 454873 URL: https://svnweb.freebsd.org/changeset/ports/454873 Log: php*-fastdfs: remove hard-code patching PR: 223714 Submitted by: Daniel Ylitalo <daniel@blodan.se> (maintainer) Approved by: tcberner (mentor) Differential Revision: https://reviews.freebsd.org/D13137 Changes: head/ftp/php56-fastdfs/Makefile head/ftp/php56-fastdfs/files/ head/ftp/php70-fastdfs/Makefile head/ftp/php70-fastdfs/files/ head/ftp/php71-fastdfs/Makefile head/ftp/php71-fastdfs/files/
Hey Daniel, bad news, sorry :( My mentors did not release the change, see: https://reviews.freebsd.org/D13123 I quote: >Technically the code is fine. Its just a bad solution. The correct one would be a >version-free port which uses the PHP defined as default either in the ports-tree >or in /etc/make.conf. >Also a slave-port would be a better variant - or a flavor. > >Its not about getting a technical good solutions, its about getting a good >solution. The code is just the implementation. It doesn't make much sense to add >this port wrongly, because it just works while planning to do it correctly in the >future.
I was waiting for flavors to arrive to the public to merge these ports but I guess I have to do a master/slave setup before that then, I got a few pecl- ports that I aready have flavor patches for too :) New patch coming up in a bit.
(In reply to Daniel Ylitalo from comment #7) Thanks for your feedback. Then I wait for the new patch ;-)
(In reply to Daniel Ylitalo from comment #7) > I was waiting for flavors to arrive to the public to merge these ports but I > guess I have to do a master/slave setup before that then, > I got a few pecl- ports that I aready have flavor patches for too :) Since the question wasn't forwarded: why don't you use the default way be defining the default PHP version in /etc/make.conf and compile the module (and pecl-modules) against it? This way works for years and is intended. Slave-Ports are just slightly better because they are not full copies. But of course they are unneeded. So can you please provide more inside to allow us helping to find the best solution? [Also may you share your pecl-flavored ports via email to tz@FreeBSD.org? I'm also working on them :D]
Okay so same deal as last time :) Note that IGNORE_WITH_PHP needs to be kept, else for example php70-fastdfs can be built when php56 is installed, you can reproduce it this way if you'd like to try; cd lang/php56 && make install clean cd ftp/php70-fastdfs && make install clean IGNORE_WITH_PHP stops the above scenario from happening because PHP_VER only pulls the correct php version if the php binary does not already exist/is installed. Part 1: Apply patch 1: Remake to Master/Slave Part 2: svn cp php71-fastdfs php72-fastdfs Part 3: Apply patch 2: Version number changes All 3 ports passes poudriere testport with the patches applied Note that portlint does not seem to have MASTERDIR support so it will whine about it in the slave ports, the master port passes portlint
Created attachment 188421 [details] Part 1: Master/Slave changes
Created attachment 188422 [details] Second part: version number changes in new port
Hi guys! Any chance we could commit this soon? The patches should be just as requested.
I'm fine with the Master/Slave changes. Since the ticket is owned by joneum: please go ahead. If testing is fine, i approve the commit.
A commit references this bug: Author: joneum Date: Wed Dec 13 17:22:46 UTC 2017 New revision: 456222 URL: https://svnweb.freebsd.org/changeset/ports/456222 Log: ftp/php*-fastdfs: Remake to Master/Slave Ports PR: 223714 Submitted by: Daniel Ylitalo <daniel@blodan.se> (maintainer) Approved by: tz (mentor) Differential Revision: https://reviews.freebsd.org/D13454 Changes: head/ftp/php56-fastdfs/Makefile head/ftp/php70-fastdfs/Makefile head/ftp/php70-fastdfs/distinfo head/ftp/php70-fastdfs/pkg-descr head/ftp/php70-fastdfs/pkg-message head/ftp/php71-fastdfs/Makefile head/ftp/php71-fastdfs/distinfo head/ftp/php71-fastdfs/pkg-descr head/ftp/php71-fastdfs/pkg-message
@joneum any idea when you might have time to commit the php72-port?
A commit references this bug: Author: joneum Date: Mon Dec 18 19:12:34 UTC 2017 New revision: 456668 URL: https://svnweb.freebsd.org/changeset/ports/456668 Log: New slaveport: ftp/php72-fastdfs PR: 223714 Submitted by: Daniel Ylitalo <daniel@blodan.se> (maintainer) Approved by: tcberner (mentor) Differential Revision: https://reviews.freebsd.org/D13479 Changes: head/ftp/Makefile head/ftp/php72-fastdfs/ head/ftp/php72-fastdfs/Makefile
(In reply to Daniel Ylitalo from comment #16) now all done, thx :-)