DRAFT: Add Optional Config File Package Creation/Installation to Exemplar

[chayden83]

This PR adds the ability to optionally create and install a config file package for the Beman exemplar project. Config file package creation and installation is enabled by default when the project is top-level. Basic usage is:

% cmake -S . -B build -DCMAKE_BUILD_TYPE=Debug

After this command, you should see the following files in $PWD/build/cmake.

% ls -l build/cmake
total 24
-rw-rw-r-- 1 chris chris 1129 Feb  7 17:56 beman.exemplar-config.cmake
-rw-r--r-- 1 chris chris 2762 Feb  7 17:56 beman.exemplar-version.cmake

If you continue building like so, you should see the config file package and target export installed.

% cmake --build build
...

% DESTDIR=/tmp/exemplar cmake --install build
-- Install configuration: "Debug"
-- Installing: /tmp/exemplar/usr/local/debug/lib/libbeman.exemplar.a
-- Up-to-date: /tmp/exemplar/usr/local/include/beman/exemplar/identity.hpp
-- Installing: /tmp/exemplar/usr/local/lib/cmake/beman/beman.exemplar-static.cmake
-- Installing: /tmp/exemplar/usr/local/lib/cmake/beman/beman.exemplar-static-debug.cmake
-- Installing: /tmp/exemplar/usr/local/lib/cmake/beman/beman.exemplar-config.cmake
-- Installing: /tmp/exemplar/usr/local/lib/cmake/beman/beman.exemplar-version.cmake

Notice that the target export variant name is static. This variant name was automatically determined based on the fact that the project was not configured to build shared libs or to use position independent code. If we configure the project to build shared libs, we get a shared target export variant name.

% cmake -S . -B build -DBUILD_SHARED_LIBS=ON -DCMAKE_BUILD_TYPE=Debug
...

% cmake --build build
...

% DESTDIR=/tmp/exemplar cmake --install build
-- Install configuration: "Debug"
-- Installing: /tmp/exemplar/usr/local/debug/lib/libbeman.exemplar.so
-- Up-to-date: /tmp/exemplar/usr/local/include/beman/exemplar/identity.hpp
-- Installing: /tmp/exemplar/usr/local/lib/cmake/beman/beman.exemplar-shared.cmake
-- Installing: /tmp/exemplar/usr/local/lib/cmake/beman/beman.exemplar-shared-debug.cmake
-- Installing: /tmp/exemplar/usr/local/lib/cmake/beman/beman.exemplar-config.cmake
-- Installing: /tmp/exemplar/usr/local/lib/cmake/beman/beman.exemplar-version.cmake

It's also possible to explicitly set the name of the target export variant using the BEMAN_EXEMPLAR_TARGET_EXPORT_VARIANT cache variable.

% cmake -S . -B build -DBUILD_SHARED_LIBS=ON -DCMAKE_BUILD_TYPE=Debug -DBEMAN_EXEMPLAR_TARGET_EXPORT_VARIANT=custom
...

% cmake --build build
...

% DESTDIR=/tmp/exemplar cmake --install build
-- Install configuration: "Debug"
-- Installing: /tmp/exemplar/usr/local/debug/lib/libbeman.exemplar.so
-- Up-to-date: /tmp/exemplar/usr/local/include/beman/exemplar/identity.hpp
-- Installing: /tmp/exemplar/usr/local/lib/cmake/beman/beman.exemplar-custom.cmake
-- Installing: /tmp/exemplar/usr/local/lib/cmake/beman/beman.exemplar-custom-debug.cmake
-- Installing: /tmp/exemplar/usr/local/lib/cmake/beman/beman.exemplar-config.cmake
-- Installing: /tmp/exemplar/usr/local/lib/cmake/beman/beman.exemplar-version.cmake

I have not yet implemented the ability to select a specific variant when using the command find_package(beman.exemplar) because I wanted to get feedback on the direction up to this point. AFAIK, CMake doesn't really have explicit support for target export variants. I think there are a couple options, which are not necessarily mutually exclusive.

• Option 1: Use the COMPONENTS argument of find_package to select the desired target export variant. This is only viable if we don't want to use COMPONENTS for something else.
• Option 2: Select the target export variant based on the value of the following variables in order of preference.
  • BEMAN_EXEMPLAR_TARGET_EXPORT_VARIANT
  • BEMAN_TARGET_EXPORT_VARIANT

If the user requests a specific target export variant using either of the above methods and the specified target export variant doesn't exist, that's a hard error. If a user calls find_package(beman.exemplar) without doing either of the above, then the target export variant is determined as follows.

• If only one target export variant is installed in the config file package, then that variant is selected.
• If multiple target export variants are installed in the config file package, then the name of the variant is determined using the same method as if the user did not specify a target export variant name when building beman.exemplar.
  • If beman.exemplar is directed to use shared libs, then the selected target export variant name is shared.
  • If beman.exemplar is directed to use static libs and position independent code, then the selected target export variant name is static-pic.
  • Otherwise, the selected target export variant name is static.

