Skip to content

Add PHP 8.4 RC - #1526

Merged
yosifkit merged 5 commits into
docker-library:masterfrom
jnoordsij:add-php8.4-rc
Jul 8, 2024
Merged

Add PHP 8.4 RC#1526
yosifkit merged 5 commits into
docker-library:masterfrom
jnoordsij:add-php8.4-rc

Conversation

@jnoordsij

@jnoordsijjnoordsij commented Jul 2, 2024

Copy link
Copy Markdown
Contributor

Note the second commit should be the result of the bot when the QA releases page is updated, but I manually added it to test image builds a little ahead.

This means we should probably watch https://github.com/php/web-qa/commits/master for the official release announcement before merging.

Edit: given the build failures which require php/php-src#14797 to be solved, this most likely won't land before the next alpha release which is scheduled for July 18th (see also https://wiki.php.net/todo/php84). The last debug commit should be removed before merging as this should no longer be required on next alpha release.

@jnoordsij
jnoordsijforce-pushed the add-php8.4-rc branch 2 times, most recently from 2984693 to 48d2406CompareJuly 2, 2024 14:49
@jnoordsij

Copy link
Copy Markdown
ContributorAuthor

Note: tests seem to be failing in CI, but it's not immediately apparent to me what's happening. Might investigate tomorrow.

@tianon

tianon commented Jul 2, 2024

Copy link
Copy Markdown
Member

I'm not 100% sure why it's failing, but it's definitely pdo_mysql doing it:

