Add back 8-zts-alpine, take 2#1076
Conversation
11e1dc5 to
6724d14
Compare
|
blargh |
6724d14 to
025d39d
Compare
|
The error I keep coming back to is: Some Googling seems to suggest a musl issue, but I've also tried building with Alpine 3.11 and got the same error, as with Alpine 3.10. Full error: |
|
Also just tried edge which comes with musl 1.2 (where 3.12 comes with 1.1) and still getting the same error. Getting the feeling, this issue purely comes from PHP 8 itself :( |
|
After reading https://wiki.php.net/zts-improvement and not having a clue what it means I also found https://bugs.freedesktop.org/show_bug.cgi?id=35268 which seems to suggest this needs to change in PHP itself. (As far as I understand.) |
|
@carusogabriel If you have any clue about this your help would be appreciated. |
|
And the issue is also present in 8.0.0 final |
025d39d to
b3bbb9b
Compare
b3bbb9b to
6dd757e
Compare
50d86f2 to
ba537dd
Compare
|
Any progress on this PR so TS PHP needed for threaded apps is available? |
ba537dd to
d26bb98
Compare
|
To be clear -- any actual progress on this PR is likely to happen in PHP upstream, not in the image (however, Alpine is not exactly well-supported, so I wouldn't suggest holding your breath). I've rebased and added the 8.1 variants too so we can see whether anything has progressed. |
|
Is the issue still present /w Alpine latest/3.15? |
d26bb98 to
427cfd9
Compare
|
Thanks for the reminder! I've just rebased so we can find out. 😄 |
427cfd9 to
7706f84
Compare
|
I have opened php/php-src#8160 in php-src. I do not know the issue directly and the CI output seems very limited, so if anyone knows the problem, please describe in the php-src issue, preferably with minimalistic Dockerfile to reproduce. |
|
Can you please try with the following patch: ext/pdo/config.m4 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ext/pdo/config.m4 b/ext/pdo/config.m4
index 9b9a3e36df..63a2f7ed0f 100644
--- a/ext/pdo/config.m4
+++ b/ext/pdo/config.m4
@@ -9,7 +9,7 @@ if test "$PHP_PDO" != "no"; then
dnl Make sure $PHP_PDO is 'yes' when it's not 'no' :)
PHP_PDO=yes
- PHP_NEW_EXTENSION(pdo, pdo.c pdo_dbh.c pdo_stmt.c pdo_sql_parser.c pdo_sqlstate.c, $ext_shared)
+ PHP_NEW_EXTENSION(pdo, pdo.c pdo_dbh.c pdo_stmt.c pdo_sql_parser.c pdo_sqlstate.c, $ext_shared,,-DZEND_ENABLE_STATIC_TSRMLS_CACHE=1)
PHP_ADD_EXTENSION_DEP(pdo, spl, true)
PHP_INSTALL_HEADERS(ext/pdo, [php_pdo.h php_pdo_driver.h php_pdo_error.h])
|
a02c99a to
0979638
Compare
|
Here's the updated warning on 8.1 with that patch:
|
|
Something similar (but on
(Maybe that fix wasn't quite enough / in the right place for 8.0?) |
|
(I've also just pushed docker-library/official-images@ce73036 which should make the next run of these tests in CI actually show the error/warning correctly, which will be a lot more useful. 🤦) Edit: success (at getting the test to print the warning/error): https://github.com/docker-library/php/runs/5382861733?check_suite_focus=true#step:7:9 🤘 |
|
Thanks for checking, @tianon! I'm a bit surprised due to |
|
See also php/php-src@804420b. Since @derickr committed this, maybe he has some ideas. |
|
@tianon please test patching PHP 8.0+ versions with php/php-src@PHP-8.0...mvorisek:abd95290272087285dc2827d0fa0fbe141db6ff7 (with both commits applied) |
fde89dc to
aa8104e
Compare
|
Looks like that still did the trick for 8.1, but not 8.0. 😅 |
weird, very weird, my build is passing even for PHP 8.0 see mvorisek/image-php@29fdca7 here is a test: https://github.com/atk4/data/runs/5456421886?check_suite_focus=true#step:4:5 , the CI confirms ZTS here is the full patching code: mvorisek/image-php@29fdca7#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fR55-R108 , it is based on |
|
I build from source mvorisek/image-php@29fdca7#diff-b803fcb7f17ed9235f1e5cb1fcd2f5d3b2838429d4368ae4c57ce4436577f03fR81 I belive that is the issue here, as the patching in this PR does not change the prebuilt |
|
Ahhh good catch -- we build from the released source tarballs, so indeed, regenerating |
aa8104e to
ed03ce8
Compare
|
There we go; much better. 😄 Edit: specifically, php/php-src@php:8b15858...mvorisek:abd9529 applied on 8.0 and 8.1 fixes it 😄 |
|
php/php-src#8180 merged, so ZTS build should pass starting with upcoming PHP 8.0.18 and 8.1.5 releases. |
ed03ce8 to
2027b5d
Compare
|
The releases are out, can you please rebase/retest? |
2027b5d to
83dea60
Compare
|
🟢 😱 🎉 ! |
|
Oops, forgot to come back to this -- thanks for the reminder! 👍 |
Changes: - docker-library/php@4aac9a2: Update 8.1-rc - docker-library/php@2a12c7c: Update 8.0-rc - docker-library/php@16b3a9d: Merge pull request docker-library/php#1076 from infosiftr/8-zts-alpine-take-2
Changes: - docker-library/php@4aac9a2: Update 8.1-rc - docker-library/php@2a12c7c: Update 8.0-rc - docker-library/php@16b3a9d: Merge pull request docker-library/php#1076 from infosiftr/8-zts-alpine-take-2

#1075, but this time with proper gusto
Closes #1074