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

fix bugs in igntion-tools recipe #11309

Closed
wants to merge 27 commits into from
Closed
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
755c364
fix: bugs in igntion-tools recipe
ggulgulia Jun 21, 2022
8d0da44
use correct ruby from CCI
ggulgulia Jun 21, 2022
5d4bfef
fix: hook errors, remove include paths in package info
ggulgulia Jun 21, 2022
746d389
Update conanfile.py
ggulgulia Jun 21, 2022
a2b8f6b
test with find_package_multi
ggulgulia Jun 21, 2022
492fff5
test with find_package_multi
ggulgulia Jun 21, 2022
6b5a21a
fix: specify empty include dir
ggulgulia Jun 24, 2022
a199f37
fix: not building for gcc 11 compiler
ggulgulia Jun 24, 2022
22a6191
fix: not building for gcc 11 compiler, again
ggulgulia Jun 24, 2022
5a2203d
fix: not building for gcc >= 11 compiler
ggulgulia Jun 24, 2022
eb98ff0
fix: use tools.Version to compare versions numbers
ggulgulia Jun 24, 2022
5acf4f6
fix: limit clang compiler < 12
ggulgulia Jun 24, 2022
db3772e
fix: limit build to apple clang less < 13
ggulgulia Jun 24, 2022
27a7f97
fix: check min cpp std in validate method
ggulgulia Jun 28, 2022
972cd5a
chore: apply review suggestions
ggulgulia Jun 30, 2022
432f607
fix: remove ruby requirement
ggulgulia Jul 12, 2022
862cede
fix: remove ruby requirement
ggulgulia Jul 12, 2022
7a71c2b
fix: component names
ggulgulia Jul 19, 2022
25ca98c
fix: ignore linting test package for conan 2.0
ggulgulia Jul 19, 2022
645a507
fix: add pylint to skip linting
ggulgulia Jul 19, 2022
2efe7cd
fix: test package name
ggulgulia Jul 19, 2022
8379fd7
building with gcc-11, clang-12 and apple clang-13
ggulgulia Jul 21, 2022
566276a
remove commeted lines from validate method
ggulgulia Jul 21, 2022
ae68bd3
chore: clean package method in recipe
ggulgulia Jul 23, 2022
dfd79a3
chore: apply reivew comments
ggulgulia Jul 26, 2022
3b54cde
fix: package_info remove cmake_paths
ggulgulia Jul 26, 2022
1ff89f4
remove extra test package
ggulgulia Jul 29, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions recipes/ignition-tools/all/conandata.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
sources:
"1.4.0":
url: "https://github.com/ignitionrobotics/ign-tools/archive/refs/tags/ignition-tools_1.4.0.tar.gz"
sha256: "fa3f7984ebb8f412133ea93368395adce426ae36c715a9f94f9509af7dac3b03"
url: "https://github.com/gazebosim/gz-tools/archive/refs/tags/ignition-tools_1.4.0.tar.gz"
sha256: "8ceebefcd1977dc2e26beede347a9c06d851b89ec0fd7d5c86126f43a49ac178"
Copy link
Contributor

Choose a reason for hiding this comment

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

@jcar87 We have another check sum change for you to label 👏

patches:
"1.4.0":
- base_path: "source_subfolder"
Expand Down
70 changes: 55 additions & 15 deletions recipes/ignition-tools/all/conanfile.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
import os
from conans import CMake, ConanFile, tools
from conans.errors import ConanInvalidConfiguration
import conan.tools.files
import textwrap

required_conan_version = ">=1.29.1"
ggulgulia marked this conversation as resolved.
Show resolved Hide resolved