$ docker buildx build --pull https://github.com/docker-library/php.git#refs/pull/1526/head:8.4-rc/bookworm/cli --tag php:8.4-rc...
$ docker run -it --rm php:8.4-rc docker-php-ext-install pdo_mysqlConfiguring for:PHP Version: 8.4PHP Api Version: 20230901Zend Module Api No: 20230901Zend Extension Api No: 420230901checking for grep that handles long lines and -e... /usr/bin/grepchecking for egrep... /usr/bin/grep -Echecking for a sed that does not truncate output... /usr/bin/sedchecking for pkg-config... /usr/bin/pkg-configchecking pkg-config is at least version 0.9.0... yeschecking for cc... ccchecking whether the C compiler works... yeschecking for C compiler default output file name... a.outchecking for suffix of executables...checking whether we are cross compiling... nochecking for suffix of object files... ochecking whether the compiler supports GNU C... yeschecking whether cc accepts -g... yeschecking for cc option to enable C11 features... none neededchecking how to run the C preprocessor... cc -Echecking for icc... nochecking for suncc... nochecking for system library directory... libchecking if compiler supports -Wl,-rpath,... yeschecking build system type... x86_64-pc-linux-gnuchecking host system type... x86_64-pc-linux-gnuchecking target system type... x86_64-pc-linux-gnuchecking for PHP prefix... /usr/localchecking for PHP includes... -I/usr/local/include/php -I/usr/local/include/php/main -I/usr/local/include/php/TSRM -I/usr/local/include/php/Zend -I/usr/local/include/php/ext -I/usr/local/include/php/ext/date/libchecking for PHP extension directory... /usr/local/lib/php/extensions/no-debug-non-zts-20230901checking for PHP installed headers prefix... /usr/local/include/phpchecking if debug is enabled... nochecking if zts is enabled... nochecking for gawk... nochecking for nawk... nawkchecking if nawk is broken... nochecking for MySQL support for PDO... yes, sharedchecking for MySQL UNIX socket location...checking for PDO includes... /usr/local/include/php/extchecking for a sed that does not truncate output... /usr/bin/sedchecking for ld used by cc... /usr/bin/ldchecking if the linker (/usr/bin/ld) is GNU ld... yeschecking for /usr/bin/ld option to reload object files... -rchecking for BSD-compatible nm... /usr/bin/nm -Bchecking whether ln -s works... yeschecking how to recognize dependent libraries... pass_allchecking for stdio.h... yeschecking for stdlib.h... yeschecking for string.h... yeschecking for inttypes.h... yeschecking for stdint.h... yeschecking for strings.h... yeschecking for sys/stat.h... yeschecking for sys/types.h... yeschecking for unistd.h... yeschecking for dlfcn.h... yeschecking the maximum length of command line arguments... 1572864checking command to parse /usr/bin/nm -B output from cc object... okchecking for objdir... .libschecking for ar... archecking for ranlib... ranlibchecking for strip... stripchecking if cc supports -fno-rtti -fno-exceptions... nochecking for cc option to produce PIC... -fPICchecking if cc PIC flag -fPIC works... yeschecking if cc static flag -static works... yeschecking if cc supports -c -o file.o... yeschecking whether the cc linker (/usr/bin/ld -m elf_x86_64) supports shared libraries... yeschecking whether -lc should be explicitly linked in... nochecking dynamic linker characteristics... GNU/Linux ld.sochecking how to hardcode library paths into programs... immediatechecking whether stripping libraries is possible... yeschecking if libtool supports shared libraries... yeschecking whether to build shared libraries... yeschecking whether to build static libraries... nocreating libtoolappending configuration tag "CXX" to libtoolconfigure: creating build directoriesconfigure: creating Makefileconfigure: patching config.h.inconfigure: creating ./config.statusconfig.status: creating config.h/bin/bash /usr/src/php/ext/pdo_mysql/libtool --tag=CC --mode=compile cc -I. -I/usr/src/php/ext/pdo_mysql -I/usr/local/include/php -I/usr/local/include/php/main -I/usr/local/include/php/TSRM -I/usr/local/include/php/Zend -I/usr/local/include/php/ext -I/usr/local/include/php/ext/date/lib -fstack-protector-strong -fpic -fpie -O2 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -DHAVE_CONFIG_H -fstack-protector-strong -fpic -fpie -O2 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1 -DZEND_COMPILE_DL_EXT=1 -c /usr/src/php/ext/pdo_mysql/pdo_mysql.c -o pdo_mysql.lo -MMD -MF pdo_mysql.dep -MT pdo_mysql.lomkdir .libs cc -I. -I/usr/src/php/ext/pdo_mysql -I/usr/local/include/php -I/usr/local/include/php/main -I/usr/local/include/php/TSRM -I/usr/local/include/php/Zend -I/usr/local/include/php/ext -I/usr/local/include/php/ext/date/lib -fstack-protector-strong -fpic -fpie -O2 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -DHAVE_CONFIG_H -fstack-protector-strong -fpic -fpie -O2 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1 -DZEND_COMPILE_DL_EXT=1 -c /usr/src/php/ext/pdo_mysql/pdo_mysql.c -MMD -MF pdo_mysql.dep -MT pdo_mysql.lo -fPIC -DPIC -o .libs/pdo_mysql.o/bin/bash /usr/src/php/ext/pdo_mysql/libtool --tag=CC --mode=compile cc -I. -I/usr/src/php/ext/pdo_mysql -I/usr/local/include/php -I/usr/local/include/php/main -I/usr/local/include/php/TSRM -I/usr/local/include/php/Zend -I/usr/local/include/php/ext -I/usr/local/include/php/ext/date/lib -fstack-protector-strong -fpic -fpie -O2 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -DHAVE_CONFIG_H -fstack-protector-strong -fpic -fpie -O2 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1 -DZEND_COMPILE_DL_EXT=1 -c /usr/src/php/ext/pdo_mysql/mysql_driver.c -o mysql_driver.lo -MMD -MF mysql_driver.dep -MT mysql_driver.lo cc -I. -I/usr/src/php/ext/pdo_mysql -I/usr/local/include/php -I/usr/local/include/php/main -I/usr/local/include/php/TSRM -I/usr/local/include/php/Zend -I/usr/local/include/php/ext -I/usr/local/include/php/ext/date/lib -fstack-protector-strong -fpic -fpie -O2 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -DHAVE_CONFIG_H -fstack-protector-strong -fpic -fpie -O2 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1 -DZEND_COMPILE_DL_EXT=1 -c /usr/src/php/ext/pdo_mysql/mysql_driver.c -MMD -MF mysql_driver.dep -MT mysql_driver.lo -fPIC -DPIC -o .libs/mysql_driver.o/bin/bash /usr/src/php/ext/pdo_mysql/libtool --tag=CC --mode=compile cc -I. -I/usr/src/php/ext/pdo_mysql -I/usr/local/include/php -I/usr/local/include/php/main -I/usr/local/include/php/TSRM -I/usr/local/include/php/Zend -I/usr/local/include/php/ext -I/usr/local/include/php/ext/date/lib -fstack-protector-strong -fpic -fpie -O2 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -DHAVE_CONFIG_H -fstack-protector-strong -fpic -fpie -O2 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1 -DZEND_COMPILE_DL_EXT=1 -c /usr/src/php/ext/pdo_mysql/mysql_statement.c -o mysql_statement.lo -MMD -MF mysql_statement.dep -MT mysql_statement.lo cc -I. -I/usr/src/php/ext/pdo_mysql -I/usr/local/include/php -I/usr/local/include/php/main -I/usr/local/include/php/TSRM -I/usr/local/include/php/Zend -I/usr/local/include/php/ext -I/usr/local/include/php/ext/date/lib -fstack-protector-strong -fpic -fpie -O2 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -DHAVE_CONFIG_H -fstack-protector-strong -fpic -fpie -O2 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1 -DZEND_COMPILE_DL_EXT=1 -c /usr/src/php/ext/pdo_mysql/mysql_statement.c -MMD -MF mysql_statement.dep -MT mysql_statement.lo -fPIC -DPIC -o .libs/mysql_statement.o/bin/bash /usr/src/php/ext/pdo_mysql/libtool --tag=CC --mode=compile cc -I. -I/usr/src/php/ext/pdo_mysql -I/usr/local/include/php -I/usr/local/include/php/main -I/usr/local/include/php/TSRM -I/usr/local/include/php/Zend -I/usr/local/include/php/ext -I/usr/local/include/php/ext/date/lib -fstack-protector-strong -fpic -fpie -O2 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -DHAVE_CONFIG_H -fstack-protector-strong -fpic -fpie -O2 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1 -DZEND_COMPILE_DL_EXT=1 -c /usr/src/php/ext/pdo_mysql/mysql_sql_parser.c -o mysql_sql_parser.lo -MMD -MF mysql_sql_parser.dep -MT mysql_sql_parser.lo cc -I. -I/usr/src/php/ext/pdo_mysql -I/usr/local/include/php -I/usr/local/include/php/main -I/usr/local/include/php/TSRM -I/usr/local/include/php/Zend -I/usr/local/include/php/ext -I/usr/local/include/php/ext/date/lib -fstack-protector-strong -fpic -fpie -O2 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -DHAVE_CONFIG_H -fstack-protector-strong -fpic -fpie -O2 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_GNU_SOURCE -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1 -DZEND_COMPILE_DL_EXT=1 -c /usr/src/php/ext/pdo_mysql/mysql_sql_parser.c -MMD -MF mysql_sql_parser.dep -MT mysql_sql_parser.lo -fPIC -DPIC -o .libs/mysql_sql_parser.o/usr/src/php/ext/pdo_mysql/mysql_sql_parser.c:21:10: fatal error: ext/pdo/php_pdo_int.h: No such file or directory 21 | #include "ext/pdo/php_pdo_int.h" | ^~~~~~~~~~~~~~~~~~~~~~~compilation terminated.make: *** [Makefile:220: mysql_sql_parser.lo] Error 1

