friendlyanon / cmake-init

The missing CMake project initializer
GNU General Public License v3.0
2.03k stars 78 forks source link

Make build directory compatible with `find_package` #38

Closed ekilmer closed 2 years ago

ekilmer commented 2 years ago

Hello! First, thank you for the wonderful project! This is a great reference for modern and essential CMake practices.

I am wondering whether it would be useful to have the build directory be compatible with find_package. With the changes, a user would be able to find and use the targets of this project from the build directory instead of requiring installation.

Not requiring installation can help to reduce the development cycle if this project is being actively developed to support features of the project depending on this project.

As a motivating concrete example, using the test directory since it's also a standalone project, a user would be able to do the following:

cmake-init /tmp/shared -s

cd /tmp/shared
cmake --preset=dev
cmake --build --preset=dev

cmake -S /tmp/shared/test \
  -B /tmp/shared/test/build \
  -Dshared_DIR=/tmp/shared/build/dev
cmake --build /tmp/shared/test/build
ctest --test-dir /tmp/shared/test/build

Notice that the installation command is not executed.

I have a working branch (feature/export-build-dir) in my fork, but I wanted to check here first to see whether a PR would have a chance of acceptance. The branch uses CMake's export and configure_package_config_file to achieve this.

Thank you!

friendlyanon commented 2 years ago

Just a note, but configure_package_config_file is pretty much useless in majority of cases and can be harmful as well. It uses the value of CMAKE_INSTALL_PREFIX at configure time, but that can change at install time.

The fuzzer example project already uses export and it's a trivial amount of code to include. It really only saves a minuscule amount of time in that project and the only reason I even included it there was to have an example for https://github.com/SerenityOS/serenity/issues/10055

On Windows, it makes even less sense, since there is no RPATH to setup there. Shared libraries are surrounded with a lot of "it depends" when looking at the utility of something.

ekilmer commented 2 years ago

configure_package_config_file is pretty much useless in majority of cases and can be harmful as well. It uses the value of CMAKE_INSTALL_PREFIX at configure time, but that can change at install time.

Ah! Good to know.

Thank you for the link to the fuzzer example. It uses configure_file instead of configure_package_config_file 👍 . I should grep through all of your examples first next time 🙂 (at least I would have found the export use).

On Windows, it makes even less sense, since there is no RPATH to setup there.

Yes, Windows is difficult in that respect.

Okay, well I've definitely learned more now, and the linked example looks good, thank you!

friendlyanon commented 2 years ago

Are the examples really not that visible? I have a link to it in cmake-init output and at the very top of the README.

ekilmer commented 2 years ago

Are the examples really not that visible?

They are very much visible. In fact, I've been reading most of them as reference material. Thank you for putting them together. Unfortunately, I did miss reading where in the cmake-init-gif-engine example export was used, which is my fault, so thank you for linking to the lines where it is used.

My thought process behind opening this issue was to ask whether making the build directory compatible with find_package should be included in this init script, and also whether the branch I had was how you would do it.

Your replies about how configure_package_config_file is dangerous taught me something new. The lack of support in Windows for RPATH-like functionality and how usage of shared libraries depends on a lot of things makes me think that this feature should not be included in this init script.