| Message ID | 20190530213022.3878-1-fontaine.fabrice@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/1] package/i2pd: fix static build with atomic | expand |
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
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
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
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
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)
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(-)