jll63 / yomm2

Fast, orthogonal, open multi-methods. Solve the Expression Problem in C++17.
Boost Software License 1.0
343 stars 18 forks source link

build(cmake): fix case where project is imported into existing CMake environment #40

Closed larkwiot closed 8 months ago

larkwiot commented 9 months ago

When using something based in FetchContent like CPM (Cmake Package Manager), CMAKE_SOURCE_DIR points to the top-level of the user's project and not the root of yomm2. This results in find_or_download_package not being found. By specifying explicitly the relative path to the file, this can be avoided.

jll63 commented 9 months ago

Thanks for this. CI is broken, nothing related (clang 12 is not available in ubuntu-latest). I will fix it and then we can merge this.

jll63 commented 8 months ago

Sorry for the delay. I have been busy with a long chain of changes required for some really nice improvements.

Firstly, I am no cmake expert. I asked a more knowledgeable colleague to take a look at your PR, and he suggested doing:

include("${CMAKE_CURRENT_SOURCE_DIR}/cmake/find_or_download_package.cmake")

...instead.

But I would like to understand what is going on, so I read the doc about FetchContent and created this small project:

# tests/cmake_fetchcontent/CMakeLists.txt

cmake_minimum_required(VERSION 3.20)
cmake_policy(SET CMP0057 NEW)
project(YOMM2_FetchContent VERSION 1.0.0)

include(FetchContent)
FetchContent_Declare(
  YOMM2
  GIT_REPOSITORY https://github.com/jll63/yomm2.git
)

FetchContent_MakeAvailable(YOMM2)

add_executable(adventure adventure.cpp)
target_link_libraries(adventure YOMM2::yomm2)

However, it fails with:

jll@moss:~/dev/yomm2/tests/cmake_fetchcontent/build$ cmake ..
CMake Error at build/_deps/yomm2-src/CMakeLists.txt:17 (include):
  include could not find requested file:

    find_or_download_package

CMake Error at build/_deps/yomm2-src/CMakeLists.txt:18 (find_or_download_package):
  Unknown CMake command "find_or_download_package".

-- Configuring incomplete, errors occurred!
See also "/home/jll/dev/yomm2/tests/cmake_fetchcontent/build/CMakeFiles/CMakeOutput.log".

...even with your patch.

It looks like something I really want to work though. How can I reproduce the problem your patch is solving? And, do you see what's wrong with my example, and, if nothing is wrong, how we could make it work?

[EDIT] Ah but of course it doesn't work because I am fetching from canonical master, and it does not contain your change. This works:

cmake_minimum_required(VERSION 3.20)
cmake_policy(SET CMP0057 NEW)
project(YOMM2_FetchContent VERSION 1.0.0)

include(FetchContent)
FetchContent_Declare(
  YOMM2
  GIT_REPOSITORY git@github.com:larkwiot/yomm2.git
  GIT_TAG patch-1
)

FetchContent_MakeAvailable(YOMM2)
larkwiot commented 8 months ago

Thank you so much for looking at this!

I am no CMake master, so I'd happily accept your colleague's change. I see you have replicated the problem with raw FetchContent. I am using CPM, which uses FetchContent in the background, so any patch that fixes this for FetchContent as you did it should work for CPM. But I will try to make a minimal example using CPM for your reference.

jll63 commented 8 months ago

Just curious, in which context do you use YOMM2?

larkwiot commented 7 months ago

Just curious, in which context do you use YOMM2?

A personal project, a toy compiler