Debian Bug report logs - #1100147
libstartup-notification0-dev: request to change API/ABI of sn_startup_sequence_get_last_active_time on 32-bit

Reply or subscribe to this bug.

Display info messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Adrian Bunk <bunk@debian.org>

To: Debian Bug Tracking System <submit@bugs.debian.org>

Subject: marco FTBFS on 32-bit with 64-bit time_t with gcc 14

Date: Thu, 31 Oct 2024 01:17:41 +0200

Source: marco
Version: 1.26.2-4
Severity: serious
Tags: ftbfs

https://buildd.debian.org/status/fetch.php?pkg=marco&arch=armhf&ver=1.26.2-4%2Bb5&stamp=1730247644&raw=0

...
core/screen.c: In function 'collect_timed_out_foreach':
core/screen.c:2673:55: error: passing argument 2 of 'sn_startup_sequence_get_last_active_time' from incompatible pointer type [-Wincompatible-pointer-types]
 2673 |   sn_startup_sequence_get_last_active_time (sequence, &tv_sec, &tv_usec);
      |                                                       ^~~~~~~
      |                                                       |
      |                                                       time_t * {aka long long int *}
In file included from /usr/include/startup-notification-1.0/libsn/sn.h:32,
                 from core/display-private.h:40,
                 from core/screen-private.h:36,
                 from core/screen.c:32:
/usr/include/startup-notification-1.0/libsn/sn-monitor.h:83:79: note: expected 'long int *' but argument is of type 'time_t *' {aka 'long long int *'}
   83 |                                                            long              *tv_sec,
      |                                                            ~~~~~~~~~~~~~~~~~~~^~~~~~
make[5]: *** [Makefile:1258: core/libmarco_private_la-screen.lo] Error 1

Message #10 received at 1086472@bugs.debian.org (full text, mbox, reply):

From: Bastian Germann <bage@debian.org>

To: 1086472@bugs.debian.org

Subject: marco FTBFS on 32-bit with 64-bit time_t with gcc 14

Date: Tue, 11 Mar 2025 20:13:10 +0100

Control: clone -1 -2
Control: reassign -2 libstartup-notification0-dev

Please change sn_startup_sequence_get_last_active_time (and for
consistency, sn_startup_sequence_get_initiated_time as well) argument *tv_sec
to have a time_t type so it can be used properly on 32 bit systems with
time_t being 64 bit wide, which is the configuration for armel and armhf
release architectures.

Message #21 received at 1100147@bugs.debian.org (full text, mbox, reply):

From: Bastian Germann <bage@debian.org>

To: 1100147@bugs.debian.org

Subject: marco FTBFS on 32-bit with 64-bit time_t with gcc 14

Date: Tue, 11 Mar 2025 20:23:27 +0100

Control: tags -1 patch
Control: forwarded -1 https://cgit.freedesktop.org/startup-notification/commit/?id=a7e49fefde18ea8d5bada8096d32f23bcfb5a6dc

Upstream has fixed this with
https://cgit.freedesktop.org/startup-notification/commit/?id=a7e49fefde18ea8d5bada8096d32f23bcfb5a6dc
and
https://cgit.freedesktop.org/startup-notification/commit/?id=ea9f7e4cc6fd8c08d175ed7774ed2c5bd11c8ef0

Message #30 received at 1100147@bugs.debian.org (full text, mbox, reply):

From: Simon McVittie <smcv@debian.org>

To: Bastian Germann <bage@debian.org>, 1100147@bugs.debian.org

Cc: 1100147@bugs.debian.org, Adrian Bunk <bunk@debian.org>

Subject: Re: Bug#1100147: libstartup-notification0-dev: request to change API/ABI of sn_startup_sequence_get_last_active_time on 32-bit

Date: Wed, 12 Mar 2025 10:56:23 +0000

Control: retitle 1100147 libstartup-notification0-dev: request to change API/ABI of sn_startup_sequence_get_last_active_time on 32-bit
Control: tags 1100147 = moreinfo
Control: severity 1100147 wishlist
Control: submitter 1100147 bage@debian.org
Control: forwarded 1100147 https://gitlab.freedesktop.org/xdg/startup-notification/-/issues/6
Control: affects 1100147 + src:compiz src:marco src:libwnck3 src:xfwm4

