-
Notifications
You must be signed in to change notification settings - Fork 2.4k
#3907: add component info in conanfile.py, add cmake helper module, e… #3908
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
Changes from all commits
170951c
7230038
0d2bed3
7b5e3ae
4fa8c6b
3421de3
e2c00ef
84f3b23
76a6068
6c630aa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import os | ||
| import os, glob | ||
| from conans import CMake, ConanFile, tools | ||
| from conans.errors import ConanInvalidConfiguration | ||
|
|
||
|
|
@@ -35,7 +35,7 @@ class FlatccConan(ConanFile): | |
| } | ||
| settings = "os", "arch", "compiler", "build_type" | ||
| generators = "cmake" | ||
| exports_sources = ["CMakeLists.txt"] | ||
| exports_sources = ["CMakeLists.txt", "patches/**"] | ||
|
|
||
| _cmake = None | ||
|
|
||
|
|
@@ -48,6 +48,10 @@ def _source_subfolder(self): | |
| def _build_subfolder(self): | ||
| return "build_subfolder" | ||
|
|
||
| def _patch_sources(self): | ||
| for patch in self.conan_data.get("patches", {}).get(self.version, []): | ||
| tools.patch(**patch) | ||
|
|
||
| def config_options(self): | ||
| if self.settings.os == "Windows": | ||
| del self.options.fPIC | ||
|
|
@@ -64,8 +68,10 @@ def configure(self): | |
|
|
||
| def source(self): | ||
| tools.get(**self.conan_data["sources"][self.version]) | ||
| extracted_dir = self.name + "-" + self.version | ||
| os.rename(extracted_dir, self._source_subfolder) | ||
| #TODO: restore extracted_dir when new release of flatcc is available | ||
| #extracted_dir = self.name + "-" + self.version | ||
| import glob | ||
| os.rename(glob.glob("flatcc-*")[0], self._source_subfolder) | ||
|
|
||
| def _configure_cmake(self): | ||
| if not self._cmake: | ||
|
|
@@ -86,10 +92,10 @@ def _configure_cmake(self): | |
| return self._cmake | ||
|
|
||
| def build(self): | ||
| self._patch_sources() | ||
| cmake = self._configure_cmake() | ||
| cmake.build() | ||
|
|
||
|
|
||
| def package(self): | ||
| cmake = self._configure_cmake() | ||
| cmake.install() | ||
|
|
@@ -99,12 +105,39 @@ def package(self): | |
| os.path.join(self.package_folder, "bin", "flatcc")) | ||
| # Copy license file | ||
| self.copy("LICENSE", dst="licenses", src=self._source_subfolder) | ||
| # Remove cmake config files | ||
| tools.rmdir(os.path.join(self.package_folder, "lib", "cmake", "flatcc")) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nevermind, it is!!!! Doh. |
||
| tools.rmdir(os.path.join(self.package_folder, "lib", "cmake", "flatccruntime")) | ||
| os.remove(os.path.join(self.package_folder, "lib", "cmake", "flatcccli", "flatcccli-config.cmake")) | ||
| os.remove(os.path.join(self.package_folder, "lib", "cmake", "flatcccli", "flatcccli-config-version.cmake")) | ||
| tools.remove_files_by_mask(os.path.join(self.package_folder, "lib", "cmake", "flatcccli"), "flatcccli-targets*.cmake") | ||
|
|
||
| def package_info(self): | ||
| bin_path = os.path.join(self.package_folder, "bin") | ||
| self.output.info('Appending PATH environment variable: %s' % bin_path) | ||
| self.env_info.PATH.append(bin_path) | ||
| debug_suffix = "_d" if self.settings.build_type == "Debug" else "" | ||
| #flatcc package provides two components: the flatcc compiler binary and the runtime library | ||
| if not self.options.runtime_lib_only: | ||
| self.cpp_info.libs.append("flatcc%s" % debug_suffix) | ||
| self.cpp_info.libs.append("flatccrt%s" % debug_suffix) | ||
| self.cpp_info.components["cli"].names["cmake_find_package"] = "cli" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe add a FIXME about missing
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Can you elaborate a bit? What exactly is missing ?
Ok, I'll add a comment that
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe it's unnecessary, but right now it's installing 3 cmake packages:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But aren't flatcccli-config.cmake and flatccruntime-config.cmake more like 'internal' config files?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've documented flatcccli for cross building purposes.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I noticed. Further down in the README there is also a section called 'Cross-compilation' that I am updating now. Will create a pull request to your branch when done tomorrow.
Will do. |
||
| self.cpp_info.components["cli"].libs = ["flatcc%s" % debug_suffix] | ||
| #FIXME: in the FlatccGenerateSources.cmake module the flatcc compiler exe is called via cmake target flatcc:cli. | ||
| #Currently this doesn't work when using Conan i.s.o. the (removed) flatcc cmake config files. | ||
| #We patch the FlatccGenerateSources.cmake module for this until Conan has support for executable targets. | ||
| #This workaround only succeeds when creating the package via 'conan create'. When calling 'conan install' | ||
| #and then manually build the flatcc package the FLATCC_CLI_EXE environment variable is not set (see below) and | ||
| #as a result the flatcc_generate_sources function in the FlatccGenerateSources.cmake module will fail. | ||
|
|
||
| #Our FlatccGenerateSources.cmake should be found when using the cmake_find_package generator | ||
| self.cpp_info.components["cli"].builddirs.append(os.path.join(self.package_folder, "lib", "cmake", "flatcccli")) | ||
| bin_path = os.path.join(self.package_folder, "bin") | ||
| self.env_info.PATH.append(bin_path) | ||
| #When we are cross-compiling cmake needs to know the location of the flatbuffer compiler executable | ||
| #compiled for the build architecture. Provide it via environment variable flatccCli_ROOT that will be | ||
| #picked up by the find_package(flatcc ...) command. | ||
| settings_target = getattr(self, 'settings_target', None) | ||
| if settings_target != None: | ||
| self.env_info.flatccCli_ROOT = self.package_folder | ||
| #Temporarily also export flatcc cli executable location, see patch 0001_workaround_no_exe_target_support_in_conan.patch. | ||
| #Don't overwrite it if already set by build env_info (when cross compiling). | ||
| if not self.env_info.FLATCC_CLI_EXE: | ||
| self.env_info.FLATCC_CLI_EXE = os.path.join(self.package_folder, "bin", "flatcc") | ||
| self.cpp_info.components["runtime"].names["cmake_find_package"] = "runtime" | ||
| self.cpp_info.components["runtime"].libs = ["flatccrt%s" % debug_suffix] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| diff --git a/cmake/FlatccGenerateSources.cmake b/cmake/FlatccGenerateSources.cmake | ||
| index 0607449..e86c365 100644 | ||
| --- a/cmake/FlatccGenerateSources.cmake | ||
| +++ b/cmake/FlatccGenerateSources.cmake | ||
| @@ -324,8 +324,8 @@ function(flatcc_generate_sources) | ||
|
|
||
| add_custom_command(OUTPUT ${OUTPUT_FILES} | ||
| COMMAND "${CMAKE_COMMAND}" -E make_directory "${FLATCC_OUTPUT_DIR}" | ||
| - COMMAND flatcc::cli ${FLATCC_ARGS} ${ABSOLUTE_SCHEMA_FILES} | ||
| - DEPENDS flatcc::cli ${ABSOLUTE_DEFINITIONS_DEPENDENCIES} | ||
| + COMMAND $ENV{FLATCC_CLI_EXE} ${FLATCC_ARGS} ${ABSOLUTE_SCHEMA_FILES} | ||
| + DEPENDS $ENV{FLATCC_CLI_EXE} ${ABSOLUTE_DEFINITIONS_DEPENDENCIES} | ||
| ) | ||
|
|
||
| add_custom_target("flatcc_generated_${FLATCC_NAME}" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,32 +1,33 @@ | ||
| cmake_minimum_required(VERSION 2.8) | ||
| cmake_minimum_required(VERSION 3.1) | ||
| project(flatcc_example) | ||
|
|
||
| include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake) | ||
| conan_basic_setup() | ||
|
|
||
| set(INC_DIR "${PROJECT_SOURCE_DIR}/include") | ||
| set(GEN_DIR "${CMAKE_CURRENT_BINARY_DIR}/generated") | ||
| set(FBS_DIR "${CMAKE_CURRENT_SOURCE_DIR}") | ||
|
|
||
| include_directories("${GEN_DIR}" "${INC_DIR}") | ||
| set(FBS_DIR "${CMAKE_CURRENT_SOURCE_DIR}") | ||
|
|
||
| add_executable(monster monster.c) | ||
|
|
||
| #On MacOS System Integrity Protection (SIP) will clear the DYLD_LIBRARY_PATH variable. | ||
| #As a result calling flatcc from cmake will currently not work if the flatcc executable | ||
| # is linked shared. As a workaround we generate the flatbuffer C files in the Conan recipe | ||
| # when on MacOS and flatcc option 'shared' is True. | ||
| if (NOT MACOS_SIP_WORKAROUND) | ||
| add_custom_target(gen_monster_fbs ALL) | ||
| add_custom_command ( | ||
| TARGET gen_monster_fbs | ||
| COMMAND cmake -E make_directory "${GEN_DIR}" | ||
| COMMAND flatcc -a -o "${GEN_DIR}" "${FBS_DIR}/monster.fbs" | ||
| DEPENDS flatcc "${FBS_DIR}/monster.fbs" | ||
| if (MACOS_SIP_WORKAROUND) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make sure that cmake in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alternative is adding flatcc as build requirement in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wait, that won't work because the flatcc version in build_requires should be the same as the injected flatcc requirement.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test_package (=running def build_requirements(self):
if tools.cross_building(self.settings):
self.build_requires("flatcc/{}"/format(some_version)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But I don't think conan supports this.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I already tried to retrieve the flatcc version that is tested from self.requires (since Conan injects it for the test_package). But both in config_options() and configure() self.requires is empty. I'll ask @jgsogo , else the following works when not using strange versions like 0.7.0pre:
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That might work, but is not really reproducible.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a big fan of this interface, but it is documented: you can use Conan injects the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @jgsogo , What we are looking for is finding out the version of the package that is being tested at a point where we can inject a build_requirement for the same package/version. In order to build the test_package for flatcc (and similar tools) when cross-compiling, flatcc is also needed in the build architecture to generate the flatbuffer header files. Then the test executable is linked to the flatcc host arch library. I suppose in the build() function it is too late to inject a build_requirement. Is it possible to do that earlier ?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. This is not possible today. The recipe in the BUT, I agree this is a Would you mind opening an issue with this feature request? Meanwhile, I cannot see a documented way to achieve this behavior. |
||
| set(INC_DIR "${PROJECT_SOURCE_DIR}/include") | ||
| set(GEN_DIR "${CMAKE_CURRENT_BINARY_DIR}/generated") | ||
| include_directories("${GEN_DIR}" "${INC_DIR}") | ||
|
Comment on lines
+15
to
+18
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this SIP workaround still needed for the latest version? Maybe change the if to include a version comparison/conditional?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Flatcc depends on the flatcc compiler shared lib when configuring with BUILD_SHARED_LIBS=On. The SIP workaround is only used then: Its very well possible that the workaround is not needed anymore now that RPATH is set but at the moment I don't have access to a Mac that is recent enough to have SIP. I can check next week at work.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please do, or just let c3i test it for you 😛 Unless cmake is configured to not set RPATH on install, which is possible: |
||
| else() | ||
| include(FlatccGenerateSources) | ||
| flatcc_generate_sources( | ||
| NAME monster_sample | ||
| SCHEMA_FILES monster.fbs | ||
| ALL | ||
| OUTPUT_DIR "${GEN_DIR}" | ||
| ) | ||
|
|
||
| add_dependencies(monster gen_monster_fbs) | ||
| endif() | ||
|
|
||
| add_executable(monster monster.c) | ||
|
|
||
| target_link_libraries(monster ${CONAN_LIBS}) | ||
| target_link_libraries(monster ${CONAN_LIBS} | ||
| $<$<NOT:$<BOOL:${MACOS_SIP_WORKAROUND}>>:flatcc_generated::monster_sample> | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| versions: | ||
| 0.6.0: | ||
| folder: all | ||
| 0.7.0pre: | ||
| folder: all |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, move the files you want to keep to some temporary folder, remove
os.path.join(self.package_folder("lib", "cmake"))and re-create acmakefolder