Build system: CMake-generated files does not honor CMAKE_INSTALL_INCLUDEDIR

[dbermond]

When building CopperSpice with -DCMAKE_INSTALL_INCLUDEDIR=<path>, the CMake-generated files does not honor the specified headers path. Instead, it always generates CMake files pointing the header files to <prefix>/include/Qt<component>. Namely, CopperSpiceLibraryTargets.cmake will still have INTERFACE_INCLUDE_DIRECTORIES referring the wrong header path <prefix>/include/Qt<component>.

That's because the path include/Qt<component> is hardcoded across CMakeLists.txt files.

As a result, when building CopperSpice with a custom include directory, applications will fail to find the CopperSpice headers, as CMake will still report the wrong path <prefix>/include/Qt<component>.

For example, when trying to build the Diamond editor against such a CopperSpice that has been built with -DCMAKE_INSTALL_INCLUDEDIR=include/copperspice, CMake will wrongly report the QtCore headers path at /usr/include/QtCore, while the expected would be /usr/include/copperspice/QtCore. And this will lead to the following error:

CMake Error in src/CMakeLists.txt:
  Imported target "CopperSpice::CsCore" includes non-existent path

    "/usr/include/QtCore"

  in its INTERFACE_INCLUDE_DIRECTORIES.  Possible reasons include:

  * The path was deleted, renamed, or moved to another location.

  * An install or uninstall procedure did not complete successfully.

  * The installation package was faulty and references files it does not
  provide.

[dbermond]

I've created pull request #171 which fixes this issue for me. Tested on Arch Linux x86_64.

[ThomasKrenn]

In most cases it should not be necessary to run cmake 'install'. Outside projects can use the exported targets from the build tree, if the BUILD_INTERFACE is setup correctly.

https://cmake.org/cmake/help/latest/prop_tgt/INTERFACE_INCLUDE_DIRECTORIES.html

Another issue is that the interface files are duplicated during 'build' time using the 'macro_generate_public' This confuses the compiler/ide due to the different timestamp of files used to build the target and 'interface' files used by the other targets in the (same) project.

[ThomasKrenn]

Hello Daniel, please also add the BUILD_INTERFACE. Here is an example:

`

$<BUILD_INTERFACE:${CMAKE_BINARY_DIR}/include/Qt"Component">

`

[dbermond]

@ThomasKrenn This is not needed in my opinion, because there is already an include_directories statement specifying ${CMAKE_BINARY_DIR}/include/Qt<component> for the build-time include path. For example, you can see it here for QtCore.

Besides, this would be unrelated to the topic of this issue.

[ThomasKrenn]

@dbermond
INSTALL_INTERFACE is used to export(!) the include dir from the install tree. BUILD_INTERFACE is used to export(!) the include dir from build tree. ( ${CMAKE_BINARY_DIR}/include/Qt<component> this is needed build the target but it does not(!) export the include path.)

The current cmake files require that copperspice is built and(!) installed, but the install step is not necessary. To allow a client to import copperspice targets from the build tree one need to set: BUILD_INTERFACE
and Export the targets of the build tree. https://cmake.org/cmake/help/latest/command/export.html

Here is an example from Pablo Arias:

target_include_directories(JSONUtils
    PUBLIC 
        $<INSTALL_INTERFACE:include>    
        $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
    PRIVATE
        ${CMAKE_CURRENT_SOURCE_DIR}/src

)

https://pabloariasal.github.io/2018/02/19/its-time-to-do-cmake-right/

A JSONUtils client that consumes (imports) the target from the build tree import also the include dir from the BUILD_INTERFACE.

[bgeller]

The current cmake files require that copperspice is built and installed, but the install step is not necessary.

As per our CS Overview documentation the install step is necessary and always has been. Nothing has changed in our procedures.

I adore the passion we are all feeling and it is very exciting to see so many great comments and new people posting. Just trying to bring the heat down a tiny bit. The CS team is happy to discuss why some choices were made and where we can make improvements. We are passionate about CopperSpice and want to work with users and other developers to achieve the goal of creating a set of great C++ libraries.

Lets try to focus us what you are looking for and what problem you and others want to solve. Then we can consider the alternatives.

[bgeller]

I would like to expand on my previous comment. We strongly suggest you do install CS so all of the files which are intended for distribution are located in your "install" folder. Our CMake files for KitchenSink, DoxyPress, and Diamond were written based on all the CS files being located in the install folder. Of course you can modify this if something else works better for your project.

However, there are some very good reasons to do the install. As an example, there are private include files in the "build" folder which are not designed and should not be included in a user project. Also, on Unix platforms the RPATH will not be set correctly for executables and libraries until they are installed.

[ThomasKrenn]

Hello Barbara, thanks for your answer.

We strongly suggest you do install CS so all of the files ...

That is why I wrote the comment. The install step is not really necessary, every copy makes the setup more error prone.

are private include files in the "build" folder

There is no need to add them to the export set. Cmake transitive compile dependencies apply to the build tree and the install tree.

(Private - Needed by me but not my dependers Public - Needed by me and my dependers Interface - Needed not by me but my dependers )

RPATH will not be set correctly

There is a BUILD_RPATH and a INSTALL_RPATH and countless other cmake and system options to solve rpath issues.

... Diamond were written based on all the CS files being located in the install folder.

Why? The build tree can be exported like the install tree. I use the install step only if I want to share binaries with coworkers on the same machine.

[agserm]

@ThomasKrenn, CopperSpice was designed to be installed and not used from a build folder. The idea of using the build folder for anything other than building CopperSpice itself is not practical. That build folder contains a large number of files which are used internally in CMake and Ninja. They are not CopperSpice files. Since not everyone will build from source, our binary distributions consist of exactly the files you would find in the install folder.

[agserm]

@dbermond, I believe we understand your point of view. Our goal is to provide as much flexibility as possible while maintaining consistency for our supported platforms.

It turns out the install layout is slightly different for every platform. This can be challenging for us when providing user support. Although your changes are interesting, it would add a lot of complexity to our support process. We want to keep all of this in mind as we are looking at things like precompiled headers, modules, and changes to better support packaging.

[ThomasKrenn]

@agserm , the suggested change from @dbermond (and my changes) are options. The don't change the default deployment (install). There is no need to officially support them, it simply makes the build more versatile and would give you a competitive advantage over Qt.

That build folder contains a large number of files which are used internally in CMake and Ninja.

It seems that you are to much focused on the content the (build/install) directory. A CMake package client can only consume targets that have been exported. Exporting the build tree would export the same targets as the install tree.

When using CMake one must think in targets and properties of the targets. Getting them right at 'configure' time makes the life easier for all users. A simple test would e.g. run CMake configure Copperspice (without build) and then CMake configure Kitchensink. The configure step of Kitchensink must be able to import all needed targets from the copperspice package. (Again: without building anything(!))

Another issue of the current build setup is that there are no CMake targets for third party libraries. On the file system you have a '3rdparty' folder, but without CMake targets 3rdparty cpp files are part of the copperspice targets. Current Copperspice user with an IDE 'see' copperspice targets as an ugly mix of copperspice files and third party files. All third party libaries should be CMake targets (probably static libraries), should not be exported and should have a 'folder' property '3rdparty'.

[dbermond]

@agserm Thank you for the reply.

I understand that this is your software, hence your rules. Fair enough.

But I cannot see how providing the correct support for a well know CMake option like CMAKE_INSTALL_INCLUDEDIR can add complexity to your support process. At the contrary, I think that you add complexity when such a standard CMake option does not work as expected on your project. You are already supporting other similar options like CMAKE_INSTALL_PREFIX and CMAKE_INSTALL_BINDIR, so what makes CMAKE_INSTALL_INCLUDEDIR different? Your CMake setup is broken in this way, as it's not working as expected.

Besides, in my humble opinion, it does not look like a good practice to hardcode a specific install path across CMakeLists.txt files. This way, you are removing from system packagers the possibility to use CMAKE_INSTALL_INCLUDEDIR in order to make packages compliant with distribution filesystem layout policies, as CMake documentation recommends. Otherwise, system packagers may be forced to carry downstream a patch to fix your CMake code that does not work as expected with a standard CMake option.