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

[CMake] FetchContent on missing submodule #262

Merged
merged 2 commits into from
Oct 26, 2021

Conversation

nim65s
Copy link
Contributor

@nim65s nim65s commented Oct 25, 2021

Hi,

This proposition allows users of CMake >= 3.14 to dynamically fetch jrl-cmakemodules when the submodule is not already available.

This also allows eigenpy to be used in other projects with FetchContent, because FetchContent is not able to handle submodules at all.

eg:

cmake_minimum_required(VERSION 3.14)

project(test-eigenpy)

include(FetchContent)
FetchContent_Declare("eigenpy"
  GIT_REPOSITORY "https://github.com/nim65s/eigenpy"
  GIT_TAG "devel")
FetchContent_MakeAvailable("eigenpy")
FetchContent_GetProperties("eigenpy" SOURCE_DIR EIGENPY_SOURCE_DIR)

add_library(test_eigenpy SHARED test_eigenpy.cpp)
add_dependencies(test_eigenpy eigenpy)
target_link_libraries(test_eigenpy PRIVATE eigenpy)
target_include_directories(test_eigenpy PRIVATE "${EIGENPY_SOURCE_DIR}/include")

Additional CMake code can also be used to FetchContent only if eigenpy is not already found, with something like:

find_package(eigenpy QUIET)
if(eigenpy_FOUND)
  target_link_libraries(test_eigenpy PRIVATE eigenpy::eigenpy)
else()
  FetchContent_Declare(eigenpy …)
  FetchContent_MakeAvailable(eigenpy)
  target_link_libraries(test_eigenpy PRIVATE eigenpy)
  target_include_directories(test_eigenpy PRIVATE "${EIGENPY_SOURCE_DIR}/include")
endif()

@jcarpent
Copy link
Contributor

Could you add a unit test in the CI for this feature?

@nim65s nim65s force-pushed the devel branch 4 times, most recently from 3a8980b to c763bea Compare October 26, 2021 16:22
@nim65s
Copy link
Contributor Author

nim65s commented Oct 26, 2021

@jcarpent : done

Copy link
Contributor

@jcarpent jcarpent left a comment

Choose a reason for hiding this comment

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

Perfect! Thanks a lot.

@jcarpent jcarpent merged commit b65e428 into stack-of-tasks:devel Oct 26, 2021
message(STATUS "JRL cmakemodules not found. Let's fetch it.")
include(FetchContent)
FetchContent_Declare("jrl-cmakemodules"
GIT_REPOSITORY "https://github.com/jrl-umi3218/jrl-cmakemodules.git")
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally, you should pull the correct commit too. I don't think it is too hard to get the hash. I am not sure if it is wise to do so.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

GIT_TAG can easily be added, with a branch, a tag, or a hash. But I'm not sure about whether this is best to add one and having to manage it, or if we can just be happy with whatever last version we'll get this way.

Copy link
Contributor

Choose a reason for hiding this comment

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

I had in mind getting the expected hash from some git command when git is available and the command succeeds and fall back to master when everything you tried failed.

No, don't add the hash explicitly in the file.

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.

3 participants