Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add recipe for dbus-cxx #24251

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add recipe for dbus-cxx #24251

wants to merge 1 commit into from

Conversation

ErikHons
Copy link

@ErikHons ErikHons commented Jun 6, 2024

dbus-cxx/2.5.1

At NI we are using conan as our C++ packaging tool. We dbus-cxx for native C++ bindings to DBus with a path to Windows support.


@CLAassistant
Copy link

CLAassistant commented Jun 6, 2024

CLA assistant check
All committers have signed the CLA.

@conan-center-bot

This comment has been minimized.

@AbrilRBS AbrilRBS self-assigned this Jun 6, 2024
@AbrilRBS
Copy link
Member

AbrilRBS commented Jun 6, 2024

Hi @ErikHons thanks a lot for taking the time to submit the recipe, we appreciate it :)

I was going to submit some cleanups for the recipe, but I realized that you didn't grant us permission to directly push, so here is the patch (Have no time to open as PR, sorry!)

diff --git a/recipes/dbus-cxx/2.x.x/conanfile.py b/recipes/dbus-cxx/2.x.x/conanfile.py
index 50d6b6324..ec1f5e99c 100644
--- a/recipes/dbus-cxx/2.x.x/conanfile.py
+++ b/recipes/dbus-cxx/2.x.x/conanfile.py
@@ -1,13 +1,14 @@
 from conan import ConanFile
-from conan.errors import ConanInvalidConfiguration
-from conan.tools.cmake import cmake_layout, CMake, CMakeToolchain
+from conan.tools.build import check_min_cppstd
+from conan.tools.cmake import cmake_layout, CMake, CMakeToolchain, CMakeDeps
+from conan.tools.files import get
 from conan.tools.scm import Version
-import conan.tools.files
+
 
 class DbusCXX(ConanFile):
     name = "dbus-cxx"
-    license = "LGPL-3.0-only, BSD-3-Clause"
-    url = "https://github.com/dbus-cxx/dbus-cxx"
+    license = "LGPL-3.0-only", "BSD-3-Clause"
+    url = "https://github.com/conan-io/conan-center-index"
     homepage = "http://dbus-cxx.github.io"
     description = "DBus-cxx provides an object-oriented interface to DBus"
     topics = "bus", "interprocess", "message"
@@ -28,30 +29,34 @@ class DbusCXX(ConanFile):
         "glib_support": False,
         "qt_support": False,
         "uv_support": False,
-   }
+    }
+
+    @property
+    def _min_cppstd(self):
+        return 17
 
     def config_options(self):
         if self.settings.os == "Windows":
             del self.options.fPIC
+        if Version(self.version) < 2.5.0:
+            del self.options.uv_support
 
     def validate(self):
-        if self.options.uv_support and Version(self.version) < "2.5.0":
-            raise ConanInvalidConfiguration("UV support requires verion >= 2.5.0")
+        if self.settings.compiler.get_safe("cppstd"):
+            check_min_cppstd(self, self._min_cppstd)
 
     def requirements(self):
-        self.requires("libsigcpp/[^3.0.7]", transitive_headers=True)
-        self.requires("expat/[^2.5.0]")
+        self.requires("libsigcpp/3.0.7", transitive_headers=True)
+        self.requires("expat/[>=2.6.2 <3]")
 
         if self.options.uv_support:
             self.requires("libuv/[>=1.0 <2.0]")
 
     def source(self):
-        conan.tools.files.get(self, **self.conan_data["sources"][self.version], strip_root=True)
+        get(self, **self.conan_data["sources"][self.version], strip_root=True)
 
     def layout(self):
-        cmake_layout(self, src_folder="src")
-
-    generators = "PkgConfigDeps"
+        cmake_layout(self)
 
     def generate(self):
         tc = CMakeToolchain(self)
