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

feat: Add CONAN_RUNTIME_LIB_DIRS to the conan_toolchain.cmake #15914

Open
wants to merge 18 commits into
base: develop2
Choose a base branch
from

Conversation

ErniGH
Copy link
Contributor

@ErniGH ErniGH commented Mar 21, 2024

Changelog: Feature: Add CONAN_RUNTIME_LIB_DIRS variable to the conan_toolchain.cmake
Docs: conan-io/docs#3698

Close: #15810

@ErniGH ErniGH added this to the 2.3.0 milestone Mar 21, 2024
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

Is it possible to make the libdirs multi-config with a generator expression, as done with MSVC_RUNTIME and flags? It would be important for VS, otherwise it might be failing and collecting Debug runtime libs even if in a Release build, just because Debug configuration was the last installed one.

Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

I still think that it would be important to make CONAN_RUNTIME_LIB_DIRS a generator expression for multi-config systems, or at least a _RELEASE, _DEBUG one. There is risk of bringing the wrong runtime just because -s build_type=Debug was executed last.

conan/tools/cmake/toolchain/blocks.py Outdated Show resolved Hide resolved
@jcar87
Copy link
Contributor

jcar87 commented Apr 9, 2024

I still think that it would be important to make CONAN_RUNTIME_LIB_DIRS a generator expression for multi-config systems, or at least a _RELEASE, _DEBUG one. There is risk of bringing the wrong runtime just because -s build_type=Debug was executed last.

indeed! This was a further step - we first need to check whether the file(GET_RUNTIME_DEPENDENCIES ...) functionality accepts a generator expression - probably works during install, but unsure about configure time

conan/tools/cmake/toolchain/blocks.py Outdated Show resolved Hide resolved
conan/tools/cmake/toolchain/blocks.py Outdated Show resolved Hide resolved
conan/tools/cmake/toolchain/blocks.py Outdated Show resolved Hide resolved
conan/tools/cmake/toolchain/blocks.py Outdated Show resolved Hide resolved
Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

This is much better, getting close, just a couple of minor comments.

conan/tools/cmake/toolchain/blocks.py Outdated Show resolved Hide resolved
conan/tools/cmake/toolchain/blocks.py Outdated Show resolved Hide resolved
conan/tools/cmake/toolchain/blocks.py Outdated Show resolved Hide resolved
@jcar87
Copy link
Contributor

jcar87 commented Apr 29, 2024

The changes in this PR cause a warning to be issued:

CMake Warning (dev) at build/generators/conan_toolchain.cmake:106:
  Syntax Warning in cmake code at column 213

  Argument not separated from preceding token by whitespace.
Call Stack (most recent call first):
  build/CMakeFiles/3.27.4/CMakeSystem.cmake:6 (include)
  CMakeLists.txt:3 (project)

I can get it to not show that warning by doing semi-colon ; separated lists inside the generator expression, rather than space separated. However, then it doesn't work

The cmake_install.cmake file then generates something like this:

  elseif(CMAKE_INSTALL_CONFIG_NAME MATCHES "^([Rr][Ee][Ll][Ee][Aa][Ss][Ee])$")
    file(GET_RUNTIME_DEPENDENCIES
      RESOLVED_DEPENDENCIES_VAR _CMAKE_DEPS
      EXECUTABLES
        "C:/Users/xxx/conan-cmake-install-runtime-dependencies/build/Release/my_app.exe"
      DIRECTORIES
        "\$<1:\"C:/Users/xxx/.conan2/p/b/libpn0bfb1d18b1c7b/p/bin"
        "C:/Users/xxx/.conan2/p/zlib47a1fa7d4bdbe/p/bin"
        "C:/Users/xxx/.conan2/p/gtestd45050d57d0d9/p/bin\">\$<0:\"C:/Users/xxx/.conan2/p/b/libpn185873a63e4d5/p/bin"
        "C:/Users/xxx/.conan2/p/b/zlib00e783ae4582e/p/bin"
        "C:/Users/xxx/.conan2/p/b/gtest2d6a2ca931073/p/bin\">"

the 0 and the 1 in the generator expression here seems correct in the sense that it matches the configuration in the generated file - but something is off and it doesn't seem that cmake is picking up the correct directories because the install command then fails.
Using this https://github.com/jcar87/conan-cmake-install-runtime-dependencies as a test project on windows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature] CMakeToolchain: add a variable with all runtime library directories
3 participants