Reconsider install location

[friendlyanon]

This came up in the C++ Slack earlier and reminded me that this library could cause the same issue as well.

Consider that include/fplus gets installed as the <prefix>/include/fplus directory and adds <prefix>/include to the include paths of the installed target. If you link against the installed FunctionalPlus::fplus target, then you get the whole of <prefix>/include added to your include paths. I'm sure you can guess what kind of trouble that could cause.

My preferred install path where I have control is <prefix>/include/<project-name>-<project-version>, so when linking against the CMake package target, you don't get the whole world in your include paths implicitly, just the library you linked against and possibly dependencies.

I propose that this be changed and instead of <prefix>/include FunctionalPlus be installed to <prefix>/include/FunctionalPlus. Anyone not using CMake (why would anyone want to do that???) can still reach the library with #include <FunctionalPlus/fplus/fplus.hpp> via the implicit system include paths, but anyone using the CMake package will get the correct include path that makes only FunctionalPlus available, so #include <fplus/fplus.hpp> can be used like usual.

[Dobiasd]

So if I understand correctly, assuming somebody installs fplus like the following on a normal Ubuntu

git clone https://github.com/Dobiasd/FunctionalPlus
cd FunctionalPlus
mkdir build
cd build
cmake ..
make
sudo make install

it would not land in /usr/local/include/fplus/ but in /usr/local/include/FunctionalPlus/fplus/ instead.

I don't know about the percentage of fplus users not using CMake, but writing #include <FunctionalPlus/fplus/fplus.hpp> instead of #include <fplus/fplus.hpp> seems a bit unusual, at least in my experience. Also, it would be a change that could break old code already using fplus, so I'm a bit hesitant here.

Is the kind of trouble, that having it in /usr/local/include/fplus/ can cause, more of theoretical nature, or does it actually happen regularly?

Asking, because glancing at the readme files from some other libraries (using JSON libs as a random example), they all seem to use the way we currently also have:

• https://github.com/Tencent/rapidjson #include "rapidjson/document.h"
• https://github.com/nlohmann/json #include <nlohmann/json.hpp>
• https://github.com/jeaye/jeayeson #include <jeayeson/jeayeson.hpp>
• http://tgockel.github.io/json-voorhees/ #include <jsonv/value.hpp>
• https://github.com/danielaparker/jsoncons/ #include <jsoncons/json.hpp>
• https://github.com/bk192077/struct_mapping #include "struct_mapping/struct_mapping.h"

Or maybe I'm missing something here?

[friendlyanon]

The #includes would not change at all from a client's PoV who's using the CMake package. Let's assume that I have fplus installed to my system to the /usr/include location, in my project I have some dependency, which adds /usr/include to my includes and then fplus installed by say Conan to some other prefix like ~/.conan/data/fplus/<hash>/include. Now in my source files if I type #include <fplus/fplus.hpp>, that include will resolve to /usr/include/fplus/fplus.hpp first instead of ~/.conan/data/fplus/<hash>/include/fplus/fplus.hpp, because of the order of the flags.

If there was another folder that uniquely identified this library, then including an unrelated dependency would not cause this issue.

Of course, a conscious user can just pass the necessary flag to define the location ahead of time:

cmake -S . -B build -D CMAKE_INSTALL_INCLUDEDIR=include/FunctionalPlus
cmake --build build
sudo cmake --install build

but of course this requires prior knowledge about the issue and how it can be solved. This is the kind of (arcane) knowledge that comes with dealing with distros and managing packages for them.

[Dobiasd]

Or maybe I'm missing something here?

Oh yes, I did. 😬
Just because the example code of a library uses #include <library/something.hpp> does not mean that it would install a file /usr/local/include/library/something.hpp and expect the compiler to be invoked with (simplified) g++ example.cpp. It ould also install to /usr/local/include/what/ever/here/library/something.hpp and expect the compiler to be invoked with g++ -I/usr/local/include/what/ever/here example.cpp

---

[...] If there was another folder that uniquely identified this library, then including an unrelated dependency would not cause this issue.

Thanks for the good explanation and example. So having fplus installed at multiple locations alone suffices to create problems, if one is not careful about the priority of the different include paths given to the compiler. Makes sense, however

The #includes would not change at all from a client's PoV who's using the CMake package.

I'm worried about the clients not using CMake. I know you say "why would anyone want to do that???". 🙂 But even if we might not agree with the reason for doing that, I'd like to avoid breaking their workflow nonetheless. Is there some way we could estimate the percentage of such users? I'm especially worried about the people using FunctionalPlus only because of frugally-deep. They often come from a data-science/Python background and might not be used to building C++ projects using CMake.

[friendlyanon]

Since I'm not aware of fplus being distributed in distro package managers based on the INSTALL.md, this might not be such a huge problem right now, but I wanted to let you know that there is a possibility of such interaction resulting in unintended includes of libraries, where the behaviour depends on the order of the include flags (you can't really control these from CMake).

Unfortunately, I do not know of a way to gauge non-CMake usage of the library, so I'm not going to push you to change things. It's enough if you know the problem exists and how it can be solved.

[Dobiasd]

Thanks. ❤️
Yeah, I now understand the problem, and I don't like it either. But currently, I'm just way too scared of support hell opening up for me in the frugally-deep issues in case we'd make that change here in FunctionalPlus. 😬

[xtofl]

"then you get the whole of /include added to your include paths"...

There are two world views with regard to include paths and install prefixes:

