| Message ID | 1514969543-20452-2-git-send-email-heyleke@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3,1/1] opentracing-cpp: new package | expand |
Hi Jan, On Wed, Jan 03, 2018 at 09:52:23AM +0100, Jan Heylen wrote: > From: Jan Heylen <jan.heylen@nokia.com> > > Signed-off-by: Jan Heylen <jan.heylen@nokia.com> > --- [...] > diff --git a/package/opentracing-cpp/opentracing-cpp.mk b/package/opentracing-cpp/opentracing-cpp.mk > new file mode 100644 > index 0000000..9b2d473 > --- /dev/null > +++ b/package/opentracing-cpp/opentracing-cpp.mk > @@ -0,0 +1,22 @@ > +################################################################################ > +# > +# opentracing-cpp > +# > +################################################################################ > + > +OPENTRACING_CPP_VERSION = v1.2.0 > +OPENTRACING_CPP_SITE = $(call github,opentracing,opentracing-cpp,$(OPENTRACING_CPP_VERSION)) > +OPENTRACING_CPP_LICENSE = MIT > +OPENTRACING_CPP_LICENSE_FILES = COPYING > + > +OPENTRACING_CPP_INSTALL_STAGING = YES > + > +ifeq ($(BR2_STATIC_LIBS),y) > +OPENTRACING_CPP_CONF_OPTS += -DBUILD_SHARED_LIBS=OFF -DBUILD_STATIC_LIBS=ON > +else ifeq ($(BR2_SHARED_STATIC_LIBS),y) > +OPENTRACING_CPP_CONF_OPTS += -DBUILD_SHARED_LIBS=ON -DBUILD_STATIC_LIBS=ON > +else ifeq ($(BR2_SHARED_LIBS),y) > +OPENTRACING_CPP_CONF_OPTS += -DBUILD_SHARED_LIBS=ON -DBUILD_STATIC_LIBS=OFF > +endif The -DBUILD_SHARED_LIBS setting should be handled correctly by the Buildroot cmake infrastructure at package/pkg-cmake.mk. Doesn't that work for you? baruch > + > +$(eval $(cmake-package))
I copied that infra from another package. Thomas also indicated already that there is something already generalised but not fully yet. So he adviced to keep it like this for the time being. If this is not nok, please advice differently. Br, Jan On 3 Jan 2018 10:04, "Baruch Siach" <baruch@tkos.co.il> wrote: > Hi Jan, > > On Wed, Jan 03, 2018 at 09:52:23AM +0100, Jan Heylen wrote: > > From: Jan Heylen <jan.heylen@nokia.com> > > > > Signed-off-by: Jan Heylen <jan.heylen@nokia.com> > > --- > > [...] > > > diff --git a/package/opentracing-cpp/opentracing-cpp.mk > b/package/opentracing-cpp/opentracing-cpp.mk > > new file mode 100644 > > index 0000000..9b2d473 > > --- /dev/null > > +++ b/package/opentracing-cpp/opentracing-cpp.mk > > @@ -0,0 +1,22 @@ > > +########################################################### > ##################### > > +# > > +# opentracing-cpp > > +# > > +########################################################### > ##################### > > + > > +OPENTRACING_CPP_VERSION = v1.2.0 > > +OPENTRACING_CPP_SITE = $(call github,opentracing, > opentracing-cpp,$(OPENTRACING_CPP_VERSION)) > > +OPENTRACING_CPP_LICENSE = MIT > > +OPENTRACING_CPP_LICENSE_FILES = COPYING > > + > > +OPENTRACING_CPP_INSTALL_STAGING = YES > > + > > +ifeq ($(BR2_STATIC_LIBS),y) > > +OPENTRACING_CPP_CONF_OPTS += -DBUILD_SHARED_LIBS=OFF > -DBUILD_STATIC_LIBS=ON > > +else ifeq ($(BR2_SHARED_STATIC_LIBS),y) > > +OPENTRACING_CPP_CONF_OPTS += -DBUILD_SHARED_LIBS=ON > -DBUILD_STATIC_LIBS=ON > > +else ifeq ($(BR2_SHARED_LIBS),y) > > +OPENTRACING_CPP_CONF_OPTS += -DBUILD_SHARED_LIBS=ON > -DBUILD_STATIC_LIBS=OFF > > +endif > > The -DBUILD_SHARED_LIBS setting should be handled correctly by the > Buildroot > cmake infrastructure at package/pkg-cmake.mk. Doesn't that work for you? > > baruch > > > + > > +$(eval $(cmake-package)) > > -- > http://baruch.siach.name/blog/ ~. .~ Tk Open > Systems > =}------------------------------------------------ooO--U-- > Ooo------------{= > - baruch@tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il - > <div dir="auto">I copied that infra from another package. Thomas also indicated already that there is something already generalised but not fully yet. So he adviced to keep it like this for the time being. If this is not nok, please advice differently.<div dir="auto"><br></div><div dir="auto">Br, </div><div dir="auto"><br></div><div dir="auto">Jan</div></div><div class="gmail_extra"><br><div class="gmail_quote">On 3 Jan 2018 10:04, "Baruch Siach" <<a href="mailto:baruch@tkos.co.il">baruch@tkos.co.il</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Jan,<br> <br> On Wed, Jan 03, 2018 at 09:52:23AM +0100, Jan Heylen wrote:<br> > From: Jan Heylen <<a href="mailto:jan.heylen@nokia.com">jan.heylen@nokia.com</a>><br> ><br> > Signed-off-by: Jan Heylen <<a href="mailto:jan.heylen@nokia.com">jan.heylen@nokia.com</a>><br> > ---<br> <br> [...]<br> <br> > diff --git a/package/opentracing-cpp/<a href="http://opentracing-cpp.mk" rel="noreferrer" target="_blank">open<wbr>tracing-cpp.mk</a> b/package/opentracing-cpp/<a href="http://opentracing-cpp.mk" rel="noreferrer" target="_blank">open<wbr>tracing-cpp.mk</a><br> > new file mode 100644<br> > index 0000000..9b2d473<br> > --- /dev/null<br> > +++ b/package/opentracing-cpp/<a href="http://opentracing-cpp.mk" rel="noreferrer" target="_blank">open<wbr>tracing-cpp.mk</a><br> > @@ -0,0 +1,22 @@<br> > +#############################<wbr>##############################<wbr>#####################<br> > +#<br> > +# opentracing-cpp<br> > +#<br> > +#############################<wbr>##############################<wbr>#####################<br> > +<br> > +OPENTRACING_CPP_VERSION = v1.2.0<br> > +OPENTRACING_CPP_SITE = $(call github,opentracing,<wbr>opentracing-cpp,$(OPENTRACING_<wbr>CPP_VERSION))<br> > +OPENTRACING_CPP_LICENSE = MIT<br> > +OPENTRACING_CPP_LICENSE_FILES = COPYING<br> > +<br> > +OPENTRACING_CPP_INSTALL_<wbr>STAGING = YES<br> > +<br> > +ifeq ($(BR2_STATIC_LIBS),y)<br> > +OPENTRACING_CPP_CONF_OPTS += -DBUILD_SHARED_LIBS=OFF -DBUILD_STATIC_LIBS=ON<br> > +else ifeq ($(BR2_SHARED_STATIC_LIBS),y)<br> > +OPENTRACING_CPP_CONF_OPTS += -DBUILD_SHARED_LIBS=ON -DBUILD_STATIC_LIBS=ON<br> > +else ifeq ($(BR2_SHARED_LIBS),y)<br> > +OPENTRACING_CPP_CONF_OPTS += -DBUILD_SHARED_LIBS=ON -DBUILD_STATIC_LIBS=OFF<br> > +endif<br> <br> The -DBUILD_SHARED_LIBS setting should be handled correctly by the Buildroot<br> cmake infrastructure at package/<a href="http://pkg-cmake.mk" rel="noreferrer" target="_blank">pkg-cmake.mk</a>. Doesn't that work for you?<br> <br> baruch<br> <br> > +<br> > +$(eval $(cmake-package))<br> <br> --<br> <a href="http://baruch.siach.name/blog/" rel="noreferrer" target="_blank">http://baruch.siach.name/blog/</a> ~. .~ Tk Open Systems<br> =}----------------------------<wbr>--------------------ooO--U--<wbr>Ooo------------{=<br> - <a href="mailto:baruch@tkos.co.il">baruch@tkos.co.il</a> - tel: +972.2.679.5364, <a href="http://www.tkos.co.il" rel="noreferrer" target="_blank">http://www.tkos.co.il</a> -<br> </blockquote></div></div>
Hi Jan, On Wed, Jan 03, 2018 at 10:07:51AM +0100, Jan Heylen wrote: > I copied that infra from another package. Thomas also indicated already > that there is something already generalised but not fully yet. So he > adviced to keep it like this for the time being. If this is not nok, please > advice differently. I see that rabbitmq-c is also doing that. I think that the -DBUILD_SHARED_LIBS part is redundant. But the -DBUILD_STATIC_LIBS part might be needed, since the generic code does not handle that. baruch > On 3 Jan 2018 10:04, "Baruch Siach" <baruch@tkos.co.il> wrote: > > On Wed, Jan 03, 2018 at 09:52:23AM +0100, Jan Heylen wrote: > > > From: Jan Heylen <jan.heylen@nokia.com> > > > > > > Signed-off-by: Jan Heylen <jan.heylen@nokia.com> > > > --- > > > > [...] > > > > > diff --git a/package/opentracing-cpp/opentracing-cpp.mk > > b/package/opentracing-cpp/opentracing-cpp.mk > > > new file mode 100644 > > > index 0000000..9b2d473 > > > --- /dev/null > > > +++ b/package/opentracing-cpp/opentracing-cpp.mk > > > @@ -0,0 +1,22 @@ > > > +########################################################### > > ##################### > > > +# > > > +# opentracing-cpp > > > +# > > > +########################################################### > > ##################### > > > + > > > +OPENTRACING_CPP_VERSION = v1.2.0 > > > +OPENTRACING_CPP_SITE = $(call github,opentracing, > > opentracing-cpp,$(OPENTRACING_CPP_VERSION)) > > > +OPENTRACING_CPP_LICENSE = MIT > > > +OPENTRACING_CPP_LICENSE_FILES = COPYING > > > + > > > +OPENTRACING_CPP_INSTALL_STAGING = YES > > > + > > > +ifeq ($(BR2_STATIC_LIBS),y) > > > +OPENTRACING_CPP_CONF_OPTS += -DBUILD_SHARED_LIBS=OFF > > -DBUILD_STATIC_LIBS=ON > > > +else ifeq ($(BR2_SHARED_STATIC_LIBS),y) > > > +OPENTRACING_CPP_CONF_OPTS += -DBUILD_SHARED_LIBS=ON > > -DBUILD_STATIC_LIBS=ON > > > +else ifeq ($(BR2_SHARED_LIBS),y) > > > +OPENTRACING_CPP_CONF_OPTS += -DBUILD_SHARED_LIBS=ON > > -DBUILD_STATIC_LIBS=OFF > > > +endif > > > > The -DBUILD_SHARED_LIBS setting should be handled correctly by the > > Buildroot > > cmake infrastructure at package/pkg-cmake.mk. Doesn't that work for you? > > > > baruch > > > > > + > > > +$(eval $(cmake-package))
Hello, On Wed, 3 Jan 2018 11:50:32 +0200, Baruch Siach wrote: > Hi Jan, > > On Wed, Jan 03, 2018 at 10:07:51AM +0100, Jan Heylen wrote: > > I copied that infra from another package. Thomas also indicated already > > that there is something already generalised but not fully yet. So he > > adviced to keep it like this for the time being. If this is not nok, please > > advice differently. > > I see that rabbitmq-c is also doing that. I think that the -DBUILD_SHARED_LIBS > part is redundant. But the -DBUILD_STATIC_LIBS part might be needed, since the > generic code does not handle that. The generic code in pkg-cmake.mk is indeed not very consistent. It is strange to pass BUILD_SHARED_LIBS, but not BUILD_STATIC_LIBS. On the other hand, there doesn't seem to be a real standardization of such options in the CMake world. I.e it's really up to each individual package to obey to BUILD_SHARED_LIBS/BUILD_STATIC_LIBS, and many CMake packages have their own custom options to enable/disable shared/static library build. But I guess for the sake of consistency pkg-cmake.mk should also pass BUILD_STATIC_LIBS, and we should accordingly cleanup the CMake packages that rely on BUILD_SHARED_LIBS/BUILD_STATIC_LIBS to not pass them explicitly, and rely on what the pkg-cmake infra is doing. Best regards, Thomas
Hi all, Apologies for being late back in this thread :-/ On Wed, Jan 3, 2018 at 11:10 AM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > Hello, > > On Wed, 3 Jan 2018 11:50:32 +0200, Baruch Siach wrote: >> Hi Jan, >> >> On Wed, Jan 03, 2018 at 10:07:51AM +0100, Jan Heylen wrote: >> > I copied that infra from another package. Thomas also indicated already >> > that there is something already generalised but not fully yet. So he >> > adviced to keep it like this for the time being. If this is not nok, please >> > advice differently. >> >> I see that rabbitmq-c is also doing that. I think that the -DBUILD_SHARED_LIBS >> part is redundant. But the -DBUILD_STATIC_LIBS part might be needed, since the >> generic code does not handle that. > > The generic code in pkg-cmake.mk is indeed not very consistent. It is > strange to pass BUILD_SHARED_LIBS, but not BUILD_STATIC_LIBS. Well, I think the code in pkg-cmake.mk has been done to stick as close as possible to what is the default behavior of CMake. But, it is true its default behavior is not really consistent since CMake only offers a boolean variable driving the way the libraries are built (see [1]) > > On the other hand, there doesn't seem to be a real standardization of > such options in the CMake world. I.e it's really up to each individual > package to obey to BUILD_SHARED_LIBS/BUILD_STATIC_LIBS, and many CMake > packages have their own custom options to enable/disable shared/static > library build. > > But I guess for the sake of consistency pkg-cmake.mk should also pass > BUILD_STATIC_LIBS, and we should accordingly cleanup the CMake packages > that rely on BUILD_SHARED_LIBS/BUILD_STATIC_LIBS to not pass them > explicitly, and rely on what the pkg-cmake infra is doing. Adding BUILD_STATIC_LIBS will make thing a bit nicer (at least symetric), though it is not standard in any way, which means some projects may choose another name for a similar option and there is not much we can do about this. BTW, some projects still seems ignoring the existence of the BUILD_SHARED_LIBS variable since they add a similar option with a different name. [1] https://cmake.org/cmake/help/v3.8/manual/cmake-variables.7.html#variables-that-change-behavior Regards,
Hello, On Wed, 3 Jan 2018 21:37:03 +0100, Samuel Martin wrote: > > On the other hand, there doesn't seem to be a real standardization of > > such options in the CMake world. I.e it's really up to each individual > > package to obey to BUILD_SHARED_LIBS/BUILD_STATIC_LIBS, and many CMake > > packages have their own custom options to enable/disable shared/static > > library build. > > > > But I guess for the sake of consistency pkg-cmake.mk should also pass > > BUILD_STATIC_LIBS, and we should accordingly cleanup the CMake packages > > that rely on BUILD_SHARED_LIBS/BUILD_STATIC_LIBS to not pass them > > explicitly, and rely on what the pkg-cmake infra is doing. > > Adding BUILD_STATIC_LIBS will make thing a bit nicer (at least > symetric), though it is not standard in any way, which means some > projects may choose another name for a similar option and there is not > much we can do about this. > > BTW, some projects still seems ignoring the existence of the > BUILD_SHARED_LIBS variable since they add a similar option with a > different name. > > > [1] https://cmake.org/cmake/help/v3.8/manual/cmake-variables.7.html#variables-that-change-behavior So BUILD_SHARED_LIBS is somewhat standardized by CMake, but not BUILD_STATIC_LIBS. If that's the case, then we should not handle BUILD_STATIC_LIBS in pkg-cmake.mk, contrary to what I said previously. And we should add a comment explaining this in pkg-cmake.mk. Thomas
diff --git a/DEVELOPERS b/DEVELOPERS index fe989c0..f17fcdd 100644 --- a/DEVELOPERS +++ b/DEVELOPERS @@ -790,6 +790,9 @@ F: package/pangomm/ F: package/rpm/ F: package/yad/ +N: Jan Heylen <jan.heylen@nokia.com> +F: package/opentracing-cpp/ + N: Jan Kraval <jan.kraval@gmail.com> F: board/orangepi/orangepi-lite F: configs/orangepi_lite_defconfig diff --git a/package/Config.in b/package/Config.in index bd39a37..1150a3f 100644 --- a/package/Config.in +++ b/package/Config.in @@ -1288,6 +1288,7 @@ menu "Logging" source "package/log4cplus/Config.in" source "package/log4cpp/Config.in" source "package/log4cxx/Config.in" + source "package/opentracing-cpp/Config.in" source "package/zlog/Config.in" endmenu diff --git a/package/opentracing-cpp/0001-CMake-make-shared-static-target-a-configurable-optio.patch b/package/opentracing-cpp/0001-CMake-make-shared-static-target-a-configurable-optio.patch new file mode 100644 index 0000000..90a945b --- /dev/null +++ b/package/opentracing-cpp/0001-CMake-make-shared-static-target-a-configurable-optio.patch @@ -0,0 +1,74 @@ +From 9462847f23a25524fdc2112cbc8de3f2c02a1669 Mon Sep 17 00:00:00 2001 +From: Jan Heylen <jan.heylen@nokia.com> +Date: Fri, 22 Dec 2017 22:04:29 +0100 +Subject: [PATCH] CMake: make shared/static target a configurable option + +Signed-off-by: Jan Heylen <jan.heylen@nokia.com> +--- + CMakeLists.txt | 40 ++++++++++++++++++++++++++++------------ + 1 file changed, 28 insertions(+), 12 deletions(-) + +diff --git a/CMakeLists.txt b/CMakeLists.txt +index aadf2f9..d03bd00 100644 +--- a/CMakeLists.txt ++++ b/CMakeLists.txt +@@ -70,18 +70,36 @@ endif() + include_directories(include) + include_directories(SYSTEM 3rd_party/include) + ++option(BUILD_SHARED_LIBS "Build as a shared library" ON) ++option(BUILD_STATIC_LIBS "Build as a static library" ON) ++ ++if (NOT BUILD_SHARED_LIBS AND NOT BUILD_STATIC_LIBS) ++ message(FATAL_ERROR "One or both of BUILD_SHARED_LIBS or BUILD_STATIC_LIBS must be set to ON to build") ++endif() ++ + set(SRCS src/propagation.cpp src/noop.cpp src/tracer.cpp) +-add_library(opentracing SHARED ${SRCS}) +-target_include_directories(opentracing INTERFACE "$<INSTALL_INTERFACE:include/>") +-set_target_properties(opentracing PROPERTIES VERSION ${OPENTRACING_VERSION_STRING} ++ ++if (BUILD_SHARED_LIBS) ++ add_library(opentracing SHARED ${SRCS}) ++ target_include_directories(opentracing INTERFACE "$<INSTALL_INTERFACE:include/>") ++ set_target_properties(opentracing PROPERTIES VERSION ${OPENTRACING_VERSION_STRING} + SOVERSION ${OPENTRACING_VERSION_MAJOR}) +-add_library(opentracing-static STATIC ${SRCS}) +-set_target_properties(opentracing-static PROPERTIES OUTPUT_NAME opentracing) +-target_include_directories(opentracing-static INTERFACE "$<INSTALL_INTERFACE:include/>") +-if (CLANG_TIDY_EXE) +- set_target_properties(opentracing PROPERTIES ++ install(TARGETS opentracing EXPORT OpenTracingTargets ++ LIBRARY DESTINATION lib ++ ARCHIVE DESTINATION lib) ++ if (CLANG_TIDY_EXE) ++ set_target_properties(opentracing PROPERTIES + CXX_CLANG_TIDY "${DO_CLANG_TIDY}") +-endif() ++ endif() ++endif(BUILD_SHARED_LIBS) ++ ++if (BUILD_STATIC_LIBS) ++ add_library(opentracing-static STATIC ${SRCS}) ++ set_target_properties(opentracing-static PROPERTIES OUTPUT_NAME opentracing) ++ target_include_directories(opentracing-static INTERFACE "$<INSTALL_INTERFACE:include/>") ++ install(TARGETS opentracing-static EXPORT OpenTracingTargets ++ ARCHIVE DESTINATION lib) ++endif(BUILD_STATIC_LIBS) + + + install(DIRECTORY 3rd_party/include/opentracing DESTINATION include +@@ -89,9 +107,7 @@ install(DIRECTORY 3rd_party/include/opentracing DESTINATION include + PATTERN "*.h") + install(DIRECTORY include/opentracing DESTINATION include + FILES_MATCHING PATTERN "*.h") +-install(TARGETS opentracing opentracing-static EXPORT OpenTracingTargets +- LIBRARY DESTINATION lib +- ARCHIVE DESTINATION lib) ++ + + # ============================================================================== + # Package configuration setup +-- +2.7.4 + diff --git a/package/opentracing-cpp/Config.in b/package/opentracing-cpp/Config.in new file mode 100644 index 0000000..f13d21b --- /dev/null +++ b/package/opentracing-cpp/Config.in @@ -0,0 +1,16 @@ +config BR2_PACKAGE_OPENTRACING_CPP + bool "opentracing-cpp" + depends on BR2_INSTALL_LIBSTDCPP + depends on BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 # C++11 + depends on !BR2_TOOLCHAIN_HAS_GCC_BUG_64735 # exception_ptr + depends on !BR2_m68k_cf # exception_ptr + help + OpenTracing API for C++ + + http://opentracing.io + +comment "opentracing-cpp needs a toolchain w/ C++11" + depends on !BR2_INSTALL_LIBSTDCPP || !BR2_TOOLCHAIN_GCC_AT_LEAST_4_8 + +comment "opentracing-cpp needs exception_ptr" + depends on BR2_TOOLCHAIN_HAS_GCC_BUG_64735 || BR2_m68k_cf diff --git a/package/opentracing-cpp/opentracing-cpp.hash b/package/opentracing-cpp/opentracing-cpp.hash new file mode 100644 index 0000000..d25dbaf --- /dev/null +++ b/package/opentracing-cpp/opentracing-cpp.hash @@ -0,0 +1,3 @@ +# Locally calculated +sha256 c77041cb2f147ac81b2b0702abfced5565a9cebc318d045c060a4c3e074009ee opentracing-cpp-v1.2.0.tar.gz +sha256 b80bffcfee825a69645f7ca97ddba48714031ea5c845198d184714d5490798b6 COPYING diff --git a/package/opentracing-cpp/opentracing-cpp.mk b/package/opentracing-cpp/opentracing-cpp.mk new file mode 100644 index 0000000..9b2d473 --- /dev/null +++ b/package/opentracing-cpp/opentracing-cpp.mk @@ -0,0 +1,22 @@ +################################################################################ +# +# opentracing-cpp +# +################################################################################ + +OPENTRACING_CPP_VERSION = v1.2.0 +OPENTRACING_CPP_SITE = $(call github,opentracing,opentracing-cpp,$(OPENTRACING_CPP_VERSION)) +OPENTRACING_CPP_LICENSE = MIT +OPENTRACING_CPP_LICENSE_FILES = COPYING + +OPENTRACING_CPP_INSTALL_STAGING = YES + +ifeq ($(BR2_STATIC_LIBS),y) +OPENTRACING_CPP_CONF_OPTS += -DBUILD_SHARED_LIBS=OFF -DBUILD_STATIC_LIBS=ON +else ifeq ($(BR2_SHARED_STATIC_LIBS),y) +OPENTRACING_CPP_CONF_OPTS += -DBUILD_SHARED_LIBS=ON -DBUILD_STATIC_LIBS=ON +else ifeq ($(BR2_SHARED_LIBS),y) +OPENTRACING_CPP_CONF_OPTS += -DBUILD_SHARED_LIBS=ON -DBUILD_STATIC_LIBS=OFF +endif + +$(eval $(cmake-package))