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

mediawiki: fix ldd parsing logic #17709

Merged

Conversation

christian-heusel
Copy link
Contributor

As it turns out the liblua5 shared object is needed for runtime aswell
and having it missing produces a warning i.e. when running "php -v". Fix
this by installing it before the build time dependencies that are to be
removed later on.

Fixes https://phabricator.wikimedia.org/T376447

Signed-off-by: Christian Heusel [email protected]

This comment has been minimized.

@tianon
Copy link
Member

tianon commented Oct 10, 2024

@tianon
Copy link
Member

tianon commented Oct 10, 2024

Confirmed, you can see that's what's happening in https://github.com/wikimedia/mediawiki-docker/actions/runs/11122723665/job/30904612353#step:3:2226:

#7 75.57 + ldd /usr/local/lib/php/extensions/no-debug-non-zts-20210902/apcu.so /usr/local/lib/php/extensions/no-debug-non-zts-20210902/calendar.so /usr/local/lib/php/extensions/no-debug-non-zts-20210902/intl.so /usr/local/lib/php/extensions/no-debug-non-zts-20210902/luasandbox.so /usr/local/lib/php/extensions/no-debug-non-zts-20210902/mbstring.so /usr/local/lib/php/extensions/no-debug-non-zts-20210902/mysqli.so /usr/local/lib/php/extensions/no-debug-non-zts-20210902/opcache.so /usr/local/lib/php/extensions/no-debug-non-zts-20210902/sodium.so
#7 75.61 dpkg-query: no path found matching pattern /lib/x86_64-linux-gnu/libicudata.so.72
#7 75.61 dpkg-query: no path found matching pattern /lib/x86_64-linux-gnu/libicui18n.so.72
#7 75.61 dpkg-query: no path found matching pattern /lib/x86_64-linux-gnu/libicuio.so.72
#7 75.61 dpkg-query: no path found matching pattern /lib/x86_64-linux-gnu/libicuuc.so.72
#7 75.61 dpkg-query: no path found matching pattern /lib/x86_64-linux-gnu/liblua5.1.so.0
#7 75.61 dpkg-query: no path found matching pattern /lib/x86_64-linux-gnu/libonig.so.5
#7 75.61 dpkg-query: no path found matching pattern /lib/x86_64-linux-gnu/libsodium.so.23
#7 75.61 dpkg-query: no path found matching pattern /lib/x86_64-linux-gnu/libstdc++.so.6
#7 75.61 apt-mark manual libc6 libgcc-s1

If I apply this patch to 1.42/apache:

diff --git a/1.42/apache/Dockerfile b/1.42/apache/Dockerfile
index 46fbed2..8ec9e20 100644
--- a/1.42/apache/Dockerfile
+++ b/1.42/apache/Dockerfile
@@ -15,14 +15,10 @@ RUN set -eux; \
 
 # Install the PHP extensions we need
 RUN set -eux; \
-	\
-	apt-get update; \
-	apt-get install -y --no-install-recommends \
-		liblua5.1-0 \
-	; \
 	\
 	savedAptMark="$(apt-mark showmanual)"; \
 	\
+	apt-get update; \
 	apt-get install -y --no-install-recommends \
 		libicu-dev \
 		libonig-dev \
