Hi Thomas

On 2024-03-23 15:24:58 +0100, Thomas Orgis wrote:
> Hi Sebastian,
> 
> 
> Am Sat, 23 Mar 2024 10:40:43 +0100
> schrieb Sebastian Ramacher <sramac...@debian.org>: 
> 
> > +#MISSING: 1.32.5-1+b1# (arch-bits=32|arch=!x32)mpg123_decode_frame_32@Base 
> > 1.13.7
> > +#MISSING: 1.32.5-1+b1# (arch-bits=32|arch=!x32)mpg123_feedseek_32@Base 
> > 1.13.7
> > +#MISSING: 1.32.5-1+b1# 
> > (arch-bits=32|arch=!x32)mpg123_framebyframe_decode_32@Base 1.13.7
> > +#MISSING: 1.32.5-1+b1# (arch-bits=32|arch=!x32)mpg123_framelength_32@Base 
> > 1.23.8
> > +#MISSING: 1.32.5-1+b1# (arch-bits=32|arch=!x32)mpg123_framepos_32@Base 
> > 1.14.0
> > +#MISSING: 1.32.5-1+b1# (arch-bits=32|arch=!x32)mpg123_index_32@Base 1.13.7
> > +#MISSING: 1.32.5-1+b1# (arch-bits=32|arch=!x32)mpg123_length_32@Base 1.13.7
> > +#MISSING: 1.32.5-1+b1# (arch-bits=32|arch=!x32)mpg123_open_32@Base 1.13.7
> > +#MISSING: 1.32.5-1+b1# (arch-bits=32|arch=!x32)mpg123_open_fd_32@Base 
> > 1.13.7
> > +#MISSING: 1.32.5-1+b1# (arch-bits=32|arch=!x32)mpg123_open_fixed_32@Base 
> > 1.26.0
> > +#MISSING: 1.32.5-1+b1# (arch-bits=32|arch=!x32)mpg123_open_handle_32@Base 
> > 1.13.7
> > +#MISSING: 1.32.5-1+b1# (arch-bits=32|arch=!x32)mpg123_position_32@Base 
> > 1.13.7
> > +#MISSING: 1.32.5-1+b1# 
> > (arch-bits=32|arch=!x32)mpg123_replace_reader_32@Base 1.13.7
> > +#MISSING: 1.32.5-1+b1# 
> > (arch-bits=32|arch=!x32)mpg123_replace_reader_handle_32@Base 1.13.7
> > +#MISSING: 1.32.5-1+b1# (arch-bits=32|arch=!x32)mpg123_seek_32@Base 1.13.7
> > +#MISSING: 1.32.5-1+b1# (arch-bits=32|arch=!x32)mpg123_seek_frame_32@Base 
> > 1.13.7
> > +#MISSING: 1.32.5-1+b1# (arch-bits=32|arch=!x32)mpg123_set_filesize_32@Base 
> > 1.13.7
> > +#MISSING: 1.32.5-1+b1# (arch-bits=32|arch=!x32)mpg123_set_index_32@Base 
> > 1.13.7
> > +#MISSING: 1.32.5-1+b1# (arch-bits=32|arch=!x32)mpg123_tell_32@Base 1.13.7
> > +#MISSING: 1.32.5-1+b1# (arch-bits=32|arch=!x32)mpg123_tell_stream_32@Base 
> > 1.13.7
> > +#MISSING: 1.32.5-1+b1# (arch-bits=32|arch=!x32)mpg123_tellframe_32@Base 
> > 1.13.7
> > +#MISSING: 1.32.5-1+b1# (arch-bits=32|arch=!x32)mpg123_timeframe_32@Base 
> > 1.13.7
> > +#MISSING: 1.32.5-1+b1# 
> > (arch-bits=32|arch=!x32)syn123_resample_intotal_32@Base 1.26.2
> > +#MISSING: 1.32.5-1+b1# 
> > (arch-bits=32|arch=!x32)syn123_resample_total_32@Base 1.26.2
> > Do you know what's happenig here?
> 
> This looks just like the list of wrapper functions offered for
> applications that do not enable LFS (_FILE_OFFSET_BITS unset or 32).
> 
> Steve mentioned this:
> 
> > (you can't enable 64-bit time_t compatibility to use the other
> > libraries it calls, without also turning on LFS).
> 
> So I take it you build mpg123 with enforced LFS so that the build does
> not detect ambiguity in off_t and hence doesn't offer 32 bit wrappers.

On all 32 bit architectures except i386 we are now building with
-D_FILE_OFFSET_BITS=64 and -D_TIME_BITS=64. Both gcc and dpkg now set
both flags. So unless the build system of a package forcibly unsets
_FILE_OFFSET_BITS and _TIME_BITS, all Debian release architectures
(except i386) now build with with 64 bit time_t and off_t.

i386 stays unchanged for historic reasons.

