Bug 240883 - Mk/Uses/php.mk: fix build of all PHP modules when using php74 on GCC architectures
Summary: Mk/Uses/php.mk: fix build of all PHP modules when using php74 on GCC architec...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Ports Framework (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: Port Management Team
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-09-27 20:47 UTC by Piotr Kubaj
Modified: 2019-11-06 17:02 UTC (History)
2 users (show)

See Also:
ale: maintainer-feedback+
pkubaj: exp-run?


Attachments
patch (762 bytes, patch)
2019-09-27 20:47 UTC, Piotr Kubaj
no flags Details | Diff
v2 (817 bytes, patch)
2019-09-29 09:34 UTC, Piotr Kubaj
no flags Details | Diff
v3 (815 bytes, patch)
2019-11-05 10:20 UTC, Piotr Kubaj
ale: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Piotr Kubaj freebsd_committer 2019-09-27 20:47:44 UTC
Created attachment 207902 [details]
patch

php74 introduced C11 requirement and all optional PHP modules available in the ports tree need a patch like this:

.include <bsd.port.options.mk>
.if ${FLAVOR} == php74
USES+=compiler:c11
.endif

With over 400 ports that need, it's not feasible to fix. And there will be php75 (or 80) next year which will require all those ports to be fixed again.

Change php.mk to fix it in one place.

Additionally, fix a typo.
Comment 1 Piotr Kubaj freebsd_committer 2019-09-29 09:31:26 UTC
Don't commit it yet, it looks like the ports that I was testing it for were inheriting compiler:c11 from lang/php74. It doesn't seem to work for ports that don't.

root@talos:$/usr/ports/security/pecl-mcrypt$ make FLAVOR=php74 -V USES
php:pecl compiler:c11
root@talos:$/usr/ports/security/pecl-mcrypt$ make FLAVOR=php74 -V CC
cc
Comment 2 Piotr Kubaj freebsd_committer 2019-09-29 09:34:32 UTC
Created attachment 207931 [details]
v2

This seems to work, but powerpc* switches to LLVM when LLVM9 is merged to head, so it will need some modifications in the coming weeks.
Comment 3 Alex Dupre freebsd_committer 2019-10-07 07:32:36 UTC
Why the previous patch doesn't work and why do we not have USE_GCC for those archs in lang/php74 Makefile?
Comment 4 Piotr Kubaj freebsd_committer 2019-10-07 14:32:12 UTC
(In reply to Alex Dupre from comment #3)
"Why the previous patch doesn't work"
It's probably because USES is added too late, it should be in the Makefile instead. When added to Mk/Uses, USES value is actually changed, but CC doesn't change.

Look at Mk/Uses/qt-dist.mk, it's done the same way.

"why do we not have USE_GCC for those archs in lang/php74 Makefile"
We do have USES=compiler:c11 (which sets USE_GCC on GCC platforms), but that only works for php74 and the ports that include lang/php74/Makefile (all */php74-* ports, e.g. archivers/php74-bz2).
Comment 5 Mark Linimon freebsd_committer freebsd_triage 2019-10-26 13:43:58 UTC
Approved as mentor (builds on ppcports and a generic x86 system) pending portmgr approval.
Comment 6 Antoine Brodin freebsd_committer 2019-10-26 14:12:08 UTC
I think the patch is not good,  it should be PHP_FLAVOR, not FLAVOR.
Comment 7 Piotr Kubaj freebsd_committer 2019-10-26 20:19:37 UTC
(In reply to Antoine Brodin from comment #6)
PHP_FLAVOR causes:
make: "/usr/home/pkubaj/ports/Mk/Uses/php.mk" line 162: Malformed conditional (${PHP_FLAVOR} == php74 && (${ARCH:Mmips*} || ${A
RCH:Mpowerpc*} || ${ARCH} == sparc64))

FLAVOR works.
Comment 8 Antoine Brodin freebsd_committer 2019-10-26 20:23:11 UTC
(In reply to Piotr Kubaj from comment #7)
sure, because you have to put it line 165 or something
Comment 9 Piotr Kubaj freebsd_committer 2019-10-26 20:36:30 UTC
I also tried that, but didn't report, because it didn't change the behaviour.
pkubaj@talos:$/usr/home/pkubaj/ports$ make FLAVOR=php74 -C archivers/pecl-rar -V CC 
cc
Comment 10 Piotr Kubaj freebsd_committer 2019-10-26 20:37:47 UTC
Also, putting FLAVOR works, I'm just finishing a bulk -a build with this patch:
https://talos.anongoth.pl/build.html?mastername=powerpc64-default&build=2019-10-18_09h28m35s
Comment 11 Mathieu Arnold freebsd_committer 2019-10-31 12:20:34 UTC
Using FLAVOR is probably wrong.  It should check that the current PHP version is 7.4.

For example, if a PHP module does not have flavors for some reason, it still need to change compiler if the default PHP version is 7.4.
Comment 12 Alex Dupre freebsd_committer 2019-11-05 10:08:18 UTC
(In reply to Mathieu Arnold from comment #11)

It should be enough to replace:

.if ${FLAVOR} == php74

with

.if ${PHP_VER} == 74

in the pkubaj patch.
Comment 13 Piotr Kubaj freebsd_committer 2019-11-05 10:20:24 UTC
Created attachment 208875 [details]
v3

Yes, this seems enough:
root@talos:$/usr/ports$ make -C archivers/pecl-rar -V CC
gcc9

Thanks for pointing it out.
Comment 14 Mathieu Arnold freebsd_committer 2019-11-05 14:04:34 UTC
As a side note, do you keep track of all thoses places you added ".if powerpc/sparc/mips -> USE_GCC" in case one day llvm works on one of those architectures one day?
Comment 15 Piotr Kubaj freebsd_committer 2019-11-05 14:30:00 UTC
(In reply to Mathieu Arnold from comment #14)
In Mk there's only one such file (qt-dist.mk). I don't keep track of those .ifs put into ports' Makefiles, but they are very easy to find. I always keep those architectures listed in the same order, so "find /usr/ports -depth 3 -name Makefile -exec grep ...." will find it.
Comment 16 Piotr Kubaj freebsd_committer 2019-11-05 19:44:27 UTC
(In reply to Mathieu Arnold from comment #14)
Since you're in this bug anyway, do you accept this patch as a mentor?
Comment 17 Mathieu Arnold freebsd_committer 2019-11-06 15:12:20 UTC
Sure, go ahead.
Comment 18 commit-hook freebsd_committer 2019-11-06 17:02:39 UTC
A commit references this bug:

Author: pkubaj
Date: Wed Nov  6 17:01:40 UTC 2019
New revision: 516904
URL: https://svnweb.freebsd.org/changeset/ports/516904

Log:
  Mk/Uses/php.mk: fix build of all PHP modules when using php74 on GCC architectures

  php74 introduced C11 requirement and all optional PHP modules available in the ports tree need to use C11 compiler.

  PR:		240883
  Approved by:	portmgr

Changes:
  head/Mk/Uses/php.mk