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

Document this project's CMake integration options #69

Closed
claremacrae opened this issue Jan 1, 2020 · 8 comments
Closed

Document this project's CMake integration options #69

claremacrae opened this issue Jan 1, 2020 · 8 comments
Assignees
Labels
documentation infrastructure CMake, Builds, CI, and similar

Comments

@claremacrae
Copy link
Collaborator

As our CMake scripts become more standard and more flexible, we should have a page of documentation describing the supported options.

This might be a good example to look at:
https://github.com/catchorg/Catch2/blob/master/docs/cmake-integration.md#top

See also this evolving project, in which I'm exploring, and testing, options for using 3rd-party dependencies:
https://github.com/claremacrae/ApprovalTests.cpp.Builds

@jwillikers
Copy link
Contributor

@claremacrae, a namespaced CMake target for ApprovalTests would be good to create before this documentation is done. By adding an aliased CMake target for ApprovalTests that prefixes a namespace, users can switch between using an installed version of the library - if support for find_package is added - and add_subdirectory / FetchContent without having to change the ApprovalTests library name. That's dense when I put it into words 🙄

An example might make this more clear. 🤔
The following example links to a hypothetically installed version of ApprovalTests, though this not supported at the moment.

# Use an installed version of ApprovalTests
find_package(ApprovalTests 7.0.0 REQUIRED)
add_executable(my-test PRIVATE main.cpp)
# According to standard practice, imported targets are prefixed with a namespace
target_link_libraries(my-test PRIVATE ApprovalTests::ApprovalTests)

This links to our current setup of ApprovalTests when using add_subdirectory.

add_subdirectory(ApprovalTests)
add_executable(my-test PRIVATE main.cpp)
# Notice the target name is different. This requires the user to change the library name.
target_link_libraries(my-test PRIVATE ApprovalTests)

This links to our current setup of ApprovalTests when using add_subdirectory after adding an aliased target.

add_subdirectory(ApprovalTests)
add_executable(my-test PRIVATE main.cpp)
# Target name is the same and this requires no change by the user.
target_link_libraries(my-test PRIVATE ApprovalTests::ApprovalTests)

Aliasing the ApprovalTests library in CMake is as simple as adding the following somewhere after defining the ApprovalTests interface library, probably in the same directory where it is defined.

add_library(ApprovalTests::ApprovalTests ALIAS ApprovalTests)

@claremacrae
Copy link
Collaborator Author

@athrun22 Thanks! I've created #75 to track this suggestion, and I'll add comments there...

@claremacrae claremacrae added the infrastructure CMake, Builds, CI, and similar label Jan 2, 2020
@claremacrae
Copy link
Collaborator Author

claremacrae commented Jan 5, 2020

Things we could include:

  • Different ways to get this repo - mainly worth documenting as I think lots of users won't know these specific mechanisms...
    • git submodules
    • CMake FetchContent
    • CMake ExternalProject
    • CMake find_package() - not supported yet
    • add_subdirectory()
  • If you use any of the above
    • you don't have access to the single header, and you have to specify the actual individual header files in the source code - which we don't document
    • we don't currently add any of the third-party sub-directories to the search path automatically - so it looks like users need to then supply their own Catch etc. - it's still possible to add one of our third-party sub-dirs to your build

@jwillikers
Copy link
Contributor

jwillikers commented Jan 8, 2020

@claremacrae I think add_subdirectory would be worth including. Because FetchContent and ExternalProject are similar, I think documenting FetchContent would be simpler and sufficient. I'm guessing that explanations for add_subdirectory, FetchContent, and the single header would be sufficient for most users that wish to use git submodules to incorporate the project.

@claremacrae
Copy link
Collaborator Author

This comment will be helpful for these docs, too:
73c09d0#commitcomment-36709438

@claremacrae claremacrae self-assigned this Jan 12, 2020
@claremacrae claremacrae added this to the vNext release milestone Jan 13, 2020
@claremacrae
Copy link
Collaborator Author

Here's the evolving CMake docs page I've started on:

https://github.com/approvals/ApprovalTests.cpp/blob/master/doc/CMakeIntegration.md#top

claremacrae added a commit that referenced this issue Jan 15, 2020
Now most of our tests are not executed if this projected has
been added inside another one.
@jwillikers
Copy link
Contributor

@claremacrae Thanks for the CMake Integrations section! It looks good. My only quibble is the two occurrences of "on" in the last paragraph should probably be ON in order to follow the convention used throughout.

@claremacrae
Copy link
Collaborator Author

Thanks @jwillikers

I'm putting this down now, as of 37f07dd.

https://github.com/approvals/ApprovalTests.cpp/blob/37f07dd26c1e69c8abb0984be1f35e275d81ff69/doc/CMakeIntegration.md#top

I included a bunch of examples as they were initially slightly hard-won figuring out the different options, so I didn't want to lose them, and I think they may help others.

I couldn't see the lower-case "on" you mentioned, so I suspect I capitalised it - or removed it - along the way.

Even though I've closed the issue, I would definitely welcome any feedback from anyone who spends the time to read the doc!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation infrastructure CMake, Builds, CI, and similar
Projects
None yet
Development

No branches or pull requests

2 participants