Hi,
On 2024-04-09 07:56, Helmut Grohne wrote:
> Hi Aurelien,
>
> On Mon, Apr 08, 2024 at 11:24:40PM +0200, Aurelien Jarno wrote:
> > Thanks for you analysis and your patch. In short your proposal is to
> > extend the initial patch from Steve to fully hide the fact that the
> > compiler default to -D_TIME_BITS=64 -D_FILE_OFFSET_BITS=64.
> >
> > This indeed works and fixes the FTBFS. However it seems that, at least
> > for some of the issues, it just hides them. For instance the wrong type
> > for timeval.tv_usec, reported by Simon upstream [1], was detected by the
> > conformance tests. Quoting utmpx.h/conform.out:
>
> I think this needs a more nuanced look. From the comments in the
> conformance test suite, it is evident that it expects to be run without
> _FILE_OFFSET_BITS and _TIME_BITS. Many of the symbols it requires to
> exist only exist when these macros are unset. The conformance test suite
> has a comment saying that it should be testing the case where
> _FILE_OFFSET_BITS is defined, but it currently does not provide
> expectations for that case.
>
> Before we can reasonably run the conformance test suite with these
> macros set (and really, the test suite should be in control of these
> macros), we cannot reasonably use it with them set. Let us now imagine a
> future where the conformance test suite has been extended to provide
> expectations (which in lots of cases means that *64 symbols disappear
> when -D_FILE_OFFSET_BITS=64). Then what still remains is Simon's issue:
>
> > | /tmp/tmp98wzaavx/test.c:4:35: error: conflicting types for ‘b2_10’; have ‘__suseconds64_t’ {aka ‘long long int’}
> > | 4 | extern __typeof__ (a2_10.tv_usec) b2_10;
> > | | ^~~~~
> > | /tmp/tmp98wzaavx/test.c:3:20: note: previous declaration of ‘b2_10’ with type ‘suseconds_t’ {aka ‘long int’}
> > | 3 | extern suseconds_t b2_10;
> > | | ^~~~~
> > | FAIL: Type of member tv_usec
>
> Indeed, this is not something that can easily be fixed and where
> upstream is still debating on what the correct solution should be. It
> also is an issue that existed for a long time. If you head over to a
> bookworm glibc and enable -D_TIME_BITS=64 there, you'll also notice that
> tv_usec and suseconds_t have different sizes. So yes, this is a bug, but
> it is not one that is directly related to Debian changing the default.
> We merely increased the visibility of this problem that existed earlier
> already.
I agree that this is an existing issue. My point there was mostly that
your solution is just hiding a real issue that is found by the
testsuite, and basically the testsuite runs in a different configuration
than the one used on the system. We might just miss new issues for new
upstream versions, which is not something very comfortable for a base
library.
> Given that
> * the issue is already present in bookworm,
> * there are two mutually exclusive solutions and
> * upstream is still discussing the best solution
> I recommend deferring this aspect.
While the issue is already present in bookworm, it is not visible
because the toolchain has different defaults.
> > And we know it is the reason for the FTBFS of libflorist [2], so should
> > we just ignore that issue anyway?
>
> The libflorist issue likely is a consequence. It arises from
> gnat_to_gnu_field where GNAT verifies that the value (of type
> suseconds_t) to be assigned to a field (tv_usec) has the matching size.
> This is directly based on the POSIX expectation and will be fixed with
> the glibc issue.
>
> How about filing a separate bug with glibc that tracks this POSIX
> divergence and mark the libflorist bug as being blocked by this other
> glibc bug? It can be RC or not, but since it affects bookworm, it won't
> block testing migration.
Yes we can do that. That said I am not sure we can claim the issue
affects bookworm, as again the toolchain does not have the same defaults
and therefore libflorist does not FTBFS in bookworm.
Regards
Aurelien
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://aurel32.net
Attachment:
signature.asc
Description: PGP signature