My best guess is something related to php/php-src@11accb5 / php/php-src#13516 but that's 100% an educated stab in the dark (based on https://github.com/php/php-src/commits/php-8.4.0alpha1/ext/pdo_mysql + it being a failure of #include).

@tianon

Copy link
Copy Markdown
Member

From what I see/understand in php/php-src#13516, we should (still) see -I/usr/src/php in the arguments to cc, but I am not seeing it there 🤔

@jnoordsij

jnoordsij commented Jul 3, 2024

Copy link
Copy Markdown
ContributorAuthor

Very much not a conclusive find here (and definitely far outside the scope of my knowledge), but here's what I just noted:

  • the -I/usr/src/php argument is not present when trying 8.3-rc either, so possibly might not be the cause here
  • possibly related is php/php-src@715b9aa, which introduces the mysql_sql_parser file causing the error

I'll open an issue over at the PHP repo to see if they may be able to help and spot the issue here.

@tianon

Copy link
Copy Markdown
Member

I wonder if we could do something like docker-php-source extract and apply that PR as a patch, just to make sure we've verified it fixes our issue too? 🤔

(I'm off today, but I might explore this tomorrow if you don't beat me to it! 😄)

@jnoordsij

Copy link
Copy Markdown
ContributorAuthor

See php/php-src#14792 (comment): I've done a quick attempt, but can't seem to get it working yet...