@@ -60,6 +65,8 @@ class DbusCXX(ConanFile):
         if Version(self.version) >= "2.5.0":
             tc.cache_variables["ENABLE_UV_SUPPORT"] = self.options.uv_support
         tc.generate()
+        deps = CMakeDeps(self)
+        deps.generate()
 
     def build(self):
         cmake = CMake(self)

Some comments:

  • I've removed the generators = "PkgConfigDeps" and used CMakeDeps in the generate method, but this stumped me a bit. Why do you need pkgconfigdeps? Any insight on that?
  • As per https://github.com/conan-io/conan-center-index/blob/master/docs/adding_packages/dependencies.md#version-ranges I've pinned and changed the version ranges you added, except for libuv, which we're currently discussing on whether to allow it as version range or not, will let you know once we come to a conclusion
  • Instead of validating bad versions depending on the version, it's better if we outright delete the option for those versions where it does not exist - We only need to take care of guarding with the same logic its access in the rest of the recipe, or using `self.options.get_safe("uv_support")

Also some more question:

  • You have a mismatched config.yml and conandata.yml files, both files in this case should contain the same versions
  • I see the qt_support option being defined, but not used in the recipe
  • usually for options that toggle optional dependencies, the convention in CCI has been to call them with_xxx, so uv_support would be with_uv

@ErikHons
Copy link
Author

ErikHons commented Jun 6, 2024

Hi @ErikHons thanks a lot for taking the time to submit the recipe, we appreciate it :)

I love conan. Glad to give back!

I was going to submit some cleanups for the recipe, but I realized that you didn't grant us permission to directly push, so here is the patch (Have no time to open as PR, sorry!)

Yeah. We run our corporate builds off that fork. I'm happy to apply the patch.

Some comments:

  • I've removed the generators = "PkgConfigDeps" and used CMakeDeps in the generate method, but this stumped me a bit. Why do you need pkgconfigdeps? Any insight on that?

The library uses pkg_check_modules() by default. It's supposed to use find_package() when a toolchain is specified bit that wasn't working for me. I'll take another look.

I've pinned and changed the version ranges you added, except for libuv, which we're currently discussing on whether to allow it as version range or not, will let you know once we come to a conclusion

I don't grok how that all works. My main concern is that libuv 1.45 drops support for an old version of RedHat I have to build for still. I think I can override all that in the client build though right?

Instead of validating bad versions depending on the version, it's better if we outright delete the option for those versions where it does not exist

Makes sense!

You have a mismatched config.yml and conandata.yml files, both files in this case should contain the same versions

Oopise. I'm working off a branch of 2.4.0 (we patch this recipe in our fork of the index). I added 2.5.1 for this PR and just missed this step.

I see the qt_support option being defined, but not used in the recipe

That option sets the ENABLE_QT_SUPPORT cmake variable and adds the dbus-cxx-qt library to self.cpp_info.libs

usually for options that toggle optional dependencies, the convention in CCI has been to call them with_xxx, so uv_support would be with_uv

Nice. I'll change it.

Thanks for the quick response!

@ErikHons
Copy link
Author

ErikHons commented Jun 6, 2024

@RubenRBS the dbus-cxx top-level cmake contains stuff like:

if( CMAKE_TOOLCHAIN_FILE )
find_package( sigc++-3 REQUIRED )
else()
pkg_check_modules( sigc REQUIRED sigc++-3.0 )
endif( CMAKE_TOOLCHAIN_FILE )

Conan passes the -DCMAKE_TOOLCHAIN_FILE="..." parameter that should define that cmake variable but cmake keeps using the pkg-config path. I'm still looking at it.

Edit: Nevermind. This got fixed in 2.5.1. I'll make this change too.

Edit: Nevermind all of that. My guys added the condition above in our fork of dbus-cxx, and it's broke. The recipe uses PkgConfigDeps because dbus-cxx uses that!

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@ErikHons
Copy link
Author

ErikHons commented Jun 7, 2024

