php8: fix and enhance configuration and dependencies by danielfdickinson · Pull Request #28234 · openwrt/packages

danielfdickinson · 2026-01-01T06:55:52Z

📦 Package Details

Maintainer: @mhei

Description:

This commit cleans up and enhances the changes from 6d6233b, f9591b8, e6c59b5, and properly implements the attempt from 996046e.

By reasoning carefully about the impact of combinations of configuration options, and thoroughly setting config options and checking actual dynamic dependencies using readelf --dynamic, the configuration and dependency handling of these options has been improved.

In addition, cleanups and enhancements like reorganizing CONFIG_DEPENDS, adding more Kconfig help text, and cleaning up some whitespace have been performed.

A missing conditional BUILD_DEPENDS on package icu when PHP8_GETTEXT is defined has been added.
The PHP8_GETTEXT, PHP8_INTL, PHP8_FULLICUDATA, php8-mod-gettext, and php8-mod-intl interactions have been verified, fixed, and improved.
A circular dependency with php8-mod-xmlreader, PHP8_DOM, and php8-mod-dom when php8-mod-xmlreader is depended on by another package (e.g. zabbix-server-frontend) has been corrected
As these changes potentially impact binaries, there is a PKG_RELEASE bump.

🧪 Run Testing Details

OpenWrt Version: SNAPSHOT (r32450-8b93c563ad)
OpenWrt Target/Subtarget: bcm27xx/bcm2712
OpenWrt Device: rpi-5

The following have been tested using Zabbix server frontend as the PHP test application:

Included php8-mod-gettext but not php8-mod-intl, set PHP8_GETTEXT and PHP8_INTL, but not PHP8_FULLICU_DATA.
Included php8-mod-gettext but not php8-mod-intl, set PHP8_GETTEXT, not PHP8_INTL, and not PHP8_FULLICU_DATA.
Included php8-mod-gettext (compiled and in image) and php8-mod-intl (compiled only), set PHP8_GETTEXT and PHP8_INTL, but not PHP8_FULLICU_DATA.
Included php8-mod-gettext (compiled and in image) and php8-mod-intl (compiled and in image), set PHP8_GETTEXT and PHP8_INTL, and PHP8_FULLICU_DATA.
Included php8-mod-gettext (compiled and in image) and php8-mod-intl (compiled and in image), set PHP8_GETTEXT and PHP8_INTL, but not PHP8_FULLICU_DATA.

✅ Formalities

I have reviewed the CONTRIBUTING.md file for detailed contributing guidelines.

mhei · 2026-01-03T12:43:55Z

Nice! At first glance, it looks good to me. I would like to have a deeper look tomorrow evening.

lang/php8/Makefile

danielfdickinson · 2026-01-04T04:54:33Z

Noticed an issue with configuring with openssl when php8-mod-openssl is not selected, so fixed that.

danielfdickinson · 2026-01-04T05:04:32Z

I've dropped the openssl changes - this looks like another can of worms.

lang/php8/Makefile

mhei · 2026-01-04T21:22:19Z

lang/php8/Makefile

+		depends on PHP8_LIBXML
 		default y
+		help
+			Without the mod-dom, this option does not provide a PHP8


Here, I'd use "Without the dom module,..." or we could use the complete package names, e.g. "Without php8-mod-dom, this..."

mhei · 2026-01-04T23:20:56Z

lang/php8/Makefile

+	CONFIG_PHP8_INTL \
+	CONFIG_PHP8_SYSTEMTZDATA
+
+PKG_BUILD_DEPENDS:= PHP8_GETTEXT:icu


Do you mean PHP8_INTL here?

No, due to this configure line, which needs icu to compile, but icu does not become a php8 dependency for php8-mod-gettext because of it, nor is php8-mod-gettext required to be built (in the case php8-mod-gettext is not built, it affects php8-cli, php8-*cgi, and apache-mod-php8, so is still relevant).

ifeq ($(CONFIG_PHP8_GETTEXT),y) CONFIGURE_ARGS+= --with-gettext=shared,"$(STAGING_DIR)/usr/lib/libintl-full" else CONFIGURE_ARGS+= --without-gettext endif

prior to 6d6233b

this was

ifneq ($(SDK)$(CONFIG_PACKAGE_php8-mod-gettext),) CONFIGURE_ARGS+= --with-gettext=shared,"$(STAGING_DIR)/usr/lib/libintl-full" else CONFIGURE_ARGS+= --without-gettext endif

Here I'm still confused because:

before we had: PACKAGE_php8-mod-intl → --enable-intl=shared and +PACKAGE_php8-mod-intl:icu which added the runtime dep and thus kind of automatically added a build-dep only when the module was selected, parallel CONFIG_PACKAGE_php8-mod-gettext → --with-gettext=shared,"$(STAGING_DIR)/usr/lib/libintl-full" and the corresponding runtime dep to libintl-full, but without any reference to icu

now we have PHP8_INTL depends on PHP8_GETTEXT with: PHP8_INTL → --enable-intl=shared and PHP8_GETTEXT → --with-gettext=shared,"$(STAGING_DIR)/usr/lib/libintl-full"; independent of whether the modules are selected or not

In my tests, even when I don't declare the build dependency, then icu is built (I don't know why yet). But gettext seems to not have a runtime dep on icu (at least readelf does not show it), only intl does. So I assumed that it would make more sense to use PKG_BUILD_DEPENDS:= PHP8_INTL:icu here.

This is why I'm still confused - do you have other results/findings?

For me the GETTEXT "$(STAGING_DIR)/usr/lib/libintl-full fails with complaint about missing icu unless I use PKG_BUILD_DEPENDS:=PHP8_GETTEXT:icu.

gettext does not acquire a runtime dependency on icu despite that.

I have not tried to build mod-gettext without that config - perhaps it could be moved to PHP8_INTL? It was there from the original version, and I didn't try to change that part.

This is what I see with the package as it is in master now:

checking whether to enable internationalization support... yes, shared checking for icu-uc >= 50.1 icu-io icu-i18n... no configure: error: Package requirements (icu-uc >= 50.1 icu-io icu-i18n) were not met: Package 'icu-uc' not found Package 'icu-io' not found Package 'icu-i18n' not found Consider adjusting the PKG_CONFIG_PATH environment variable if you installed software in a non-standard prefix.

That is with PHP8_GETTEXT and PHP8_INTL both =y, and PACKAGE_php8-mod-gettext=y but PACKAGE_php8-mod-intl not set.

First, thanks for spending time on this and sorting the issues out - appreciated.
Yes, I can confirm this and this is expected. Adding PKG_BUILD_DEPENDS:= PHP8_INTL:icu fixes the issue.
Even for the configuration PHP8_GETTEXT=y and PHP8_INTL not set, PACKAGE_php8-mod-gettext and PACKAGE_php8-mod-intl both not set.

I've cherry-picked your commit from your branch here, rebased & applied to current master and slightly adapted according to what I think should work. Maybe you can have a look at https://github.com/mhei/packages/commits/pr-fix-php8-gettext-icu-always-selected It works for me and I hope that it also works on your side as well?

Will check tonight. Thank you for working with me on this - appreciated here as well.

I have confirmed it works for me.

mhei · 2026-01-04T23:26:44Z

lang/php8/Makefile


 	config PHP8_FULLICUDATA
 		bool "Add dependency to full ICU Data"
-		depends on PHP8_INTL


PHP8_INTL already depends on PHP8_GETTEXT so this should not be required? And after looking at 72dd9ee again, since this flag is only affecting the php8-mod-intl, I think we should revert to use Package/php8-mod-intl/config stuff, so that this option is displayed near the package.

I moved it because Kconfig did not want to behave properly with it as Package/php8-mod-intl/config. I don't know if it's just the vagaries of Kconfig or if there is a bug to chase down, and in another couple of instances.

mhei · 2026-01-04T23:31:23Z

lang/php8/Makefile

 $(eval $(call BuildModule,gmp,GMP,+PACKAGE_php8-mod-gmp:libgmp))
 $(eval $(call BuildModule,iconv,iConv,$(ICONV_DEPENDS)))
-$(eval $(call BuildModule,intl,Internationalization Functions,@PHP8_INTL +PACKAGE_php8-mod-intl:icu +PHP8_FULLICUDATA:icu-full-data))
+$(eval $(call BuildModule,intl,Internationalization Functions,@PHP8_GETTEXT @PHP8_INTL +PACKAGE_php8-mod-intl:php8-mod-gettext +PHP8_FULLICUDATA:icu-full-data +!PHP8_FULLICUDATA:icu))


@PHP_GETTEXT should not be required here since @PHP8_INTL already depends on it, right?

I'd prefer to leave the dependency to icu and icu-full-data as is: the dependency to icu is always required, and the dependency with PHP8_FULLICUDATA to icu-full-data is in my eyes just a "convenience" helper.

Kconfig issues again.

mhei · 2026-01-04T23:36:11Z

lang/php8/Makefile

 	config PHP8_INTL
 		bool "Enable Internationalization"
-    depends on PHP8_GETTEXT
-		default y


I have to do more tests, but at the moment, I don't see that PHP8_INTL must depend on PHP8_GETTEXT.
But on the other side, I think we should add to PHP8_LIBXML, PHP8_GETTEXT and PHP8_INTL a depends on depends on PACKAGE_php8 so that all options are hidden, if the "top-level" config option is not selected.

This was because Kconfig was behaving strangely without it.

Also adding a depends on PACKAGE_php8 gave me recursive dependency Kconfig problems.

I am not sure if I did try PHP8_INTL without PHP8_GETTEXT, that may be able to be disentangled.

