You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Compilation failure on pdo_mysql extension for 8.4alpha Docker image #14792
After tagging of the first alpha for 8.4 occurred yesterday, I proceeded to provide an upstream PR to the official Docker image repository: docker-library/php#1526.
However the test for the builds fail when attempting to compile the pdo_mysql extension, which can be reproduced as follows (see also docker-library/php#1526 (comment)):
Possibly related commits are 11accb5 and 715b9aa, which may require a change in the upstream configuration?
Note: given that there's no official announcement for this alpha release yet and this is thus an overly eager attempt from my side, this might be a known issue that will be resolved before the announcement; if so, feel free to close!
I see. Docker image already has generated re2c files from the downloaded archive. Ok. That simplifies things a bit. I've split the bugfix and enhancement into separate PRs.
(First off: thanks for the quick action and your efforts to try and help solving this issue!)
I've tried applying this as a patch to the extracted source in the Docker build with some quick patching at jnoordsij/docker-library-php@f9d0ddc, but it still not seems to be working. Am I missing something?
Yes, the issue is that this patch needs to be run when building the main Docker image. When it is applied only on the step at docker run -it --rm php:8.4-rc docker-php-ext-install pdo_mysql it will patch the file but the missing PDO headers won't be installed because they were installed in the previous step when building the main Docker image.
Or instead of applying the patch this can be done:
@@ -19,7 +19,7 @@ case "$1" in
mkdir -p "$dir"
if [ ! -f "$dir/.docker-extracted" ]; then
tar -Jxf /usr/src/php.tar.xz -C "$dir" --strip-components=1
- curl -s https://patch-diff.githubusercontent.com/raw/php/php-src/pull/14797.patch | git -C "$dir" apply --reject || true+ cp $dir/ext/pdo/pdo_sql_parser.h $dir/ext/pdo/php_pdo_int.h /usr/local/include/php/ext/pdo/
touch "$dir/.docker-extracted"
fi
;;
Moreover, even if I remove the post-compile deletion of the source code that happens after building from the image build (https://github.com/jnoordsij/docker-library-php/blob/f9d0ddc5ca9f3ea4742bcb50f7418f7cb9d73896/8.4-rc/bookworm/cli/Dockerfile#L194), so that the build of the pdo_mysql extension happens with the exact same code state (which we would not want to do for the actual build, since this would heavily bloat the image size), I can see that the same (patched) source code is used for both building operations, but still the pdo_mysql build is failing as above.
@jnoordsij sorry. Now I understand. You also need to rebuild the configure script :D Eh, how could I've missed that. This *.m4 files create a configure script with Autoconf and there is a buildconf script that does that. So, after patching run also this:
extract)
mkdir -p "$dir"if [ !-f"$dir/.docker-extracted" ];then
tar -Jxf /usr/src/php.tar.xz -C "$dir" --strip-components=1
curl -s https://patch-diff.githubusercontent.com/raw/php/php-src/pull/14797.patch | git -C "$dir" apply --reject ||true
touch "$dir/.docker-extracted"# --> Add this one. This will recreate the configure script: <--$dir/buildconf -f
fi;;
Thanks, that seems to do the trick! Just added docker-library/php@24d1dd7 to verify CI builds all work now.
If I understand correctly from the release process docs, the (re)built configure script should be part of the next alpha release tarball, so all of this patching will no longer be required then and everything should build "just as before"?
Thanks, that seems to do the trick! Just added docker-library/php@24d1dd7 to verify CI builds all work now.
If I understand correctly from the release process docs, the (re)built configure script should be part of the next alpha release tarball, so all of this patching will no longer be required then and everything should build "just as before"?
Yes, this configure script is always regenerated and updated for the current sources in the current downloaded archive. You only need to regenerate it like here if you change some Autoconf (*.m4 or configure.ac) file.
Hi @petk , is it still true that a lets say pdo_mysql phpize build does not have the RE2C variable in Makefile (at least it seems so)? If so, are you by any chance apprised of any ETAs re this issue since we already have php-8.4.0RC2? :)
Hi @petk , is it still true that a lets say pdo_mysql phpize build does not have the RE2C variable in Makefile (at least it seems so)? If so, are you by any chance apprised of any ETAs re this issue since we already have php-8.4.0RC2? :)
Thanks!
We could add it to the master branch as this is now more of a new feature than a bug fix. It would be needed for PHP 8.4 already perhaps? Because, I'm a bit concerned whether to change the build system in the "released" branches with such change. And the change would also wrap that configure option --enable-re2c-cgoto for extensions (probably).
One would think that being able to build those extensions that require re2c lexer generator (namely all those mentioned here) should be a given in PHP 8.4, especially if they are part of the PHP source code, as it were :)
If they should be built (in PHP 8.4) by employing some other technique would you be so kind as to suggest a preferred one?
Activity
petk commentedon Jul 3, 2024
The issue is that phpize build doesn't have the RE2C variable in Makefile. In PHP-8.3 there wasn't this need yet.
petk commentedon Jul 3, 2024
I see. Docker image already has generated re2c files from the downloaded archive. Ok. That simplifies things a bit. I've split the bugfix and enhancement into separate PRs.
Fix phpGH-14792: Compilation failure on pdo_* extensions (php#14797)
jnoordsij commentedon Jul 4, 2024
(First off: thanks for the quick action and your efforts to try and help solving this issue!)
I've tried applying this as a patch to the extracted source in the Docker build with some quick patching at jnoordsij/docker-library-php@f9d0ddc, but it still not seems to be working. Am I missing something?
petk commentedon Jul 4, 2024
Yes, the issue is that this patch needs to be run when building the main Docker image. When it is applied only on the step at
docker run -it --rm php:8.4-rc docker-php-ext-install pdo_mysqlit will patch the file but the missing PDO headers won't be installed because they were installed in the previous step when building the main Docker image.Or instead of applying the patch this can be done:
jnoordsij commentedon Jul 5, 2024
The patch is part of main Docker image build as well with the change I mentioned above, as the
docker-php-sourcecommand is being run before building:https://github.com/jnoordsij/docker-library-php/blob/f9d0ddc5ca9f3ea4742bcb50f7418f7cb9d73896/8.4-rc/bookworm/cli/Dockerfile#L125-L126
Moreover, even if I remove the post-compile deletion of the source code that happens after building from the image build (https://github.com/jnoordsij/docker-library-php/blob/f9d0ddc5ca9f3ea4742bcb50f7418f7cb9d73896/8.4-rc/bookworm/cli/Dockerfile#L194), so that the build of the
pdo_mysqlextension happens with the exact same code state (which we would not want to do for the actual build, since this would heavily bloat the image size), I can see that the same (patched) source code is used for both building operations, but still thepdo_mysqlbuild is failing as above.petk commentedon Jul 5, 2024
@jnoordsij sorry. Now I understand. You also need to rebuild the configure script :D Eh, how could I've missed that. This *.m4 files create a configure script with Autoconf and there is a buildconf script that does that. So, after patching run also this:
DEBUG: Apply suggestion per php/php-src#14792 (comment)
DEBUG: Apply suggestions per php/php-src#14792 to test CI
jnoordsij commentedon Jul 5, 2024
Thanks, that seems to do the trick! Just added docker-library/php@24d1dd7 to verify CI builds all work now.
If I understand correctly from the release process docs, the (re)built configure script should be part of the next alpha release tarball, so all of this patching will no longer be required then and everything should build "just as before"?
petk commentedon Jul 5, 2024
Yes, this configure script is always regenerated and updated for the current sources in the current downloaded archive. You only need to regenerate it like here if you change some Autoconf (*.m4 or configure.ac) file.
KaseyJenkins commentedon Oct 14, 2024
Hi @petk , is it still true that a lets say pdo_mysql phpize build does not have the RE2C variable in Makefile (at least it seems so)? If so, are you by any chance apprised of any ETAs re this issue since we already have php-8.4.0RC2? :)
Thanks!
petk commentedon Oct 14, 2024
We could add it to the master branch as this is now more of a new feature than a bug fix. It would be needed for PHP 8.4 already perhaps? Because, I'm a bit concerned whether to change the build system in the "released" branches with such change. And the change would also wrap that configure option --enable-re2c-cgoto for extensions (probably).
KaseyJenkins commentedon Oct 14, 2024
Thank you @petk for your swift reply!
One would think that being able to build those extensions that require re2c lexer generator (namely all those mentioned here) should be a given in PHP 8.4, especially if they are part of the PHP source code, as it were :)
If they should be built (in PHP 8.4) by employing some other technique would you be so kind as to suggest a preferred one?
petk commentedon Oct 14, 2024
One quick solution would be to set the RE2C environment variable in the make step, something like this:
KaseyJenkins commentedon Oct 15, 2024
Thank you @petk!