diff mbox series

[1/1] cutelyst: link with libatomic when needed

Message ID 20180827164102.21147-1-fontaine.fabrice@gmail.com
State Superseded
Headers show
Series [1/1] cutelyst: link with libatomic when needed | expand

Commit Message

Fabrice Fontaine Aug. 27, 2018, 4:41 p.m. UTC
On some architectures, atomic binutils are provided by the libatomic
library from gcc. Linking with libatomic is therefore necessary,
otherwise the build fails with:

sparc-buildroot-linux-uclibc/sysroot/lib/libatomic.so.1: error adding symbols: DSO missing from command line

This is often for example the case on sparcv8 32 bit.

Fixes:
 - http://autobuild.buildroot.net/results/9e307ab9c7067b26d7b33a572204394808e25772

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 package/cutelyst/cutelyst.mk | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Peter Korsgaard Aug. 27, 2018, 10 p.m. UTC | #1
>>>>> "Fabrice" == Fabrice Fontaine <fontaine.fabrice@gmail.com> writes:

 > On some architectures, atomic binutils are provided by the libatomic
 > library from gcc. Linking with libatomic is therefore necessary,
 > otherwise the build fails with:

 > sparc-buildroot-linux-uclibc/sysroot/lib/libatomic.so.1: error adding symbols: DSO missing from command line

 > This is often for example the case on sparcv8 32 bit.

 > Fixes:
 >  - http://autobuild.buildroot.net/results/9e307ab9c7067b26d7b33a572204394808e25772

 > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
 > ---
 >  package/cutelyst/cutelyst.mk | 4 ++++
 >  1 file changed, 4 insertions(+)

 > diff --git a/package/cutelyst/cutelyst.mk b/package/cutelyst/cutelyst.mk
 > index e8695a7b3a..1f1a5249be 100644
 > --- a/package/cutelyst/cutelyst.mk
 > +++ b/package/cutelyst/cutelyst.mk
 > @@ -16,6 +16,10 @@ CUTELYST_CONF_OPTS += \
 >  	-DPLUGIN_CSRFPROTECTION=ON \
 >  	-DPLUGIN_VIEW_GRANTLEE=OFF
 
 > +ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
 > +CUTELYST_CONF_OPTS += -DCMAKE_EXE_LINKER_FLAGS=-latomic

I know we are doing this elsewhere as well, but doesn't this override
TARGET_LDFLAGS then?
Fabrice Fontaine Aug. 28, 2018, 6:52 a.m. UTC | #2
Dear Peter,

Le mar. 28 août 2018 à 00:00, Peter Korsgaard <peter@korsgaard.com> a
écrit :

> >>>>> "Fabrice" == Fabrice Fontaine <fontaine.fabrice@gmail.com> writes:
>
>  > On some architectures, atomic binutils are provided by the libatomic
>  > library from gcc. Linking with libatomic is therefore necessary,
>  > otherwise the build fails with:
>
>  > sparc-buildroot-linux-uclibc/sysroot/lib/libatomic.so.1: error adding
> symbols: DSO missing from command line
>
>  > This is often for example the case on sparcv8 32 bit.
>
>  > Fixes:
>  >  -
> http://autobuild.buildroot.net/results/9e307ab9c7067b26d7b33a572204394808e25772
>
>  > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
>  > ---
>  >  package/cutelyst/cutelyst.mk | 4 ++++
>  >  1 file changed, 4 insertions(+)
>
>  > diff --git a/package/cutelyst/cutelyst.mk b/package/cutelyst/
> cutelyst.mk
>  > index e8695a7b3a..1f1a5249be 100644
>  > --- a/package/cutelyst/cutelyst.mk
>  > +++ b/package/cutelyst/cutelyst.mk
>  > @@ -16,6 +16,10 @@ CUTELYST_CONF_OPTS += \
>  >      -DPLUGIN_CSRFPROTECTION=ON \
>  >      -DPLUGIN_VIEW_GRANTLEE=OFF
>
>  > +ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
>  > +CUTELYST_CONF_OPTS += -DCMAKE_EXE_LINKER_FLAGS=-latomic
>
> I know we are doing this elsewhere as well, but doesn't this override
> TARGET_LDFLAGS then?
>
Indeed, we're already doing this for gnuradio but you're right, I will
update my patch to use CMAKE_CXX_FLAGS.

