Page MenuHomeFreeBSD

lang/php73: Simplify with options framework
Needs ReviewPublic

Authored by brnrd on Sep 17 2017, 3:08 PM.

Details

Reviewers
tz
mat
ale
Summary
lang/php72: Simplify with options framework

 - Convert all PORT_OPTIONS:M checks to OPT_something
 - Order alphabetically where possible
 - Fold double PHP_MODNAME checks into one

Diff Detail

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

Event Timeline

brnrd created this revision.Sep 17 2017, 3:08 PM
brnrd edited the summary of this revision. (Show Details)Sep 17 2017, 3:11 PM
brnrd edited the summary of this revision. (Show Details)Sep 17 2017, 6:13 PM
mat added inline comments.Sep 18 2017, 12:29 PM
lang/php71/Makefile
22 ↗(On Diff #33167)

USE_AUTOTOOLS is deprecatd, it should be really better to make it work with USES=autoreconf.

77 ↗(On Diff #33167)

debug leftover ?

brnrd updated this revision to Diff 33204.Sep 18 2017, 7:03 PM
brnrd marked an inline comment as done.

Small fixups

brnrd marked an inline comment as done.Sep 18 2017, 7:05 PM
brnrd added inline comments.
lang/php71/Makefile
22 ↗(On Diff #33167)

I know.
Tried building it with USES= autoreconf but that results in a libtool version error
Trying to find a PR for it...

brnrd edited the summary of this revision. (Show Details)Sep 18 2017, 7:20 PM
mat added inline comments.Sep 20 2017, 7:55 AM
lang/php71/Makefile
79 ↗(On Diff #33204)

No such thing as USES=apache. It is always better to not mix 50 different kind of changes in a patch. Especially in case of a new USES where the old usage is not removed and would probably be better off being done in batch.

Also note that 2.2+ is irrelevant, the only versions we have are 2.2 and 2.4, and 2.2 is deprecated so it will soon be 2.4 only.

157 ↗(On Diff #33204)

Makefile.ext does not .include <bsd.port.mk> put it back at the end.

joneum added a subscriber: joneum.Jan 15 2018, 10:48 AM
brnrd edited the summary of this revision. (Show Details)Mar 11 2018, 9:18 PM
mat requested changes to this revision.Mar 12 2018, 10:16 AM
mat added inline comments.
lang/php72/Makefile
100–101 ↗(On Diff #40178)

Options helper must no be defined after including bsd.port.options.mk.

This revision now requires changes to proceed.Mar 12 2018, 10:16 AM
brnrd updated this revision to Diff 40199.Mar 12 2018, 3:27 PM

OPT_* before bsd.port.pre.mk

Turns out the include of apache.mk is not needed at all.
It seems to have been there so APXS is defined, this is no longer
a requirement. Include moves down to just before where options
need checking

Additionally for Makefile.ext:

  • Remove _DESC that are in Mk/bsd.options.desc.mk
  • Fix faulty REGEX_REGEX_*
brnrd retitled this revision from lang/php71: Simplify with options framework to lang/php72: Simplify with options framework.Mar 19 2018, 8:54 PM
brnrd added a reviewer: ale.Apr 8 2018, 1:00 PM
brnrd added a comment.Apr 8 2018, 1:05 PM

Hi @tz, @ale,

Can you provide feedback on this review? I think I can commit this under "Approved by: infrastructure modernization blanket"

Rather have your feedback and support though...

Cheers, Bernard.

tz added a comment.Apr 12 2018, 3:17 PM

Aloha Bernard,
i will take a look at it next week. I'm currently on very low time, but will try to through all your PHP reviews (i'm assigned to) next week.
Thanks,
Torsten

tz accepted this revision.Jul 25 2018, 12:43 PM

Aloha Bernard,
i'm very sorry that this ticket is stuck. Since i will be away for some time, i took a short, but not too deep, look at it. It will need an rebase, but i can't see any showstopper or obvious issues. So: please go ahead and big thank you for your work and patience!
Greetings,
Torsten

brnrd updated this revision to Diff 50484.Nov 16 2018, 11:34 AM

Update for 7.3

brnrd retitled this revision from lang/php72: Simplify with options framework to lang/php73: Simplify with options framework.Nov 16 2018, 11:35 AM
brnrd edited the summary of this revision. (Show Details)
brnrd updated this revision to Diff 50485.Nov 16 2018, 11:36 AM

Remove crud, restore CONFLICTS

ale added a comment.Nov 16 2018, 3:04 PM

I have a few doubts, see my comments

lang/php73/Makefile
64

This looks duplicated, line 54

68

Why php_sapi is lowercase? There are many others below.

69

Why not using XXX_CONFIGURE_ENABLE=cgi ? There are other occurrences below

90

Why --with-apx2=${APXS} was removed?