-
Notifications
You must be signed in to change notification settings - Fork 12
CMake improvements #14
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
Conversation
dvzrv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good and will be an improvement to packaging this project!
I guess this closes #15
| - name: Run tests | ||
| run: | | ||
| cd build | ||
| make test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can do make -C build and drop the cd.
Alternatively: ctest --test-dir build --output-on-failure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's used to make it similar with other steps.
| if(CMAKE_MAJOR_VERSION LESS 3) | ||
| cmake_minimum_required(VERSION 2.6) | ||
| else() | ||
| cmake_minimum_required(VERSION 2.8.12) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason to support a cmake version from 10 years ago? 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's still used on one of the systems on which I'm testing roc (some old raspberry pi). While there's no big maintaining burden, I prefer to keep compatibility with older systems, because often people are too lazy to update them, like me :)
Co-authored-by: David Runge <dave@sleepmap.de>
|
@dvzrv thanks for review! |
tools and tests are created in
EXECUTABLE_OUTPUT_PATH(exceptdescr_stats, which have to be inside source dir)EXECUTABLE_OUTPUT_PATHandLIBRARY_OUTPUT_PATHare explicitly markedCACHEdescr_statsis excluded from installation, because it is useful when it is located insideperf_evaldir; it is now the only binary that usesPROJECT_SOURCE_DIRas destinationeperftool,simple_server,simple_clientare by default excluded by installation; new optionINSTALL_DEVTOOLSis added; when it's enabled, those tools are installed intoCMAKE_INSTALL_FULL_BINDIR.hheaders and.pcfile are now installed toominimum cmake version is set back to 2.6 to preserve compatibility with outdated environments (which are checked in roc CI)
-Ilib_advancedis added to.pcfiletests now work and are added to CI