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

find_package and CPMFindPackage have different behaviors #132

Closed
phillipjohnston opened this issue Jun 16, 2020 · 9 comments · Fixed by #141
Closed

find_package and CPMFindPackage have different behaviors #132

phillipjohnston opened this issue Jun 16, 2020 · 9 comments · Fixed by #141

Comments

@phillipjohnston
Copy link

phillipjohnston commented Jun 16, 2020

Hello,

I'm using CPMFindPackage() to check for a dependency on CMocka and noticing some strange behavior with the find_package portion. Do you know what I might be doing wrong here?

CPMFindPackage(
  NAME CMOCKA
  GIT_REPOSITORY https://git.cryptomilk.org/projects/cmocka.git/
  VERSION 1.1.5
  GIT_TAG cmocka-1.1.5
  OPTIONS
    "WITH_EXAMPLES OFF"
)

message("CMOCKA_FOUND: ${CMOCKA_FOUND}")
message("CMOCKA_ADDED: ${CMOCKA_ADDED}")
message("CMOCKA_INCLUDES: ${CMOCKA_INCLUDE_DIR}")
message("CMOCKA_LIBRARIES: ${CMOCKA_LIBRARIES}")

The library is installed on my system with a CMake config file. I see this output from CPM, but the FOUND/ADDED variables are not set, and the other variables defined by the config file aren't used.

-- CPM: using local package [email protected]
CMOCKA_FOUND:
CMOCKA_ADDED:
CMOCKA_INCLUDES:
CMOCKA_LIBRARIES:

If I remove CPMFindPackage and add find_package:

find_package(CMOCKA)

This time, CMOCKA_FOUND will be defined, and the values defined by the config file are used.

CMOCKA_FOUND: 1
CMOCKA_ADDED:
CMOCKA_INCLUDES: /usr/local/include
CMOCKA_LIBRARIES: /usr/local/lib/libcmocka.dylib
@phillipjohnston
Copy link
Author

phillipjohnston commented Jun 16, 2020

I think is a scoping problem because find_package is called inside of a function.

@TheLartians
Copy link
Member

Hey, thanks for the report! The problem is indeed that CPMFindPackage is a function and has it's own scope. Therefore, any variables exported to the parent scope by the FindXXX.cmake script get lost in the inner scope. This is expected behaviour.

Basically, CPMFindPackage guarantees that the dependency has been added after the call and we can use any targets defined inside. In modern CMake, the targets themselves should contain all libraries and include directories, so that no variables need to be exported.

If the dependency does depend on variables, I'm not really sure what we can do about this as usually the generated CMake module scripts (used by find_package) differ strongly from the source scripts (CMakeLists.txt) and there would be no good way to get reliable build behaviour.

@phillipjohnston
Copy link
Author

Thanks for the explanation. Those are nice notes for the documentation too, I have a better understanding of the requirements for using CPMFindPackage

Perhaps this can be a suggested workaround in the documentation for cases where the dependency's config.cmake file doesn't export targets.

find_package(cmocka QUIET)

if(NOT cmocka_FOUND)
  CPMAddPackage(
    NAME cmocka
    GIT_REPOSITORY https://git.cryptomilk.org/projects/cmocka.git/
    VERSION 1.1.5
    GIT_TAG cmocka-1.1.5
    OPTIONS
      "WITH_EXAMPLES OFF"
  )

  # Maintain build compatibility between find_package and CMakeLists.txt variants
  set(CMOCKA_LIBRARIES cmocka)
endif()

If the dependency does depend on variables, I'm not really sure what we can do about this as usually the generated CMake module scripts (used by find_package) differ strongly from the source scripts (CMakeLists.txt) and there would be no good way to get reliable build behaviour.

Scoping aside, I think that's the user's responsibility. I work around this for the current dependency by setting a single variable when CPMAddPackage is used instead of find_package.

# Maintain build compatibility between find_package and CMakeLists.txt variants
set(CMOCKA_LIBRARIES cmocka)

@TheLartians
Copy link
Member

Thanks for the feedback, I agree that this should be mentioned in the documentation. Also your user-side workaround makes sense, I'll see that we add this to the docs soon.

@Neumann-A
Copy link

@TheLartians You could export those variables by inspecting the directory property https://cmake.org/cmake/help/latest/prop_dir/VARIABLES.html before and after the find_package call and promote any new variables to PARENT_SCOPE

@apGribble
Copy link

I'm having a similar issue with a project "A" that gets Catch2 via CPM but also includes project "B" that gets Catch2 via Fetchcontent. When I configure the project, the FetchContent call in project B doesn't see that CPM has already got Catch2 so tries to get it again and fails with the following error:

The binary directory
[cmake]     /build/_deps/catch2-build
[cmake]   is already used to build a source directory.  It cannot be used to build source directory
[cmake]     /build/_deps/catch2-src
[cmake]   Specify a unique binary directory name.

The relevant section of the cmake file for project A is:

CPMAddPackage(
  NAME catch2
  GIT_REPOSITORY https://github.com/catchorg/Catch2
  VERSION 2.10.2
)

if(${catch2_ADDED})
  # Path to module "Catch" containing catch_discover_tests()
  list(APPEND CMAKE_MODULE_PATH ${catch2_SOURCE_DIR}/contrib)
  set( CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} PARENT_SCOPE)
endif()

And project B:

include(FetchContent)
FetchContent_Declare(catch2
GIT_REPOSITORY "https://github.com/catchorg/Catch2"
GIT_TAG 87b745da6625b7469e61de5910978a097caf8b74 # v2.10.2
)

FetchContent_GetProperties(catch2)

if(NOT catch2_POPULATED)
  FetchContent_Populate(catch2)
  add_subdirectory(${catch2_SOURCE_DIR} ${catch2_BINARY_DIR})
  # Path to module "Catch" containing catch_discover_tests()
  list(APPEND CMAKE_MODULE_PATH ${catch2_SOURCE_DIR}/contrib)
  set( CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} PARENT_SCOPE)
endif()

I've tried setting CATCH2_LIBRARIES as per the above example and also tried setting catch2_POPULATED but this just gets set back to false by the call to FetchContent_GetProperties(catch2)

@TheLartians
Copy link
Member

TheLartians commented Jun 23, 2021

Hey @apGribble thanks for the comment, that is an interesting use-case. I think the issue could be due CPM.cmake currently bypassing FetchContent for performance reasons. Could you try disabling the cache by changing the first CPMAddPackage call to the following?

  CPMAddPackage(
    NAME catch2
    GIT_REPOSITORY https://github.com/catchorg/Catch2
    VERSION 2.10.2
++  NO_CACHE TRUE
  )

@apGribble
Copy link

Yeah that removes the error.
Is there actually much overhead with calling fetchcontent when the SOURCE_DIR is in the local cache.

@TheLartians
Copy link
Member

Yeah, each invocation of FetchContent currently comes with a significant (100 - 1000 ms) performance penalty, even if all sources are already cached. The CMake maintainers are aware of this and already working on a fix though.

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

Successfully merging a pull request may close this issue.

4 participants