bitcoin-core / secp256k1

Optimized C library for EC operations on curve secp256k1
MIT License
2.02k stars 977 forks source link

build: Replace hardcoded "auto" value with default one #1535

Closed hebasto closed 2 weeks ago

hebasto commented 1 month ago

"auto" implies that a value is being chosen based on build system introspection or host system capabilities. However, for the --with-ecmult-window and --with-ecmult-gen-kb options, the values "auto" are hardcoded, which might lead to confusion.

This PR replaces "auto" with more appropriate default values.

If Concept ACKed, I'll add equivalent commits for CMake.

real-or-random commented 1 month ago

Concept ACK from my side

I think having a default is good enough, we don't need two layers of indirection (default -> auto -> some number).

hebasto commented 1 month ago

If Concept ACKed, I'll add equivalent commits for CMake.

The equivalent commits for CMake have been added.

fanquake commented 2 weeks ago

Concept ACK

sipa commented 2 weeks ago

utACK 4d9645bee06e732f5d7d9e72b0c26c22c5006eb8

real-or-random commented 2 weeks ago

Does this need a changelog entry? Strictly speaking yes because this removes a valid config option. But pragmatically, I tend to say no: the breakage occurs only for people who passed =auto (the default) explicitly, so this should hardly affect anyone.

edit: Let me just add the label, we can revisit this when we do the release.