class IgnitionToolsConan(ConanFile):
name = "ignition-tools"
license = "Apache-2.0"
homepage = "https://ignitionrobotics.org/libs/tools"
homepage = "https://gazebosim.org/libs/tools"
url = "https://github.com/conan-io/conan-center-index"
description = "Provides general purpose classes and functions designed for robotic applications.."
topics = ("ignition", "robotics", "tools")
settings = "os", "compiler", "build_type", "arch"
options = {"shared": [True, False], "fPIC": [True, False]}
default_options = {"shared": False, "fPIC": True}
generators = "cmake", "cmake_find_package_multi"
generators = "cmake", "cmake_find_package"
exports_sources = "CMakeLists.txt", "patches/**"
_cmake = None

Expand Down Expand Up @@ -50,9 +52,7 @@ def validate(self):
min_version = self._minimum_compilers_version.get(str(self.settings.compiler))
if not min_version:
self.output.warn(
"{} recipe lacks information about the {} compiler support.".format(
self.name, self.settings.compiler
)
f"{self.name} recipe lacks information about the {self.settings.compiler} compiler support."
)
else:
if tools.Version(self.settings.compiler.version) < min_version:
Expand All @@ -64,8 +64,12 @@ def validate(self):
)
)

def requirements(self):
pass
ggulgulia marked this conversation as resolved.
Show resolved Hide resolved

def source(self):
tools.get(**self.conan_data["sources"][self.version],destination=self._source_subfolder, strip_root=True)
tools.get(**self.conan_data["sources"][self.version],
strip_root=True, destination=self._source_subfolder)

def _configure_cmake(self):
if self._cmake is not None:
Expand Down Expand Up @@ -93,13 +97,49 @@ def package(self):
for dll_pattern_to_remove in ["concrt*.dll", "msvcp*.dll", "vcruntime*.dll"]:
tools.remove_files_by_mask(os.path.join(self.package_folder, "bin"), dll_pattern_to_remove)

self._create_cmake_module_variables(
os.path.join(self.package_folder, self._module_file_rel_path),
tools.Version(self.version))

@staticmethod
def _create_cmake_module_variables(module_file, version):
content = textwrap.dedent("""\
set(IGN_TOOLS_CONAN_PACKAGE_DIR ${{CMAKE_CURRENT_LIST_DIR/../..}})
list(APPEND IGNITION-TOOLS_BINARY_DIRS ${{IGN_TOOLS_CONAN_PACKAGE_DIR}}/bin)
list(APPEND IGNITION-TOOLS_INCLUDE_DIRS ${{IGN_TOOLS_CONAN_PACKAGE_DIR}}/include)
list(APPEND IGNITION-TOOLS_LIBRARY_DIRS ${{IGN_TOOLS_CONAN_PACKAGE_DIR}}/libs)
list(APPEND IGNITION-TOOLS_CFLAGS -I/${{IGN_TOOLS_CONAN_PACKAGE_DIR}}/include)
list(APPEND IGNITION-TOOLS_CXX_FLAGS -std=c++11)
set(ignition-tools{major}_VERSION_MAJOR {major})
set(ignition-tools{major}_VERSION_MINOR {minor})
set(ignition-tools{major}_VERSION_PATCH {patch})
set(ignition-tools{major}_VERSION_STRING "{major}.{minor}.{patch}")
""".format(major=version.major, minor=version.minor, patch=version.patch))
tools.save(module_file, content)

def package_info(self):
version_major = tools.Version(self.version).major
self.cpp_info.names["cmake_find_package"] = "ignition-tools{}".format(version_major)
self.cpp_info.names["cmake_find_package_multi"] = "ignition-tools{}".format(version_major)

self.cpp_info.components["libignition-tools"].libs = ["ignition-tools-backward"]
self.cpp_info.components["libignition-tools"].includedirs.append("include/ignition/tools{}".format(version_major))
self.cpp_info.components["libignition-tools"].names["cmake_find_package"] = "ignition-tools{}".format(version_major)
self.cpp_info.components["libignition-tools"].names["cmake_find_package_multi"] = "ignition-tools{}".format(version_major)
self.cpp_info.components["libignition-tools"].names["pkg_config"] = "ignition-tools{}".format(version_major)
lib_name = "ignition-tools"
self.cpp_info.names["cmake_find_package"] = lib_name
self.cpp_info.names["cmake_find_package_multi"] = lib_name
self.cpp_info.set_property("cmake_file_name", "ignition-tools")
self.cpp_info.components["backward"].names["cmake_find_package"] = "backward"
Copy link
Contributor

