open-quantum-safe / liboqs

C library for prototyping and experimenting with quantum-resistant cryptography
https://openquantumsafe.org/
Other
1.87k stars 459 forks source link

Use modern CMake syntax #1494

Open vsoftco opened 1 year ago

vsoftco commented 1 year ago

Instead of depending on make/msbuild etc, we should use only modern cmake and ctest, such as

cmake -B build -DCMAKE_FLAGS...
cmake --build build --parallel 4 [--target ... ]
ctest --test-dir build

This is platform-independent and saves a lot of headaches, and completely removes make and msbuild explicit dependencies. See https://github.com/open-quantum-safe/liboqs-cpp/blob/main/.github/workflows/cmake.yml for an example.

dstebila commented 1 year ago

So cmake --build build would also replace ninja?

I don't mind, though I haven't had any trouble with our current build sequence.

The first two commands work fine for me, but ctest --test-dir build doesn't do anything. I'm guessing this is because our test suite is under a custom target (run_tests) rather than some default name?

vsoftco commented 1 year ago

Yes, it'll replace ninja, and is completely platform-independent. I'll look into the test suite...

cmake --build build --target run_tests runs the test suite, but it'll be preferably to be able to use ctest instead.

baentsch commented 1 year ago

I like the platform-independence aspect (particularly looking at oqs-provider for Windows right now). What makes me wonder, though, is how this will impact build+test/CI runtime: ninja always nicely & automatically takes the maximum amount of CPUs available. The proposal above seems to require a hardcoded number of CPUs (that may or may not be available, i.e., run too slow if more CPUs were available and may overwhelm the machine if too many are spec'd).

dstebila commented 1 year ago

I did --parallel without a number, and it seemed to use the maximum number of CPUs on my machine, running about as fast as ninja does.

vsoftco commented 1 year ago

@dstebila Thanks, that's good to know, I didn't know we can use the flag without a number

SWilson4 commented 1 month ago

@Martyrshot took an initial stab at this in https://github.com/open-quantum-safe/liboqs/pull/1700. That PR would be a good jumping-off point for anyone who would like to take this up in the future.