Discussion:
How to conditionally patch a package based on architecture with debhelper?
Add Reply
Ben Collins
2025-01-16 16:20:02 UTC
Reply
Permalink
Greetings.
I have a situation with mumble where the build is breaking on armel
architecture. Upstream has identified that this bug is due to the mumble
"link" plugin containing atomic memory operations. I would like to
conditionally patch the source plugins/CMakeLists.txt file based on
architecutre to not build that one plugin for armel. CDBS apparently has a
way of doing this [1] and I would like to find a solution for doing this
with debhelper. I've been searching and haven't found anything quite
fitting.
Can the patched code just look like:

#ifdef __ARMEL__
// Patched code
#else
// original code with atomic memory ops
#endif
--
Ben Collins
https://debian.org | https://ubuntu.com
https://ben-collins.blogspot.com
https://catchmobilehealth.com
--
3EC9 7598 1672 961A 1139 173A 5D5A 57C7 242B 22CF
Andrey Rakhmatullin
2025-01-16 15:50:01 UTC
Reply
Permalink
I have a situation with mumble where the build is breaking on armel
architecture. Upstream has identified that this bug is due to the mumble
"link" plugin containing atomic memory operations. I would like to
conditionally patch the source plugins/CMakeLists.txt file based on
architecutre to not build that one plugin for armel.
Consider instead unconditionally patching it in a way that disables the
building of that plugin on that arch.
CDBS apparently has a way of doing this [1] and I would like to find a
solution for doing this with debhelper.
Nowadays changing the upstream source is handled by dpkg, not by d/rules.
I suppose in a pinch I could build-depend on the 'patch' command and do
ifeq ($(DEB_HOST_ARCH),armel)
        patch < debian/patches/armel/95-fix-build-armel.patch
endif
If you do this don't forget to do the opposite in clean, as mandated by
the Policy.
--
WBR, wRAR
Simon Richter
2025-01-16 16:00:02 UTC
Reply
Permalink
Hi,

atomic operations require linking against libatomic — always have. Some architectures inline a few functions, which is how you get away with omitting the library on amd64 most of the time, but this is incorrect.

No architecture specific patch should be required here, adding libatomic everywhere is fine, possibly via
-Wl,--push-options,--as-needed,-latomic,--pop-options

(although as-needed is likely default anyway)

   Simon
Chris Knadle
2025-01-16 19:00:01 UTC
Reply
Permalink
Hi,
atomic operations require linking against libatomic — always have.
Some architectures inline a few functions, which is how you get away
with omitting the library on amd64 most of the time, but this is
incorrect.
No architecture specific patch should be required here, adding
libatomic everywhere is fine, possibly via
-Wl,--push-options,--as-needed,-latomic,--pop-options
(although as-needed is likely default anyway)
   Simon
I'm assuming this also requires adding libatomic1 as a build dependency.
Huh. So there's a chance this an fix the build for armel too. Nice.

Thanks.

  -- Chris

Chris Knadle
***@coredump.us
Andrey Rakhmatullin
2025-01-16 20:00:02 UTC
Reply
Permalink
Post by Chris Knadle
Post by Simon Richter
atomic operations require linking against libatomic — always have. Some
architectures inline a few functions, which is how you get away with
omitting the library on amd64 most of the time, but this is incorrect.
No architecture specific patch should be required here, adding libatomic
everywhere is fine, possibly via
-Wl,--push-options,--as-needed,-latomic,--pop-options
(although as-needed is likely default anyway)
   Simon
I'm assuming this also requires adding libatomic1 as a build dependency.
ITYM libgcc-14-dev, and it's in build-essential.
--
WBR, wRAR
Chris Knadle
2025-01-16 20:30:02 UTC
Reply
Permalink
Post by Andrey Rakhmatullin
Post by Chris Knadle
atomic operations require linking against libatomic — always have. Some
architectures inline a few functions, which is how you get away with
omitting the library on amd64 most of the time, but this is incorrect.
No architecture specific patch should be required here, adding libatomic
everywhere is fine, possibly via
-Wl,--push-options,--as-needed,-latomic,--pop-options
(although as-needed is likely default anyway)
   Simon
I'm assuming this also requires adding libatomic1 as a build dependency.
ITYM libgcc-14-dev, and it's in build-essential.
i.e. no new build depends needed. Understood, thanks

Chris Knadle
***@coredump.us
Johannes Schauer Marin Rodrigues
2025-01-16 19:20:01 UTC
Reply
Permalink
Hi Simon,

