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

Enable CMake integration with FetchContent #260

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

jeanchristopheruel
Copy link

@jeanchristopheruel jeanchristopheruel commented Oct 17, 2022

Integrate autodiff in CMake using FetchContent

include(FetchContent)
FetchContent_Declare(
        autodiff
        GIT_REPOSITORY https://github.com/autodiff/autodiff
        GIT_TAG        v0.6.12)
FetchContent_MakeAvailable(autodiff)

And for more granular control:

include(FetchContent)
FetchContent_Declare(
        autodiff
        GIT_REPOSITORY https://github.com/autodiff/autodiff
        GIT_TAG        v0.6.12)
FetchContent_GetProperties(autodiff)
if(NOT autodiff_POPULATED)
    FetchContent_Populate(autodiff)

    # Bring the populated content into the build
    add_subdirectory(${autodiff_SOURCE_DIR} ${autodiff_BINARY_DIR})
    set(AUTODIFF_BUILD_TESTS OFF)
    set(AUTODIFF_BUILD_PYTHON OFF)
    set(AUTODIFF_BUILD_EXAMPLES OFF)
    set(AUTODIFF_BUILD_PYTHON OFF)
endif()

Fixes #259

@allanleal
Copy link
Member

allanleal commented Oct 18, 2022

Hi @jeanchristopheruel , thanks for proposing and implementing these changes. This is how I think we should do it. All CMake changes related to FetchContent should exist in a CMake module within the cmake directory. Say, EnableFetchContent.cmake. Then, in the main/root CMakeLists.txt file, we have:

if(AUTODIFF_ENABLE_FETCH_CONTENT)
    include(EnableFetchContent)
endif()

I see that you have changed options from ON to OFF (build tests, python, docs, etc) because if you are integrating autodiff with your lib, you probably don't need to build those. But in development mode, I'd hate to have to set these options ON explicitly all the time! :) Thus, please let them ON but add commands in the EnableFetchContent.cmake to set them OFF.

Also, I will not rely on this fetching mechanism of CMake for installing dependencies. I rely on conda (conda-forge more exactly) as a dependency manager, which you should give it a try.

A common misconception is that conda is for python only; I use conda to install even compilers! Have a look at the environment.devenv.yml file.

@jeanchristopheruel
Copy link
Author

Also, I will not rely on this fetching mechanism of CMake for installing dependencies. I rely on conda (conda-forge more exactly) as a dependency manager, which you should give it a try.

Interesting indeed. I'm not that familiar with conda. Personally, I think FetchContent makes the code more portable and avoids adding dependencies. Thank you for your answer, I will suggest an improvement.

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.

Enable CMake integration with FetchContent
2 participants