sdbus-cpp/0.8.3 #4148

bobrofon · 2021-01-07T09:51:18Z

Specify library name and version: sdbus-cpp/0.8.3

I've read the guidelines for contributing.
I've followed the PEP8 style guides for Python code in the recipes.
I've used the latest Conan client version.
I've tried at least one configuration locally with the
conan-center hook activated.

Croydon · 2021-01-07T09:56:46Z

recipes/sdbus-cpp/all/conanfile.py

+        tools.check_min_cppstd(self, "17")
+
+    def build_requirements(self):
+        self.build_requires("cmake/3.19.1")


We usually don't require CMake as this is one of very few dependencies we pre-require. Exceptions can be made when the CMake version required is very recent or only supports a specific old versions or something like that.

This projects seems to be fine with CMake >= 3.13.0 so I would argue that it should be removed here

https://github.com/Kistler-Group/sdbus-cpp/blob/d6fdacafbe5e490d540956fe78f8df808723f49e/CMakeLists.txt#L5

conan-center-bot · 2021-01-07T09:57:11Z

Failure in build 1 (53d7b452dffe018b62ea6741359ea52d4f097c2f):

sdbus-cpp/0.8.3:
- Zero packages generated. Looks like all configurations have been discarded by ConanInvalidConfiguration exceptions

recipes/sdbus-cpp/all/conanfile.py

conan-center-bot · 2021-01-07T13:35:55Z

Failure in build 2 (1cc1fb3e2339de44e4998031f6cde4feb97f0ee6):

sdbus-cpp/0.8.3:
- Zero packages generated. Looks like all configurations have been discarded by ConanInvalidConfiguration exceptions

prince-chrismc · 2021-01-07T22:26:21Z

recipes/sdbus-cpp/all/conanfile.py

+            raise ConanInvalidConfiguration("Only Linux supported")
+        if self.options.shared:
+            del self.options.fPIC
+        tools.check_min_cppstd(self, "17")


This needs an if check and some boilerplate

You can copy this conan-io/conan#8002 (comment)

prince-chrismc · 2021-01-07T22:28:44Z

recipes/sdbus-cpp/all/conanfile.py

+        self._cmake.definitions["BUILD_TESTS"] = False
+        self._cmake.definitions["BUILD_LIBSYSTEMD"] = False
+        self._cmake.configure(source_folder=self._source_subfolder,
+                              build_folder=self._build_subfolder)


It looks like you are missing the cmake wrapper which is required to inject the right flags

prince-chrismc · 2021-01-07T22:30:12Z

recipes/sdbus-cpp/all/test_package/CMakeLists.txt

+conan_basic_setup()
+
+add_executable(example example.cpp)
+target_link_libraries(example ${CONAN_LIBS})


It would be nice to test the cmake components you added

conan-center-bot · 2021-01-08T07:01:58Z

All green in build 3 (898c3397faa6d4e311c083ae5ec094de73f2285b)! 😊

sdbus-cpp/0.8.3: Generated 56 packages. All logs here

recipes/sdbus-cpp/all/conanfile.py

uilianries · 2021-01-08T13:02:32Z

recipes/sdbus-cpp/all/conanfile.py

+    def configure(self):
+        if self.settings.os != "Linux":
+            raise ConanInvalidConfiguration("Only Linux supported")
+        if self.options.shared:
+            del self.options.fPIC


Suggested change

def configure(self):

if self.settings.os != "Linux":

raise ConanInvalidConfiguration("Only Linux supported")

if self.options.shared:

del self.options.fPIC

def configure(self):

if self.options.shared:

del self.options.fPIC

def validate(self):

if self.settings.os != "Linux":

raise ConanInvalidConfiguration("Only Linux supported")

https://blog.conan.io/2020/12/14/New-conan-release-1-32.html

uilianries · 2021-01-08T13:03:45Z

recipes/sdbus-cpp/all/conanfile.py

+        self._cmake.configure()
+        return self._cmake
+
+    def fix_tools_cmake(self):


Suggested change

def fix_tools_cmake(self):

def _patch_sources(self):

Any custom method should be declared as protected to avoid future name collisions.

_patch_sources is no official name or a pattern, but it's the most popular in CCI.

uilianries · 2021-01-08T13:05:13Z

recipes/sdbus-cpp/all/conanfile.py