Quoting Simon Richter (2025-01-16 16:52:19)
Post by Simon Richter
atomic operations require linking against libatomic — always have. Some
architectures inline a few functions, which is how you get away with omitting
the library on amd64 most of the time, but this is incorrect.
No architecture specific patch should be required here, adding libatomic everywhere is fine, possibly via
-Wl,--push-options,--as-needed,-latomic,--pop-options
(although as-needed is likely default anyway)
I find this very interesting. I am fighting -latomic linking issues for quite a
while now and what you just said makes me think that I am in severe lack of
understanding here. Can you elaborate?

My specific situation is:

src:vcmi FTBFS on armel unless I apply this patch:

https://sources.debian.org/src/vcmi/1.5.7%2Bdfsg-1/debian/patches/fix-armel-atomics.patch/

After more research together with vcmi upstream involvement I found out that
linking vcmi against atomic is *only* required because it uses
tbb::parallel_for from oneTBB: https://github.com/uxlfoundation/oneTBB/issues/1454

And then others have linked me to this GCC bug:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81358

So I'm at a loss at deciding who is at fault and who should be fixing
something. You say this should work out-of-the-box on amd64 mostly but
everything I have come across handling std::atomic types builds fine on all
architectures -- except armel. So which is the thing that gets things wrong
here?

- armel?
- g++?
- cmake?
- oneTBB?
- vcmi?

Who should be patched? Relevant Debian bugs are #1089991 and #1088922

I'd very much welcome to be enlightened by you or anybody else on this list. :)

Thanks!

cheers, josch
Andrey Rakhmatullin
2025-01-16 19:50:01 UTC
Reply
Permalink
Post by Johannes Schauer Marin Rodrigues
Post by Simon Richter
atomic operations require linking against libatomic — always have. Some
architectures inline a few functions, which is how you get away with omitting
the library on amd64 most of the time, but this is incorrect.
No architecture specific patch should be required here, adding libatomic everywhere is fine, possibly via
-Wl,--push-options,--as-needed,-latomic,--pop-options
(although as-needed is likely default anyway)
I find this very interesting. I am fighting -latomic linking issues for quite a
while now and what you just said makes me think that I am in severe lack of
understanding here. Can you elaborate?
https://sources.debian.org/src/vcmi/1.5.7%2Bdfsg-1/debian/patches/fix-armel-atomics.patch/
After more research together with vcmi upstream involvement I found out that
linking vcmi against atomic is *only* required because it uses
tbb::parallel_for from oneTBB: https://github.com/uxlfoundation/oneTBB/issues/1454
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81358
So I'm at a loss at deciding who is at fault and who should be fixing
something. You say this should work out-of-the-box on amd64 mostly but
everything I have come across handling std::atomic types builds fine on all
architectures -- except armel. So which is the thing that gets things wrong
here?
- armel?
- g++?
- cmake?
- oneTBB?
- vcmi?
Without having any special knowledge about this and skimming through the
linked GCC bug I assume that until (unless?) GCC starts adding -latomic
implicitly (which is what the bug and patches linked in it are about)
it's the software upstream's fault, and no upstreams actually care about
armel (and why would they).

See also the armel/upstream-support cell on
https://release.debian.org/testing/arch_qualify.html
--
WBR, wRAR
Ángel
2025-01-16 23:40:01 UTC
Reply
Permalink
Post by Johannes Schauer Marin Rodrigues
Hi Simon,
Quoting Simon Richter (2025-01-16 16:52:19)
atomic operations require linking against libatomic — always have..
Some
architectures inline a few functions, which is how you get away with omitting
the library on amd64 most of the time, but this is incorrect.
No architecture specific patch should be required here, adding
libatomic everywhere is fine, possibly via
-Wl,--push-options,--as-needed,-latomic,--pop-options
(although as-needed is likely default anyway)
I find this very interesting. I am fighting -latomic linking issues
for quite a while now and what you just said makes me think that I am
in severe lack of understanding here. Can you elaborate?
Hi Josch,

It's not that complex.¹ If you use an atomic, you should also be
linking with atomic (i.e. -latomic). This is similar to using
-pthreads if you are using multiple threads, or -lresolv if you use
gethostbyname()

What makes things complicated here is that in most architectures, gcc
is able to inline everything, and libatomic is not really needed, so
you don't need to add -latomic and it just works.

...until it doesn't, such as in armel.

Just like you can use gethostbyname() directly, since this is provided
by libc, which is implicitly linked by default.
But you would need to specify -lresolv in Solaris, or it won't link.

So the solution would be just to add -latomic

