Make corrosion_install install header files and export configurations

[gtker]

This PR makes corrosion_install also install header files attached to the Rust target. It works both for files that are added through target_include_directories and through target_sources file sets for 3.23+.

Progresses #415, #63, #64, and #117.

I believe that this will also solve #261 since the headers are added through target_include_directories and target_sources for 3.23+, although I'm not sure how to setup cxxbridge so I can't test it.

This also correctly installs headers added through corrosion_experimental_cbindgen, although I had errors relating to cbindgen not running when testing it. If the files are generated in the correct location they will be installed.

This uses commits from #543, but since it has been approved and is just waiting for a CI run before merging I assumed it was OK to put this up.

Adding header files the old way:

    target_include_directories(is_odd INTERFACE
            $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
            $<INSTALL_INTERFACE:include>
    )
    target_sources(is_odd INTERFACE
            $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include/is_odd/is_odd.h>
            $<INSTALL_INTERFACE:include/is_odd/is_odd.h>
    )

will lead to

-- Installing: /tmp/install/./include
-- Installing: /tmp/install/./include/is_odd
-- Installing: /tmp/install/./include/is_odd/is_odd.h

while using file sets:

    target_sources(is_odd
            INTERFACE
            FILE_SET HEADERS
            BASE_DIRS include
            FILES
            include/is_odd/is_odd.h
    )

will lead to

-- Installing: /tmp/install/include/is_odd/is_odd.h

The distinction between file sets and include directories is in preparation for EXPORT functionality.

Caveat

For include directories added through target_include_directories the PUBLIC_HEADER DESTINATION argument does not determine the install location, but the name of the include directory does.
So for target_include_directories(my-lib INTERFACE wacky-files) and corrosion_install(TARGETS my-lib PUBLIC_HEADER includerinos) the file would go into ./wacky-files rather than ./includerinos.

This is because CMake doesn't seem to offer any real way of installing entire directories without doubling nesting them, and we can't file(GLOB) through the directories to find files/directories since they can be generator expressions (and must be in order to be relocatable).

From what I can tell just CMake doesn't have a good way of installing header files if you're not using 3.23+.

3.23+ file sets work completely fine.

Open questions

• This currently installs all PUBLIC/INTERFACE file sets for a target, should it do that, or only the HEADERS file set?
• Is PUBLIC_HEADER DESTINATION not working for target_include_directories too confusing for it to be included? Or should we find another way. I don't have a clue how that would be done, but I'm open to suggestions.

[gtker]

I'm extending this PR with functionality for EXPORT since it's a small change and pretty related to the header installation.

The current implementation creates a file at ${CMAKE_BINARY_DIR}/corrosion/<export-name>TargetsCorrosion.cmake that can be included in the main config file as shown in the docs.

There's still an awful lot of boilerplate, but almost all of it is just the "normal" amount of pointless CMake configuration.

I'm creating the COR_FILE_NAME property on the *-static/*-shared targets in order to avoid duplicating all the logic in corrosion_import_crate. This will be externally visible, but I don't think there's anything wrong with leaking the file name of the library.

Sadly, because we need the base path of the install prefix in the *Corrosion.cmake file, users will either need to use configure_package_config_file or manually create the variable. I couldn't think of a better way to get it without requiring the relative destination of the file, which would scope creep the corrosion_install function significantly.

I have tested this with both static and shared libraries using the https://github.com/gtker/corrosion_export/ repo.

With this PR the planned functionality of corrosion_install should be complete.

[gtker]

I completely forgot Windows IMPLIB support.

If IMPORTED_RUNTIME_ARTIFACTS correctly copies the IMPLIB it should work, although I can't test since I'm on Linux.

[jschwe]

Haven't reviewed yet, but one short comment / question

This is because CMake doesn't seem to offer any real way of installing entire directories without doubling nesting them

I'm not sure I understood what you mean, but did you try specifying the target directory with a trailing slash?

The last component of each directory name is appended to the destination directory but a trailing slash may be used to avoid this because it leaves the last component empty.

Source: install(DIRECTORY)

[jschwe]

I left some comments inline

This currently installs all PUBLIC/INTERFACE file sets for a target, should it do that, or only the HEADERS file set?

is there a need to install all file sets by default? I would start with HEADERS only first, unless there is a reason to install more by default.

[jschwe]

Let's add this example code as part of a test, and include it via the anchor feature of the book.
I just added a first test project which installs a Rust executable to test/corrosion_install/install_rust_bin/rust_bin

Could you perhaps add a demo project which installs a shared library / headers? If can help with integrating it into the testing system.

[gtker]

I'm not sure I understood what you mean, but did you try specifying the target directory with a trailing slash?

I considered doing this like this, but we need to allow generator expressions, so it won't be a simple append-./ operation. We could replace > with ./>, but that would break the moment nested generator expressions are used.

is there a need to install all file sets by default? I would start with HEADERS only first, unless there is a reason to install more by default.

I don't see it as completely necessary, but there had to be some default behavior so I just chose to install everything. I'll make it only install the HEADERS file set.

I don't have a lot of time right now, but I'll reply to your other comments later and make the changes.

I also tested it out on Windows, and I don't believe the IMPLIB is installed correctly with IMPORTED_RUNTIME_ARTIFACTS, so we'll need to fix that as well.

[gtker]

Still working on remaining change requests, just wanted to update.

[gtker]

I fixed all issues, except creating a test.

As far as I can see there's already a test for libraries in test/corrosion_install/, do I need to do anything else?

[jschwe]

As far as I can see there's already a test for libraries in test/corrosion_install/, do I need to do anything else?

I probably don't have the time to review today, but perhaps you could edit the test, so that the shared library has an associated header that also needs to be installed. (

corrosion/test/corrosion_install/install_lib/main.cpp

Line 5 in f6ccb71

extern "C" uint64_t add(uint64_t a, uint64_t b);

this line should be in a header associated with the library, instead of hardcoded in the executable)

[gtker]

I added the header file, but I really can't penetrate the meta levels of CMake script that the tests are made of. It might have been easier with python scripts that just called the cmake executable. :)

[gtker]

The test failures do not appear to be mine. :)

[jschwe]

Sorry for the delay, busy two weeks. I'll try to get to this MR over the weekend

[jschwe]

I think we can land this as is for now, and iterate over it in case any problems surface.
Thanks a lot for looking into this.

[gtker]

Cool. Let me know if any problems surface with corrosion_install. I won't be keeping an active eye on the issue tracker. :)