This is a cleanup which makes it easier to add further modules with dependencies, e.g., in #1452. The diff looks larger than it is because I also reordered the modules and made the order consistent between CMake and autotools.
(We noticed that the current logic could be improved in https://github.com/BlockstreamResearch/secp256k1-zkp/pull/275.)
391- SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DENABLE_MODULE_ECDH=1"
392-fi
393-
394-if test x"$enable_module_recovery" = x"yes"; then
395- SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DENABLE_MODULE_RECOVERY=1"
396+# Processing must be done in a reverse topological sorting of the dependency graph.
micronit: Since “reverse” is a matter of defining the direction of the edge, maybe
0# Processing must be done in a topological sorting of the dependency graph (dependent module first).
386@@ -387,29 +387,31 @@ SECP_CFLAGS="$SECP_CFLAGS $WERROR_CFLAGS"
387 ### Handle module options
388 ###
389
390-if test x"$enable_module_ecdh" = x"yes"; then
391- SECP_CONFIG_DEFINES="$SECP_CONFIG_DEFINES -DENABLE_MODULE_ECDH=1"
392-fi
May I ask a bit more explanation for the second commit please?
0$ cmake -B build -DSECP256K1_ENABLE_MODULE_EXTRAKEYS=OFF
1-- The C compiler identification is GNU 11.4.0
2-- Detecting C compiler ABI info
3-- Detecting C compiler ABI info - done
4-- Check for working C compiler: /usr/bin/cc - skipped
5-- Detecting C compile features
6-- Detecting C compile features - done
7CMake Error at CMakeLists.txt:71 (message):
8 schnorrsig module requires explicitly disabled extrakeys module.
9
10
11-- Configuring incomplete, errors occurred!
12See also "/home/hebasto/git/secp256k1/secp256k1/build/CMakeFiles/CMakeOutput.log".
The error message seems contradicts to -DSECP256K1_ENABLE_MODULE_EXTRAKEYS=OFF.
Or did I miss something?
@hebasto Not sure if I understand your note about the error message. I think it is correct. But I did parse it incorrectly the first time.
schnorrsig module requires explicitly disabled extrakeys module.
means that the schnorrsig module requires the extrakeys module. It does not mean that the schnorrsig module requires the extrakeys module to be explicitly disabled.
@hebasto Not sure if I understand your note about the error message. I think it is correct. But I did parse it incorrectly the first time.
schnorrsig module requires explicitly disabled extrakeys module.
means that the schnorrsig module requires the extrakeys module. It does not mean that the schnorrsig module requires the extrakeys module to be explicitly disabled.
So, why is the “disabled” word here?
The user can build the schnorrsig module either providing -DSECP256K1_ENABLE_MODULE_EXTRAKEYS=ON explicitly or relying on the default behavior when the SECP256K1_ENABLE_MODULE_EXTRAKEYS is set to ON by the CMake script, right?
The user can build the schnorrsig module either providing -DSECP256K1_ENABLE_MODULE_EXTRAKEYS=ON explicitly or relying on the default behavior when the SECP256K1_ENABLE_MODULE_EXTRAKEYS is set to ON by the CMake script, right?
No, turning on the extrakeys module should not turn on the schnorrsig module. The schnorrsig module requires the extrakeys module, not the other way around.
The user can build the schnorrsig module either providing -DSECP256K1_ENABLE_MODULE_EXTRAKEYS=ON explicitly or relying on the default behavior when the SECP256K1_ENABLE_MODULE_EXTRAKEYS is set to ON by the CMake script, right?
No, turning on the extrakeys module should not turn on the schnorrsig module. The schnorrsig module requires the extrakeys module, not the other way around.
Sorry for bad wording from my side. I mean, the schnorrsig module is enabled by default, and no extra actions, like providing -DSECP256K1_ENABLE_MODULE_SCHNORRSIG=ON, are needed, no?
--disable-module-extrakeys to ./configure or -DSECP256K1_ENABLE_MODULE_EXTRAKEYS=OFF to cmake, it is simply ignored (because schnorrsig turns it on again). So it makes sense to throw an error instead of ignoring the explicit module disabling.
On master if you provide
--disable-module-extrakeysto./configureor-DSECP256K1_ENABLE_MODULE_EXTRAKEYS=OFFto cmake, it is simply ignored (because schnorrsig turns it on again). So it makes sense to throw an error instead of ignoring the explicit module disabling.
Agree.
This also makes the order in which module options are processed
consistent between CMake and autotools (the reverse order of the listing
printed to stdout).