Saying instead
-Wl,--push-options,--as-needed,-latomic,--pop-options

is an advanced way to make the linker not to depend on libatomic if it
isn't actually needed.
Post by Johannes Schauer Marin Rodrigues
So I'm at a loss at deciding who is at fault and who should be fixing
something. You say this should work out-of-the-box on amd64 mostly but
everything I have come across handling std::atomic types builds fine on all
architectures -- except armel. So which is the thing that gets things wrong
here?
- armel?
- g++?
- cmake?
- oneTBB?
- vcmi?
Who should be patched? Relevant Debian bugs are #1089991 and #1088922
I'd very much welcome to be enlightened by you or anybody else on
this list.. :)
I think the bug is in oneTBB, it is that that should be patched.
Your last patch at
https://github.com/uxlfoundation/oneTBB/issues/1454#issuecomment-2267455977
seems appropriate.


vcmi should not need to care if oneTBB uses atomics or not.


armel is not strictly at fault (although you might blame the
architecture for not providing a way as easy as amd does)

g++ (actually gcc has the same issue) can be partially to blame.
gcc made an interface which is not too usable. Then it was reused for
g++, albeit the language specification doesn't state that requirement.
It *should* be improved at gcc level, but at the present time the spec
is that you will need to link to libatomic, and that's what oneTBB
should be doing.


Best regards


¹ Ok, after writing this whole email, maybe a bit 😅
Alexandre Rossi
2025-01-17 08:50:01 UTC
Reply
Permalink
Hi,
Post by Ángel
It's not that complex.¹ If you use an atomic, you should also be
linking with atomic (i.e. -latomic). This is similar to using
-pthreads if you are using multiple threads, or -lresolv if you use
gethostbyname()
Some upstreams have gone through build system tests to assess whether
to build with libatomic or not. Adding -latomic is simpler. I wonder if
the build system configure stage test is overkill.

To illustrate, that's what I thought was the right fix for transmission[1][2].

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1067625
[2] https://github.com/transmission/transmission/pull/6774

Thanks,

Alex
Simon Richter
2025-01-17 10:50:01 UTC
Reply
Permalink
Hi,
Post by Alexandre Rossi
Some upstreams have gone through build system tests to assess whether
to build with libatomic or not. Adding -latomic is simpler. I wonder if
the build system configure stage test is overkill.
Yes and no. A lot of the tests are just broken, like "compile and link a
program incrementing an atomic integer, try with no extra libraries and
with libatomic" -- in principle you'd need to repeat this with all data
types used, then you'd get a reliable result.

It's similar to how libm is used -- if you use fsin(), this may be a
single assembler instruction, or a call to a function that is IEEE754
conformant and sets errno, and that implementation detail determines
whether you want to link against libm or not, but if you test whether
fsin() requires -lm, that doesn't tell you whether fabs() requires -lm.

Likewise, 4-byte atomics being inlined doesn't tell you if 1-byte or
16-byte atomics are also inlined, or whether there is special handling
for signed or unsigned integers.

Within Debian, always linking with --as-needed is a simple fix, but it
depends on using a linker that understands --as-needed, so it's not
portable enough to be submitted upstream.

In my own packages, I check if libatomic exists, and if it does, I
unconditionally link if I use any atomics. I also check if the linker
accepts --push-flags, if it does I generate a
-Wl,--push-flags,--as-needed,-latomic,--pop-flags sequence, if not, the
unconditional link will generate dpkg-shlibdeps warnings about
unnecessary linking on amd64, but that's better than failing on other
platforms.

Simon
Wookey
2025-01-17 15:40:01 UTC
Reply
Permalink
Post by Simon Richter
In my own packages, I check if libatomic exists, and if it does, I
unconditionally link if I use any atomics. I also check if the linker
accepts --push-flags, if it does I generate a
-Wl,--push-flags,--as-needed,-latomic,--pop-flags sequence, if not, the
unconditional link will generate dpkg-shlibdeps warnings about unnecessary
linking on amd64, but that's better than failing on other platforms.
Can you explain why this:
-Wl,--push-flags,--as-needed,-latomic,--pop-flags
is better than
-as-needed,-latomic
I thought --as-needed on its own is sufficicent to avoid dpkg-shlibdeps warnings
about unnecessary linking (because it only links the things that are needed), so
I don't understand the need for the psuh/pop sequence (I must admit that I never
knew that existed until your message recently).

cheers