We can add additional default target export names as we see fit. For example, we could add LTO variants that differ in name according to the toolchain since different compilers have incompatible LTO formats.

I designed all of this with a Beman super-build in mind. I think it's important that we be able to have a top-level CMakeLists.txt file that looks something like the following.

project(beman DESCRIPTION "The Beman C++ libraries" LANGUAGES CXX)

add_subdirectory(beman.exemplar)
add_subdirectory(beman.optional)
...

[wusatosi]

Hey @chayden83 thanks for the contribution, you can apply linting suggestions by running pre-commit run at the root directory of your exemplar folder.

Edit: Subsequent comment marked resolved is relating to this comment.

[wusatosi]

None the less, thank you for your contribution!

Package creation is definitely something we desperately need.

This is out of my league to review, but I do like how configurable your implementation is. I will leave the review to @camio and @bretbrownjr .

Just a note, beman usuals are at iso meetings right now, it might take a while for them to circle back and review your contribution.

Edit: A while ~= One/ Two weeks.

[chayden83]

Do you mean the GitHub codespace container? I wasn't aware of that.

No, sorry, I wasn't clear. I rolled my own local image for beman dev.

Just a note, beman usuals are at iso meetings right now, it might take a while for them to circle back and review your contribution.

👍

[steve-downey]

I've been setting up a python venv and pip installing the requirements for pre-commit, gcovr, etc, into that. There's enough C++ in the python ecosystem that most C++ tools have current versions available. The problem is, of course, yet another layer of tools.

[wusatosi]

Note to maintainers: We should move this to infra repo in the future.

[wusatosi]

@chayden83 You can click on the "ready for review" button at the merge panel to update this PR to unset the draft state.

[chayden83]

General feedback:

A lot of build options are introduced in this PR, but there declaration are very readable and self documenting. I believe this is nice enough to the point where I am comfortable point to downstream users to just read through cmake/beman-configure.cmake to understand all the project build options instead of writing a separate doc.

The utilities are well designed on abstracting out the project being exemplar, though there's still some work to be done. I believe we are hitting the ceiling on how expressive CMake is, agh...

However, there's a lot of implied design decision done, that I am sure we will need to discuss before merging and may require some updates.

Some of the design options are mentioned in contributor's PR description. I am unfortunately not a CMake guru to give effective discussion.

I do like how you designed this with beman mega build in-mind.

Usually I would push for some kind of testing in CI, ideally to verify the installation work, but I understand verifying this would be non-trivial.

I left this facility for basic CMake parameter testing, this just ensure the project builds under the cmake args. I would suggest you put the options in so we all know the CMake file at least won't break build.

exemplar/.github/workflows/ci_tests.yml

Lines 143 to 152 in 5471664

	configuration-test:
	runs-on: ubuntu-latest
	strategy:
	fail-fast: false
	matrix:
	args:
	- name: "Disable build testing"
	arg: "-DBEMAN_EXEMPLAR_BUILD_TESTS=OFF"
	- name: "Disable example building"
	arg: "-DBEMAN_EXEMPLAR_BUILD_EXAMPLES=OFF"

• I added some tests to verify that the introduced options don't break the build. I found a bug, so thank you for that. I am happy to expand on the tests as much as necessary now that I know where they are (sorry, I have a lot of GitLab experience, but not much GitHub experience).
• I would be happy to expand on the documentation for any of the configuration options. I do think it's important that the documentation in the CMake file is extensive enough that it's comprehensible w/out separate documentation. That said, I don't expect most users will use most of the config options. They are aimed at people like myself who need to package libraries up for deployment into non-standard environments. In general, the default behavior accords with what I think a typical user would expect. Simple things are simple and complex things are possible.
• Yes, there is more design in this PR than I would have liked. I certainly don't mean to be presumptuous. I just thought some of the design choices would make more sense if they could be seen as part of a larger whole instead of as separate increments. I would be happy to have a call/Zoom/whatever if we want to discuss. Fridays are typically pretty good for me.

[github-actions]

[pre-commit] _{reported by reviewdog 🐶}

Suggested change

	VERSION ${PROJECT_VERSION}
	SOVERSION ${PROJECT_VERSION_MAJOR}
	VERSION ${PROJECT_VERSION}
	SOVERSION ${PROJECT_VERSION_MAJOR}

[bretbrownjr]

Hi @chayden83!

Nice CMake code. We really need changes in this space, and I'm excited to see you taking a crack at a lot of the problems here.

I think we need to downshift a bit, though. Can we iterate towards where we want to end up instead of doing it all at once? I'm OK hardcoding assumptions initially, for instance. And I'm OK on adding support for specific features (PIC, shared libs, etc.) in specific PRs.