On Thu, 31 Oct 2024 at 01:17:41 +0200, Adrian Bunk wrote (in #1086472,
a FTBFS bug report against src:marco):
>Source: marco
...
>core/screen.c: In function 'collect_timed_out_foreach':
>core/screen.c:2673:55: error: passing argument 2 of 'sn_startup_sequence_get_last_active_time' from incompatible pointer type [-Wincompatible-pointer-types]
> 2673 |   sn_startup_sequence_get_last_active_time (sequence, &tv_sec, &tv_usec);
>      |                                                       ^~~~~~~
>      |                                                       |
>      |                                                       time_t * {aka long long int *}
...
>/usr/include/startup-notification-1.0/libsn/sn-monitor.h:83:79: note: expected 'long int *' but argument is of type 'time_t *' {aka 'long long int *'}
>   83 |                                                            long              *tv_sec,
>      |                                                            ~~~~~~~~~~~~~~~~~~~^~~~~~

On Tue, 11 Mar 2025 at 20:13:10 +0100, Bastian Germann wrote
(in cloned bug #1100147 assigned to src:startup-notification):
>Please change sn_startup_sequence_get_last_active_time (and for
>consistency, sn_startup_sequence_get_initiated_time as well) argument *tv_sec
>to have a time_t type so it can be used properly on 32 bit systems with
>time_t being 64 bit wide, which is the configuration for armel and armhf
>release architectures.

I see that the change you're requesting has been made in upstream git 
but is not in any release. There is discussion on 
https://gitlab.freedesktop.org/xdg/startup-notification/-/issues/6 
pointing out that this is an API and ABI break on the 32-bit 
architectures (other than i386).

There are some packages other than marco that were previously using the 
documented API correctly (in a way that is not Y2038-safe on armel/armhf 
but follows upstream's documentation), like xfwm4 and libwnck3 upstream, 
and compiz via a Debian patch. For example:

    /* https://sources.debian.org/src/xfwm4/4.20.0-1/src/startup_notification.c/?hl=139#L125 */
    time_t tv_sec;
    suseconds_t tv_usec;
    long l_sec, l_usec;
    /* ... */
    sn_startup_sequence_get_last_active_time (sequence, &l_sec, &l_usec);
    tv_sec = l_sec; tv_usec = l_usec;
    /* continue to do things with tv_sec and tv_usec */

If the change you're proposing is made in startup-notification, then 
those three packages (and possibly more) will start to FTBFS. I think 
that's worse than one FTBFS in marco.

Instead of changing startup-notification, I think the right solution 
to #1086472 for trixie would be to apply a patch to marco, similar to 
https://salsa.debian.org/compiz-team/compiz-reloaded/-/blob/master/debian/patches/64-bit-time-t-compat.patch?ref_type=heads
in compiz.

For the longer term, we should see what startup-notification upstream 
will do, and whether they're going to break the API and ABI, or 
deprecate sn_startup_sequence_get_*_time() and add another entry point.
I suggested on the upstream issue that a new entry point returning an 
int64_t with microseconds since the epoch, similar to g_get_real_time(), 
would avoid the API/ABI issue while also being more convenient to use 
than separate seconds and microseconds.

I think we should be strict about not breaking upstream's API and ABI in 
Debian, even if upstream's API and ABI is not ideal. That way lies 
cross-distro divergence, and someone having to do a lot of work in 
several years' time to unpick it.

However, if someone wants to overrule me and break API/ABI anyway, then 
they should note that this is a transition, which would require changing 
the name of the package (probably to libstartup-notification0t64) and 
coordination with the release team to rebuild reverse-dependencies. I 
would suggest that a few days before transition freeze is not the time 
to start a previously unplanned transition, involving sourceful changes 
to at least 4 packages and a series of binNMUs, just to avoid having to 
apply a patch to src:marco.

    smcv

Send a report that this bug log contains spam.