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