> I recently took pains to make the actual library code independent of
> LFS choice, using explicit 64 bit API. But If you build with
> -D_FILE_OFFSET_BITS=64 enforced, mpg123's configure will _not_ set
> 
>   lfs_sensitive=yes
>   AC_DEFINE(LFS_SENSITIVE, 1, [ System redefines off_t when defining 
> _FILE_OFFSET_BITS to 64. ])
> 
> as defining the offset bits to 64 doesn't change the size of off_t, so
> it looks like a pure 64 bit offset system. But this doesn't control the
> symbols, only usage of libmpg123 API by the mpg123 application.
> 
> That happens in src/libmpg123/lfs_wrap.c:
> 
> off_t attribute_align_arg mpg123_tell(mpg123_handle *mh)
> {
>         int64_t pos = mpg123_tell64(mh);
>         OFF_RETURN(pos, mh)
> }
> 
> #if SIZEOF_OFF_T == 4
> 
> int attribute_align_arg mpg123_open_32(mpg123_handle *mh, const char *path)
> {
>         return mpg123_open(mh, path);
> }
> 
> #endif
> 
> #if defined(LFS_LARGEFILE_64) || (SIZEOF_OFF_T == 8)
> 
> #ifdef LFS_LARGEFILE_64
> #define OFF64 off64_t
> #else
> #define OFF64 off_t
> #endif
> 
> OFF64 attribute_align_arg mpg123_tell_64(mpg123_handle *mh)
> {
>         return mpg123_tell64(mh);
> }
> 
> #endif
> 
> So, there is the new portable mpg123_tell64 which works with int64_t,
> always. There is a 'native' off_t wrapper over this with no suffix. If 
> the 'native' off_t is 32 bits, an _32 alias to that is added. If
> the 'native' off_t is 64 bits, or if off64_t is available, a _64 alias
> is created with _64 suffix.
> 
> The changed Debian build seems to trigger the case of 'native 64 bit
> off_t'. Is that really an issue if you rebuild all software with that
> setting? It should only ever use the _64 symbols. Should we explicitly
> add 32 bit wrappers that nobody needs?

It is not necessary if we are renaming the binary packages instead. So
we'd implement Steve's original plan. 

> What compatibility considerations do you have for that transition? Is
> it expected to possibly break user-compiled software that now sees a
> changed ABI that that is why all time_t-affected libs get an soname
> change?

We are not changing the SONAMEs, only the package names. User's will
have to rebuild their user-compiled software themselves. We do change
the package names however to have an upgrade path for Debian packages
from non-LFS and 32 bit time_t systems where libraries do not provide
dual-ABI interfaces.

What we are currently implementing is:
* rename library packages with a time_t and/or off_t dependendant ABI
  with the t64 postfix.
* rebuild the world with the new ABI

> Do I need to add a
> 
>       ./configure --enable-off_t-is-really-32-bits-please-add-wrappers
> 
> ?
> 
> I could forcibly undef _FILE_OFFSET_BITS in the build to get at the
> truth, but since you set that (I presume), my build script should
> honour that you do not want to deal with 32 bit off_t. So … does Debian
> _want_ 32 bit off_t ABI at all in the 64 bit time_t world?

No, we don't. If we do not rename the package, we want the old symbols
to stay around though with the same ABI as those are the sybmols the
package promised in the past. Without the package rename, packages built
with the old 32 bit off_t and time_t ABI are expected to stay
functional for the bookworm -> trixie upgrade.

> Or can we just disable that for libmpg123 as time_t is an internal
> detail?
> 
> Glancing at
> 
>       
> https://www.gnu.org/software/libc/manual/html_node/64_002dbit-time-symbol-handling.html
> 
> suggests to me that we're repeating the same mess that already plagued
> the world with the botched LFS handling. Shape-shifting off_t … now
> shape-shifting time_t, too. Why do we have to do dual-time
> configurations?
> 
> So maybe I'll have to revise large file API/ABI crap for the dozenth
> time. It's never over, appparently. Should I go full $TRIGGERWORD and
> always add the _32 symbols using int32_t, regardless of native off_t
> size (adding senseless bloat on 64 bit systems)?

Why do you want to touch 64 bit systems? We'd need to _32 symbols only
on 32 bit systems with 64 bit time_t and off_t.

> So … now … I _do_ use time_t internally. Once in libout123, for a timeout
> while reading data (where 32 bits probably are adequate enough), but
> also in libout123 for clock_gettime(), which also is intended for short
> sleeps, but does work by subtracting full system time values, so
> possibly affected post-2038.
> 
> So, maybe I'll have to change the native off_t size check to also
> forcibly undefine _FILE_OFFSET_BITS. Then, since I apparently need (?)
> both _FILE_OFFSET_BITS and _TIME_BITS for working 64 bit time_t on 32
> bit platforms, I need to rephrase the LFS wrapper code not to touch
> off_t at all, but use int32_t instead?
> 
> This would mean adding the _32 wrappers for anyone specifying
> _FILE_OFFSET_BITS=64 at mpg123 build time, which does strike me as
> suboptimal. My assumption so far is that, if the user does specify this
> during build, it should be taken as a platform property. It should be
> always on. Right now, this also results in the non-suffixed plain
> 
>       off_t mpg123_tell()
> 
> returning a 64 bit value. Without that setting, this would be the 32
> bit variant and aliased by mpg123_tell_32(). When I ignore
> _FILE_OFFSET_BITS at build-time for that now, I break ABI with earlier
> builds where people intended to just get the 64 bit functions.
> 
> Regardless of how often I revise LFS stuff, I always end up in a mess.

We can also agree to just leave it as is and do the package rename
instead.

Cheers
-- 
Sebastian Ramacher

Reply via email to