Maybe this PR can focus on the task in its title? What's a reasonable sized PR for making sure cmake --install workflows for beman.exemplar produce files such that find_package(beman.exemplar REQUIRED) works as expected? I'd be OK with a little beman_install_target function that hardcodes the export install commands or even installing a set beman.exemplar-config.cmake file that's reasonably portable.

[bretbrownjr]

Suggested change

	- name: "Enable position-independent code?"
	arg: "-DBEMAN_EXEMPLAR_POSITION_INDEPENDENT_CODE=ON"

I'd prefer just to support CMAKE_POSITION_INDEPENDENT_CODE. As a general rule, I'd rather use standard CMake mechanisms whenever possible. Even if that means shoehorning more things into CMAKE_CXX_FLAGS in .github files.

[bretbrownjr]

Suggested change

	- name: "Override default library install directory"
	arg: "-DBEMAN_EXEMPLAR_INSTALL_LIBDIR=local/lib"
	- name: "Override default executable installation directory"
	arg: "-DBEMAN_EXEMPLAR_INSTALL_BINDIR=local/bin"
	- name: "Override default include install directory"
	arg: "-DBEMAN_EXEMPLAR_INSTALL_INCLUDEDIR=local/include"
	- name: "Override default CMake config-file package install directory"
	arg: "-DBEMAN_EXEMPLAR_INSTALL_INCLUDEDIR=local/lib/cmake/beman.exemplar"

I'd prefer just using the GNUInstallDirs mechanisms as documented upstream. CMAKE_INSTALL_INCLUDEDIR, etc. Though in most cases, we shouldn't need to reference those directly as defaults for HEADER FILE_SETS is documented to use those paths.

[bretbrownjr]

Suggested change

	- name: "Enable config-file package creation?"
	arg: "-DBEMAN_EXEMPLAR_CONFIG_FILE_PACKAGE=OFF"
	- name: "Specify config-file package version compatibility"
	arg: "-DBEMAN_EXEMPLAR_CONFIG_FILE_PACKAGE_COMPATIBILITY=SameMajorVersion"
	- name: "Specify the name of the target export variant to create"
	arg: "-DBEMAN_EXEMPLAR_TARGET_EXPORT_VARIANT=custom"
	- name: "Specify the name of the development install component"
	arg: "-DBEMAN_EXEMPLAR_DEVELOPMENT_INSTALL_COMPONENT=development"
	- name: "Specify the name of the runtime install component"
	arg: "-DBEMAN_EXEMPLAR_RUNTIME_INSTALL_COMPONENT=runtime"

For these, I think we can just hardcode and document our APIs for now. We can come back with more targeted PRs when specific use cases arise.

[bretbrownjr]

Suggested change

	- name: "Enable shared libs?"
	arg: "-DBEMAN_EXEMPLAR_SHARED_LIBS=ON"

Let's just use BUILD_SHARED_LIBS as documented upstream.

[bretbrownjr]

Suggested change

	- name: "Enable docs building"
	arg: "-DBEMAN_EXEMPLAR_BUILD_DOCS=OFF"

I like supporting docs, though let's make that an entirely different PR please. Also note that we have been discussing options on this already. It's also not clear to me that we'd have CMake drive the documentation generation. The current discussion is centered around tools like mkdocs that do not involve parsing C++ code, so there's no need to involve the C++ build system in the process.

[bretbrownjr]

I'll hold off on reviewing all this until we get on the same page about all the options being added to support this PR. I expect some obvious changes will be required after we sync up on the high level scope and design choices.

[bretbrownjr]

Quick note: I like functional testing for this better. I'd rather do a full integration test in a subdir that includes a find_package(beman.exemplar REQUIRED) in a CMakeLists.txt and exercise that. Or use equivalent functionality in a Conan package test. Or have a vcpkg arrangement that includes building at least one consumer package.

Those are more thorough test cases that actually represent what a user would do and ensure we didn't make any typos or whatnot while refactoring something.

I think it makes sense to have test calls as an intermediate step I suppose. Maybe we compromise by speccing out an issue and reference the issue here?

[bretbrownjr]

I like having a properly set version. I'm a little out of the loop on how GitHub versioning works (or not) with respect to this kind of detail. I could see this actually being a cache variable set by a GitHub action based on a git tag?

[wusatosi]

What use case are you asking here?

Do you mean substituting this field when we make a new release?

[bretbrownjr]

Suggested change

	beman_install_targets(
	TARGETS beman.exemplar.headers beman.exemplar
	)
	beman_install_export()
	beman_install_targets(TARGETS beman.exemplar)

I've had a mind to introduce something like this myself, for what it's worth. I think it's a design gap in CMake upstream that there's not a clear design seam here.

That being said, I've had good luck elsewhere leaving it at only installing the library target and letting its properties and such speak for themselves. For instance the FILE_SET HEADERS properties document that it has headers, that they are installed, where they are installed, and so on.

I would also expect that installing the beman.exemplar module would be unconditional. It's phrased that way here since there's no if statement! Similarly, we'll (in another PR) support pkg-config and CPS files in the future.