-
Notifications
You must be signed in to change notification settings - Fork 35
cmake: support default build and test workflow #204
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
base: master
Are you sure you want to change the base?
cmake: support default build and test workflow #204
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process. |
Code review 47deaef. Thanks for the PR. These changes seem worth considering, and it does seem pretty indefensible that "make test" doesn't build the tests it depends on. This behavior seems to be a longstanding bug or quirk of cmake, and it's especially surprising given how cmake's "make test" target works differently than its "make install" target when it comes to building dependencies. I think it could be good to implement the "fixtures" idea in the linked stackoverflow thread to address this, but not good to build targets by default that aren't needed for installation. I think it's best for the 'make; make install` workflow to be fast and reliable and not do unnecessary work. I'm also not sure about the other part of this change removing convenience targets. |
CI fails:
|
Make sure that the default workflow of `cmake ..; make; make test` does not fail with an error message that no tests are found. Remove any "convenience" targets that add maintenance burden and require tribal knowledge.
47deaef
to
1674fe0
Compare
Thanks for the review, @ryanofsky.
Stackoverflow is not a good source of information when it comes to CMake (neither is chatgpt, which seems to be trained on content from stackoverflow as well as projects that followed the advices found there). Too often, the accepted answers are outdated or plain wrong. In this particular case, what you are looking for is
This is exactly my concern. The project configuration is intertwined with CI scripts. What I aim for, is that projects and CI scripts are developed independently against a defined interface so that CI scripts can be reused for multiple projects. About
|
I'd be very interested in this. If there's a generic cmake CI framework that could be plugged in here to simply the CI configuration here that would be great. Absent that though, I think
Yeah that part makes sense. But it would also make sense if cmake provided a way to control what test target depends on, using something like |
I think my main objection to this is I don't think the "configure, make, make install" should be doing unnecessary work and building things that don't get installed or used. I do think the "configure, make, make test" workflow should work, though and I appreciate you pointing out that it is broken. If there's a way to fix this without using "make check", which it seems like there might be, I do think we can drop "make check" |
Make sure that the default workflow of
cmake ..; make; make test
does not fail with an error message that no tests are found.Remove any "convenience" targets that add maintenance burden and require tribal knowledge.