Bug#1065371: unable to disable bug-implicit-func for time64
Hi! On Sun, 2024-03-03 at 16:46:33 +0100, Guillem Jover wrote: > On Sun, 2024-03-03 at 16:11:36 +0100, Matthias Klose wrote: > > - please provide an opt-out option. > > This is a bug, which I should fix. The first attached patch is what I'd use to fix this. > > - turn it on on all architectures, so that everbody > >can reproduce the effects. > > I'd be fine with that. The second attached patch is what I'd use to implement this, if there's agreement. (Barring manual page updates here.) I'll wait for Steve's input, before proceeding, otherwise I might just upload the first patch for now, either later today or tomorrow, so that people can opt-out of this until there's agreement on how to proceed. (Even though I guess people could already use DEB_CFLAGS_MAINT_STRIP to forcibly disable the -Werror=implicit-function-declaration flag.) Thanks, Guillem From f747a38746cbf0fa4279e773835b7d872c0d313c Mon Sep 17 00:00:00 2001 From: Guillem Jover Date: Sun, 3 Mar 2024 18:42:34 +0100 Subject: [PATCH] Dpkg::Vendor::Debian: Make it possible to disable qa=-bug-implicit-func We do not need to forcibly enable this feature if the user explicitly specified it. Closes: #1065371 --- scripts/Dpkg/Vendor/Debian.pm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/Dpkg/Vendor/Debian.pm b/scripts/Dpkg/Vendor/Debian.pm index fcf5b1e2a..ad727d2cf 100644 --- a/scripts/Dpkg/Vendor/Debian.pm +++ b/scripts/Dpkg/Vendor/Debian.pm @@ -299,8 +299,8 @@ sub set_build_features { $use_feature{abi}{lfs} = 1 if $libc eq 'gnu'; # Require -Werror=implicit-function-declaration, to avoid linking -# against the wrong symbol. -$use_feature{qa}{'bug-implicit-func'} = 1; +# against the wrong symbol, unless it has been set explicitly. +$use_feature{qa}{'bug-implicit-func'} //= 1; } # XXX: Handle lfs alias from future abi feature area. -- 2.43.0 From 87702728876e96891d02df2d1b0419f709939190 Mon Sep 17 00:00:00 2001 From: Guillem Jover Date: Sun, 3 Mar 2024 18:53:12 +0100 Subject: [PATCH] Dpkg::Vendor::Debian: Unconditionally set qa bug-implicit-func For the time64 default change, we conditionally enabled bug-implicit-func to avoid silent ABI breakage due to implicit function declarations that end up using the time32 symbols, but that is causing confusion as the effects are not visible on the most commonly used architectures. Instead enable this globally, unless the maintainer has specified otherwise. --- scripts/Dpkg/Vendor/Debian.pm | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/scripts/Dpkg/Vendor/Debian.pm b/scripts/Dpkg/Vendor/Debian.pm index ad727d2cf..b3be69e86 100644 --- a/scripts/Dpkg/Vendor/Debian.pm +++ b/scripts/Dpkg/Vendor/Debian.pm @@ -117,7 +117,7 @@ sub set_build_features { time64 => undef, }, qa => { -bug => 0, +bug => undef, 'bug-implicit-func' => undef, canary => 0, }, @@ -297,10 +297,6 @@ sub set_build_features { if ($use_feature{abi}{time64} && ! $builtin_feature{abi}{time64}) { # On glibc 64-bit time_t support requires LFS. $use_feature{abi}{lfs} = 1 if $libc eq 'gnu'; - -# Require -Werror=implicit-function-declaration, to avoid linking -# against the wrong symbol, unless it has been set explicitly. -$use_feature{qa}{'bug-implicit-func'} //= 1; } # XXX: Handle lfs alias from future abi feature area. @@ -311,7 +307,14 @@ sub set_build_features { ## Area: qa -$use_feature{qa}{'bug-implicit-func'} //= $use_feature{qa}{bug}; +# For time64 we require -Werror=implicit-function-declaration, to avoid +# linking against the wrong symbol. Instead of enabling this conditionally +# on time64 being enabled, do it unconditionally so that the effects are +# uniform and visible on all architectures. Unless it has been set +# explicitly. +$use_feature{qa}{'bug-implicit-func'} //= $use_feature{qa}{bug} // 1; + +$use_feature{qa}{bug} //= 0; ## Area: reproducible -- 2.43.0
Bug#1065371: unable to disable bug-implicit-func for time64
On Sun, 2024-03-03 at 16:57:28 +0100, Matthias Klose wrote: > On 03.03.24 16:46, Guillem Jover wrote: > > On Sun, 2024-03-03 at 16:11:36 +0100, Matthias Klose wrote: > > > I just filed another bug report for bc, together with the one for heimdal. > > > > > > Please turn this off for a while, it's really harmful for the time64 > > > bootstrap. > > > > This was added on request by Steve, to help with the time64 changes. > > > > > When you turn it on again, > > > > > > - please provide an opt-out option. > > > > This is a bug, which I should fix. > > > > > - turn it on on all architectures, so that everbody > > > can reproduce the effects. > > > > I'd be fine with that. > > > > > - before turning it on again, please do an archive wide > > > test rebuild and file bug reports for it. > > > > My impression is that this was done as part of the time64 checks? If > > not, and the consensus is to disable the flag, I'm very unlikely to > > drive this, and someone else will need to do those rebuilds and post > > results. > > I can do that, but we will need a stable dpkg version and a dpkg upload > providing that setting on amd64 without time64 set. Then I'll ask Lucas for > two test rebuilds (at this stage, that would be testing). > Doing test rebuilds with time64 enabled on testing doesn't make sense for > now, and unstable is too unstable. Hmm, I'm not sure I understand what you are asking here, so let me try to rephrase, you'd like to see: - a dpkg 1.22.6 upload for unstable to the Debian archive with the bug-implicit-func unconditionally set? - a dpkg 1.21.x version out-of-archive with the bug-implicit-func support backported and also enabled by default? For the former you should be able to do that already by setting the flag to enable via DEB_BUILD_OPTIONS with the version already in sid, if you don't want time64 then you can also disable that there. The latter I don't understand, so perhaps I interpreted that incorrectly from what you said. > > I think making the opt-out functional might be enough to help with > > this, and I could upload a fix later today, which would not disarm > > this safety net for the time64 transition. But at this point I don't > > mind either way, and if people prefer disabling the warning then I can > > do that instead. > > at least for heimdal, three people spent several hours looking for the cause > of the failure. I'm not sure we want these kind of delays for the > transition. While that's unfortunate, I think that might be better than silently letting ABI breakage through due to the missing Werror flag. Thanks, Guillem
Bug#1065371: unable to disable bug-implicit-func for time64
[CCing Lucas] On 03.03.24 16:46, Guillem Jover wrote: Hi! On Sun, 2024-03-03 at 16:11:36 +0100, Matthias Klose wrote: Control: severity -1 serious I just filed another bug report for bc, together with the one for heimdal. Please turn this off for a while, it's really harmful for the time64 bootstrap. This was added on request by Steve, to help with the time64 changes. When you turn it on again, - please provide an opt-out option. This is a bug, which I should fix. - turn it on on all architectures, so that everbody can reproduce the effects. I'd be fine with that. - before turning it on again, please do an archive wide test rebuild and file bug reports for it. My impression is that this was done as part of the time64 checks? If not, and the consensus is to disable the flag, I'm very unlikely to drive this, and someone else will need to do those rebuilds and post results. I can do that, but we will need a stable dpkg version and a dpkg upload providing that setting on amd64 without time64 set. Then I'll ask Lucas for two test rebuilds (at this stage, that would be testing). Doing test rebuilds with time64 enabled on testing doesn't make sense for now, and unstable is too unstable. I think making the opt-out functional might be enough to help with this, and I could upload a fix later today, which would not disarm this safety net for the time64 transition. But at this point I don't mind either way, and if people prefer disabling the warning then I can do that instead. at least for heimdal, three people spent several hours looking for the cause of the failure. I'm not sure we want these kind of delays for the transition. Matthias
Bug#1065371: unable to disable bug-implicit-func for time64
Hi! On Sun, 2024-03-03 at 16:11:36 +0100, Matthias Klose wrote: > Control: severity -1 serious > I just filed another bug report for bc, together with the one for heimdal. > > Please turn this off for a while, it's really harmful for the time64 > bootstrap. This was added on request by Steve, to help with the time64 changes. > When you turn it on again, > > - please provide an opt-out option. This is a bug, which I should fix. > - turn it on on all architectures, so that everbody >can reproduce the effects. I'd be fine with that. > - before turning it on again, please do an archive wide >test rebuild and file bug reports for it. My impression is that this was done as part of the time64 checks? If not, and the consensus is to disable the flag, I'm very unlikely to drive this, and someone else will need to do those rebuilds and post results. I think making the opt-out functional might be enough to help with this, and I could upload a fix later today, which would not disarm this safety net for the time64 transition. But at this point I don't mind either way, and if people prefer disabling the warning then I can do that instead. Thanks, Guillem
Bug#1065371: unable to disable bug-implicit-func for time64
Package: dpkg-dev Version: 1.22.5 Severity: important It's not possible to disable bug-implicit-func on architectures where time64 is enabled by default: DEB_BUILD_OPTIONS=qa=-bug-implicit-func dpkg-buildflags |grep ^CFLAGS CFLAGS=-g -O2 -Werror=implicit-function-declaration -ffile-prefix-map=/root/doko/heimdal-7.8.git20221117.28daf24+dfsg=. -fstack-protector-strong -Wformat -Werror=format-security -fno-stack-clash-protection -fdebug-prefix-map=/root/doko/heimdal-7.8.git20221117.28daf24+dfsg=/usr/src/heimdal-7.8.git20221117.28daf24+dfsg-4.1ubuntu1 This is seen when building heimdal for abi=time64, the configure check for checking for the crypt library fails, and later the build fails. There should be a way to override this.