Boost target_include_directories should probably be PRIVATE #6464
Comments
|
I believe there are libtorrent header files that include boost headers, which means clients of libtorrent also need to make the boost include path available |
|
Do I understand it correctly, that it was done to save the client's CMakeLists.txt from explicitly adding Boost include directories with it's own target_include_directories directive? And libtorrent is just not supposed to be used with Boost headers and libraries if they are located in project's source tree? |
Yes. Just like a client of any library should inherit any configuration from the target that's required to link against it. libtorrent requires that clients of it also have boost includes available, so it must be propagated to clients.
I don't see how that follows. If you're suggesting that libtorrent can be built with one boost installation and then used with another, that's certainly not ok. They must be the same. |
I'm trying to use the same boost for my target and libtorrent, but which is located under project's source tree. Let me explain it in more details how I see it. CMake's target_include_directories if called with PUBLIC will add the argument to INTERFACE_INCLUDE_DIRECTORIES variable. That variable is used by CMake with assumption of portability. In particular, when it generates install scripts, it uses these paths. Since people generally misunderstand that consequence, CMake developers added a safeguard to prevent execution of CMake scripts if such public path is obviously not portable. One case of such non-portable path is the source directory of the project being compiled, if CMake sees that such path is instructed to be reexported as a public (install) location dependency it can know for sure that it's wrong and prevents compilation. In other cases such a check can't be done, and the compilation will continue. My project's source tree contains precompiled boost libraries for different architectures for cross-compiling. Both my project and libtorrent compiled use the same boost library each time. The problem is that libtorrent's CMakeLists.txt is trying to reexport Boost's location, which it obtained from FindBoost as a public include. The thing is that there's no reason to assume that such path is a public-exportable location. In fact various FindBoost hint variables are made specifically to hint to search for Boost at any custom location, which is, most likely is not re-exportable. I can easily fool CMake's safeguard check by copying Boost to some non-source prefixed directory, to work that around. But I think that assumptions made in libtorrent's CMakeLists are wrong, because using an internal (project-local) Boost is quite legitimate scenario. |
|
I'm not a cmake expert, but I don't understand what it means for an include path to "non-portable". It's a path on your computer that your compiler can use as an include path, what's not portable about it? It's possible that I've misunderstood what |
|
By portable I mean that such an include path is always valid, e.g. when you're doing a 'make install'. This stackoverflow question probably does a better job explaining it than me: https://stackoverflow.com/questions/25676277/cmake-target-include-directories-prints-an-error-when-i-try-to-add-the-source Note that they're suggesting to use generator expression to distinguish the path for build and install interface. But in our case we can't do that, because a portable path (for install interface) may just not exist. The path obtained from FindBoost is not necessarily a path that is valid for install targets. When I'm using libtorrent through add_subdirectory I'm not planning to do an "install" at all, I just link to the library, but CMake can't know that. So it assumes the install target must be generated. And that install target is checked for the validity of exported paths. There is almost no way for CMake to make such checks, but an absolute path to a source directory is obviously is not that is valid for "install". It doesn't mean that any other arbitrary is valid, though, CMake just doesn't have a way to verify the path, so it will compile without a glitch generating a possibly invalid install target. As for relying on CMake, it can't do that currently either, at least in add_subdirectory case. Pushing an include path to parent doesn't lead to any predictable result, the parent may do whatever it wants within it's CMakeLists with include directories. It may already include different Boost in include and you try to override it, or it may add a target_include_dir with it's own after calling add_subdirectory with your CMakeLists.txt. The result is not predictable. Here's a lengthy post with an explanation PUBLIC/PRIVATE pitfalls if you are interested: https://crascit.com/2016/01/31/enhanced-source-file-handling-with-target_sources/
|
|
I believe the issue here stems from incorrect dependency propagation due to the use of Among other things, #6270 is supposed to solve such problems. To be fair, I have not tested it against OP's exact case ("compiling as CMake subdirectory (through @elelel #6270 got stuck in discussion hell, but perhaps you could give it a try and see if it solves your issue. |
|
@FranciscoPombal the libtorrent target links against the boost target, so one would expect that libtorrent itself has the boost include directory visible to it. However, the libtorrent target must also propagate this include directory to its clients. I would expect this to be done by declaring the dependency on boost to be I suppose this ticket is about messing with the include directories directly, and explicitly, which I suppose is questionable. |
"Messing with the directories directly and explicitly" is exactly what the current CML does by using the But then again, nothing like testing to be sure. It is also possible that #6270 too would require some tweaks to handle this case correctly. OP's use case seems legitimate, CMake projects (libraries, in particular) are supposed to be able to be used as subprojects in larger project hierarchies without issues. |
|
@arvidn I don't think that enforcing client's include directories from a dependency is the intention behind PUBLIC includes in CMake. In fact, the whole idea of a dependency library dictating the include path/third-party libs to it's parent in dependency node is not quite safe. Imagine there're two libraries being used by a project. And each of them decides which Boost to use with it's own methods. Now if both libraries push these paths/locations up the dependency graph how the conflict will be solved? It's entirely unpredictable and depends which one was the last to be added as add_subdirectory or in any other way. If a library intends to play nice with any tree-graph dependency structure, it either has to isolate it's subdependencies from the client, which is not possible in our case, because we need Boost .h interfaces, or it has to be configured from upstream (client) which must take the responsibility to find Boost and to guarantee that it's the same Boost for all it's dependencies. PUBLIC keyword indicates that the include dirs, among other since, stay valid after you compiled the project (classic 'make build' equivalent) till you install the project ('make install'). A path within source tree is obviously not a valid path for "make install" stage. This is one of the few cases when it is clear that PUBLIC keyword is not used as supposed to be used by CMake developers, and they have the ability to catch that case and prevent compilation, which they do. If I place it to any other dir (not prefixed with source directory), there will be no problem compiling: CMake has no way to detect PUBLIC keyword usage which is still wrong. |
|
I think you are misunderstanding how a large part of CMake is supposed to work. And yes, if libtorrent is built against a certain set of boost artifacts, it is expected that projects that link against libtorrent also use the same boost artifacts. What else did you expect? If I'm misunderstanding something here, please let me know. As for build environment-dependent paths making their way into the dependency interface causing the build to fail because of that: that is obviously a problem, which as I speculated in Again, I encourage you to try to reproduce the problem with #6270. See also the concept of |
If you are using add_subdirectory style library dependencies PUBLIC keyword can't be used that way. Let's say I write a cool multinetwork p2p program. It uses libtorrent, and fictional libnapster and libedonkey. Each of them might want to use Boost. And the project on it's own also uses Boost. What makes libtorrent so special, that it has monopoly of finding Boost and pushing it upwards? Why libnapster can't do that? The only correct solution is to find boost in CMakeLists.txt of the project, and then pass the include/lib to the dependencies (to libtorrent, libnapster, libedonkey). You mention INTERFACE and that is exactly the problem, libtorrent somehow thinks that a path obtained from FindBoost is a valid INTERFACE path. But it's not. It might be a precompiled Boost that is used only inside project and will never be published by the project in install target. When someone does "make install" with my p2p program I don't know that install scipt to know anything about my /home/user/project/source/lib/arm64/boost/include path. That is explained in more detail in links of this post: #6464 (comment) I don't think I will be able to quickly reconfigure my cross-compilation project for #6270 to test it out any time soon, it requires rewriting quite a lot of CMake scripts. I'm totally okay with the hack I'm doing now, just patching the PUBLIC out of libtorrent's CMakeLists.txt EDIT:
The reason of source path used as lib path for Boost is that because the correct Boost libraries are located there. They are located there because they are precompiled boost libraries and headers for cross-compilation for differrent architecture. When using FindBoost there are ways to hint it where to search the correct Boost. If you are using custom Boost, e.g. cross-compiling it will be some path like /project/lib/precompiled/arch/riscv/boost/include. It will be both in old-style variables as a result and in lib-interface targets. If such library happens to be located under same /src dir as the project's CMakeLists.txt, it is by definition non-public (can't be in install interface) and CMake detects that. |
It doesn't make one library special, all of them should (probably) propagate the boost dependency. If they disagree, the build system better fail. You can't have multiple different boost versions linked into the same binary. If the build system doesn't, that's a problem of the build system, not of the libraries you're linking against.
That precompiled boost still needs matching headers for it, right? Using the correct headers is critical to not violate the ODR rule. It's fine if your target doesn't propagate the boost dependency by linking statically. But you can't have multiple boost versions in the same binary, that's a limitation of how programs are linked. I would expect that if your binary links statically against boost, any install rule wouldn't care about installing any header nor libraries, since they're already statically linked against your binary. But I'm not familiar with how install targets work in cmake. |
|
that said, it does sound like there's an issue in the libtorrent cmakefile where it uses include directories explicitly at all, which it shouldn't be. but I don't see how the boost dependency itself can't be made public. |
There are no such restrictions, which are leading you to a false "only correct solution". It is perfectly fine for to have the following (assume you have a recent enough boost to satisfy all minimum requirements):
All projects will link to the same boost version, provided you provide the correct hints and your environment is correctly configured. libtorrent and libnapster will propagate the dependency on Boost to foo, while libedonkey will not (but it will use it internally). Furthermore, if foo uses Boost internally independently of these libraries, it should be explicit about it (add
I'm still convinced you should try out #6270 and report your findings, it applies cleanly to Overall you seem to be trying do rather complex stuff (cross compiling for different architectures and nesting projects), which I would love to be able to debug and investigate better. Unfortunately, I can't easily replicate your setup. That being said, I would also advise you to experiment with:
Here is an excerpt of Professional CMake 10th edition (from the same author as in one of the sites you linked above) about system roots and how they are used in CMake: Note: to better support cases where libtorrent is not the top level project, we can guard certain logic behind the following condition: if(CMAKE_CURRENT_SOURCE_DIR STREQUAL CMAKE_SOURCE_DIR)
# ...
endif()
The equality test can be replaced simply by one of these variables in CMake >= 3.21: https://cmake.org/cmake/help/latest/variable/PROJECT_IS_TOP_LEVEL.html |
Correct, but that doesn't mean this very reasonable concern has to be addressed with a wrong instrument, that is, putting Boost headers into install interface. In case of add_subdirectory style I have only one single install artefect, in my client example, the statically compiled binary. This is the only file that will be installed, putting any header in it's install interface is not quite right. But add_subdirectory scenario is not the main problem, the main problem is that you can't guarantee that the path you get from FindBoost is a path which is install-stable. Even for those people who do not use add_subdirectory but instead just compile the shared library and then install it, it is still not correct. Because you should not rely to any potentially temp path from source/build environment in your install script. Nobody runs into problem with this (yet), because not many people build the library against some temp-boost in their source dir, still it doesn't mean that what is happening correct.
That will only work, if each of these library calls FindBoost and interprets it the same way. E.g. doesn't paramterize FindBoost by itself. And doesn't make wrong assumptions about it's path, for example. If you can give such guarantees, then I will trust the libraries and do it your way. Untill every library is 100% compliant in it's interpretation of FindBoost result and invokes it with same arguments, unfortunately, the only way for top project to be sure that Boost is found correctly and used correctly is to do the thing itself and pass the info down to dependencies.
Perhaps I explained my situation badly. I have no problem with building my project, I perfectly understand why CMake complains. I also understand how to fix that in many ways. I prefer patching the PUBLIC into PRIVATE in my case because I don't want any dependency contaminate any other dependency or root project. Another solution, which is more robust and is probably more preferable by people who think that CMake developers are wrong and should be fooled out with their install interface path checks, is just to FetchContent from project src dir to any temp dir the precompiled Boost (copy). That way they'll never guess you're doing the nasty thing with install interface. |
Not sure what you mean by "doesn't paramterize FindBoost by itself". But there is no requirement that the dependency libraries of a project "invoke [FindBoost] with the same arguments". They just need to use it correctly and propagate the requirements correctly as exemplified above. If they are not doing that, then you should report the bug on their issue tracker. The only way for the top project to work correctly is for all of its dependencies to correctly propagate their usage requirements. As a top-level project, you don't "pass info down to dependencies", you consume their usage requirements. It is up to you to ensure the dependencies you are using were built (and possibly installed) with the correct options that your project requires, either manually, or with some source package manager-like solution such as vcpkg.
Worth considering: maybe this isn't a "good" way of doing it in the first place? (regardless, and at the risk of sounding like a broken record, I still think the changes #6270 can make the build robust against this) Again, I don't know the details of your environment, but I'm inclined to agree that |
|
I'm not sure what's actionable here. @elelel could you propose a patch in a PR? There are examples that don't use boost, but when they link against libtorrent, the libtorrent headers they include will in turn include boost headers. Those headers will need to be found somewhere. Some dependency on boost need to propagate to the examples. Perhaps I'm misunderstanding you and you're saying that this dependency should be propagated in the form of a public target dependency, not an include path dependency. |
|
If I make this change: I get these build errors: I suppose there's supposed to be an |
I assume you are compiling only CMakeLists.txt from libtorrent and not a client's CMakeLists.txt in this example. If that is the case, the patch that is correct would be just: This compiles without the errors: But that of course means, that a client that uses such a lib's CMakeLists.txt both indirectly (when using later installed libtorrent as a system-found lib, for example) or directly (by using add_subdirectory(...) and pointing to libtorrent's source dir in it's own CMakeList.txt) now has to make target_include_directories(.... Boost) on it's own. If I understand correctly why a "PUBLIC" include is made now, it is such as to free the client of the responsibility to make target_include_directory(... Boost) statement in it's file on it's own. Again, if I understand correctly the reason for that is to make sure that the client's CMakeLists and libtorrent's CMakeLists include the same thing (Boost headers at the same path). But unfortunately it's not what "PUBLIC" keyword is for. "PUBLIC" keyword adds the headers listed in a CMakeLists.txt to CMake's install interface, for the whole project, not just for libtorrent. That means if a client has a CMakeLists.txt that uses libtorrent's source files by making a call to add_subdirectory to libtorrent's source directory, get's these paths into it's install interface. But when you create an end-user application the only public artefact is your final executable and that is the thing that will be "make install"-ed. It's exactly that reason why CMake authors prevented compiling anything they can detect as a non-public location, in my concrete example, source dir location of a public header. It happens that now nobody places Boost headers into their project's src dir, because it's rarely needed, and they are not hitting this error. But it's still ok to do that, there is no reason to say "never place your precompiled Boost libs/headers to your project's source dir". So in terms of actionability I see three options:
|
Right, you suggested keeping it, but making it private. As you point out, this change still makes users/clients of the library to fail to build, for example:
I wouldn't quite phrase it that way. It is the build systems responsibility to propagate dependencies required by the client. If libtorrent adds a dependency on some other library, libtorrent's build files should change, not all users'. From your description, it sounds like cmake does not support this. Which would surprise me because it seems like one of the most important features of a build system (and the core of "modern" cmake). How come this doesn't work? the idea being that raising the abstraction level to "targets" rather than "include paths" would allow cmake to be smarter about it. With this patch, the examples still fail to include boost. |
|
this looks promising. I need to experiment a bit: |
The above point about building I made are relevant to libtorrent's CMakeLists, not to the clients that use it (e.g. example's CMakeLists. To get them compiled each of them has to include in their CMakeLists line like: And this makes sense from pure C perspective, after all that #include that build fails on fails when compiling a example's translation unit because it's -I parameter doesn't include Boost's dir. It's not that it libtorrent's build itself fails.
I wouldn't say that it's CMake who doesn't support this, it's C compile system that does not quite support it. CMake is just a set of convinience wrappers that emulate a "build system" analogous to modern languages on top of standard C approach of specifying dependencies that comes from 1970s. And that approach comes down to two things basically: compilier invocation accepts a plain list of headers that are combined in single plain and large header source and libraries for dynamic linking passed in through linker invocation. C compiler does nothing about "structure" which is unlike other languages like Java. What it "sees" is a single .h source pseudofile that is glued together from pieces of #include that come from actual .h files. CMake tries to do it's best to provide an -I directective that would produce such a pseudofile that makes sense. It may look as if it is fully analogous to mdoern package management for CMake users, but it is still just an emulation which is limited by underlying C build system. Because of such limitations they prevent people from doing certain otherwise natural in other build systems things. Purely from graph-theoretic view, to take a directed tree of library dependencies and produce an undirected chain graph that represents ordered parameters for cc -I argument, one needs deterministic rules of resolving conflicts between the nodes (e.g. when they include the same thing but of differrent versions) and general consistency guarantees (e.g. we depend on header locations that may change AFTER the compilation has been done, for example system-wide install-time h dependencies for a library; Java doesn't have this problem for example). CMake is just trying to act within the scope where more or less formal rules hold as much as possible.
I'm not sure, but it doesn't look to be correct. target_link_libraries wouldn't add anything to includes, maybe only in some special platform-dependent cases? |
|
Maybe a better perspective to understand the issue is just to look at source of CMake's FindBoost: What it basically does, it searches the system on which that script is invoked for boost files of specific versions, taking into considiration various user-defined hint variables. So the only thing it can produce is paths to files that are valid only at build time. It's like running something like "find / | grep boost" on the system where we are building. The PUBLIC keyword in target_include_directories puts the arguments into CMake target install interface. This means that these files/paths are available and valid not only at build time, but also at install time, the time when user types "make install". That is what they mean by keeping the paths in the build "portable" in StackOverflow links I mentioned above. We can't be sure that a paths that we got by browsing build computer's filesystem is valid for every system where we run make install. |
This is how "modern" cmake works (and also other proper build systems). The documentation is here:
|
|
From the same page:
I don't know if the same check that prevents putting fishy paths in install interface exists to prevent fishy paths in link interface. It should. It can also be possible that target_link_libraries(... PUBLIC) make target_include_directories(... PRIVATE), so it will be equivalent to just taking option 2 of what I suggested, and in this case it should work. By the way, the fact that you put something as PUBLIC or PRIVATE does not affect your library's client unless it includes your CMakeLists into their CMakeLists which most people don't do. For most people on Unix-like systems Boost headers are found by their project because they are at the same location as libtorrent's headers, somehere down /usr/include. |
|
I think I came up with a way that both should work with your approach and with cross-compilation structure when part of brebuilt target arch system resides within project src. In client CMakeLists.txt must do something like this: Since the state of Boost::headers is cached, when libtorrent calls find_package(...) again it will just get the right result. For reference, I found previous discussion of the issue #3101 |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Please provide the following information
libtorrent version (or branch): master
platform/architecture: Cross-compiling from x86-64 host to various targets
compiler and compiler version: Clang
When compiling as CMake subdirectory (through add_subdriectory) with custom Boost location set through CMake's FindBoost variables, if that location is within the project source, the following error will occur:
This can be solved by changing PUBLIC to PRIVATE in this line:
libtorrent/CMakeLists.txt
Line 795 in 3623664
Was there any special reason why it was declared PUBLIC there?
The text was updated successfully, but these errors were encountered: