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 aurel...@aurel32.net http://aurel32.net
signature.asc
Description: PGP signature