>
> --
> Bye, Peter Korsgaard
>

Best Regards,

Fabrice
<div dir="ltr">Dear Peter,<br><br><div class="gmail_quote"><div dir="ltr">Le mar. 28 août 2018 à 00:00, Peter Korsgaard &lt;<a href="mailto:peter@korsgaard.com">peter@korsgaard.com</a>&gt; a écrit :<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">&gt;&gt;&gt;&gt;&gt; &quot;Fabrice&quot; == Fabrice Fontaine &lt;<a href="mailto:fontaine.fabrice@gmail.com" target="_blank">fontaine.fabrice@gmail.com</a>&gt; writes:<br>
<br>
 &gt; On some architectures, atomic binutils are provided by the libatomic<br>
 &gt; library from gcc. Linking with libatomic is therefore necessary,<br>
 &gt; otherwise the build fails with:<br>
<br>
 &gt; sparc-buildroot-linux-uclibc/sysroot/lib/libatomic.so.1: error adding symbols: DSO missing from command line<br>
<br>
 &gt; This is often for example the case on sparcv8 32 bit.<br>
<br>
 &gt; Fixes:<br>
 &gt;  - <a href="http://autobuild.buildroot.net/results/9e307ab9c7067b26d7b33a572204394808e25772" rel="noreferrer" target="_blank">http://autobuild.buildroot.net/results/9e307ab9c7067b26d7b33a572204394808e25772</a><br>
<br>
 &gt; Signed-off-by: Fabrice Fontaine &lt;<a href="mailto:fontaine.fabrice@gmail.com" target="_blank">fontaine.fabrice@gmail.com</a>&gt;<br>
 &gt; ---<br>
 &gt;  package/cutelyst/<a href="http://cutelyst.mk" rel="noreferrer" target="_blank">cutelyst.mk</a> | 4 ++++<br>
 &gt;  1 file changed, 4 insertions(+)<br>
<br>
 &gt; diff --git a/package/cutelyst/<a href="http://cutelyst.mk" rel="noreferrer" target="_blank">cutelyst.mk</a> b/package/cutelyst/<a href="http://cutelyst.mk" rel="noreferrer" target="_blank">cutelyst.mk</a><br>
 &gt; index e8695a7b3a..1f1a5249be 100644<br>
 &gt; --- a/package/cutelyst/<a href="http://cutelyst.mk" rel="noreferrer" target="_blank">cutelyst.mk</a><br>
 &gt; +++ b/package/cutelyst/<a href="http://cutelyst.mk" rel="noreferrer" target="_blank">cutelyst.mk</a><br>
 &gt; @@ -16,6 +16,10 @@ CUTELYST_CONF_OPTS += \<br>
 &gt;      -DPLUGIN_CSRFPROTECTION=ON \<br>
 &gt;      -DPLUGIN_VIEW_GRANTLEE=OFF<br>
<br>
 &gt; +ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)<br>
 &gt; +CUTELYST_CONF_OPTS += -DCMAKE_EXE_LINKER_FLAGS=-latomic<br>
<br>
I know we are doing this elsewhere as well, but doesn&#39;t this override<br>
TARGET_LDFLAGS then?<br></blockquote><div>Indeed, we&#39;re already doing this for gnuradio but you&#39;re right, I will update my patch to use CMAKE_CXX_FLAGS.<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
-- <br>
Bye, Peter Korsgaard<br></blockquote><div><br></div><div>Best Regards,</div><div><br></div><div>Fabrice<br></div></div></div>
Thomas Petazzoni Aug. 28, 2018, 9:58 a.m. UTC | #3
Hello,

