Created attachment 701760 [details, diff] glibc-2.33-patches-fix-test-arg-quoting.patch 0018-x86-Set-minimum-x86-64-level-marker-BZ-27318.patch, included in glibc-2.33-patches-3.tar.xz, introduces two new feature tests in sysdeps/x86/configure{,.ac} but does not quote the arguments sufficiently: if test $libc_cv_have_x86_lahf_sahf = yes; then ... if test $libc_cv_have_x86_movbe = yes; then These tests fail with the following errors when the variables are unset: /var/tmp/portage/sys-libs/glibc-2.33/work/glibc-2.33/configure: 168: test: =: unexpected operator /var/tmp/portage/sys-libs/glibc-2.33/work/glibc-2.33/configure: 172: test: =: unexpected operator The attached patch corrects the argument quoting and eliminates the errors.
Can you send patch upstream? https://sourceware.org/glibc/wiki/Contribution%20checklist
(In reply to Sergei Trofimovich from comment #1) > Can you send patch upstream? Upstream sources don't have the problem because it is introduced in one of the patches that Gentoo is applying.
(In reply to Matt Whitlock from comment #2) > (In reply to Sergei Trofimovich from comment #1) > > Can you send patch upstream? > > Upstream sources don't have the problem because it is introduced in one of > the patches that Gentoo is applying. I don't think gentoo applies anything special. All our relevant patches are upstream ones. glibc ebuild does set libc_cv_include_x86_isa_level=no environment variable that might get ./configure into a rarely exercised area.
Reported upstream at: https://sourceware.org/bugzilla/show_bug.cgi?id=27318
Actually, it looks like the problem is in the Gentoo ebuild. Upstream is okay, as the relevant variables do get initialized to "no" but only if libc_cv_include_x86_isa_level is not already set by the time that test is reached. The Gentoo ebuild forces that configure variable on the command line, and this is what causes the failure since then the other two variables are never initialized by the configure script. Apparently, even though Gentoo is including the fix for upstream BZ-27318, the workaround for that bug is still present in ${myconf} in the ebuild. Removing the workaround from the ebuild eliminates the errors.
I would say it's still an upstream bug. libc_cv_* are cache values. They should have a reasonable default if not set and should be overridable with external environment by an arbitrary subset of variables. We can add more assignments as a workaround (if they really change comparison semantics), but the ./configure should handle it itself.
(In reply to Sergei Trofimovich from comment #6) > libc_cv_* are cache values. They should have a reasonable default if not set While I wholeheartedly agree, Glibc is chock full of bad Autoconf cache practices, to the point that I have to override my local environment to disable Autoconf caching specifically in Glibc builds because the builds break when caching is used. So any attempt to convince the Glibc devs to improve their Autoconf cache behavior will likely fall flat, as it would be a significant undertaking.
I ran into this again when upgrading to sys-libs/glibc-2.33-r1. :( Can someone please remove the workaround in the ebuild that is no longer needed and is causing this failure? The workaround is not needed since 0018-x86-Set-minimum-x86-64-level-marker-BZ-27318.patch is included in the patchset. --- sys-libs/glibc/glibc-2.33-r1.ebuild~ 2021-06-13 14:09:44.000000000 +0000 +++ sys-libs/glibc/glibc-2.33-r1.ebuild 2021-06-16 19:12:27.947891595 +0000 @@ -937,10 +937,6 @@ # https://bugs.gentoo.org/753740 libc_cv_complocaledir='${exec_prefix}/lib/locale' - # -march= option tricks build system to infer too - # high ISA level: https://sourceware.org/PR27318 - libc_cv_include_x86_isa_level=no - ${EXTRA_ECONF} )
(In reply to Matt Whitlock from comment #8) > I ran into this again when upgrading to sys-libs/glibc-2.33-r1. :( What is the actual failure? Just warnings at configure time or a broken build? I assume broken build and would like to understand failure mode a bit more. glibc-2.33-r1 certainly works here. Please attach the: 1. emerge --info 2. build.log
(In reply to Sergei Trofimovich from comment #9) > What is the actual failure? Just warnings at configure time or a broken > build? I don't know if the build would complete if allowed to run past the configure stage. I'd guess it would. To be clear, these are not "warnings at configure time"; they are *errors* at configure time, but because of the way Autoconf is designed, they do not (normally) cause configure to return a non-zero exit status to stop the build. I have extra plumbing in place to abort builds whose configure scripts emit errors because I have /bin/sh->dash on my system, and very many Autoconf scripts use Bashisms that I try to catch and report so they can be fixed. Of course I can turn off my forced aborting upon configure errors for sys-libs/glibc, but I'd prefer to leave it in place so I can catch any new Bashisms that may creep into its configure script in the future. I ordinarily would work around this locally with a patch in /etc/portage/patches, but AFAIK Portage does not allow ebuilds themselves to be patches via that mechanism, and I don't know of a way to avoid this failure mode without removing the vestigial workaround from the ebuild. Thus why I'm posting here.
(In reply to Matt Whitlock from comment #10) > (In reply to Sergei Trofimovich from comment #9) > > What is the actual failure? Just warnings at configure time or a broken > > build? > > I don't know if the build would complete if allowed to run past the > configure stage. I'd guess it would. > > To be clear, these are not "warnings at configure time"; they are *errors* > at configure time, but because of the way Autoconf is designed, they do not > (normally) cause configure to return a non-zero exit status to stop the > build. > > I have extra plumbing in place to abort builds whose configure scripts emit > errors because I have /bin/sh->dash on my system, and very many Autoconf > scripts use Bashisms that I try to catch and report so they can be fixed. Aha, does not look like a real build failure then. My build logs contain those as: """ checking for linker x86 ISA level support... (cached) no /tmp/portage-tmpdir/portage/sys-libs/glibc-2.33-r1/work/glibc-2.33/sysdeps/x86/configure: line 168: test: =: unary operator expected /tmp/portage-tmpdir/portage/sys-libs/glibc-2.33-r1/work/glibc-2.33/sysdeps/x86/configure: line 172: test: =: unary operator expected """ > I ordinarily would work around this locally with a patch in > /etc/portage/patches, but AFAIK Portage does not allow ebuilds themselves to > be patches via that mechanism, and I don't know of a way to avoid this > failure mode without removing the vestigial workaround from the ebuild. Thus > why I'm posting here. As a wourkaround you can try something like (untested) EXTRA_ECONF=libc_cv_include_x86_isa_level= in package.env for glibc.
(In reply to Sergei Trofimovich from comment #11) > As a wourkaround you can try something like (untested) > EXTRA_ECONF=libc_cv_include_x86_isa_level= > in package.env for glibc. That doesn't work because the configure script tests whether libc_cv_include_x86_isa_level is set at all, not whether it's set to a non-null value. However, I can set the other two variables with EXTRA_ECONF='libc_cv_have_x86_lahf_sahf=no libc_cv_have_x86_movbe=no' and that does eliminate the errors! :) So that's a decent enough workaround. Thanks for the idea. I think I'll still try to convince upstream to fix their script so that caching works correctly and doesn't end up with unset *_cv_* variables in some cases.
Created attachment 716487 [details, diff] x86-fix-Autoconf-caching-when-checking-for-instruction-support.patch Patch submitted upstream. This is a real fix for the incorrect use of the AC_CACHE_CHECK macro.
(In reply to Matt Whitlock from comment #12) > (In reply to Sergei Trofimovich from comment #11) > > As a wourkaround you can try something like (untested) > > EXTRA_ECONF=libc_cv_include_x86_isa_level= > > in package.env for glibc. > > That doesn't work because the configure script tests whether > libc_cv_include_x86_isa_level is set at all, not whether it's set to a > non-null value. > > However, I can set the other two variables with > EXTRA_ECONF='libc_cv_have_x86_lahf_sahf=no libc_cv_have_x86_movbe=no' > and that does eliminate the errors! :) > > So that's a decent enough workaround. Thanks for the idea. Ah, right. Yeah, setting other dependent values is more predictable. We should do it in the ebuild as well. > I think I'll still try to convince upstream to fix their script so that > caching works correctly and doesn't end up with unset *_cv_* variables in > some cases. Thank you for the follow up!
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=0efc5ad7d63239a278a8d07c90a19787da57786e commit 0efc5ad7d63239a278a8d07c90a19787da57786e Author: Sergei Trofimovich <slyfox@gentoo.org> AuthorDate: 2021-06-17 08:15:03 +0000 Commit: Sergei Trofimovich <slyfox@gentoo.org> CommitDate: 2021-06-17 08:15:43 +0000 sys-libs/glibc: override x86_lahf_sahf=no x86_movbe=no values Reported-by: Matt Whitlock Bug: https://bugs.gentoo.org/785091 Package-Manager: Portage-3.0.20, Repoman-3.0.3 Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org> sys-libs/glibc/glibc-2.33-r1.ebuild | 5 +++++ sys-libs/glibc/glibc-9999.ebuild | 5 +++++ 2 files changed, 10 insertions(+)
OK let's consider this fixed then (it's an interaction of Gentoo and upstream, now fixed on the Gentoo side).
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=1f0fd3e2aee01e0c09e7103c8af4183b57faef49 commit 1f0fd3e2aee01e0c09e7103c8af4183b57faef49 Author: Sam James <sam@gentoo.org> AuthorDate: 2023-07-24 22:42:52 +0000 Commit: Sam James <sam@gentoo.org> CommitDate: 2023-07-24 22:43:14 +0000 sys-libs/glibc: drop stale workarounds As of >=2.35, all of these are fixed. Bug: https://bugs.gentoo.org/785091 Bug: https://sourceware.org/PR27318 Bug: https://sourceware.org/PR27991 Signed-off-by: Sam James <sam@gentoo.org> sys-libs/glibc/glibc-9999.ebuild | 10 ---------- 1 file changed, 10 deletions(-)