Wookey
--
Principal hats: Debian, Wookware
http://wookware.org/
Simon McVittie
2025-01-17 16:50:01 UTC
Reply
Permalink
Post by Simon Richter
-Wl,--push-flags,--as-needed,-latomic,--pop-flags
is better than
-as-needed,-latomic
I thought --as-needed on its own is sufficicent to avoid dpkg-shlibdeps warnings
about unnecessary linking (because it only links the things that are needed), so
I don't understand the need for the psuh/pop sequence (I must admit that I never
knew that existed until your message recently).
Two things, depending which difference between your two command lines
you are thinking about:

1. -Wl,--as-needed normally sets global state for the entire linker
invocation. If the package you're building is one that has a
functional requirement to *not* use -Wl,--as-needed for other library
dependencies, the push/pop guards ensure that the -Wl,--as-needed
only affects how libatomic is linked, without having the side-effects
of linking every other library dependency (not just libatomic!) in
"as needed" mode.

For example GLib explicitly pulls in libpthread, which is (or at least
was in the past) functionally necessary: if that wasn't done,
dlopen()ing modules that indirectly depend on libpthread would not
be safe, because internal locks in glibc were already initialized
in a non-thread-safe way before libpthread was in memory, and cannot
(or at least could not in 2009) be re-initialized with the thread-safe
equivalent after libpthread arrived. (Reference:
https://mail.gnome.org/archives/gtk-devel-list/2009-November/msg00096.html)

2. -Wl is necessary to tell the compiler driver (gcc or clang)
"it's OK that you don't understand this, just pass it through to the
linker". --as-needed is a ld option, not a gcc option; but $LDFLAGS
are passed to gcc, not ld.

smcv
Theodore Ts'o
2025-01-21 15:40:01 UTC
Reply
Permalink
A question about --as-needed as an upstream developer who wants to
care about more than just Debian or Linux. Am I correct in assuming
this will work on any system using GNU binutils? And it doesn't
matter whether you are using gcc, clang, etc. What about other OS's
such as *BSD, MacOS, etc.?

I guess I could add an autoconf test to see if the linker barfs on
--as-needed, but I'm curious how much this actually matters in
practice.

Thanks,

- Ted
Colin Watson
2025-01-21 21:10:01 UTC
Reply
Permalink
Post by Theodore Ts'o
A question about --as-needed as an upstream developer who wants to
care about more than just Debian or Linux. Am I correct in assuming
this will work on any system using GNU binutils? And it doesn't
matter whether you are using gcc, clang, etc. What about other OS's
such as *BSD, MacOS, etc.?
I often consult Gnulib for lore on this kind of thing.
https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=m4/lib-ignore.m4
seems to have some useful hints, with comments for those not fluent in
m4.

This also seems to be a standalone file, so if your project is using
Autoconf but not Gnulib, then you should be able to copy it into your
project's local macro directory, add gl_IGNORE_UNUSED_LIBRARIES to
configure.ac, and add $(IGNORE_UNUSED_LIBRARIES_CFLAGS) etc. to whatever
*_LDFLAGS variable is appropriate.
--
Colin Watson (he/him) [***@debian.org]
Matthias Klose
2025-01-22 04:10:01 UTC
Reply
Permalink
Post by Colin Watson
Post by Theodore Ts'o
A question about --as-needed as an upstream developer who wants to
care about more than just Debian or Linux. Am I correct in assuming
this will work on any system using GNU binutils? And it doesn't
matter whether you are using gcc, clang, etc. What about other OS's
such as *BSD, MacOS, etc.?
--as-needed has been there forever. The --push-state/--pop-state options
are implemented in binutils 2014 (ld), and binutils 2016 (gold). I
didn't look at other linkers.

Using these in libtool still seems to be a problem, as libtool has it's
own idea about how to re-sort command line options and libraries.
Post by Colin Watson
I often consult Gnulib for lore on this kind of thing.
https://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=m4/lib-ignore.m4
seems to have some useful hints, with comments for those not fluent in
m4.
This also seems to be a standalone file, so if your project is using
Autoconf but not Gnulib, then you should be able to copy it into your
project's local macro directory, add gl_IGNORE_UNUSED_LIBRARIES to
configure.ac, and add $(IGNORE_UNUSED_LIBRARIES_CFLAGS) etc. to whatever
*_LDFLAGS variable is appropriate.
Simon Richter
2025-01-22 08:00:01 UTC
Reply
Permalink
Hi,
Post by Theodore Ts'o
I guess I could add an autoconf test to see if the linker barfs on
--as-needed, but I'm curious how much this actually matters in
practice.
From an upstream perspective, I think the best approach is

AC_CHECK_LIB([atomic],[main])

That creates a few unnecessary dependencies, but it should work pretty
much everywhere.

If Debian maintainers feel annoyed by warnings about unnecessary
dependencies, they are welcome to replace this with a hardcoded

LDFLAGS="${LDFLAGS} -Wl,--push-state,--as-needed,-latomic,--pop-state"

because unlike upstream, Debian can assume reasonably recent GNU tools.

Someone GNU adjacent might want to build something more sophisticated
for the autoconf-archive -- but I'd draw the line at

0. if the library does not exist, assume it's not needed and explicitly
say so in the build log
1. if --push-state/--pop-state are supported, wrap them around --as-needed
2. if --as-needed is supported, but no state stack, use --no-as-needed
afterwards, and if any other macro wants to use --as-needed, they need
to add their own
3. if neither is supported, unconditionally link

Like libm, what is actually needed at link time depends on inlining,
which is affected by constant propagation, so it is impossible to build
100% reliable tests.

Simon

Chris Knadle
2025-01-16 18:30:01 UTC
Reply
Permalink
Post by Ben Collins
Greetings.
I have a situation with mumble where the build is breaking on armel
architecture. Upstream has identified that this bug is due to the mumble
"link" plugin containing atomic memory operations. I would like to
conditionally patch the source plugins/CMakeLists.txt file based on
architecutre to not build that one plugin for armel. CDBS apparently has a
way of doing this [1] and I would like to find a solution for doing this
with debhelper. I've been searching and haven't found anything quite
fitting.
#ifdef __ARMEL__
// Patched code
#else
// original code with atomic memory ops
#endif
Thank you. From this good hint I found an additional [1] piece that may
get the rest of the way
[And, ironic that this particular post on stackoverflow only received
downvotes.]

----------------------

# Store in CMAKE_DEB_HOST_ARCH var the current build architecture
# (Note: See link [2] below for details for this section)
execute_process(COMMAND
  dpkg-architecture
    -qDEB_HOST_ARCH
  OUTPUT_VARIABLE
    CMAKE_DEB_HOST_ARCH
  OUTPUT_STRIP_TRAILING_WHITESPACE
)

#And use that information later in CMakeLists

if(${CMAKE_DEB_HOST_ARCH} MATCHES "armhf")
  ...
elseif(${CMAKE_DEB_HOST_ARCH} MATCHES "i386")
  ...
else()
  ...
endif()

----------------------

Looking at the manpage for dpkg-architecture, the variable I may want to
conditionally build upon might be DEB_TARGET_ARCH rather than DEB_HOST_ARCH.

[1]
https://stackoverflow.com/questions/11944060/how-to-detect-target-architecture-using-cmake

[2] https://cmake.org/cmake/help/latest/command/execute_process.html

   -- Chris
--
Chris Knadle
***@coredump.us
Johannes Schauer Marin Rodrigues
2025-01-16 19:10:01 UTC
Reply
Permalink
Quoting Chris Knadle (2025-01-16 19:26:39)
Post by Chris Knadle
Looking at the manpage for dpkg-architecture, the variable I may want to
conditionally build upon might be DEB_TARGET_ARCH rather than DEB_HOST_ARCH.
above the VARIABLES section in that man page there is the TERMS section which
explains:


target machine
The machine the compiler is building for, or the emulator will run
code for. This is only needed when building a cross-toolchain (or
emulator), one that will be built on the build architecture, to be
run on the host architecture, and to build (or run emulated) code
for the target architecture.

Unless you are building a compiler, you do not need to concern yourself with
the "target" architecture. Debian is using the GNU terminology and wRAR already
linked the relevant GNU documentation in the other mail.

Thanks!

cheers, josch
Guillem Jover
2025-01-17 00:10:01 UTC
Reply
Permalink
Hi!
Post by Chris Knadle
Looking at the manpage for dpkg-architecture, the variable I may want to
conditionally build upon might be DEB_TARGET_ARCH rather than DEB_HOST_ARCH.
Others have already given pointers about this confusion. I'm more
interested in trying to see whether we can improve the man page to
avoid it.

I think if you search for stuff and end up in the VARIABLES section,
from there it's hard to notice the TERMS one, but in the past it
seemed excessive to me to either repeat the same or back-reference
the TERMS section on each variable definition. Perhaps a middle ground
would be adding a reference in the DEB_BUILD_ARCH and DEB_TARGET_ARCH,
like in the attached patch? Do you think that would have helped?

(I've gone over those two sections to try to further clarify and update
things I noticed, although I think the «Debian architecture» one could
do with some major update.)

Thanks,
Guillem
Loading...