Skip to content
Closed
7 changes: 7 additions & 0 deletions recipes/flatcc/all/conandata.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,10 @@ sources:
"0.6.0":
sha256: a92da3566d11e19bb807a83554b1a2c644a5bd91c9d9b088514456bb56e1c666
url: https://github.com/dvidelabs/flatcc/archive/v0.6.0.tar.gz
"0.7.0pre":
url: https://github.com/madebr/flatcc/archive/cmake_dep.zip
sha256: 78522f33d35efe291896b8a5b69b1752eadcd126a259522543cbc7f527772471
patches:
"0.7.0pre":
- patch_file: "patches/0001_workaround_no_exe_target_support_in_conan.patch"
base_path: "source_subfolder"
53 changes: 43 additions & 10 deletions recipes/flatcc/all/conanfile.py
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

Expand Down Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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:
Expand All @@ -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()
Expand All @@ -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

Copy link
Copy Markdown
Contributor

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 a cmake folder

tools.rmdir(os.path.join(self.package_folder, "lib", "cmake", "flatcc"))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is flatcc present at install time?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe add a FIXME about missing flatcccli and flatccruntime (2 files) instead of one
Conan is missing this feature.
Also FIXME about missing flatcc::cli target.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Maybe add a FIXME about missing flatcccli and flatccruntime (2 files) instead of one

Can you elaborate a bit? What exactly is missing ?

Also FIXME about missing flatcc::cli target

Ok, I'll add a comment that flatcc::cli executable target currently doesn't work with Conan.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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:
flatcc-config.cmake, flatcccli-config.cmake and flatccruntime-config.cmake. Of these, currently only flatcc-config.cmake` is re-created by conan.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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?
End-users should use find_package(flatcc).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've documented flatcccli for cross building purposes.
But anyhow, maybe add a FIXME/TODO for when conan receives support for creating multiple cmake modules.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've documented flatcccli for cross building purposes.

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.

But anyhow, maybe add a FIXME/TODO for when conan receives support for creating multiple cmake modules.

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}"
33 changes: 17 additions & 16 deletions recipes/flatcc/all/test_package/CMakeLists.txt
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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Make sure that cmake in test_package/conanfile.py is only run when not tools.cross_building(self.settings) is true.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Alternative is adding flatcc as build requirement in test_package/conanfile.py, but then when increasing the version in conanfile.py you have to remember to update the flatcc version in test_package/conanfile.py as well.

@@ -6,6 +6,7 @@ from conans import ConanFile, CMake, tools, RunEnvironment
 class FlatccTestConan(ConanFile):
     settings = "os", "compiler", "build_type", "arch"
     generators = "cmake"
+    build_requires = "flatcc/0.7.0pre"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The test_package (=running flatcc::cli) only works when not cross building.
I haven't tested, but maybe add

def build_requirements(self):
    if tools.cross_building(self.settings):
        self.build_requires("flatcc/{}"/format(some_version)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

But I don't think conan supports this.
I haven't seen recipes doing this. Maybe ask @jgsogo, the conan xbuild professional.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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:
build_requires = ("flatcc/[>=0.7.0]")

@madebr madebr Dec 20, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That might work, but is not really reproducible.
There should be some way to know what package a test recipe is testing.
e.g. as self.test_package that is available as soon as possible.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 self.deps_cpp_info["requirements"].version in the build() to get the version of the requirement (after the graph is resolved). Is this what you are looking for?

Conan injects the requirement automatically, but after that line, it is a regular requirement.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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 ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see.

This is not possible today. The recipe in the test_package works like any other recipe. The version of a requirement can be overridden by other branches of the graph (overrides, version-ranges, diamonds), so the final version of a requirement is only known when the graph is fully resolved and this is exactly the scenarios where the new validate() function plays an important role.

BUT, I agree this is a test_package and it totally makes sense to know the recipe being tested. And probably Conan should forbid modifying options in the test_package too (conan-io/conan#7547). These (together with a proposal to test build-requires using the two-profiles approach conan-io/conan#7132) are things that we should explore before Conan v2.

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

@madebr madebr Dec 20, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this SIP workaround still needed for the latest version?
flatcc does not depend on an external (shared) library.
And I made sure to set a relative RPATH on the executable.

Maybe change the if to include a version comparison/conditional?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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:
if tools.os_info.is_macos and self.options["flatcc"].shared:

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please do, or just let c3i test it for you 😛
SIP causes problems only when it needs external DYLD_LIBRARY_PATH, which is not the case here.

Unless cmake is configured to not set RPATH on install, which is possible:
https://cmake.org/cmake/help/latest/variable/CMAKE_SKIP_INSTALL_RPATH.html

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>
)
7 changes: 7 additions & 0 deletions recipes/flatcc/all/test_package/conanfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ class FlatccTestConan(ConanFile):
generators = "cmake"

def build(self):
if tools.cross_building(self):
pass
#Note: if you want to build this test_package also when cross-compiling just add
# the same flatcc version that you are testing to build_requires of
# this test_package. Then the build environment flatcc cli executable location
# will be passed automatically to the flatcc_generate_sources cmake function.

env_build = RunEnvironment(self)
with tools.environment_append(env_build.vars):
cmake = CMake(self)
Expand Down
2 changes: 2 additions & 0 deletions recipes/flatcc/config.yml
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