diff mbox series

Take into account ${sysconfdir} in elf/tst-ldconfig-p.sh

Message ID 586cec0c-4497-1029-da8f-e55d1967bba7@gmail.com
State New
Headers show
Series Take into account ${sysconfdir} in elf/tst-ldconfig-p.sh | expand

Commit Message

Romain Geissler March 13, 2023, 9:46 p.m. UTC
Hi,

While upgrading to the latest commit of the release branch 2.37, with an exotic glibc installation (I override explicitly the --prefix destination, the test tst-ldconfig-p.sh failed. Here is a small patch to take into account ${sysconfdir} so we don't hardcode it to /etc.

PS: I hope the patch will be well formed, I have just setup Thunderbird to write this mail using gmail.com's smtp, unfortunately my employer forbids using anything else than Outlook to send mails, and almost no open source tools support the Exchange protocol...

Cheers,
Romain


commit ad491ee678026eea33e4654da4ce86d4835bd240
Author: Romain Geissler <romain.geissler@amadeus.com>
Date:   Mon Mar 13 20:04:48 2023 +0000

    Take into account ${sysconfdir} in elf/tst-ldconfig-p.sh.

Comments

Adhemerval Zanella Netto March 22, 2023, 11:51 a.m. UTC | #1
On 13/03/23 18:46, Romain Geissler via Libc-alpha wrote:
> Hi,
> 
> While upgrading to the latest commit of the release branch 2.37, with an exotic glibc installation (I override explicitly the --prefix destination, the test tst-ldconfig-p.sh failed. Here is a small patch to take into account ${sysconfdir} so we don't hardcode it to /etc.
> 
> PS: I hope the patch will be well formed, I have just setup Thunderbird to write this mail using gmail.com's smtp, unfortunately my employer forbids using anything else than Outlook to send mails, and almost no open source tools support the Exchange protocol...

Unfortunately this did not work, patchwork was not able to apply neither
I manually (thunderbird without extra configuration do not keep tabs if
I recall correctly).

The patch is not wrong, but building with a --prefix other than '/usr'
triggers other issues:

FAIL: elf/check-abi-libc
FAIL: elf/tst-glibc-hwcaps-prepend-cache
FAIL: elf/tst-ldconfig-ld_so_conf-update
FAIL: elf/tst-unwind-main

Which makes me wonder if we even support check without the canonical
prefix.  The elf/check-abi-libc would need to either be unsupported or
add another hackery to ignore the patch (which is the test essentially).

> 
> Cheers,
> Romain
> 
> 
> commit ad491ee678026eea33e4654da4ce86d4835bd240
> Author: Romain Geissler <romain.geissler@amadeus.com>
> Date:   Mon Mar 13 20:04:48 2023 +0000
> 
>     Take into account ${sysconfdir} in elf/tst-ldconfig-p.sh.
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index 4d0e04b2a20..262fbd1770c 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -2421,7 +2421,7 @@ $(objpfx)tst-ldconfig-X.out : tst-ldconfig-X.sh $(objpfx)ldconfig
>      $(evaluate-test)
>  
>  $(objpfx)tst-ldconfig-p.out : tst-ldconfig-p.sh $(objpfx)ldconfig
> -    $(SHELL) $< '$(common-objpfx)' '$(test-wrapper-env)' \
> +    $(SHELL) $< '$(common-objpfx)' '$(sysconfdir)' '$(test-wrapper-env)' \
>           '$(run-program-env)' > $@; \
>      $(evaluate-test)
>  
> diff --git a/elf/tst-ldconfig-p.sh b/elf/tst-ldconfig-p.sh
> index ec937bf4ec0..e966c4d1241 100644
> --- a/elf/tst-ldconfig-p.sh
> +++ b/elf/tst-ldconfig-p.sh
> @@ -23,11 +23,12 @@
>  # involves emulation when running ldconfig).
>  
>  common_objpfx=$1
> -test_wrapper_env=$2
> -run_program_env=$3
> +sysconfdir=$2
> +test_wrapper_env=$3
> +run_program_env=$4
>  
> -if ! test -r /etc/ld.so.cache; then
> -    echo "warning: /etc/ld.so.cache does not exist, test skipped"
> +if ! test -r "${sysconfdir}/ld.so.cache"; then
> +    echo "warning: ${sysconfdir}/ld.so.cache does not exist, test skipped"
>      exit 77
>  fi
>  
> @@ -46,7 +47,7 @@ errors=0
>  case $status in
>      (0)
>      if head -n 1 "$testout" | \
> -        grep -q "libs found in cache \`/etc/ld.so.cache'\$" ; then
> +        grep -q "libs found in cache \`${sysconfdir}/ld.so.cache'\$" ; then
>          echo "info: initial string found" >>"$testout"
>      else
>          echo "error: initial string not found" >>"$testout"
>
Romain Geissler March 25, 2023, 7:23 p.m. UTC | #2
> Le 22 mars 2023 à 12:51, Adhemerval Zanella Netto via Libc-alpha <libc-alpha@sourceware.org> a écrit :
> 
> On 13/03/23 18:46, Romain Geissler via Libc-alpha wrote:
> 
> Unfortunately this did not work, patchwork was not able to apply neither
> I manually (thunderbird without extra configuration do not keep tabs if
> I recall correctly).

Ah… ok, I will need to figure out how to set up git-send-mail on my private computer then. I will re-sent the right patch later.

> The patch is not wrong, but building with a --prefix other than '/usr'
> triggers other issues:
> 
> FAIL: elf/check-abi-libc
> FAIL: elf/tst-glibc-hwcaps-prepend-cache
> FAIL: elf/tst-ldconfig-ld_so_conf-update
> FAIL: elf/tst-unwind-main
> 
> Which makes me wonder if we even support check without the canonical
> prefix.  The elf/check-abi-libc would need to either be unsupported or
> add another hackery to ignore the patch (which is the test essentially).

On my side the first 3 fail, and I apply local patches for. For 2 of them, I actually posted a patch couple of years any in https://sourceware.org/pipermail/libc-alpha/2021-August/129984.html and there was a review but I never took the time to follow-up. I will re-post this one as well, updated according to the comment made back then.

For elf/check-abi-libc I apply since many years a patch to change the size just for me, so the test pass. Checking the history, there is already bug 14664 filled for this since years. HJ’s patch generated some discussions on the list back then, some saying this symbol should actually have never been part of the ABI, but hey too late, it is now, some saying it’s part of the GLIBC ABI so using any other prefix is going to break the ABI so it’s not ok, some saying that despite it’s part of the ABI, still it would be cool that testing glibc with a custom prefix should still not result as an error. And in the end it seems it stayed like this. Honestly I don’t know what shall be done here, that’s why I had always applied a patch locally on my glibcs and never tried to fix this one. But we can fix the others tests that have a proposed patch I guess ;)

Cheers,
Romiain
diff mbox series

Patch

diff --git a/elf/Makefile b/elf/Makefile
index 4d0e04b2a20..262fbd1770c 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -2421,7 +2421,7 @@  $(objpfx)tst-ldconfig-X.out : tst-ldconfig-X.sh $(objpfx)ldconfig
     $(evaluate-test)
 
 $(objpfx)tst-ldconfig-p.out : tst-ldconfig-p.sh $(objpfx)ldconfig
-    $(SHELL) $< '$(common-objpfx)' '$(test-wrapper-env)' \
+    $(SHELL) $< '$(common-objpfx)' '$(sysconfdir)' '$(test-wrapper-env)' \
          '$(run-program-env)' > $@; \
     $(evaluate-test)
 
diff --git a/elf/tst-ldconfig-p.sh b/elf/tst-ldconfig-p.sh
index ec937bf4ec0..e966c4d1241 100644
--- a/elf/tst-ldconfig-p.sh
+++ b/elf/tst-ldconfig-p.sh
@@ -23,11 +23,12 @@ 
 # involves emulation when running ldconfig).
 
 common_objpfx=$1
-test_wrapper_env=$2
-run_program_env=$3
+sysconfdir=$2
+test_wrapper_env=$3
+run_program_env=$4
 
-if ! test -r /etc/ld.so.cache; then
-    echo "warning: /etc/ld.so.cache does not exist, test skipped"
+if ! test -r "${sysconfdir}/ld.so.cache"; then
+    echo "warning: ${sysconfdir}/ld.so.cache does not exist, test skipped"
     exit 77
 fi
 
@@ -46,7 +47,7 @@  errors=0
 case $status in
     (0)
     if head -n 1 "$testout" | \
-        grep -q "libs found in cache \`/etc/ld.so.cache'\$" ; then
+        grep -q "libs found in cache \`${sysconfdir}/ld.so.cache'\$" ; then
         echo "info: initial string found" >>"$testout"
     else
         echo "error: initial string not found" >>"$testout"