Page MenuHomeFreeBSD

PHP Flavors.
ClosedPublic

Authored by mat on Feb 5 2018, 6:41 PM.

Details

Reviewers
tz
mat
Group Reviewers
portmgr
Commits
rP463917: Introduce PHP flavors.

Diff Detail

Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 15431
Build 15480: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
mat updated this revision to Diff 38913.Feb 5 2018, 10:11 PM

rebase.

mat updated this revision to Diff 38939.Feb 6 2018, 3:14 PM
  • Uniquify pecl package names.
  • And pear.
  • Don't do pearXY- and peclXY-, do phpXY-pear/pecl-
  • force php flavors for pear/horde, and provide multiple pkgnameprefixes
  • Add pear:env.
  • Fix pear ports.
  • Flavorize the pear-channel ports.
  • Handle pear channels dependency automatically.
  • poudriere bulk */pecl-* */pear-* */horde-* starts.
tz added a comment.Feb 6 2018, 4:28 PM

It looks fine too me. I'm glad that i wasn't too far away from the Flavor code. :)

After some tests i wouldn't extend this patch further - its great enough. I would flavorize various PHP ports afterwards. :)

joneum added a comment.Feb 6 2018, 9:16 PM

why only www/wordpress?
There are more wordpress ports:
russian/wordpress
and more here: https://portscout.freebsd.org/joneum@freebsd.org.html

Please node: wordpress 4.9.3 is out and i will update it (bugfix release)

mat updated this revision to Diff 38978.Feb 6 2018, 10:59 PM
  • Add flavors.
  • more ports
mat updated this revision to Diff 38993.Feb 7 2018, 11:24 AM
  • Allow using pecl with github.
  • Always define PECL_PKGNAMEPREFIX.
  • Pass the flavor to external pecl extensions.
  • More flavors.
mat added a comment.Feb 7 2018, 12:24 PM
In D14208#298316, @tz wrote:

It looks fine too me. I'm glad that i wasn't too far away from the Flavor code. :)
After some tests i wouldn't extend this patch further - its great enough. I would flavorize various PHP ports afterwards. :)

Would be nice if it was a good idea, but for many it is not.

why only www/wordpress?
There are more wordpress ports:
russian/wordpress
and more here: https://portscout.freebsd.org/joneum@freebsd.org.html
Please node: wordpress 4.9.3 is out and i will update it (bugfix release)

Why only www/wordpress? Because it's the first PHP app that came to mind, and I wanted to toy with it.

mat updated this revision to Diff 38995.Feb 7 2018, 12:46 PM
  • more ports.
  • bulk -n -a passes
tz added a comment.Feb 7 2018, 3:43 PM
In D14208#298635, @mat wrote:
In D14208#298316, @tz wrote:

It looks fine too me. I'm glad that i wasn't too far away from the Flavor code. :)
After some tests i wouldn't extend this patch further - its great enough. I would flavorize various PHP ports afterwards. :)

Would be nice if it was a good idea, but for many it is not.

I don't know who you mean? Currently there are only 3 persons (+ portmgr) in this review.

I won't stop you. But the patch exceeds my review-time and touches many different issues. If you don't want to break it down, please go ahead, but in this case i'm out. Maybe joneum can help with more time. And it would be great if this feature enters the portstree.

But one note: please add more comments to the framework-changes. It is already hard to understand what the framework does. And with the current patch it becomes harder.

mat added a comment.Feb 7 2018, 5:08 PM
In D14208#298688, @tz wrote:
In D14208#298635, @mat wrote:
In D14208#298316, @tz wrote:

It looks fine too me. I'm glad that i wasn't too far away from the Flavor code. :)
After some tests i wouldn't extend this patch further - its great enough. I would flavorize various PHP ports afterwards. :)

Would be nice if it was a good idea, but for many it is not.

I don't know who you mean? Currently there are only 3 persons (+ portmgr) in this review.

Sorry, my sentence was less than clear. The last "it" refered to "flavorizing php ports". There are many that cannot be done afterwards.

I won't stop you. But the patch exceeds my review-time and touches many different issues. If you don't want to break it down, please go ahead, but in this case i'm out. Maybe joneum can help with more time. And it would be great if this feature enters the portstree.

The only interesting bits are the Mk changes, the rest are just making things build.

But one note: please add more comments to the framework-changes. It is already hard to understand what the framework does. And with the current patch it becomes harder.

I usually add comments later, when the dust settles a bit, because there's no point in spending time explaining stuff that will be rewritten five times during the process of the review.

mat updated this revision to Diff 39015.Feb 7 2018, 5:56 PM

rebase.

mat updated this revision to Diff 39213.Feb 12 2018, 6:54 PM

Rebase +

  • Allow the installed php version to be the default flavor.
  • Remove flavors from PHP apps.
mat updated this revision to Diff 39231.Feb 13 2018, 12:28 AM

rebase after fixing devel/pear-channel-htmlpurifier.org

mat updated this revision to Diff 39247.Feb 13 2018, 11:57 AM
  • Fail early if the installed PHP version is not supported.
mat updated this revision to Diff 39249.Feb 13 2018, 1:46 PM

rebase and

  • Add a few comments.
tz added a comment.Feb 20 2018, 4:35 PM

So whats the current status here? When will it land?

mat added a comment.Feb 21 2018, 9:20 AM
In D14208#302810, @tz wrote:

So whats the current status here?

It is ready, been ready for a while, waiting for review.

When will it land?

When I decide I am tired of waiting for a review, and after an exp-run dance :-)

tz added a comment.Feb 21 2018, 9:36 AM

Great - i will try to find some time within the next days for review :)

mat updated this revision to Diff 39561.Feb 21 2018, 9:51 AM

rebase, and:

  • Merge databases/php7*-memcache
  • Merge ftp/php*-fastdfs
  • Remove PKGNAMESUFFIX for flavorized PHP extensions when they different
  • Flavorize www/unit-php*
tz requested changes to this revision.Feb 21 2018, 4:35 PM

Fine - this is only a rough review. I found some issues, have some questions and many entries reflecting that i'm unsure what you do. :D But hopefully this helps.

MOVED
9968

Merge conflict?

9986

The 8 lines above: typo. Must be "Merge" instead of "Metge"

Mk/Uses/horde.mk
18

I don't understand why this can be removed without any change. Is this channel no longer needed?

Mk/Uses/php.mk
113

I don't understand the new conditional. Why is it used here?

203

What is WRT? Also "dark magic" is not a helpful documentation

databases/Makefile
634

php70-memcached needs to be flavorized too.

databases/pear-DoctrineCommon/Makefile
15

Why could this be removed without any replacement?

databases/pear-Horde_Mongo/Makefile
10–11

No need to update a comment. Better remove it.

databases/pecl-memcached2/Makefile
8

I'm not sure- why can this be thrown away?

databases/php-memcache/pkg-descr
8

This is not correct.

databases/php-memcached/Makefile
1

This port should be renamed to php-memcached after flavorizing.

devel/pecl-jsmin2/Makefile
7

Won't this conflict with devel/pecl-jsmin?

devel/pecl-msgpack0/Makefile
7

Conflict with devel/pecl-msgpack?

devel/pecl-propro2/Makefile
8

Conflict with devel/pecl-propro?

devel/pecl-weakref2/Makefile
7

Conflict with devel/pecl-weakref?

mail/pecl-mailparse2/Makefile
8

Conflict with mail/pecl-mailparse?

math/pecl-bitset2/Makefile
7

Conflict with math/pecl-bitset?

math/pecl-stats2/Makefile
6

Conflict with math/pecl-stats?
Okay... i'm skipping this types of questions from here on. Its a little redundant (especially if i'm wrong ;D)

This revision now requires changes to proceed.Feb 21 2018, 4:35 PM
mat added a comment.Feb 21 2018, 11:22 PM

Note that the only interesting bits to review are the Mk/ changes, all the rest is mechanical changes to make sure everything still work.

The PKGNAMESUFFIX are removed in a few ports because they do not conflict any more.

Mk/Uses/horde.mk
18

It is done automatically by pear.mk

Mk/Uses/php.mk
113

Because if a pecl port USE_GITHUB to fetch their distfile, they can still USES=php:pecl to get the right suffix and DIST_SUBDIR.

databases/Makefile
634

it is.

databases/pear-DoctrineCommon/Makefile
15

pear.mk

databases/php-memcached/Makefile
1

it could, yes.

mat updated this revision to Diff 39606.Feb 22 2018, 3:10 PM
  • Fixes after tz's review.
mat added a comment.Feb 22 2018, 3:11 PM

About the PKGNAMESUFFIX removals, right now, we have:

$ for i in devel/pecl-weakref{,2}; do make -C $i pretty-flavors-package-names; done
no flavor: pecl-weakref-0.2.6_1
no flavor: pecl-weakref2-0.3.3

So the 2 is needed.

With flavors, we have:

$ for i in devel/pecl-weakref{,2}; do make -C $i pretty-flavors-package-names; done
php56: php56-pecl-weakref-0.2.6_1
php70: php70-pecl-weakref-0.3.3
php71: php71-pecl-weakref-0.3.3
php72: php72-pecl-weakref-0.3.3

So the 2 is no longer needed, package names are already different.

mat marked 23 inline comments as done.Feb 22 2018, 3:13 PM
tz accepted this revision.Feb 26 2018, 12:08 PM

Besides the 2 cosmetic issues i'm fine with it (as far as i understand it).

I'm looking forward to see it getting committed! :)

Mk/Uses/php.mk
203

For the log: i'm insisting that "dark magic" is not a helpful documentation at all.

databases/pear-Horde_Mongo/Makefile
10–11

The issue isn't done? The line is still a comment without any functionality?

This revision is now accepted and ready to land.Feb 26 2018, 12:08 PM
mat marked an inline comment as done.Feb 26 2018, 4:42 PM
mat updated this revision to Diff 39757.Feb 26 2018, 4:43 PM

rebase.

This revision now requires review to proceed.Feb 26 2018, 4:43 PM
mat marked an inline comment as done.Feb 26 2018, 4:44 PM
tz accepted this revision.Feb 26 2018, 4:47 PM

Issues were addressed, looks fine to me! :)
Thanks!

This revision is now accepted and ready to land.Feb 26 2018, 4:47 PM
mat updated this revision to Diff 39762.Feb 26 2018, 5:27 PM
  • Maj.
This revision now requires review to proceed.Feb 26 2018, 5:27 PM
tz accepted this revision.Feb 27 2018, 8:53 AM
This revision is now accepted and ready to land.Feb 27 2018, 8:53 AM
mat updated this revision to Diff 39860.Mar 1 2018, 1:49 PM

rebase.

This revision now requires review to proceed.Mar 1 2018, 1:49 PM
mat updated this revision to Diff 40069.Mar 8 2018, 1:38 PM
  • Add CHANGES and UPDATING entries.
mat updated this revision to Diff 40070.Mar 8 2018, 1:50 PM
  • reword a bit.
  • Add a bit about package name changes.
tz added a comment.Mar 20 2018, 10:17 AM

This was already committed and could now be abandoned?

mat accepted this revision.Mar 21 2018, 1:48 PM
This revision is now accepted and ready to land.Mar 21 2018, 1:48 PM
mat closed this revision.Mar 21 2018, 1:49 PM