Conan v2 #14
Conan v2 #14
Conversation
…h CMake binary directory
eidekrist
requested review from
kyllingstad,
markaren,
THEvang,
restenb,
joakimono and
SSkjong
.github/workflows/ci-conan.yml
Outdated
| build_type: [Debug, Release] | ||
| compiler_version: [7, 8, 9] | ||
| compiler_version: [9] | ||
| compiler_libcxx: [libstdc++11] | ||
| option_shared: ['True', 'False'] |
There was a problem hiding this comment.
I suggest using containers when building. This configuration (gcc 9 and ubuntu 22) is unfortunate, since distributions using gcc 9 have an older glibc version than ubuntu 22 and pesky glibc issues are likely.
conanio have (almost) ready-made containers for various compiler versions.
container:
image: conanio/gcc${{ matrix.compiler_version }}-ubuntu18.04
options: -u 0
You would need to upgrade conan and either use conan config install or conan profile detect to setup.
There was a problem hiding this comment.
Also note that many conan environment variables, e.g. CONAN_NON_INTERACTIVE are ignored with conan 2 and should be configured with core:non_interactive = False in global.conf . I think in this case it does not make a difference.
There was a problem hiding this comment.
I agree, and we actually do use containers in our main repo, libcosim. But since this is going to be just a stop-gap solution until you get fmilibrary into conan-center, let's just leave it as-is for now.
.github/workflows/ci-conan.yml
Outdated
| build_type: [Debug, Release] | ||
| compiler_version: [7, 8, 9] | ||
| compiler_version: [9] | ||
| compiler_libcxx: [libstdc++11] | ||
| option_shared: ['True', 'False'] | ||
| steps: | ||
| - uses: actions/checkout@v2 |
test_package/CMakeLists.txt
Outdated
| @@ -1,8 +1,7 @@ | |||
| project(PackageTest CXX) | |||
| cmake_minimum_required(VERSION 2.8.12) | |||
There was a problem hiding this comment.
Minimum required version when using CMakeDeps is 3.15
| tc.cache_variables["FMILIB_BUILD_SHARED_LIB"] = True if self.options.shared else False | ||
| tc.cache_variables["FMILIB_BUILD_TESTS"] = False | ||
| tc.cache_variables["FMILIB_INSTALL_PREFIX"] = pathlib.Path(os.path.join(self.build_folder, "install")).as_posix() | ||
| tc.generate() |
There was a problem hiding this comment.
Suggest adding tc.variables["FMILIB_GENERATE_DOXYGEN_DOC"] = False to always avoid building and installing documentation.
conanfile.py
Outdated
|
|
||
| def generate(self): | ||
| tc = CMakeToolchain(self) | ||
| tc.cache_variables["FMILIB_BUILD_STATIC_LIB"] = False if self.options.shared else True |
There was a problem hiding this comment.
I believe these should be variables instead of cache_variables
There was a problem hiding this comment.
Are you sure? According to the variables documentation,
This attribute allows defining CMake variables, for multiple configurations (Debug, Release, etc). These variables should be used to define things related to the toolchain and for the majority of cases cache_variables is what you probably want to use.
kyllingstad
force-pushed
the
feature/conan-2
branch
from
4d9f7db
to
697aa5d
Compare
kyllingstad
force-pushed
the
feature/conan-2
branch
from
697aa5d
to
9f74fd5
Compare
|
I did a bit of extra work here today to build the Conan packages using the official Conan Docker images rather than building it directly on the GitHub runner. This was necessary to avoid a glibc version conflict with the libcosim package, which is also built on those images. |
|
Given that we have now merged several downstream pull requests that depend on this PR, I take it as implicit approval and will just merge this now. Feel free to revert and/or reprimand if you disagree with this decision. |
Converts recipe to Conan v2 syntax.