+            # FIXME: remove after
+            #  https://github.com/Kistler-Group/sdbus-cpp/pull/136
+            self.fix_tools_cmake()


Suggested change

# FIXME: remove after

# https://github.com/Kistler-Group/sdbus-cpp/pull/136

self.fix_tools_cmake()

# FIXME: remove after https://github.com/Kistler-Group/sdbus-cpp/pull/136

self._patch_sources()

Try to keep fixme, todo, xxx as single line. It's printed as warning on output, it's easier to be visualized.

uilianries · 2021-01-08T13:07:52Z

recipes/sdbus-cpp/all/conanfile.py

+        self.cpp_info.components["sdbus-c++"].names["cmake_find_package_multi"] = "sdbus-c++"
+        self.cpp_info.components["sdbus-c++"].names["pkg_config"] = "sdbus-c++"
+        if self.options.with_code_gen:
+            bin_path = os.path.join(self.package_folder, "bin")


Suggested change

bin_path = os.path.join(self.package_folder, "bin")

bin_path = os.path.join(self.package_folder, "bin")

self.output.info("Appending PATH env var with : {}".format(bin_path))

That's a common pattern when adding something to PATH.

conan-center-bot · 2021-01-08T13:49:19Z

All green in build 4 (679335ebbb17345f1fc65d474766c50d7c84afec)! 😊

sdbus-cpp/0.8.3: Generated 56 packages. All logs here

recipes/sdbus-cpp/all/conanfile.py

Co-authored-by: Uilian Ries <[email protected]>

Croydon reviewed Jan 7, 2021

View reviewed changes

conan-center-bot added the Failed label Jan 7, 2021

Croydon reviewed Jan 7, 2021

View reviewed changes

recipes/sdbus-cpp/all/conanfile.py Outdated Show resolved Hide resolved

prince-chrismc reviewed Jan 7, 2021

View reviewed changes

sdbus-cpp/0.8.3

898c339

bobrofon force-pushed the sdbus-cpp/0.8.3 branch from 1cc1fb3 to 898c339 Compare January 8, 2021 06:45

conan-center-bot removed the Failed label Jan 8, 2021

uilianries reviewed Jan 8, 2021

View reviewed changes

sdbus-cpp: Review fixes

679335e

prince-chrismc previously approved these changes Jan 8, 2021

View reviewed changes

github-actions bot mentioned this pull request Jan 8, 2021

Pull Requests Ready to be Reviewed prince-chrismc/conan-center-index-pending-review#1

Open

uilianries reviewed Jan 14, 2021

View reviewed changes

recipes/sdbus-cpp/all/conanfile.py Outdated Show resolved Hide resolved

Move compiller check to validate phase

27245e1

Co-authored-by: Uilian Ries <[email protected]>

bobrofon dismissed prince-chrismc’s stale review via 27245e1 January 17, 2021 08:49

prince-chrismc approved these changes Jan 17, 2021

View reviewed changes

uilianries approved these changes Jan 18, 2021

View reviewed changes

SSE4 approved these changes Jan 19, 2021

View reviewed changes

conan-center-bot merged commit 1589128 into conan-io:master Jan 19, 2021

bobrofon deleted the sdbus-cpp/0.8.3 branch January 25, 2021 05:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sdbus-cpp/0.8.3 #4148

sdbus-cpp/0.8.3 #4148

bobrofon commented Jan 7, 2021

Croydon Jan 7, 2021

conan-center-bot commented Jan 7, 2021

conan-center-bot commented Jan 7, 2021

prince-chrismc Jan 7, 2021

prince-chrismc Jan 7, 2021

prince-chrismc Jan 7, 2021

conan-center-bot commented Jan 8, 2021

uilianries Jan 8, 2021

uilianries Jan 8, 2021

uilianries Jan 8, 2021

uilianries Jan 8, 2021

conan-center-bot commented Jan 8, 2021

	bin_path = os.path.join(self.package_folder, "bin")
	bin_path = os.path.join(self.package_folder, "bin")
	self.output.info("Appending PATH env var with : {}".format(bin_path))

sdbus-cpp/0.8.3 #4148

sdbus-cpp/0.8.3 #4148

Conversation

bobrofon commented Jan 7, 2021

Choose a reason for hiding this comment

conan-center-bot commented Jan 7, 2021

conan-center-bot commented Jan 7, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

conan-center-bot commented Jan 8, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

conan-center-bot commented Jan 8, 2021