Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make header_only_library() function #262

Merged
merged 5 commits into from
Jul 10, 2024

Conversation

chiphogg
Copy link
Contributor

@chiphogg chiphogg commented Jul 10, 2024

Au is overwhelmingly built from header-only libraries. We can make this
a lot easier to maintain with a CMake function, header_only_library(),
that presents an Au-centric interface. Specifically, the function will
automatically:

  • Make the target an INTERFACE
  • Set FILE_SET HEADERS
  • Set BASE_DIRS to the base Au directory

Users must provide the HEADERS option, and we will use those to set
the FILES option. Users can optionall provide the DEPS option, and
we'll use it if present to populate a target_link_libraries call.

We also add an optional GTEST_SRCS argument. If this is provided,
then we will automatically create a _test-suffixed target, link to the
library and to gtest_main, and run gtest_discover_tests(). If the
test has any dependencies apart from the tested target, and GTest, they
can be supplied via an optional GTEST_EXTRA_DEPS argument.

Finally, we provide the option to mark a library as INTERNAL_ONLY. For
now, we borrow the EXPORT_NAME manipulation from the original CMake
branch, but we could choose to do other things later --- the important
part is marking the intent that the library is internal-only.

Helps #215.

Au is overwhelmingly built from header-only libraries.  We can make this
a lot easier to maintain with a CMake function, `header_only_library()`,
that presents an Au-centric interface.  Specifically, the function will
automatically:

- Make the target an `INTERFACE`
- Set `FILE_SET HEADERS`
- Set `BASE_DIRS` to the base Au directory

Users must provide the `HEADERS` option, and we will use those to set
the `FILES` option.  Users can optionall provide the `DEPS` option, and
we'll use it if present to populate a `target_link_libraries` call.

We also add an optional `GTEST_SRCS` argument.  If this is provided,
then we will automatically create a `_test`-suffixed target, link to the
library and to `gtest_main`, and run `gtest_discover_tests()`.  If the
test has any dependencies apart from the tested target, and GTest, they
can be supplied via an optional `GTEST_EXTRA_DEPS` argument.

Finally, we provide the option to mark a library as `INTERNAL_ONLY`.
For now, we borrow the `EXPORT_NAME` manipulation from the original
CMake branch, but we could choose to do other things later --- the
important part is marking the intent that the library is internal-only.
@chiphogg chiphogg added the release notes: ⚙️ repo PR affecting the way the repository works label Jul 10, 2024
@chiphogg chiphogg requested a review from geoffviola July 10, 2024 16:08
Copy link
Contributor

@geoffviola geoffviola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes make sense. Test still passes.

docker run --rm \
ubuntu@sha256:2e863c44b718727c860746568e1d54afd13b2fa71b160f5cd9058fc436217b30 \
/bin/bash -c \
'apt update && apt install -y clang cmake git && git clone -b chiphogg/cmake-header-only-library#215 https://github.com/aurora-opensource/au.git && cd au && cmake -S . -B cmake/build -DCMAKE_VERIFY_INTERFACE_HEADER_SETS=TRUE && cmake --build cmake/build --target all all_verify_interface_header_sets test'

${ARG_NAME}
${ARG_GTEST_EXTRA_DEPS}
GTest::gtest_main
GTest::gmock_main
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both main targets would have two main functions. You should only need GTest::gmock_main.

gmock_main has main and depends on gmock. gmock depends on gtest.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! Now I'm a little curious why it actually worked before (or at least, appeared to), with two main functions.

@chiphogg chiphogg merged commit 34c201b into main Jul 10, 2024
10 checks passed
@chiphogg chiphogg deleted the chiphogg/cmake-header-only-library#215 branch July 10, 2024 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: ⚙️ repo PR affecting the way the repository works
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants