There is analysis pending, unfortunately 90% of the -dev packages have been analyzed leaving 90% to go (~1600 -dev packages that require fixes to be
Ack, thanks. Is that last % supposed to be 10%? Otherwise I think I'm
missing something :).
From 5a861d19b1610ae82bf95e6c5142a3365436fbd2 Mon Sep 17 00:00:00 2001 From: Steve Langasek <steve.langasek@ubuntu.com>
Date: Fri, 2 Jun 2023 14:30:20 +0000
Subject: [PATCH 1/3] lfs and time64 are no longer "future", call them
"feature" instead
Ah, I actually had implemented locally the alias with your original suggestion of "abi"! :) Using "feature" here seems would be rather
more confusing as these are called feature flags, and feature areas.
I'll try to push the stuff I've got queued locally during the weekend, then I can rebase these patches on a branch or similar.
As far as I can tell, this hasn't been pushed anywhere yet?
Right, sorry got entangled in a local branch with random stuff, but
rebased that and added on top now several other fixes including these
changes or ones similar in intention. See at:
https://git.hadrons.org/git/debian/dpkg/dpkg.git/log/?h=next/time64-default
(Beware that I might rebase that branch, before merging it, although I
think I'll start pushing some of the foundation work into main already.)
Did you reach a decision here? Anything you'd like from me to move this forward?
I realized now that this cannot be set for CXXFLAGS as at least g++
will warn about that. And I've gone for now by depending on qa=+bug,
but I'm not sure whether that would cause too many regressions. OTOH
and AFAIK any such problem should be considered a genuine bug anyway,
so…
But if this is too much I guess I could split that warning into its
own feature and make both the qa=+bug and abi=+time64 depend on it
instead.
On Sun, Jul 09, 2023 at 08:06:56PM +0200, Guillem Jover wrote:
I realized now that this cannot be set for CXXFLAGS as at least g++
will warn about that. And I've gone for now by depending on qa=+bug,
but I'm not sure whether that would cause too many regressions. OTOH
and AFAIK any such problem should be considered a genuine bug anyway,
so…
But if this is too much I guess I could split that warning into its
own feature and make both the qa=+bug and abi=+time64 depend on it
instead.
Now that I've had a chance to look at the implementation, I am concerned about abi=+time64 implying qa=+bug, because I see that this turns on additional -Werror options beyond implicit-function-declaration:
my @cfamilyflags = qw(
array-bounds
clobbered
volatile-register-var
);
foreach my $warnflag (@cfamilyflags) {
$flags->append('CFLAGS', "-Werror=$warnflag");
$flags->append('CXXFLAGS', "-Werror=$warnflag");
}
While these may all be bugs, forcing the fixing of bugs unrelated to the time_t transition during the transition will inevitably slow down Debian development, so I am concerned about having such a dependency. We unfortunately also haven't done any archive rebuild analysis on this to understand how large the actual impact is (again, the focus has been on just getting the analysis right of which packages do have an ABI change).
And while there's no specific timeline required on the Debian side yet, on the Ubuntu side we have a tight timeline to get this all done in the next couple of months so that it can be included in the 24.04 LTS release.
The idea of splitting it into a separate feature seems ok, to avoid turning on unrelated -Werror options.
We still don't have a slot from the Release Team for when this can be
landed, but following up to debian-devel with a complete analysis of the library ABIs is my next step before the end of year. Is this a change you think could be uploaded to dpkg on short notice? Or would you be amenable
to an NMU, if you're unavailable for an upload?
Package: dpkg-dev
Version: 1.21.22
Tags: patch
User: ubuntu-devel@lists.ubuntu.com
Usertags: origin-ubuntu mantic patch
The discussion on debian-devel around 64-bit time_t has died down, so I figure it's time to propose some patches to implement what's been discussed there.
I'm not sure whether you were persuaded that i386 should stay with the current ABI, but anyway thought I would propose the patches and we could discuss further if necessary.
From 5a861d19b1610ae82bf95e6c5142a3365436fbd2 Mon Sep 17 00:00:00 2001
From: Steve Langasek <steve.langasek@ubuntu.com>
Date: Fri, 2 Jun 2023 14:30:20 +0000
Subject: [PATCH 1/3] lfs and time64 are no longer "future", call them
"feature" instead
From 7eff8f89b32b6921a0d86c50c6c62154c6ddc96e Mon Sep 17 00:00:00 2001
From: Steve Langasek <steve.langasek@ubuntu.com>
Date: Fri, 2 Jun 2023 16:30:19 +0000
Subject: [PATCH 3/3] Also emit -Werror=implicit-function-declaration for
feature=+time64
Per https://lists.debian.org/debian-devel/2023/05/msg00262.html et al., missing glibc includes can cause packages to link to the wrong symbols, potentially causing crashes or misbehavior. Since functions that use
time_t are fairly ubiquitous, there's a high risk of this happening for *some* package in Debian. Better to make all software with missing
function declarations fail to build now, than to spend all cycle tracking down runtime bugs.
---
scripts/Dpkg/Vendor/Debian.pm | 2 ++
1 file changed, 2 insertions(+)
diff --git a/scripts/Dpkg/Vendor/Debian.pm b/scripts/Dpkg/Vendor/Debian.pm index 20d77fea1..803949024 100644
--- a/scripts/Dpkg/Vendor/Debian.pm
+++ b/scripts/Dpkg/Vendor/Debian.pm
@@ -400,6 +400,8 @@ sub _add_build_flags {
if ($flags->use_feature('feature', 'time64')) {
$flags->append('CPPFLAGS', '-D_TIME_BITS=64');
+ $flags->append('CFLAGS', '-Werror=implicit-function-declaration');
+ $flags->append('CXXFLAGS', '-Werror=implicit-function-declaration');
}
On Mon, 2023-06-05 at 21:18:10 -0700, Steve Langasek wrote:
Package: dpkg-dev
Version: 1.21.22
Tags: patch
User: ubuntu-devel@lists.ubuntu.com
Usertags: origin-ubuntu mantic patch
The discussion on debian-devel around 64-bit time_t has died down, so I figure it's time to propose some patches to implement what's been discussed there.
Yeah, I'm not sure whether it has died off or it's just being slow.
I'm not sure whether you were persuaded that i386 should stay with the current ABI, but anyway thought I would propose the patches and we could discuss further if necessary.
In any case I've tried to reply there. Also my impression was that
there was still some analysis pending (perhaps that was a wrong
impression though)?
Thanks for the patches!
From 5a861d19b1610ae82bf95e6c5142a3365436fbd2 Mon Sep 17 00:00:00 2001 From: Steve Langasek <steve.langasek@ubuntu.com>
Date: Fri, 2 Jun 2023 14:30:20 +0000
Subject: [PATCH 1/3] lfs and time64 are no longer "future", call them
"feature" instead
Ah, I actually had implemented locally the alias with your original suggestion of "abi"! :) Using "feature" here seems would be rather
more confusing as these are called feature flags, and feature areas.
I'll try to push the stuff I've got queued locally during the weekend,
then I can rebase these patches on a branch or similar.
From 7eff8f89b32b6921a0d86c50c6c62154c6ddc96e Mon Sep 17 00:00:00 2001 From: Steve Langasek <steve.langasek@ubuntu.com>
Date: Fri, 2 Jun 2023 16:30:19 +0000
Subject: [PATCH 3/3] Also emit -Werror=implicit-function-declaration for
feature=+time64
Per https://lists.debian.org/debian-devel/2023/05/msg00262.html et al., missing glibc includes can cause packages to link to the wrong symbols, potentially causing crashes or misbehavior. Since functions that use time_t are fairly ubiquitous, there's a high risk of this happening for *some* package in Debian. Better to make all software with missing function declarations fail to build now, than to spend all cycle tracking down runtime bugs.
---
scripts/Dpkg/Vendor/Debian.pm | 2 ++
1 file changed, 2 insertions(+)
diff --git a/scripts/Dpkg/Vendor/Debian.pm b/scripts/Dpkg/Vendor/Debian.pm index 20d77fea1..803949024 100644
--- a/scripts/Dpkg/Vendor/Debian.pm
+++ b/scripts/Dpkg/Vendor/Debian.pm
@@ -400,6 +400,8 @@ sub _add_build_flags {
if ($flags->use_feature('feature', 'time64')) {
$flags->append('CPPFLAGS', '-D_TIME_BITS=64');
+ $flags->append('CFLAGS', '-Werror=implicit-function-declaration'); + $flags->append('CXXFLAGS', '-Werror=implicit-function-declaration');
}
I'm not sure I like intermingling the different semantics here, if
necessary I'd rather make time64 forcibly enable another feature flag,
like it is done for lfs.
As I mentioned on the recent thread about the modern C stuff, I do
have a branch that adds these as part of a new qa=+c99 feature, but
that's too broad. :/
<https://git.hadrons.org/git/debian/dpkg/dpkg.git/commit/?h=next/modern-c&id=3316845bf415436299d61501db655fd2c1813436>
Maybe I could add a new feature area instead and add the flags
individually, and then make time64 also enable that other specific
feature. Hmmm.
On Fri, Jun 09, 2023 at 03:56:28AM +0200, Guillem Jover wrote:
On Mon, 2023-06-05 at 21:18:10 -0700, Steve Langasek wrote:
Package: dpkg-dev
Version: 1.21.22
Tags: patch
User: ubuntu-devel@lists.ubuntu.com
Usertags: origin-ubuntu mantic patch
The discussion on debian-devel around 64-bit time_t has died down, so I figure it's time to propose some patches to implement what's been discussed
there.
Yeah, I'm not sure whether it has died off or it's just being slow.
I'm not sure whether you were persuaded that i386 should stay with the current ABI, but anyway thought I would propose the patches and we could discuss further if necessary.
In any case I've tried to reply there. Also my impression was that
there was still some analysis pending (perhaps that was a wrong
impression though)?
There is analysis pending, unfortunately 90% of the -dev packages have been analyzed leaving 90% to go (~1600 -dev packages that require fixes to be
compilable and therefore analyzable). I don't have any answer yet from the Release Team, so I'm not sure if we need this analysis to be completely done before starting the transition or if we can leave the long tail of packages with 1 or 2 reverse-build-dependencies to be figured out later.
From 5a861d19b1610ae82bf95e6c5142a3365436fbd2 Mon Sep 17 00:00:00 2001 From: Steve Langasek <steve.langasek@ubuntu.com>
Date: Fri, 2 Jun 2023 14:30:20 +0000
Subject: [PATCH 1/3] lfs and time64 are no longer "future", call them
"feature" instead
Ah, I actually had implemented locally the alias with your original suggestion of "abi"! :) Using "feature" here seems would be rather
more confusing as these are called feature flags, and feature areas.
I'll try to push the stuff I've got queued locally during the weekend,
then I can rebase these patches on a branch or similar.
As far as I can tell, this hasn't been pushed anywhere yet?
From 7eff8f89b32b6921a0d86c50c6c62154c6ddc96e Mon Sep 17 00:00:00 2001 From: Steve Langasek <steve.langasek@ubuntu.com>
Date: Fri, 2 Jun 2023 16:30:19 +0000
Subject: [PATCH 3/3] Also emit -Werror=implicit-function-declaration for
feature=+time64
Per https://lists.debian.org/debian-devel/2023/05/msg00262.html et al., missing glibc includes can cause packages to link to the wrong symbols, potentially causing crashes or misbehavior. Since functions that use time_t are fairly ubiquitous, there's a high risk of this happening for *some* package in Debian. Better to make all software with missing function declarations fail to build now, than to spend all cycle tracking down runtime bugs.
---
scripts/Dpkg/Vendor/Debian.pm | 2 ++
1 file changed, 2 insertions(+)
diff --git a/scripts/Dpkg/Vendor/Debian.pm b/scripts/Dpkg/Vendor/Debian.pm
index 20d77fea1..803949024 100644
--- a/scripts/Dpkg/Vendor/Debian.pm
+++ b/scripts/Dpkg/Vendor/Debian.pm
@@ -400,6 +400,8 @@ sub _add_build_flags {
if ($flags->use_feature('feature', 'time64')) {
$flags->append('CPPFLAGS', '-D_TIME_BITS=64');
+ $flags->append('CFLAGS', '-Werror=implicit-function-declaration');
+ $flags->append('CXXFLAGS', '-Werror=implicit-function-declaration');
}
I'm not sure I like intermingling the different semantics here, if necessary I'd rather make time64 forcibly enable another feature flag,
like it is done for lfs.
As I mentioned on the recent thread about the modern C stuff, I do
have a branch that adds these as part of a new qa=+c99 feature, but
that's too broad. :/
<https://git.hadrons.org/git/debian/dpkg/dpkg.git/commit/?h=next/modern-c&id=3316845bf415436299d61501db655fd2c1813436>
Maybe I could add a new feature area instead and add the flags individually, and then make time64 also enable that other specific
feature. Hmmm.
Did you reach a decision here? Anything you'd like from me to move this forward?
| Sysop: | Keyop |
|---|---|
| Location: | Huddersfield, West Yorkshire, UK |
| Users: | 546 |
| Nodes: | 16 (0 / 16) |
| Uptime: | 164:02:22 |
| Calls: | 10,385 |
| Calls today: | 2 |
| Files: | 14,057 |
| Messages: | 6,416,517 |