Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PHP 8.4 RC #1526

Merged
merged 5 commits into from
Jul 8, 2024
Merged

Add PHP 8.4 RC #1526

merged 5 commits into from
Jul 8, 2024

Conversation

jnoordsij
Copy link
Contributor

@jnoordsij jnoordsij commented Jul 2, 2024

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 jnoordsij force-pushed the add-php8.4-rc branch 2 times, most recently from 2984693 to 48d2406 Compare July 2, 2024 14:49
@jnoordsij
Copy link
Contributor Author

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

@tianon
Copy link
Member

tianon commented Jul 2, 2024

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_mysql
Configuring for:
PHP Version:             8.4
PHP Api Version:         20230901
Zend Module Api No:      20230901
Zend Extension Api No:   420230901
checking for grep that handles long lines and -e... /usr/bin/grep
checking for egrep... /usr/bin/grep -E
checking for a sed that does not truncate output... /usr/bin/sed
checking for pkg-config... /usr/bin/pkg-config
checking pkg-config is at least version 0.9.0... yes
checking for cc... cc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether the compiler supports GNU C... yes
checking whether cc accepts -g... yes
checking for cc option to enable C11 features... none needed
checking how to run the C preprocessor... cc -E
checking for icc... no
checking for suncc... no
checking for system library directory... lib
checking if compiler supports -Wl,-rpath,... yes
checking build system type... x86_64-pc-linux-gnu
checking host system type... x86_64-pc-linux-gnu
checking target system type... x86_64-pc-linux-gnu
checking for PHP prefix... /usr/local
checking 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/lib
checking for PHP extension directory... /usr/local/lib/php/extensions/no-debug-non-zts-20230901
checking for PHP installed headers prefix... /usr/local/include/php
checking if debug is enabled... no
checking if zts is enabled... no
checking for gawk... no
checking for nawk... nawk
checking if nawk is broken... no
checking for MySQL support for PDO... yes, shared
checking for MySQL UNIX socket location...
checking for PDO includes... /usr/local/include/php/ext
checking for a sed that does not truncate output... /usr/bin/sed
checking for ld used by cc... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for /usr/bin/ld option to reload object files... -r
checking for BSD-compatible nm... /usr/bin/nm -B
checking whether ln -s works... yes
checking how to recognize dependent libraries... pass_all
checking for stdio.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for strings.h... yes
checking for sys/stat.h... yes
checking for sys/types.h... yes
checking for unistd.h... yes
checking for dlfcn.h... yes
checking the maximum length of command line arguments... 1572864
checking command to parse /usr/bin/nm -B output from cc object... ok
checking for objdir... .libs
checking for ar... ar
checking for ranlib... ranlib
checking for strip... strip
checking if cc supports -fno-rtti -fno-exceptions... no
checking for cc option to produce PIC... -fPIC
checking if cc PIC flag -fPIC works... yes
checking if cc static flag -static works... yes
checking if cc supports -c -o file.o... yes
checking whether the cc linker (/usr/bin/ld -m elf_x86_64) supports shared libraries... yes
checking whether -lc should be explicitly linked in... no
checking dynamic linker characteristics... GNU/Linux ld.so
checking how to hardcode library paths into programs... immediate
checking whether stripping libraries is possible... yes
checking if libtool supports shared libraries... yes
checking whether to build shared libraries... yes
checking whether to build static libraries... no

creating libtool
appending configuration tag "CXX" to libtool
configure: creating build directories
configure: creating Makefile
configure: patching config.h.in
configure: creating ./config.status
config.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.lo
mkdir .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
Member

tianon commented Jul 2, 2024

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
Copy link
Contributor Author

jnoordsij commented Jul 3, 2024

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
Member

tianon commented Jul 4, 2024

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
Contributor Author

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
Contributor Author

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
Member

tianon commented Jul 5, 2024

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
Contributor Author

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
Copy link
Member

@tianon tianon left a comment

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 on lines 249 to 251
TEMPFILE=$(mktemp); \
filterdiff -p1 -x 'NEWS' < php-pdo.patch >$TEMPFILE; \
mv $TEMPFILE php-pdo.patch; \
Copy link
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 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
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 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
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
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
Contributor Author

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 yosifkit July 8, 2024 21:44
@yosifkit yosifkit merged commit 69f491f into docker-library:master Jul 8, 2024
58 checks passed
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
Sign up for free to 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