On Mon, 27 Aug 2018 18:41:02 +0200, Fabrice Fontaine wrote:
> On some architectures, atomic binutils are provided by the libatomic
> library from gcc. Linking with libatomic is therefore necessary,
> otherwise the build fails with:
> 
> sparc-buildroot-linux-uclibc/sysroot/lib/libatomic.so.1: error adding symbols: DSO missing from command line
> 
> This is often for example the case on sparcv8 32 bit.
> 
> Fixes:
>  - http://autobuild.buildroot.net/results/9e307ab9c7067b26d7b33a572204394808e25772
> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>

If this package unconditionally needs atomic intrinsics, then it also
needs to depends on BR2_TOOLCHAIN_HAS_ATOMIC. Indeed, libatomic is not
available in gcc < 4.8.

Best regards,

Thomas
Fabrice Fontaine Aug. 28, 2018, 11:31 a.m. UTC | #4
Dear Thomas,

Le mar. 28 août 2018 à 11:59, Thomas Petazzoni <thomas.petazzoni@bootlin.com>
a écrit :

> Hello,
>
> On Mon, 27 Aug 2018 18:41:02 +0200, Fabrice Fontaine wrote:
> > On some architectures, atomic binutils are provided by the libatomic
> > library from gcc. Linking with libatomic is therefore necessary,
> > otherwise the build fails with:
> >
> > sparc-buildroot-linux-uclibc/sysroot/lib/libatomic.so.1: error adding
> symbols: DSO missing from command line
> >
> > This is often for example the case on sparcv8 32 bit.
> >
> > Fixes:
> >  -
> http://autobuild.buildroot.net/results/9e307ab9c7067b26d7b33a572204394808e25772
> >
> > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
>
> If this package unconditionally needs atomic intrinsics, then it also
> needs to depends on BR2_TOOLCHAIN_HAS_ATOMIC. Indeed, libatomic is not
> available in gcc < 4.8.
>
cutelyst does not directly depends on atomic, this dependency is linked to
Qt 5.8.
Here is an extrat of qt5base.mk:
ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC)$(BR2_PACKAGE_QT5_VERSION_LATEST),yy)
# Qt 5.8 needs atomics, which on various architectures are in -latomic
define QT5BASE_CONFIGURE_ARCH_CONFIG
        printf 'LIBS += -latomic\n' >$(QT5BASE_ARCH_CONFIG_FILE)
endef

So what should we do?
Should we add a dependency to BR2_TOOLCHAIN_HAS_ATOMIC if
BR2_PACKAGE_QT5_VERSION_LATEST in qt5/Config.in?

>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
Best Regards,

Fabrice
<div dir="ltr">Dear Thomas,<br><br><div class="gmail_quote"><div dir="ltr">Le mar. 28 août 2018 à 11:59, Thomas Petazzoni &lt;<a href="mailto:thomas.petazzoni@bootlin.com">thomas.petazzoni@bootlin.com</a>&gt; a écrit :<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello,<br>
<br>
On Mon, 27 Aug 2018 18:41:02 +0200, Fabrice Fontaine wrote:<br>
&gt; On some architectures, atomic binutils are provided by the libatomic<br>
&gt; library from gcc. Linking with libatomic is therefore necessary,<br>
&gt; otherwise the build fails with:<br>
&gt; <br>
&gt; sparc-buildroot-linux-uclibc/sysroot/lib/libatomic.so.1: error adding symbols: DSO missing from command line<br>
&gt; <br>
&gt; This is often for example the case on sparcv8 32 bit.<br>
&gt; <br>
&gt; Fixes:<br>
&gt;  - <a href="http://autobuild.buildroot.net/results/9e307ab9c7067b26d7b33a572204394808e25772" rel="noreferrer" target="_blank">http://autobuild.buildroot.net/results/9e307ab9c7067b26d7b33a572204394808e25772</a><br>
&gt; <br>
&gt; Signed-off-by: Fabrice Fontaine &lt;<a href="mailto:fontaine.fabrice@gmail.com" target="_blank">fontaine.fabrice@gmail.com</a>&gt;<br>
<br>
If this package unconditionally needs atomic intrinsics, then it also<br>
needs to depends on BR2_TOOLCHAIN_HAS_ATOMIC. Indeed, libatomic is not<br>
available in gcc &lt; 4.8.<br></blockquote><div>cutelyst does not directly depends on atomic, this dependency is linked to Qt 5.8.</div><div>Here is an extrat of <a href="http://qt5base.mk">qt5base.mk</a>:</div><div>ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC)$(BR2_PACKAGE_QT5_VERSION_LATEST),yy)<br># Qt 5.8 needs atomics, which on various architectures are in -latomic<br>define QT5BASE_CONFIGURE_ARCH_CONFIG<br>        printf &#39;LIBS += -latomic\n&#39; &gt;$(QT5BASE_ARCH_CONFIG_FILE)<br>endef</div><div><br></div><div>So what should we do?</div><div>Should we add a dependency to BR2_TOOLCHAIN_HAS_ATOMIC if BR2_PACKAGE_QT5_VERSION_LATEST in qt5/Config.in?<br></div><div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Best regards,<br>
<br>
Thomas<br>
-- <br>
Thomas Petazzoni, CTO, Bootlin<br>
Embedded Linux and Kernel engineering<br>
<a href="https://bootlin.com" rel="noreferrer" target="_blank">https://bootlin.com</a><br></blockquote><div></div><div>Best Regards,</div><div><br></div><div>Fabrice<br></div></div></div>
Thomas Petazzoni Aug. 28, 2018, 1:24 p.m. UTC | #5
Hello,