1. I don't care because I trust the maintainers to do 'the right thing' so that the compiler finds installed libraries in default places. This is the way I have seen traditional C developers work (think upfront, and trust others will do so, too), and it makes it possible for newcomers to 'just start developing' without needing to know too much of the nitty gritty details.
2. I am meticulously composing my development environment so conflicts don't happen and I am willing to have a more complex include path. This is modern professional C++ development audience, and they know they should tinker with the install prefix (or use a package manager for this, or docker, or ...).

When the install prefix stays as is, both audiences are served well: it does 'just works' out of the box, and can be tweaked to 'do the right thing' by specifying the prefix at build and install time.

Aside...

Actually, the "I'm sure you can guess what kind of trouble that could cause." problem is only a consequence of the way compilers (and linkers, for that matter) expect the user to set up their environment correctly: fplus/xyz.h must only be found in a single location on the include path. When you have

/usr/shared/fplus-1.0.0/include/fplus/xyz.h
/usr/shared/fplus-1.1.0/include/fplus/xyz.h

The preprocessor will not complain about having two matches when looking on -I /usr/shared/fplus-1.0.0/include -I /usr/shared/fplus-1.1.0/include. (Nor does the linker, for that matter)

I think this problem is not up to library/package maintainers to solve, but to the C++ committee.

[friendlyanon]

This isn't really something the committee can change, people writing libraries should excercise some level of discipline and have knowledge about packages. I admit the resources to acquire this kind of knowledge is scarce and not of great quality.

I myself plan to work on something like a modern CMake tutorial that would ideally cover topics like this, as I believe the current availability of documentation surrounding such topics is in a dire situation. Unless you trip into the problems you'd have never known about them.

Regarding your example of having different versions of the same library, you can still separate things into libraries from the CMake side and link to the appropriate target. Again, this requires some level of discipline.

[xtofl]

You're right; I first thought 'compiler vendors' should solve, but why would they if the compilation model leaves this issue in the open? So I aimed a little higher and wrote 'committee' :). But then again, I'm not sure the standard doesn't mention this.

15.3 Source file inclusion [cpp.include]

A preprocessing directive of the form
# include < h-char-sequence > new-line
searches a sequence of implementation-defined places for a header identified uniquely by the specified sequence
between the < and > delimiters

So if the header is not uniquely identified, can we say the implementation is in contradiction with the standard?

[alexreinking]

Of course, a conscious user can just pass the necessary flag to define the location ahead of time:

cmake -S . -B build -D CMAKE_INSTALL_INCLUDEDIR=include/FunctionalPlus
cmake --build build
sudo cmake --install build

but of course this requires prior knowledge about the issue and how it can be solved. This is the kind of (arcane) knowledge that comes with dealing with distros and managing packages for them.

It is true that the CMake install process is not widely understood, but one must be very careful with the implementation of this feature so that a package maintainer can still set CMAKE_INSTALL_INCLUDEDIR to override your rules. For example, the following is totally wrong because it prevents a package maintainer from removing the FunctionalPlus folder without patching the build files:

include(GNUInstallDirs)
set(CMAKE_INSTALL_INCLUDEDIR "${CMAKE_INSTALL_INCLUDEDIR}/FunctionalPlus")

To be correct, you need to check that CMAKE_INSTALL_INCLUDEDIR is not defined, then include GNUInstallDirs, then override it if it wasn't set to begin with:

if (NOT DEFINED CMAKE_INSTALL_INCLUDEDIR)
  set(override_includes YES)
else ()
  set(override_includes NO)
endif ()
include(GNUInstallDirs)
# CMAKE_INSTALL_INCLUDEDIR is defined as a cache variable now
if (override_includes)
  # Force override. Only happens when the user wasn't trying to override it the first time.
  # Won't be executed multiple times because override_includes is always false after first run.
  set(CMAKE_INSTALL_INCLUDEDIR "${CMAKE_INSTALL_INCLUDEDIR}/FunctionalPlus"
      CACHE PATH "C header files (include)" FORCE)
endif ()

But wouldn't it be simpler to just teach people CMake better? I'm not sure this sort of thing belongs anywhere besides CMake's documentation & supporting blog posts.

[friendlyanon]

That's a lot of code to replicate what cache variables do already:

set(CMAKE_INSTALL_INCLUDEDIR "include/FunctionalPlus" CACHE PATH "")
include(GNUInstallDirs)

CMAKE_INSTALL_INCLUDEDIR has never meant anything other than include, so the above will never misbehave and if the user already provided a different path, then that path will be used, since the user always knows the outside requirements the best.

And that is actually a really elegant way to solve this problem. Thanks for the idea!

[alexreinking]

CMAKE_INSTALL_INCLUDEDIR has never meant anything other than include, so the above will never misbehave and if the user already provided a different path, then that path will be used, since the user always knows the outside requirements the best.

I didn't bother to dig through the CMake source code. I suppose they wouldn't be able to change it without a policy so given that it has always been include, just creating the cache variable before include(GNUInstallDirs) is correct.

And that is actually a really elegant way to solve this problem. Thanks for the idea!

Happy to be of service, even if it isn't my preferred approach🙂. Generally speaking, the argument to DESTINATION in any of the install() commands should be an unadorned cache variable. In my own projects, I always create a MyProj_INSTALL_CMAKEDIR to allow relocation of my CMake package configs and target export files (some people want them in lib, others in share, etc.)