[Buildroot] [PATCH 1/1] package/i2pd: fix static build with atomic

[Buildroot] [PATCH 1/1] package/i2pd: fix static build with atomic

[Fabrice Fontaine]

Use CMAKE_REQUIRED_LIBRARIES otherwise -latomic won't be at the end of
the line

Fixes:
 - http://autobuild.buildroot.org/results/9dd1c34917d4c67bdf4b1aaff3e9c087c8149f6f

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 package/i2pd/i2pd.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/i2pd/i2pd.mk b/package/i2pd/i2pd.mk
index 5fa815c21d..9755752c02 100644
--- a/package/i2pd/i2pd.mk
+++ b/package/i2pd/i2pd.mk
@@ -22,7 +22,7 @@ I2PD_CONF_OPTS += -DWITH_GUI=OFF
 I2PD_CONF_OPTS += -DTHREADS_PTHREAD_ARG=OFF
 
 ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
-I2PD_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -latomic"
+I2PD_CONF_OPTS += -DCMAKE_REQUIRED_LIBRARIES=-latomic
 endif
 
 ifeq ($(BR2_STATIC_LIBS),y)
-- 
2.20.1

[Buildroot] [PATCH 1/1] package/i2pd: fix static build with atomic

[Thomas Petazzoni]

Hello Fabrice,

On Thu, 30 May 2019 23:30:22 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
>  ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
> -I2PD_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -latomic"
> +I2PD_CONF_OPTS += -DCMAKE_REQUIRED_LIBRARIES=-latomic
>  endif

Can we settle on what is the right way to add -latomic for CMake-based
packages ? We already have a mix of CMAKE_CXX_FLAGS and
CMAKE_EXE_LINKER_FLAGS:

$ git grep "CMAKE.*atomic"
package/cutelyst/cutelyst.mk:CUTELYST_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -latomic"
package/gerbera/gerbera.mk:GERBERA_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -latomic"
package/gnuradio/gnuradio.mk:GNURADIO_CONF_OPTS += -DCMAKE_EXE_LINKER_FLAGS=-latomic
package/gqrx/gqrx.mk:GQRX_CONF_OPTS += -DCMAKE_EXE_LINKER_FLAGS=-latomic
package/grpc/grpc.mk:GRPC_CONF_OPTS += -DCMAKE_EXE_LINKER_FLAGS=-latomic
package/i2pd/i2pd.mk:I2PD_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -latomic"
package/kf5/kf5-modemmanager-qt/kf5-modemmanager-qt.mk:KF5_MODEMMANAGER_QT_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -latomic"
package/libcpprestsdk/libcpprestsdk.mk:LIBCPPRESTSDK_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -latomic"
package/wampcc/wampcc.mk:WAMPCC_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -latomic"

It would be nice to not introduce a third way of doing the same
thing :-)

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

[Buildroot] [PATCH 1/1] package/i2pd: fix static build with atomic

[Fabrice Fontaine]

Hello Thomas,

Le ven. 31 mai 2019 ? 16:41, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> a ?crit :
>
> Hello Fabrice,
>
> On Thu, 30 May 2019 23:30:22 +0200
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
> >  ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
> > -I2PD_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -latomic"
> > +I2PD_CONF_OPTS += -DCMAKE_REQUIRED_LIBRARIES=-latomic
> >  endif
>
> Can we settle on what is the right way to add -latomic for CMake-based
> packages ? We already have a mix of CMAKE_CXX_FLAGS and
> CMAKE_EXE_LINKER_FLAGS:
>
> $ git grep "CMAKE.*atomic"
> package/cutelyst/cutelyst.mk:CUTELYST_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -latomic"
> package/gerbera/gerbera.mk:GERBERA_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -latomic"
> package/gnuradio/gnuradio.mk:GNURADIO_CONF_OPTS += -DCMAKE_EXE_LINKER_FLAGS=-latomic
> package/gqrx/gqrx.mk:GQRX_CONF_OPTS += -DCMAKE_EXE_LINKER_FLAGS=-latomic
> package/grpc/grpc.mk:GRPC_CONF_OPTS += -DCMAKE_EXE_LINKER_FLAGS=-latomic
> package/i2pd/i2pd.mk:I2PD_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -latomic"
> package/kf5/kf5-modemmanager-qt/kf5-modemmanager-qt.mk:KF5_MODEMMANAGER_QT_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -latomic"
> package/libcpprestsdk/libcpprestsdk.mk:LIBCPPRESTSDK_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -latomic"
> package/wampcc/wampcc.mk:WAMPCC_CONF_OPTS += -DCMAKE_CXX_FLAGS="$(TARGET_CXXFLAGS) -latomic"
>
> It would be nice to not introduce a third way of doing the same
> thing :-)
I know but in this case, this is the only solution that does not
impact upstream.
upstream choose to link through
target_link_libraries( "${PROJECT_NAME}" libi2pd libi2pdclient
${DL_LIB} ${Boost_LIBRARIES} ${OPENSSL_LIBRARIES} ${ZLIB_LIBRARY}
${CMAKE_THREAD_LIBS_INIT} ${MINGW_EXTRA} ${DL_LIB}
${CMAKE_REQUIRED_LIBRARIES})

Because of this, if we pass -latomic through CMAKE_EXE_LINKER_FLAGS or
CMAKE_CXX_FLAGS, it'll always be placed before Boost_Libraries and all
the other dependencies.
I can try to find a way to fix this but I didn't take time to do so as
it was quicker to just use CMAKE_REQUIRED_LIBRARIES.
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Best Regards,

Fabrice

[Buildroot] [PATCH 1/1] package/i2pd: fix static build with atomic

[Thomas Petazzoni]

Hello,

On Fri, 31 May 2019 16:54:15 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> > It would be nice to not introduce a third way of doing the same
> > thing :-)  
> I know but in this case, this is the only solution that does not
> impact upstream.
> upstream choose to link through
> target_link_libraries( "${PROJECT_NAME}" libi2pd libi2pdclient
> ${DL_LIB} ${Boost_LIBRARIES} ${OPENSSL_LIBRARIES} ${ZLIB_LIBRARY}
> ${CMAKE_THREAD_LIBS_INIT} ${MINGW_EXTRA} ${DL_LIB}
> ${CMAKE_REQUIRED_LIBRARIES})
> 
> Because of this, if we pass -latomic through CMAKE_EXE_LINKER_FLAGS or
> CMAKE_CXX_FLAGS, it'll always be placed before Boost_Libraries and all
> the other dependencies.
> I can try to find a way to fix this but I didn't take time to do so as
> it was quicker to just use CMAKE_REQUIRED_LIBRARIES.

Shouldn't we find a canonical way of adding the -latomic dependency
handling directly in the CMakeLists.txt files of all those packages ?
Ultimately, this should be the right thing to do, no ?

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

[Buildroot] [PATCH 1/1] package/i2pd: fix static build with atomic

[Arnout Vandecappelle]

On 31/05/2019 21:29, Thomas Petazzoni wrote:
> Hello,
> 
> On Fri, 31 May 2019 16:54:15 +0200
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
> 
>>> It would be nice to not introduce a third way of doing the same
>>> thing :-)  
>> I know but in this case, this is the only solution that does not
>> impact upstream.
>> upstream choose to link through
>> target_link_libraries( "${PROJECT_NAME}" libi2pd libi2pdclient
>> ${DL_LIB} ${Boost_LIBRARIES} ${OPENSSL_LIBRARIES} ${ZLIB_LIBRARY}
>> ${CMAKE_THREAD_LIBS_INIT} ${MINGW_EXTRA} ${DL_LIB}
>> ${CMAKE_REQUIRED_LIBRARIES})
>>
>> Because of this, if we pass -latomic through CMAKE_EXE_LINKER_FLAGS or
>> CMAKE_CXX_FLAGS, it'll always be placed before Boost_Libraries and all
>> the other dependencies.
>> I can try to find a way to fix this but I didn't take time to do so as
>> it was quicker to just use CMAKE_REQUIRED_LIBRARIES.
> 
> Shouldn't we find a canonical way of adding the -latomic dependency
> handling directly in the CMakeLists.txt files of all those packages ?
> Ultimately, this should be the right thing to do, no ?

 I won't comment (yet) about how we should do it in Buildroot, but I think the
proper way to do this in CMake itself would be something like:

target_link_libraries(<target> PUBLIC atomic)

 The PUBLIC bit makes sure that the library is not just linked with, but that is
it also included in the generated .cmake file that is used by other packages to
find the include and libs options. As I mentioned earlier, though, I don't think
CMake has support for the equivalent of pkg-onfig Libs.private.

 For packages that provide a custom CMake config file, like boost, we could
update that config file and add something like:

set_property(TARGET <target> APPEND PROPERTY INTERFACE_LINK_LIBRARIES atomic)

 This gets complicated by three elements, though:
1. The boost cmake config files are generated.
2. There's also a boost library called 'atomic', it might clash.
3. For upstream boost, they'd want to detect the "must link with atomic" case
within their build system, probably.

 As to the cases we have already: CMAKE_EXE_LINKER_FLAGS is definitely more
appropriate than CMAKE_CXX_FLAGS.

 Regards,
 Arnout