On Tue, 28 Aug 2018 13:31:51 +0200, Fabrice Fontaine wrote:

> cutelyst does not directly depends on atomic, this dependency is linked to
> Qt 5.8.

Argh. Ideally, cutelyst should pick this up from Qt's pkg-config file
or something like that. But I guess "ideal" and "build system" are not
always aligned :-)

> Here is an extrat of qt5base.mk:
> ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC)$(BR2_PACKAGE_QT5_VERSION_LATEST),yy)
> # Qt 5.8 needs atomics, which on various architectures are in -latomic
> define QT5BASE_CONFIGURE_ARCH_CONFIG
>         printf 'LIBS += -latomic\n' >$(QT5BASE_ARCH_CONFIG_FILE)
> endef
> 
> So what should we do?
> Should we add a dependency to BR2_TOOLCHAIN_HAS_ATOMIC if
> BR2_PACKAGE_QT5_VERSION_LATEST in qt5/Config.in?

It depends whether Qt needs atomic intrinsics unconditionally or not.
If it does need atomic intrinsics unconditionally, then yes, it needs a
BR2_TOOLCHAIN_HAS_ATOMIC dependency.

We should perhaps have an older toolchain that doesn't provide atomic
intrinsics to be able to verify this.

*But*, BR2_PACKAGE_QT5_VERSION_LATEST already depends on
BR2_TOOLCHAIN_GCC_AT_LEAST_4_8, so anyway libatomic will always be
available, which makes the entire discussion moot.

Except for cutelyst built against Qt5.6, which doesn't need libatomic.
So perhaps the link of cutelyst on libatomic should be conditional on
the version of Qt5 ?

Best regards,

Thomas
diff mbox series

Patch

diff --git a/package/cutelyst/cutelyst.mk b/package/cutelyst/cutelyst.mk
index e8695a7b3a..1f1a5249be 100644
--- a/package/cutelyst/cutelyst.mk
+++ b/package/cutelyst/cutelyst.mk
@@ -16,6 +16,10 @@  CUTELYST_CONF_OPTS += \
 	-DPLUGIN_CSRFPROTECTION=ON \
 	-DPLUGIN_VIEW_GRANTLEE=OFF
 
+ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
+CUTELYST_CONF_OPTS += -DCMAKE_EXE_LINKER_FLAGS=-latomic
+endif
+
 ifeq ($(BR2_PACKAGE_LIBPWQUALITY),y)
 CUTELYST_CONF_OPTS += -DPLUGIN_VALIDATOR_PWQUALITY=ON
 CUTELYST_DEPENDENCIES += libpwquality