Can't get cmake FetchContent to work

[maxmarsc]

Hi, first thanks a lot for this amazing library.

I'm trying to integrate this library as a dependency of my CMake project, but I can't get it to work the way I thought it would.

I'm trying like this :

include(FetchContent)

FetchContent_Declare(
  cppitertools
  GIT_REPOSITORY [email protected]:ryanhaining/cppitertools.git
  GIT_TAG 556fca33235b6ad1fba6e37df104ca23a01a5b45
  GIT_SHALLOW ON
)

FetchContent_MakeAvailable(cppitertools)

target_link_libraries(
  mylib
  PUBLIC cppitertools::cppitertools
)

I checked and the cppitertools::cppitertools target exists (although I can't build it). But when trying to compile my library it can't find the headers :

[build] /foo/bar.cpp:11:10: fatal error: cppitertools/enumerate.hpp: No such file or directory
[build]    11 | #include <cppitertools/enumerate.hpp>

I'm kinda new to this workflow (previously I would have used target_include_directories manually), but from what I understood FetchContent on a header-only library would not require me to do anything else.

Is it a bug or am I missing something here ?

[ryanhaining]

I don't maintain the cmake files because I barely understand them, they are a community contribution, sorry. If you figure out what needs changing I'll accept whatever PR fixes it

[maxmarsc]

Even if this is primarily about cmake issues, I'd like your opinion @ryanhaining, as I think it points to a bigger issue (you can go to the second part directly if you don't care at all about cmake stuff)

The way the include directories are specified for a cmake configuration is the following :

target_include_directories(cppitertools INTERFACE
  $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>
  $<INSTALL_INTERFACE:${cppitertools_INSTALL_INCLUDE_DIR}/cppitertools>)

What that means is that when installing with CMake, you should be able to find the headers in the expected way, as long as you make sure you include the install include directory, which is like this :

#include <cppitertools/itertools.hpp>

However, if you decide to use FetchContent, which is the more recent CMake's standard for adding dependencies, then it will clone the repo and add it as a subdirectory, not installing it. In that case it will use the BUILD_INTERFACE settings, which points to the project directory itself. What that means is that you can no longer include it in the expected way, since you only see inside the cppitertools directory. You'd have to do it like this :

#include <itertools.hpp>

So this is bad because if you use cppitertools in a library, you'd need consistency between building and installing your library.

At first I though a way to workaround this would be to change the BUILD_INTERFACE include directory, like this :

# Get the parent directory
get_filename_component(
  cppitertools_BUILD_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR} DIRECTORY
)
target_include_directories(
  cppitertools
  INTERFACE
    $<BUILD_INTERFACE:${cppitertools_BUILD_INCLUDE_DIR}>
    $<INSTALL_INTERFACE:${cppitertools_INSTALL_INCLUDE_DIR}/cppitertools>
)

But this has two issues :

1. It forces to add to include paths the whole parent directory, and who knows what's there, it could creates conflicts
2. Using FetchContent it couldn't even work, because the git clone part renames the directory to cppitertools-src (which would need a #include <cppitertools-src/itertools.hpp>

---

The second issues actually highlight a bigger issue : the current design doesn't allow for the user to git clone the repository under another folder name than cppitertools which imho is an issue (it forces the user to git clone in a restricted way).

Potential fix : From what I understand the trouble here is because "expected" way of including files uses paths that are not related to the code structure itself, but rather the name of the git repo in github.

I see two potentials way to evolve to follow what I think is considered a good practice :

1. Putting all files under a cppitertools folder inside the repo and continue using #include <cppitertools/itertools.hpp>
2. Let the code structure as it is and move to an include strategy like this #include <itertools.hpp>

I understand that this might be a breaking change but I think it could be important. Please let me know what you think about it.

[ryanhaining]

I am very sorry that I never saw that you had replied to this issue.

the current design doesn't allow for the user to git clone the repository under another folder name than cppitertools which imho is an issue (it forces the user to git clone in a restricted way).

I think I'm misunderstanding the problem here, can't anyone do

git clone https://github.com/ryanhaining/cppitertools.git my_custom_name

WRT the potential fixes, obviously anything breaking is not ideal, and dumping all the .hpp files at the top level is probably worse. I'm guessing nothing has changed with cmake since November. I could probably stand for moving all the .hpp files into another cppitertools directory if that would definitely fix the cmake issues.

[maxmarsc]

Hi, no problem it happens.

I'll try to explain it without talking about cmake. Yes anyone can technically do this

git clone https://github.com/ryanhaining/cppitertools.git my_custom_name

=> but it can create issues

Why ?:

In the documentation you mention the user should include this way : #include <cppitertools/itertools.hpp> which, since every header is at the root of your repo, only works if you clone under the exact name cppitertools.

Which means your include strategy (which impact how you can share/reuse your code), depends on the name you give to the cloned repo, which could vary for various reasons :

1. avoid duplicates, you might want to have mirrors of a same repo on your local machine (ex: cppitertools-github, cppitertools-custom)
2. use of CMake's FetchContent which will rename the repo to cppitertools-src
3. possibles other reasons ? I think forcing the user to use a specific repo name is not really a good thing

---

I would agree that moving everything to a cppitertools folder inside the repo would be the best idea, because I think having a "prefix" <cppitertools/... in the include directive helps prevent naming conflicts, but that's a personal opinion and some library authors don't do this.

I cannot guarantee that there would not be a need for a few changes in the CMakeLists.txt to accommodate for such a change, but I'd be happy to help. However I probably won't be able to help on other packaging tools (like conan or else, if you use them, I'm not sure)

[ryanhaining]

Thanks for this explanation, the custom naming to avoid duplicates makes sense as a possibility to avoid.

I'll have to figure out the conan file stuff. Might need to tag this as a 3.0 release to avoid the concerning breakages.

The only other package management is the bazel BUILD file which I can of course do. cmake is the one I'm most concerned about.

[maxmarsc]

Ok so I created a fork with the following modifications :

• all headers have been moved to a cppitertools folder
• all include directives in the test and examples files have been changed from #include <HEADER.hpp> to #include <cppitertools/HEADER.hpp
• CMakeLists.txt now reflects such a change

This fixes:

• the issues we mentionned
• another one I discover : you could not install with cmake in a subfolder of the git repo (ex: in cppitertools/install) due to a recursivity in the cmake config

What's left are the bazel & conan config to adjust. All the tests and examples compiles and run. I tested the new CMake configuration with both install workflow and FetchContent workflow.

I did not created a PR yet because it would break the bazel and conan build on the master branch. How would you like to proceed ? Create a branch for this issue on which I'll merge my fork ?

[ryanhaining]

Thanks. I created https://github.com/ryanhaining/cppitertools/tree/buildfix buildfix branch for this. If there are build_tests/CMakeFiles that should never get tracked, can we add a .gitignore for that?

[maxmarsc]

When using cmake, the user can use any folder as its build folder (here build_tests is just how I do it), so almost impossible to track all possible uses.

[ryanhaining]

ah I see the second commit is removing files from the first one. Would you be able to amend the first commit to exclude those files instead? For the sake of keeping the history cleaner on this

[maxmarsc]

Done

[ryanhaining]

Great, tyvm

[ryanhaining]

merged to master