Fixes: php8: global package dependency changes based on module selection Fixes: #28078 As described in #28078 and #28075, Some binaries gain a dependency on libstdcpp when mod-gettext is included in the build, however this was not explicitly declared, so packaging fails with (e.g.): Package php8-cgi is missing dependencies for the following libraries: libstdc++.so.6 In contrast to #28075, this commit takes the approach: * Make use of --with-gettext depend on a configure flag (enabled by default, since that matches current full build behaviour) * Make sub-packages which require --with-gettext depend on the configure flag This means that e.g. php-cgi would not have gettext support if the configure flag was disabled, and e.g. php-mod-gettext and php-mod-intl would not be selectable. Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>

danielfdickinson · 2026-01-07T15:28:50Z

@mhei I have not given up on this - I've just not had a chance to try again. Kconfig can be a pain to work with.

Switch to a single CONFIG_ per line, and alphabetize. Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>

xmlreader was selecting package php8-mod-dom as well as depending on PHP8_DOM, while php8-mod-dom also depended on PHP8_DOM (and therefore selected PHP8_DOM when php8-mod-dom was selected). This is a Kconfig recursive dependency, so break the recursion by noting that because php8-mod-xmlreader selects php8-mod-dom, PHP8_DOM is a transitive depends, so php8-mod-xmlreader should not depend on PHP8_DOM itself. Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>

Add more menuconfig help text descriptions, and convert some mixed tabs and spaces to spaces. Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>

* Add a needed BUILD_DEPENDENCY on icu package, when PHP8_INTL is defined. * Make PHP8_DOM selecting PHP8_LIBXML instead of depending on it. * PHP8_INTL does not depend on PHP8_GETTEXT, it builds also without gettext. * Always show option for choosing PHP8_FULLUCIDATA * For php8-cgi, php-cli, etc, a libstdcpp dependency is only gained when PHP8_INTL is selected, therefore update those conditional depends. As some combinations of these changes can change the binaries output, PKG_RELEASE has been bumped. Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca> Signed-off-by: Michael Heimpold <mhei@heimpold.de>

danielfdickinson · 2026-01-16T05:02:51Z

LGTM! Thank you.

danielfdickinson marked this pull request as ready for review January 1, 2026 10:41

GeorgeSapkin reviewed Jan 3, 2026

View reviewed changes

lang/php8/Makefile Outdated Show resolved Hide resolved

danielfdickinson force-pushed the pr-fix-php8-gettext-icu-always-selected branch from 72ef85e to be62019 Compare January 4, 2026 03:43

GeorgeSapkin reviewed Jan 4, 2026

View reviewed changes

lang/php8/Makefile Outdated Show resolved Hide resolved

danielfdickinson force-pushed the pr-fix-php8-gettext-icu-always-selected branch from be62019 to faae8bd Compare January 4, 2026 03:53

danielfdickinson force-pushed the pr-fix-php8-gettext-icu-always-selected branch from bf8f4bc to 23ae03c Compare January 4, 2026 05:04

mhei requested changes Jan 4, 2026

View reviewed changes

mhei mentioned this pull request Jan 10, 2026

php8: the PHP8 compilation options are only saved if PHP8 is also built #28288

Merged

1 task

danielfdickinson added 4 commits January 13, 2026 21:04

php8: reformat CONFIG_DEPENDS

0433116

Switch to a single CONFIG_ per line, and alphabetize. Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>

php8: add more help text and tweak whitespace

d057dab

Add more menuconfig help text descriptions, and convert some mixed tabs and spaces to spaces. Signed-off-by: Daniel F. Dickinson <dfdpublic@wildtechgarden.ca>

mhei force-pushed the pr-fix-php8-gettext-icu-always-selected branch from 23ae03c to 46f0e43 Compare January 15, 2026 20:55

mhei merged commit f8b8ce6 into openwrt:master Jan 16, 2026
12 checks passed

danielfdickinson deleted the pr-fix-php8-gettext-icu-always-selected branch January 17, 2026 05:54

danielfdickinson mentioned this pull request Feb 2, 2026

zabbix: fix compile issues from #28428 final tweaks #28479

Merged

1 task

Conversation

danielfdickinson commented Jan 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📦 Package Details

🧪 Run Testing Details

✅ Formalities

Uh oh!

mhei commented Jan 3, 2026

Uh oh!

Uh oh!

Uh oh!

danielfdickinson commented Jan 4, 2026

Uh oh!

danielfdickinson commented Jan 4, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielfdickinson Jan 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielfdickinson Jan 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielfdickinson commented Jan 7, 2026

Uh oh!

danielfdickinson commented Jan 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

danielfdickinson commented Jan 1, 2026 •

edited

Loading

danielfdickinson Jan 11, 2026 •

edited

Loading

danielfdickinson Jan 11, 2026 •

edited

Loading