3f31ac43e0 doc: Promote "Building with CMake" to standard procedure (Hennadii Stepanov)
Pull request description:
The CMake-based build system has demonstrated its maturity through its use in Bitcoin Core 29.0.
ACKs for top commit:
real-or-random:
utACK 3f31ac43e0
Tree-SHA512: be83494b60f4fd3ff08f4199d9cda4663b89efff32f3ec3bb856843707eeb6592ffdc6c84fdc18242cad422795901fb21e13cb15edd23a5d9cf2784324f8f7e0
3af71987a8 cmake: Bump minimum required CMake version to 3.22 (Hennadii Stepanov)
Pull request description:
Ubuntu 20.04 LTS has [reached](https://wiki.ubuntu.com/Releases) the End of Standard Support. There no longer appear to be compelling reasons to maintain compatibility with CMake 3.16.
The new suggested minimum, CMake 3.22, is shipped with Ubuntu 22.04 LTS, which is supported until April 2027.
This PR also introduces new CMake policies, from CMP0098 to CMP0128. Some of these may warrant the reviewers' attention:
- [CMP0099: Link properties are transitive over private dependencies of static libraries.](https://cmake.org/cmake/help/latest/policy/CMP0099.html)
- [CMP0117: MSVC RTTI flag /GR is not added to CMAKE_CXX_FLAGS by default.](https://cmake.org/cmake/help/latest/policy/CMP0117.html)
- [CMP0126: set(CACHE) does not remove a normal variable of the same name.](https://cmake.org/cmake/help/latest/policy/CMP0126.html)
- [CMP0128: Selection of language standard and extension flags improved.](https://cmake.org/cmake/help/latest/policy/CMP0128.html)
ACKs for top commit:
real-or-random:
utACK 3af71987a8
Tree-SHA512: f0c70dd5beafe830513895f076cafa6902dfcaab79d40bf9e7b27f14d9c4818f91d75f6aa993ba843f1d28ccd13cf466ad11dca46ca022cab1b43aace17d3ff7
Ubuntu 20.04 LTS has reached the end of standard support. There no
longer appear to be compelling reasons to maintain compatibility with
CMake 3.16.
The new suggested minimum, CMake 3.22, is shipped with Ubuntu 22.04 LTS,
which is supported until April 2027.
This change also introduces new CMake policies, from CMP0098 to CMP0128.
20b05c9d3f configure: Show exhaustive tests in summary (Tim Ruffing)
Pull request description:
ACKs for top commit:
hebasto:
ACK 20b05c9d3f, it aligns now with the CMake script: e56716a3bc/CMakeLists.txt (L348-L350)
sipa:
utACK 20b05c9d3f
jonasnick:
ACK 20b05c9d3f
Tree-SHA512: 30744ea5e5b7441ad252868c83cebfec2b02b75786b9ea55d4b0b0a696f1c7df39c48c243b29b13839a9f3a7757c192da8be0dd95412678a7583b123db6e99ac
1b6e081538 include: remove WARN_UNUSED_RESULT for functions always returning 1 (Jonas Nick)
Pull request description:
This makes the usage of the atribute consistent. In the musig and ellswift module, functions that return 1 always already don't have the WARN_UNUSED_RESULT attribute. In secp256k1.h and the extrakeys module, this has only been the case partially.
In all cases where this was removed, the function only returns 0 if the illegal callback has been called.
Fixes#1379
ACKs for top commit:
real-or-random:
utACK 1b6e081538
sipa:
utACK 1b6e081538
Tree-SHA512: 5d1f2563ddde34fb721dd0b96622e0888a9c72f95af6f1c8a94f7f1f72ca934b6af98a3357c1e922d8611a9869264bb0f3ceb7bed0164c6c3a6f92f9950d4f19
d87c3bc58f ci: Fix exiting from ci.sh on error (Tim Ruffing)
Pull request description:
Fixes the following bash error when make fails:
./ci/ci.sh: line 100: return: can only `return' from a function or
sourced script
ACKs for top commit:
hebasto:
ACK d87c3bc58f
Tree-SHA512: 5ecd0f550f7659cc41b403fdb7d5d3d37d1a167d585cca02b0aca209c8b9592bb3067cf11aeb80775666e7232f31bf05cf1bb97fec8c67f3bc5fe2243ddbbcfa
This makes the usage of the atribute consistent. In the musig and ellswift
module, functions that return 1 always already don't have the WARN_UNUSED_RESULT
attribute. In secp256k1.h and the extrakeys module, this has only been the case
partially.
In all cases where this was removed, the function only returns 0 if the illegal
callback has been called.
59860bcc24 gha: Print all *.log files, in a separate action (Tim Ruffing)
Pull request description:
Before this commit, we didn't print *_example.log files and
test_suite.log.
Printing is now handled in a separate action, which avoids code
duplication and makes the ci.yml file more readable. This changes the
folding/grouping of the log output in the GitHub Actions CI, but I
think the new variant is as good as the old one.
Furthermore, the condition for printing the logs is changed from
"always()" to "!cancelled()". This ensures that logs will still be
printed if previous steps such as the CI script failed, but that they
won't be printed if the entire run is cancelled (e.g., by clicking a
button in the UI or through a force-push to the PR). This is in line
with a recommendation in the GHA docs:
https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/evaluate-expressions-in-workflows-and-actions#always
ACKs for top commit:
hebasto:
ACK 59860bcc24.
sipa:
ACK 59860bcc24
Tree-SHA512: ca11f5e5f01660964276b9c2e11c22caeed8492e9c7ffaa2078aaaa733005c63242fc93a1056124fb8f1f83019d46818c12b10142fb10f43270a8562fd10885a
Before this commit, we didn't print *_example.log files and
test_suite.log.
Printing is now handled in a separate action, which avoids code
duplication and makes the ci.yml file more readable. This changes the
folding/grouping of the log output in the GitHub Actions CI, but I
think the new variant is as good as the old one.
Furthermore, the condition for printing the logs is changed from
"always()" to "!cancelled()". This ensures that logs will still be
printed if previous steps such as the CI script failed, but that they
won't be printed if the entire run is cancelled (e.g., by clicking a
button in the UI or through a force-push to the PR). This is in line
with a recommendation in the GHA docs:
https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/evaluate-expressions-in-workflows-and-actions#always
4c50d73dd9 ci: Add new "Windows (clang-cl)" job (Hennadii Stepanov)
84c0bd1f72 cmake: Adjust diagnostic flags for clang-cl (Hennadii Stepanov)
Pull request description:
When building with `clang-cl` on Windows, the output is cluttered with warning messages because compiler diagnostic flags are not applied correctly:
```
> cmake -B build -G Ninja -DCMAKE_C_COMPILER="C:\Users\hebasto\Downloads\clang+llvm-18.1.8-x86_64-pc-windows-msvc\bin\clang-cl.exe"
> cmake --build build
[1/16] Building C object src\CMakeFiles\bench.dir\bench.c.obj
In file included from C:\Users\hebasto\secp256k1\src\bench.c:11:
C:\Users\hebasto\secp256k1\src\util.h(34,13): warning: unused function 'print_buf_plain' [-Wunused-function]
34 | static void print_buf_plain(const unsigned char *buf, size_t len) {
| ^~~~~~~~~~~~~~~
1 warning generated.
[2/16] Building C object src\CMakeFiles\secp256k1_precomputed.dir\precomputed_ecmult_gen.c.obj
In file included from C:\Users\hebasto\secp256k1\src\precomputed_ecmult_gen.c:3:
In file included from C:\Users\hebasto\secp256k1\src\group.h:10:
In file included from C:\Users\hebasto\secp256k1\src\field.h:10:
C:\Users\hebasto\secp256k1\src\util.h(34,13): warning: unused function 'print_buf_plain' [-Wunused-function]
34 | static void print_buf_plain(const unsigned char *buf, size_t len) {
| ^~~~~~~~~~~~~~~
```
This PR resolves this issue.
---
**Additional note for reviewers:** The VS builtin clang can also be used assuming that the following VS components are installed:
The user can generate a build system on Windows as follows:
- Using the default "Visual Studio" generator:
```
cmake -B build -T ClangCL
```
- Using the "Ninja" generator:
```
cmake -B build -G Ninja -DCMAKE_C_COMPILER=clang-cl
```
---
Required for downstream projects which aim to build with `clang-cl` (see https://github.com/bitcoin/bitcoin/issues/31456).
ACKs for top commit:
real-or-random:
utACK 4c50d73dd9
Tree-SHA512: 439eb53afd7be65d538cd569f3d095f58325bd26ffc5014ca5f94320689a45b20c9a5a963170578214a20fd3233ec15ef6ab75ab96ce3a4314c282b1b6229ca1
These function aliases have been described as DEPRECATED in the public
API docs already many years ago (see #701, commit 41fc7856), and in
addition explicit deprecation warnings are shown by the compiler at
least since the first official release 0.2.0 (see PR #1089, commit
fc94a2da), so it should be fine to just remove them by now.
Co-authored-by: Tim Ruffing <crypto@timruffing.de>
1823594761 Verify `compressed` argument in `secp256k1_eckey_pubkey_serialize` (Sebastian Falbesoner)
Pull request description:
Due to similarity to the public API function `secp256k1_ec_pubkey_serialize`, public API flags like `SECP256K1_EC_COMPRESSED` are sometimes mistakingly passed to `secp256k1_eckey_pubkey_serialize` in newly proposed code (this is currently the case for several modules in secp256k1-zkp, see https://github.com/BlockstreamResearch/secp256k1-zkp/pull/300), which is currently not detected. To avoid this in the future, a VERIFY_CHECK is added to check that the `compressed` argument is either 0 or 1.
ACKs for top commit:
real-or-random:
utACK 1823594761
stratospher:
ACK 1823594. Got tests failures when passing public API flags to `secp256k1_eckey_pubkey_serialize`.
Tree-SHA512: ca542afc87f33e436ba33dc55b285dfe3759007c446ef94503bc1044c7a0a7f7b2208ae82e2c9743fc5fa38cf386127f3fbfa02d2c242f28fab3041ee46f153b
13d389629a CONTRIBUTING: mention that `EXIT_` codes should be used (Sebastian Falbesoner)
c855581728 test, bench, precompute_ecmult: use `EXIT_...` constants for `main` return values (Sebastian Falbesoner)
965393fcea examples: use `EXIT_...` constants for `main` return values (Sebastian Falbesoner)
Pull request description:
This simple PR addresses #1609 for all example and internal binaries. Alternative to #1618, which is stale (the author confirmed to me that they are not working on that PR anymore). The last commits adds a suggestion to CONTRIBUTING.md, not sure though if we want to go that far.
ACKs for top commit:
jonasnick:
ACK 13d389629a
real-or-random:
utACK 13d389629a
Tree-SHA512: 513eba4b712ba3d5f23a5fdc51cb27c5347b29bcaba39501345913c220be6f093a41186911032d2ddc898b848de84f05f374b3554ffcf92610728b2a23c0bb36