Hey @RubenRBS, I see that the "Linux, GCC-legacy" action is failing on the gcc 7 build but not why. When I first created a docker image to test that case it failed because the pkg-config tools weren't installed. After adding the pkgconf package the build works.

Is it possible that your pipelines don't have the pkgconf package installed? If so, what do you recommend going forward: update CCI pipelines, patch dbus-cxx in the recipe, something else?

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ❌

Failure in build 8 (d39f3bc9f3e1bf23b83fb966bb3755784d5292bf):

  • dbus-cxx/2.5.1:
    CI failed to create some packages (All logs)

    Logs for packageID 834ea2cc25954a55a16f54099256bddae09e5465:
    [settings]
    arch=x86_64
    build_type=Release
    compiler=gcc
    compiler.libcxx=libstdc++11
    compiler.version=9
    os=Linux
    
    [...]
    [ 37%] Building CXX object CMakeFiles/dbus-cxx.dir/dbus-cxx/sendmsgtransport.cpp.o
    [ 39%] Building CXX object CMakeFiles/dbus-cxx.dir/dbus-cxx/transport.cpp.o
    [ 40%] Building CXX object CMakeFiles/dbus-cxx.dir/dbus-cxx/threaddispatcher.cpp.o
    [ 41%] Building CXX object CMakeFiles/dbus-cxx.dir/dbus-cxx/sasl.cpp.o
    [ 43%] Building CXX object CMakeFiles/dbus-cxx.dir/dbus-cxx/validator.cpp.o
    [ 44%] Building CXX object CMakeFiles/dbus-cxx.dir/dbus-cxx/daemon-proxy/DBusDaemonProxy.cpp.o
    [ 45%] Building CXX object CMakeFiles/dbus-cxx.dir/dbus-cxx/variantappenditerator.cpp.o
    [ 46%] Building CXX object CMakeFiles/dbus-cxx.dir/dbus-cxx/variantiterator.cpp.o
    [ 48%] Building CXX object CMakeFiles/dbus-cxx.dir/dbus-cxx/property.cpp.o
    [ 49%] Building CXX object CMakeFiles/dbus-cxx.dir/dbus-cxx/propertyproxy.cpp.o
    [ 50%] Building CXX object CMakeFiles/dbus-cxx.dir/dbus-cxx/matchrule.cpp.o
    [ 51%] Building CXX object CMakeFiles/dbus-cxx.dir/dbus-cxx/standard-interfaces/peerinterfaceproxy.cpp.o
    [ 53%] Building CXX object CMakeFiles/dbus-cxx.dir/dbus-cxx/standard-interfaces/introspectableinterfaceproxy.cpp.o
    [ 54%] Building CXX object CMakeFiles/dbus-cxx.dir/dbus-cxx/standard-interfaces/propertiesinterfaceproxy.cpp.o
    [ 55%] Linking CXX shared library libdbus-cxx.so
    [ 55%] Built target dbus-cxx
    Scanning dependencies of target data-tests
    Scanning dependencies of target introspection-tests
    Scanning dependencies of target signal-tests
    [ 56%] Building CXX object unit-tests/CMakeFiles/data-tests.dir/datatests.cpp.o
    [ 58%] Building CXX object unit-tests/CMakeFiles/signal-tests.dir/signaltests.cpp.o
    [ 59%] Building CXX object unit-tests/CMakeFiles/introspection-tests.dir/introspection-test.cpp.o
    [ 60%] Building CXX object unit-tests/CMakeFiles/data-tests.dir/custom-type.cpp.o
    [ 62%] Linking CXX executable signal-tests
    [ 62%] Built target signal-tests
    [ 63%] Linking CXX executable data-tests
    [ 63%] Built target data-tests
    dbus-cxx/2.5.1: 
    /home/conan/workspace/prod-v1/bsr/90471/ffdee/.conan/data/dbus-cxx/2.5.1/_/_/build/834ea2cc25954a55a16f54099256bddae09e5465/dbus-cxx/utility.cpp: In function ‘void DBus::log_std_err(const char*, const SL_LogLocation*, SL_LogLevel, const char*)’:
    /home/conan/workspace/prod-v1/bsr/90471/ffdee/.conan/data/dbus-cxx/2.5.1/_/_/build/834ea2cc25954a55a16f54099256bddae09e5465/dbus-cxx/utility.cpp:65:35: warning: format ‘%X’ expects argument of type ‘unsigned int’, but argument 4 has type ‘std::thread::id’ [-Wformat=]
       65 |     snprintf( buffer, 4096, "0x%08X %s [%s] - %s(%s:%d)", this_id, logger_name, stringLevel, log_string,
          |                                ~~~^                       ~~~~~~~
          |                                   |                       |
          |                                   unsigned int            std::thread::id
    /home/conan/workspace/prod-v1/bsr/90471/ffdee/.conan/data/dbus-cxx/2.5.1/_/_/build/834ea2cc25954a55a16f54099256bddae09e5465/unit-tests/introspection-test.cpp:23:10: fatal error: expat.h: No such file or directory
       23 | #include <expat.h>
          |          ^~~~~~~~~
    compilation terminated.
    make[2]: *** [unit-tests/CMakeFiles/introspection-tests.dir/build.make:82: unit-tests/CMakeFiles/introspection-tests.dir/introspection-test.cpp.o] Error 1
    make[1]: *** [CMakeFiles/Makefile2:958: unit-tests/CMakeFiles/introspection-tests.dir/all] Error 2
    make[1]: *** Waiting for unfinished jobs....
    make: *** [Makefile:182: all] Error 2
    WARN: *** Conan 1 is legacy and on a deprecation path ***
    WARN: *** Please upgrade to Conan 2 ***
    dbus-cxx/2.5.1: WARN: Using the new toolchains and generators without specifying a build profile (e.g: -pr:b=default) is discouraged and might cause failures and unexpected behavior
    dbus-cxx/2.5.1: ERROR: Package '834ea2cc25954a55a16f54099256bddae09e5465' build failed
    dbus-cxx/2.5.1: WARN: Build folder /home/conan/workspace/prod-v1/bsr/90471/ffdee/.conan/data/dbus-cxx/2.5.1/_/_/build/834ea2cc25954a55a16f54099256bddae09e5465/build/Release
    ERROR: dbus-cxx/2.5.1: Error in build() method, line 74
    	cmake.build()
    	ConanException: Error 2 while executing cmake --build "/home/conan/workspace/prod-v1/bsr/90471/ffdee/.conan/data/dbus-cxx/2.5.1/_/_/build/834ea2cc25954a55a16f54099256bddae09e5465/build/Release" '--' '-j3'
    
  • dbus-cxx/2.4.0:
    Didn't run or was cancelled before finishing


Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability.


Conan v2 pipeline ❌

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

The v2 pipeline failed. Please, review the errors and note this is required for pull requests to be merged. In case this recipe is still not ported to Conan 2.x, please, ping @conan-io/barbarians on the PR and we will help you.

Failure in build 8 (d39f3bc9f3e1bf23b83fb966bb3755784d5292bf):

  • dbus-cxx/2.5.1:
    CI failed to create some packages (All logs)

    Logs for packageID 77c0c65f0429516b71e1162a18554e8def765598:
    [settings]
    arch=x86_64
    build_type=Release
    compiler=gcc
    compiler.cppstd=17
    compiler.libcxx=libstdc++11
    compiler.version=11
    os=Linux
    [options]
    */*:shared=False
    
    [...]
    [ 55%] Linking CXX shared library libdbus-cxx.so
    [ 55%] Built target dbus-cxx
    Scanning dependencies of target customaddress-tests
    Scanning dependencies of target test-connection
    [ 56%] Building CXX object unit-tests/CMakeFiles/customaddress-tests.dir/customaddress-test.cpp.o
    Scanning dependencies of target object-tests
    [ 58%] Building CXX object unit-tests/CMakeFiles/test-connection.dir/connectiontests.cpp.o
    [ 59%] Building CXX object unit-tests/CMakeFiles/object-tests.dir/objecttests.cpp.o
    [ 60%] Linking CXX executable customaddress-tests
    [ 60%] Built target customaddress-tests
    Scanning dependencies of target test-callmessage
    [ 62%] Building CXX object unit-tests/CMakeFiles/test-callmessage.dir/callmessagetests.cpp.o
    [ 63%] Linking CXX executable object-tests
    [ 63%] Built target object-tests
    Scanning dependencies of target test-path
    [ 64%] Building CXX object unit-tests/CMakeFiles/test-path.dir/pathclasstests.cpp.o
    [ 65%] Linking CXX executable test-callmessage
    [ 67%] Linking CXX executable test-connection
    [ 67%] Built target test-callmessage
    Scanning dependencies of target test-messageiterator
    [ 68%] Building CXX object unit-tests/CMakeFiles/test-messageiterator.dir/messageiteratortests.cpp.o
    [ 68%] Built target test-connection
    Scanning dependencies of target test-validation
    [ 69%] Building CXX object unit-tests/CMakeFiles/test-validation.dir/validationtests.cpp.o
    [ 70%] Linking CXX executable test-path
    [ 70%] Built target test-path
    Scanning dependencies of target introspection-tests
    [ 72%] Building CXX object unit-tests/CMakeFiles/introspection-tests.dir/introspection-test.cpp.o
    /home/conan/workspace/prod-v2/bsr/81037/dabff/p/b/dbus-a10fdbd616687/b/unit-tests/introspection-test.cpp:23:10: fatal error: expat.h: No such file or directory
       23 | #include <expat.h>
          |          ^~~~~~~~~
    compilation terminated.
    unit-tests/CMakeFiles/introspection-tests.dir/build.make:62: recipe for target 'unit-tests/CMakeFiles/introspection-tests.dir/introspection-test.cpp.o' failed
    make[2]: *** [unit-tests/CMakeFiles/introspection-tests.dir/introspection-test.cpp.o] Error 1
    CMakeFiles/Makefile2:1166: recipe for target 'unit-tests/CMakeFiles/introspection-tests.dir/all' failed
    make[1]: *** [unit-tests/CMakeFiles/introspection-tests.dir/all] Error 2
    make[1]: *** Waiting for unfinished jobs....
    [ 73%] Linking CXX executable test-validation
    [ 73%] Built target test-validation
    [ 74%] Linking CXX executable test-messageiterator
    [ 74%] Built target test-messageiterator
    Makefile:162: recipe for target 'all' failed
    make: *** [all] Error 2
    
    dbus-cxx/2.5.1: ERROR: 
    Package '77c0c65f0429516b71e1162a18554e8def765598' build failed
    dbus-cxx/2.5.1: WARN: Build folder /home/conan/workspace/prod-v2/bsr/81037/dabff/p/b/dbus-a10fdbd616687/b/build/Release
    ERROR: dbus-cxx/2.5.1: Error in build() method, line 74
    	cmake.build()
    	ConanException: Error 2 while executing
    
  • dbus-cxx/2.4.0:
    Didn't run or was cancelled before finishing


Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability.

@ErikHons
Copy link
Author

All checks have passed!!!!!!! Woo!!!!

homepage = "http://dbus-cxx.github.io"
description = "DBus-cxx provides an object-oriented interface to DBus"
topics = "bus", "interprocess", "message"
package_type = "static-library"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this correct? this seems to be unconditionally shared
https://github.com/dbus-cxx/dbus-cxx/blob/master/CMakeLists.txt#L250

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants