PackageProject.cmake icon indicating copy to clipboard operation
PackageProject.cmake copied to clipboard

add support for FILE_SET HEADERS

Open ClausKlein opened this issue 3 years ago • 11 comments

to support new CMake feature for interface-libraries

this would fix #32

ClausKlein avatar Aug 25 '22 19:08 ClausKlein

The use case is that a header only project may check that every header is self contented and compiles.

Too it is possible to run-clang-tidy for each header, because CMake generate an object target for each header file in this set.

see https://discourse.cmake.org/t/verifying-header-sets-not-supported-for-header-only-interface-libraries/6139/8

and https://github.com/ClausKlein/asio/pull/3/commits/7701d95f38a09b196670fd70ef2e7d98e864c513

ClausKlein avatar Sep 03 '22 04:09 ClausKlein

So if I understand correctly from the docs, the advantage is that we don't need to specify the include destination explicitly but can now use the BASE_DIRS flag in the library definition? How about adding a test that relies on this feature?

TheLartians avatar Sep 13 '22 20:09 TheLartians

So if I understand correctly from the docs, the advantage is that we don't need to specify the include destination explicitly but can now use the BASE_DIRS flag in the library definition? How about adding a test that relies on this feature?

No, the main advantages are

  • for an interface library you may check, that every header is self-contained
  • you may also check them with run-clang-tidy without writing tests.

see too https://github.com/aminya/project_options/issues/143 and CMAKE_VERIFY_INTERFACE_HEADER_SETS

ClausKlein avatar Sep 15 '22 18:09 ClausKlein

I still don't really understand the feature then, do you think you could add an independent test case that would fail with the current version, but pass using the suggested changes?

TheLartians avatar Sep 20 '22 20:09 TheLartians

I still don't really understand the feature then, do you think you could add an independent test case that would fail with the current version, but pass using the suggested changes?

@TheLartians I push a version that is not usable with CMake v3.24.2 (pip install cmake)

How will you support to install a header only library using this new CMake feature?

ClausKlein avatar Sep 20 '22 21:09 ClausKlein

@TheLartians may you think about this MR again?

ClausKlein avatar Oct 22 '22 08:10 ClausKlein

@TheLartians Q: Is the project not maintained anymore?

ClausKlein avatar Jan 28 '23 12:01 ClausKlein

Hey @ClausKlein I apologise for not being able to respond so far. I'm still planning to maintain this project, however I don't have any experience with FILE_SET HEADERS and don't yet understand what problem it solves. From a quick look at the readme it allows omitting the target_include_directories?

As before it would be great if you could avoid changing the test behaviour based on the CMake version. You can assume that tests will be performed with CMake > 3.23.0, but I still want to keep the existing test code that doesn't use the new feature. Perhaps you instead create another dependency test/file_set_dependency/CMakeLists.txt using file set instead?

TheLartians avatar Jan 28 '23 16:01 TheLartians

FYI: from import-cmake-c20-modules

Once you have built a compiler that supports p1689 and a version of CMake that supports C++20 modules and have run the tests to make sure things are set up correctly. To use modules in CMake you need to use target_sources and FILE_SETS. https://cmake.org/cmake/help/latest/command/target_sources.html

ClausKlein avatar Feb 01 '23 20:02 ClausKlein

I see, so the files sets are required to support modules? In that case how about adding a separate test case that demonstrates this functionality by using modules instead of modifying the existing tests?

TheLartians avatar Feb 19 '23 13:02 TheLartians

I can not longer use this cmake module because it has no support for moderern CMake file sets!

ClausKlein avatar Mar 23 '24 18:03 ClausKlein