build: Replace hardcoded “auto” value with default one #1535

pull hebasto wants to merge 4 commits into bitcoin-core:master from hebasto:240527-auto changing 4 files +16 −36
  1. hebasto commented at 9:52 am on May 27, 2024: member

    “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.

  2. autotools: Remove "auto" value of `--with-ecmult-window` option
    "auto" implies that a value is being chosen based on build system
    introspection or host system capabilities. However, for the
    `--with-ecmult-window` option, the value "auto" is hardcoded, which
    might lead to confusion.
    
    This change replaces "auto" with a more appropriate default value.
    122dbaeb37
  3. autotools: Remove "auto" value of `--with-ecmult-gen-kb` option
    "auto" implies that a value is being chosen based on build system
    introspection or host system capabilities. However, for the
    `--with-ecmult-gen-kb` option, the value "auto" is hardcoded, which
    might lead to confusion.
    
    This change replaces "auto" with a more appropriate default value.
    26b94ee92a
  4. hebasto force-pushed on May 27, 2024
  5. real-or-random commented at 12:17 pm on May 27, 2024: contributor

    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).

  6. cmake: Remove "AUTO" value of `SECP256K1_ECMULT_WINDOW_SIZE` option
    "AUTO" implies that a value is being chosen based on build system
    introspection or host system capabilities. However, for the
    `SECP256K1_ECMULT_WINDOW_SIZE` option, the value "AUTO" is hardcoded,
    which might lead to confusion.
    
    This change replaces "AUTO" with a more appropriate default value.
    a06805ee74
  7. cmake: Remove "AUTO" value of `SECP256K1_ECMULT_GEN_KB` option
    "AUTO" implies that a value is being chosen based on build system
    introspection or host system capabilities. However, for the
    `SECP256K1_ECMULT_GEN_KB` option, the value "AUTO" is hardcoded, which
    might lead to confusion.
    
    This change replaces "AUTO" with a more appropriate default value.
    4d9645bee0
  8. hebasto commented at 12:35 pm on May 27, 2024: member

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

    The equivalent commits for CMake have been added.

  9. real-or-random added the label build on May 27, 2024
  10. real-or-random renamed this:
    autotools: Replace hardcoded "auto" value with default one
    build: Replace hardcoded "auto" value with default one
    on May 27, 2024
  11. real-or-random approved
  12. real-or-random commented at 8:52 pm on May 27, 2024: contributor

    utACK 4d9645bee06e732f5d7d9e72b0c26c22c5006eb8 good from my side, but let’s see if we can get more (Concept) ACKs

    I think we’d still want to have something like profiles (e.g., “desktop”, “embedded”, “minimal”) eventually, but this change makes sense independently of this.

  13. bitcoin-core deleted a comment on Jun 13, 2024
  14. fanquake commented at 8:37 am on June 25, 2024: member
    Concept ACK
  15. sipa commented at 1:02 pm on June 25, 2024: contributor
    utACK 4d9645bee06e732f5d7d9e72b0c26c22c5006eb8
  16. real-or-random merged this on Jun 25, 2024
  17. real-or-random closed this on Jun 25, 2024

  18. hebasto deleted the branch on Jun 25, 2024
  19. real-or-random commented at 2:03 pm on June 25, 2024: contributor

    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.

  20. real-or-random added the label needs-changelog on Jun 25, 2024
  21. fanquake referenced this in commit 1408944d2e on Jun 25, 2024
  22. achow101 referenced this in commit 9ac4f69ec2 on Jun 26, 2024
  23. josibake referenced this in commit 6fa30ee136 on Jun 29, 2024
  24. vmta referenced this in commit bafdd96c0a on Jul 3, 2024
  25. janus referenced this in commit 411aef6677 on Jul 26, 2024
  26. vmta referenced this in commit 871c80c433 on Sep 6, 2024
  27. vmta referenced this in commit 4d1f6d5635 on Oct 29, 2024
  28. real-or-random removed the label needs-changelog on Nov 4, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-21 17:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me