@jnoordsij
jnoordsij marked this pull request as draft July 5, 2024 08:01
@jnoordsij

Copy link
Copy Markdown
ContributorAuthor

Thanks to the upstream help at php/php-src#14792 I think I've managed to hack something that should at least show build succeeds for the current alpha release now.

@tianon feel free to turn my hacking into something more subtle/stable to use if you want to attempt and already build an official 8.4-rc variant for this alpha; otherwise waiting for the next alpha also sounds very reasonable to me ;)

@tianon

Copy link
Copy Markdown
Member

It's honestly moderately tempting to just ignore our test failure since it only affects PDO and pdo_mysql just happens to be one of the two extensions we explicitly test (and we know it's fixed upstream already / in the next release). It does make me wonder if we just got really lucky to catch this or whether there are more such cases, so it does seem worthwhile to get this alpha release in somehow so folks can start testing it now instead of in two weeks when the next one comes (and thus increase the chances of finding more of these kinds of issues in the meantime). 🤔

Maybe we take the middle ground and only apply the patch in our Dockerfiles for now (so that the headers get installed correctly) and leave a comment here we can point users at if they're testing the alpha and run into this problem for how they can do the dance with docker-php-source + patch + regen + docker-php-ext-install to work around it? Or perhaps we download/verify the patch in the Dockerfile and leave it in /usr/src next to the PHP tarball so docker-php-source can just apply it directly after extracting the (otherwise completely unmodified) upstream source tarball? It doesn't seem great for every invocation of docker-php-source extract to have to regenerate all the configure scripts for just this one minor issue on the off chance that the module the user is trying to install happens to be PDO-related. 😄

(Your work testing/pressing this is really appreciated either way! 👍)

@jnoordsij

Copy link
Copy Markdown
ContributorAuthor

After some puzzling I've managed to add, verify and use the patch in the new 8.4 build. I've made all templating changes to use "8.4.0alpha1" explicitly, so that on the next alpha and automated update all the changes should be automatically discarded from the generated Dockerfiles without requiring immediate manual action. The template changes can then be safely reverted afterwards.

As the patch ensures that pdo already installs the correct headers upon initial build, it looks like the patch is no longer required after the build. It's still part of the image just in case someone still needs it for some build step and they find this PR.

WyriHaximus added a commit to WyriHaximus/github-action-supported-php-versions that referenced this pull request Jul 7, 2024

@tianontianon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nicely done, thank you! I've left a few minor nits that I'm happy to take care of if you'd prefer, but I think this is in pretty good shape and I'm glad we don't have to deal with the problem of runtime patching too! 😄

Comment threadDockerfile-linux.template Outdated
Comment on lines +249 to +251
TEMPFILE=$(mktemp); \
filterdiff -p1 -x 'NEWS' < php-pdo.patch >$TEMPFILE; \
mv $TEMPFILE php-pdo.patch; \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL patchutils! How have I not known of this before? 😅 ❤️

Minor nit, but I think we should do this with just a .xxx or similar instead of a whole mktemp:

Suggested change
TEMPFILE=$(mktemp); \
filterdiff -p1 -x 'NEWS' < php-pdo.patch >$TEMPFILE; \
mv $TEMPFILE php-pdo.patch; \
filterdiff -p1 -x 'NEWS' < php-pdo.patch > php-pdo.patch.filtered; \
mv php-pdo.patch.filtered php-pdo.patch; \

