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

[feature] Add CONAN_DOWNLOAD and CONAN_ISOLATE_HOME optional features #651

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

Conversation

valgur
Copy link
Contributor

@valgur valgur commented Jun 30, 2024

One quite frequent complaint about Conan, especially from Windows users, is that it requires a Python installation with Conan installed, which adds friction to the build process. This PR proposes a simple fix to that: optionally download and use one of the self-contained Conan packages from https://github.com/conan-io/conan/releases/latest, if Conan is not found on the system. The config options for this feature are always, if-missing and never.

In addition to that, it also adds an option to override CONAN_HOME to a folder inside the build dir. This might be useful in other scenarios as well, but if Conan is not available on the system or it's an outdated version, then it's generally preferable if the build process does not add unexpected files to ~/.conan2. Its config options are always, if-downloaded and never.

Whether the default config options for these features should be if-missing and if-downloaded, respectively, or never and never is debatable, but I preferred and set them to the prior one.

The added tests should cover both of these features comprehensively, I believe.

I also moved all config options of conan_provider.cmake to the top of the file for better visibility.

@valgur
Copy link
Contributor Author

valgur commented Jul 1, 2024

One more use case that the CONAN_ISOLATE_HOME feature unlocks (at least as far as I'm concerned) is the ability to precisely control the remotes used during conan install:

set(CUSTOM_REMOTE_PATH "${CMAKE_BINARY_DIR}/cci-valgur" )
include(FetchContent)
FetchContent_Declare(
    cci_valgur
    URL "https://github.com/valgur/conan-center-index/archive/11c1fadf6f659bb6ba208e79352be24bf1077385.zip"
    SOURCE_DIR "${CUSTOM_REMOTE_PATH}"
    DOWNLOAD_EXTRACT_TIMESTAMP TRUE
)
FetchContent_MakeAvailable(cci_valgur)
file(WRITE "${CMAKE_BINARY_DIR}/conan_home/remotes.json" "
{
 \"remotes\": [
  {
   \"name\": \"cci-valgur\",
   \"url\": \"${CUSTOM_REMOTE_PATH}\",
   \"verify_ssl\": true,
   \"remote_type\": \"local-recipes-index\"
  }
 ]
}
")

Hopefully there are better ways to accomplish the same now or in the future, though, but I have not found any.

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.

Thanks for your contribution @valgur

I think this is getting too much responsibility for a single provider, and is covering functionalities that are out of the scope of the current conan_provider.cmake intention.

The installation of Conan should be a separate use case. While I understand some convenience for some cmake-conan users, specially those that are not using Conan but they attempt to use a project that uses the Conan provider, this is not enough reason to add this extra complexity to the current one.

The CMAKE_PROJECT_TOP_LEVEL_INCLUDES accepts a list of files. If anything this functionality could make sense as an independent and extra CMAKE_PROJECT_TOP_LEVEL_INCLUDES file, but not in the conan_provider.cmake file.

@valgur
Copy link
Contributor Author

valgur commented Jul 16, 2024

Hmm... that's a fair point. Would this be considered for inclusion if I refactored it into a separate .cmake module?

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.

2 participants