@prince-chrismc prince-chrismc Jul 25, 2022

Choose a reason for hiding this comment

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

I am not sure whats the best option here but the targets are not correctly exporting from the upstream project... Does this match the projects intentions? I am not sure I can answer that...

https://cmake.org/cmake/help/latest/guide/importing-exporting/index.html#exporting-targets calls out install(EXPORT...) and install(TARGETS ...) neither exist 😕

https://github.com/gazebosim/gz-tools/blob/ignition-tools_1.4.0/src/CMakeLists.txt#L29 main gets closer but it's still not doing something consumers would rely on AFAIK.

self.cpp_info.components["backward"].names["cmake_find_package_multi"] = "backward"
self.cpp_info.components["backward"].names["cmake_paths"] = "backward"
ggulgulia marked this conversation as resolved.
Show resolved Hide resolved
self.cpp_info.components["backward"].bindirs = ["bin"]

self.cpp_info.components["backward"].libs = []
self.cpp_info.components["backward"].includedirs = []
if int(tools.Version(self.version).minor) > 2:
self.cpp_info.components["backward"].libs.append(lib_name +"-backward")
self.cpp_info.components["backward"].builddirs.append(self._module_dir_rel_path)
self.cpp_info.set_property("cmake_build_modules", [self._module_file_rel_path])
self.cpp_info.components["backward"].build_modules["cmake_find_package"] = [self._module_file_rel_path]
self.cpp_info.components["backward"].build_modules["cmake_find_package_multi"] = [self._module_file_rel_path]
self.cpp_info.components["backward"].build_modules["cmake_paths"] = [self._module_file_rel_path]

@property
def _module_dir_rel_path(self):
return os.path.join("lib", "cmake")
@property
def _module_file_rel_path(self):
return os.path.join(self._module_dir_rel_path, f"conan-official-{self.name}-variables.cmake")
10 changes: 2 additions & 8 deletions recipes/ignition-tools/all/test_package/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,10 @@ project(test_package)
include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake)
conan_basic_setup(TARGETS)

set(IGN_TOOLS_MAJOR_VER "" CACHE STRING "Version of igition-tools")

if(NOT IGN_TOOLS_MAJOR_VER)
message(FATAL_ERROR "IGN_MAJOR_MAJOR_VER not set")
endif()

find_package(ignition-tools${IGN_TOOLS_MAJOR_VER} REQUIRED CONFIG)
find_package(ignition-tools REQUIRED COMPONENTS backward CONFIG)

add_executable(${PROJECT_NAME} test_package.cpp)
target_link_libraries(${PROJECT_NAME} ignition-tools${IGN_TOOLS_MAJOR_VER}::ignition-tools${IGN_TOOLS_MAJOR_VER})
target_link_libraries(${PROJECT_NAME} ignition-tools::backward)
set_target_properties(${PROJECT_NAME} PROPERTIES
CXX_STANDARD 17
CXX_STANDARD_REQUIRED ON
Expand Down
2 changes: 1 addition & 1 deletion recipes/ignition-tools/all/test_package/conanfile.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# pylint: skip-file
from conans import ConanFile, CMake, tools
import os

Expand All @@ -7,7 +8,6 @@ class TestPackageConan(ConanFile):

def build(self):
cmake = CMake(self)
cmake.definitions["IGN_TOOLS_MAJOR_VER"] = tools.Version(self.deps_cpp_info["ignition-tools"].version).major
cmake.configure()
cmake.build()

Expand Down