Comment threadDockerfile-linux.template Outdated
Comment on lines +247 to +248
curl -fsSL -o php-pdo.patch https://patch-diff.githubusercontent.com/raw/php/php-src/pull/14797.patch; \
echo "d9f33beda1ffffc66299377a0946b1fe2916382e320084568ae6ee2a6d1fb19e php-pdo.patch" | sha256sum -c -; \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't using the ENVs that you added, but I think that's probably fine since it makes the amount of things to remove later smaller (and I don't think the ENVs add much value to downstream users). 😄

I do have a few notes here though:

  • we should include the undocumented full_index=1 to help combat GitHub non-reproducibility
  • single quotes
  • * in sha256sum to avoid text-mode parsing
Suggested change
curl -fsSL -o php-pdo.patch https://patch-diff.githubusercontent.com/raw/php/php-src/pull/14797.patch; \
echo "d9f33beda1ffffc66299377a0946b1fe2916382e320084568ae6ee2a6d1fb19e php-pdo.patch" | sha256sum -c -; \
curl -fsSL -o php-pdo.patch 'https://github.com/php/php-src/pull/14797.patch?full_index=1'; \
echo '3a95762048a56ec0f59cb9ee18df49751ffb0bdd0e91e021b57f69ac7af62996 *php-pdo.patch' | sha256sum -c -; \

Comment threadDockerfile-linux.template Outdated
Comment on lines +201 to +204
{{ if .version == "8.4.0alpha1" then ( -}}
ENV PHP_PDO_PATCH_URL="https://patch-diff.githubusercontent.com/raw/php/php-src/pull/14797.patch"
ENV PHP_PDO_PATCH_SHA256="d9f33beda1ffffc66299377a0946b1fe2916382e320084568ae6ee2a6d1fb19e"
{{ ) else "" end -}}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{{ if .version == "8.4.0alpha1" then ( -}}
ENV PHP_PDO_PATCH_URL="https://patch-diff.githubusercontent.com/raw/php/php-src/pull/14797.patch"
ENV PHP_PDO_PATCH_SHA256="d9f33beda1ffffc66299377a0946b1fe2916382e320084568ae6ee2a6d1fb19e"
{{ ) else "" end -}}

👀 ❤️

"C28D937575603EB4ABB725861C0779DC5C0A9DE4", # bukka
"AFD8 691F DAED F03B DF6E 4605 63F1 5A9B 7153 76CA" # ericmann
"1198 C011 7593 497A 5EC5 C199 286A F1F9 8974 69DC", # pierrick
"C28D937575603EB4ABB725861C0779DC5C0A9DE4", # bukka

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wild, I guess this was probably just to match the format on the linked page specifically because we can. 😄 👍

@jnoordsij

Copy link
Copy Markdown
ContributorAuthor

Very nicely done, thank you! I've left a few minor nits that I'm happy to take care of if you'd prefer, but I think this is in pretty good shape and I'm glad we don't have to deal with the problem of runtime patching too! 😄

Thanks for the reviewing. Please go ahead with finishing this, all fine with me!

@tianon
tianon marked this pull request as ready for review July 8, 2024 21:44
@tianon
tianon requested a review from yosifkitJuly 8, 2024 21:44
@yosifkit
yosifkit merged commit 69f491f into docker-library:masterJul 8, 2024
docker-library-bot added a commit to docker-library-bot/official-images that referenced this pull request Jul 8, 2024
Changes:
- docker-library/php@69f491fb: Merge pull request docker-library/php#1526 from jnoordsij/add-php8.4-rc
- docker-library/php@6de7c598: Apply review nits
- docker-library/php@a1dce0f8: Add pdo build batch for 8.4.0alpha1
- docker-library/php@a40f4d1a: Add PHP 8.4-rc variant
- docker-library/php@9315df94: Update in preparation for first PHP 8.4 alpha release
- docker-library/php@05941afc: Improve formatting on 8.3 GPG_KEYS
@jnoordsij
jnoordsij deleted the add-php8.4-rc branch July 9, 2024 08:24
@jnoordsijjnoordsij mentioned this pull request Jul 2, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jnoordsij@tianon@yosifkit