-
Notifications
You must be signed in to change notification settings - Fork 1k
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
cmake: Emulate GNU Autotools 'make check -jN' #1294
Conversation
# Emulate GNU Autotools 'make check -jN'. | ||
add_custom_target(check | ||
COMMAND @$(CMAKE_COMMAND) -E cmake_echo_color --switch=$(COLOR) --cyan \"Running tests...\" | ||
COMMAND ${CMAKE_CTEST_COMMAND} --force-new-ctest-process -$(MAKEFLAGS) |
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.
Passing $MAKEFLAGS
to ctest is indeed pretty hackish (and ctest doesn't like -j
without an integer and some users have more MAKEFLAGS
set in their env...)
Yep, we should just update the CMake build docs in the README. We could even remove the "make check" emulation entirely if this is more CMake-ish. What do you think? |
As we discussed at CoreDev, it would be better for the common good to use a unified approach for both projects, Bitcoin Core and libsecp256k1. A similar discussion is currently taking place in Bitcoin Core (here, here and here). Regarding your question, I think it would be advisable to ask for the opinions of developers who expressed concerns about |
I've updated the PR description with a couple of alternatives. |
@hebasto Has been any progress on this point in Core? |
In the https://github.com/hebasto/bitcoin/tree/cmake-staging, we lean to use the native CMake's invocations in all scripts (CI, Guix etc). This approach is agnostic to the underlying build system, and However, it might change in the future :) |
Resolves one item in bitcoin-core#1235. Closes bitcoin-core#1294.
Let's stick with this for now. Then people can use |
Resolves one item in bitcoin-core#1235. Closes bitcoin-core#1294.
It seems parallelism is requested for
make check
target when building with CMake and the "Unix Makefiles" generator which is the default on Linux and macOS. See:With this PR, it is possible to build like that:
Making this PR a draft as it seems low priority and the diff seems a bit hackish.
My personal preference is to use the CMake's native
ctest
command :)Anyway, this PR fixes an item in #1235.
UPD. Here is a couple of alternative approaches:
CTEST_PARALLEL_LEVEL
environment variable:test
target and (undocumented)ARGS
variable: