cmake: Switch to crc32c upstream build system #30905

1. 
   hebasto commented at 6:55 pm on September 14, 2024: member

   A few things still need to be addressed:

   • The minimum supported version 3.1 is too low and raises a warning.
   • Some compiler flags (hardening, sanitizers etc) have to be passed to the subtree build system.
   • https://github.com/google/crc32c/pull/61.
2. hebasto added the label Build system on Sep 14, 2024
3. 
   DrahtBot commented at 6:55 pm on September 14, 2024: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #30595 (kernel: Introduce initial C header API by TheCharlatan)
   • #29852 ([WIP] build: remove need to test for endianness by fanquake)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

4. DrahtBot added the label CI failed on Sep 14, 2024
5. 
   DrahtBot commented at 8:07 pm on September 14, 2024: contributor

   🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/30150430662

   Make sure to run all tests locally, according to the documentation.

   The failure may happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

6. hebasto force-pushed on Sep 15, 2024
7. DrahtBot removed the label CI failed on Sep 15, 2024
8. 
   fanquake commented at 11:02 am on September 16, 2024: member

   A few things still need to be addressed:

   This PR is also changing the flags used for code generation. Is that intentional? If yes, would be good to mention in the PR description (and why).

9. 
   hebasto commented at 12:55 pm on September 17, 2024: member

   This PR is also changing the flags used for code generation.

   Yes. The crc32c upstream build system modifies flags in many places, for example:

   0  # Disable C++ exceptions.
   1  string(REGEX REPLACE "-fexceptions" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
   2  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-exceptions")
   3
   4
   5  # Disable RTTI.
   6  string(REGEX REPLACE "-frtti" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
   7  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-rtti")
   

   Is that intentional?

   The intention isn’t mine.

10. 
    fanquake commented at 9:16 am on September 18, 2024: member

    Ok. Well the current changes would be unsafe to merge, and I don’t think we should be adjusting our build system to work around the subtree, if anything, we should just change the subtree directly.
11. DrahtBot added the label Needs rebase on Sep 20, 2024
12. cmake: Make `BUILD_SHARED_LIBS` a cache variable

    This change allows overriding the cached value with local variables when
    building in subtrees.

    e220292ebe
13. cmake: Switch to crc32c upstream build system 59618541ff
14. hebasto force-pushed on Sep 23, 2024
15. DrahtBot removed the label Needs rebase on Sep 23, 2024
16. 
    fanquake commented at 10:25 am on October 21, 2024: member

    Closing, as the consensus between myself, @hebasto, @theuni and @TheCharlatan was to not make this change (for now, or not until we’ve adjusted upstream further).
17. fanquake closed this on Oct 21, 2024

    ---

Contributors
hebasto DrahtBot fanquake

Labels
Build system