@@ -49,9 +45,9 @@ RUN set -eux; \
 	apt-mark auto '.*' > /dev/null; \
 	apt-mark manual $savedAptMark; \
 	ldd "$(php -r 'echo ini_get("extension_dir");')"/*.so \
-		| awk '/=>/ { print $3 }' \
+		| awk '/=>/ { so = $(NF-1); if (index(so, "/usr/local/") == 1) { next }; gsub("^/(usr/)?", "", so); printf "*%s\n", so }' \
 		| sort -u \
-		| xargs -r dpkg-query -S \
+		| xargs -r dpkg-query --search \
 		| cut -d: -f1 \
 		| sort -u \
 		| xargs -rt apt-mark manual; \

I get this instead:

+ ldd /usr/local/lib/php/extensions/no-debug-non-zts-20210902/apcu.so /usr/local/lib/php/extensions/no-debug-non-zts-20210902/calendar.so /usr/local/lib/php/extensions/no-debug-non-zts-20210902/intl.so /usr/local/lib/php/extensions/no-debug-non-zts-20210902/luasandbox.so /usr/local/lib/php/extensions/no-debug-non-zts-20210902/mbstring.so /usr/local/lib/php/extensions/no-debug-non-zts-20210902/mysqli.so /usr/local/lib/php/extensions/no-debug-non-zts-20210902/opcache.so /usr/local/lib/php/extensions/no-debug-non-zts-20210902/sodium.so
apt-mark manual libc6 libgcc-s1 libicu72 liblua5.1-0 libonig5 libsodium23 libstdc++6
libc6 was already set to manually installed.
libgcc-s1 was already set to manually installed.
libicu72 was already set to manually installed.
libonig5 was already set to manually installed.
libsodium23 was already set to manually installed.
libstdc++6 was already set to manually installed.
liblua5.1-0 set to manually installed.

(and php -v no longer throws a warning)

@christian-heusel
Copy link
Contributor Author

@tianon thanks for investigating! 😊 I will have a look and incorporate your patch into our repository 😅

christian-heusel added a commit to wikimedia/mediawiki-docker that referenced this pull request Oct 10, 2024
This reverts commit 434eb6f.
As it turns out the solutions is more complicated than wha I've
implemented in this patch, see [0] for details.

[0]: docker-library/official-images#17709 (comment)
christian-heusel added a commit to wikimedia/mediawiki-docker that referenced this pull request Oct 10, 2024
These ensure that no needed shared object dependencies are removed in
the cleanup steps.

Link: docker-library/official-images#17709 (comment)
Fixes: https://phabricator.wikimedia.org/T376447
Co-authored-by: Tianon Gravi <[email protected]>
Signed-off-by: Christian Heusel <[email protected]>
@christian-heusel christian-heusel changed the title mediawiki: fix missing liblua5.1.so.0 mediawiki: fix ldd parsing logic Oct 10, 2024
Copy link

Diff for cd0e75c:
diff --git a/_bashbrew-cat b/_bashbrew-cat
index b03d017..6614c8b 100644
--- a/_bashbrew-cat
+++ b/_bashbrew-cat
@@ -2,7 +2,7 @@ Maintainers: Kunal Mehta <[email protected]> (@legoktm), addshore <addshorewiki
 Architectures: amd64, arm32v5, arm32v7, arm64v8, i386, ppc64le
 GitRepo: https://github.com/wikimedia/mediawiki-docker.git
 GitFetch: refs/heads/main
-GitCommit: 0b41a035fabebeda3d6b4ccc39e2c5fc3ce1cc19
+GitCommit: 4708aabf22334d933a45f714e28a78722b36427a
 
 Tags: 1.39.10, 1.39, lts
 Directory: 1.39/apache
diff --git a/mediawiki_latest/Dockerfile b/mediawiki_latest/Dockerfile
index 9b06a0a..8ec9e20 100644
--- a/mediawiki_latest/Dockerfile
+++ b/mediawiki_latest/Dockerfile
@@ -45,9 +45,9 @@ RUN set -eux; \
 	apt-mark auto '.*' > /dev/null; \
 	apt-mark manual $savedAptMark; \
 	ldd "$(php -r 'echo ini_get("extension_dir");')"/*.so \
-		| awk '/=>/ { print $3 }' \
+		| awk '/=>/ { so = $(NF-1); if (index(so, "/usr/local/") == 1) { next }; gsub("^/(usr/)?", "", so); printf "*%s\n", so }' \
 		| sort -u \
-		| xargs -r dpkg-query -S \
+		| xargs -r dpkg-query --search \
 		| cut -d: -f1 \
 		| sort -u \
 		| xargs -rt apt-mark manual; \
diff --git a/mediawiki_legacy-fpm/Dockerfile b/mediawiki_legacy-fpm/Dockerfile
index 25f9161..92b2184 100644
--- a/mediawiki_legacy-fpm/Dockerfile
+++ b/mediawiki_legacy-fpm/Dockerfile
@@ -45,9 +45,9 @@ RUN set -eux; \
 	apt-mark auto '.*' > /dev/null; \
 	apt-mark manual $savedAptMark; \
 	ldd "$(php -r 'echo ini_get("extension_dir");')"/*.so \
-		| awk '/=>/ { print $3 }' \
+		| awk '/=>/ { so = $(NF-1); if (index(so, "/usr/local/") == 1) { next }; gsub("^/(usr/)?", "", so); printf "*%s\n", so }' \
 		| sort -u \
-		| xargs -r dpkg-query -S \
+		| xargs -r dpkg-query --search \
 		| cut -d: -f1 \
 		| sort -u \
 		| xargs -rt apt-mark manual; \
diff --git a/mediawiki_legacy/Dockerfile b/mediawiki_legacy/Dockerfile
index d5938a3..ce8452e 100644
--- a/mediawiki_legacy/Dockerfile
+++ b/mediawiki_legacy/Dockerfile
@@ -45,9 +45,9 @@ RUN set -eux; \
 	apt-mark auto '.*' > /dev/null; \
 	apt-mark manual $savedAptMark; \
 	ldd "$(php -r 'echo ini_get("extension_dir");')"/*.so \
-		| awk '/=>/ { print $3 }' \
+		| awk '/=>/ { so = $(NF-1); if (index(so, "/usr/local/") == 1) { next }; gsub("^/(usr/)?", "", so); printf "*%s\n", so }' \
 		| sort -u \
-		| xargs -r dpkg-query -S \
+		| xargs -r dpkg-query --search \
 		| cut -d: -f1 \
 		| sort -u \
 		| xargs -rt apt-mark manual; \
diff --git a/mediawiki_lts-fpm/Dockerfile b/mediawiki_lts-fpm/Dockerfile
index dda51c0..93d071f 100644
--- a/mediawiki_lts-fpm/Dockerfile
+++ b/mediawiki_lts-fpm/Dockerfile
@@ -45,9 +45,9 @@ RUN set -eux; \
 	apt-mark auto '.*' > /dev/null; \
 	apt-mark manual $savedAptMark; \
 	ldd "$(php -r 'echo ini_get("extension_dir");')"/*.so \
-		| awk '/=>/ { print $3 }' \
+		| awk '/=>/ { so = $(NF-1); if (index(so, "/usr/local/") == 1) { next }; gsub("^/(usr/)?", "", so); printf "*%s\n", so }' \
 		| sort -u \
-		| xargs -r dpkg-query -S \
+		| xargs -r dpkg-query --search \
 		| cut -d: -f1 \
 		| sort -u \
 		| xargs -rt apt-mark manual; \
diff --git a/mediawiki_lts/Dockerfile b/mediawiki_lts/Dockerfile
index 4927808..998ba5f 100644
--- a/mediawiki_lts/Dockerfile
+++ b/mediawiki_lts/Dockerfile
@@ -45,9 +45,9 @@ RUN set -eux; \
 	apt-mark auto '.*' > /dev/null; \
 	apt-mark manual $savedAptMark; \
 	ldd "$(php -r 'echo ini_get("extension_dir");')"/*.so \
-		| awk '/=>/ { print $3 }' \
+		| awk '/=>/ { so = $(NF-1); if (index(so, "/usr/local/") == 1) { next }; gsub("^/(usr/)?", "", so); printf "*%s\n", so }' \
 		| sort -u \
-		| xargs -r dpkg-query -S \
+		| xargs -r dpkg-query --search \
 		| cut -d: -f1 \
 		| sort -u \
 		| xargs -rt apt-mark manual; \
diff --git a/mediawiki_stable-fpm/Dockerfile b/mediawiki_stable-fpm/Dockerfile
index 6662ce3..e295656 100644
--- a/mediawiki_stable-fpm/Dockerfile
+++ b/mediawiki_stable-fpm/Dockerfile
@@ -45,9 +45,9 @@ RUN set -eux; \
 	apt-mark auto '.*' > /dev/null; \
 	apt-mark manual $savedAptMark; \
 	ldd "$(php -r 'echo ini_get("extension_dir");')"/*.so \
-		| awk '/=>/ { print $3 }' \
+		| awk '/=>/ { so = $(NF-1); if (index(so, "/usr/local/") == 1) { next }; gsub("^/(usr/)?", "", so); printf "*%s\n", so }' \
 		| sort -u \
-		| xargs -r dpkg-query -S \
+		| xargs -r dpkg-query --search \
 		| cut -d: -f1 \
 		| sort -u \
 		| xargs -rt apt-mark manual; \

Relevant Maintainers:

@christian-heusel
Copy link
Contributor Author

christian-heusel commented Oct 10, 2024

@tianon could you re-review? 😊 I have reverted the wrong fix I initially thought was right and implemented your suggestions 😃

@tianon tianon merged commit 00e7678 into docker-library:master Oct 10, 2024
14 checks passed
@tianon
Copy link
Member

tianon commented Oct 10, 2024

Glad we got it fixed, and hopefully in a way that makes sure you don't see other cases of it in the future (if more libraries need to be added). 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants