Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmake: Port PR29890 from the master branch #180

Merged
merged 4 commits into from
Apr 30, 2024
Merged

Conversation

hebasto
Copy link
Owner

@hebasto hebasto commented Apr 30, 2024

This PR ports bitcoin#29890 and fixes cross-compiling for macOS been broken since the recent sync/rebase PR.

@hebasto
Copy link
Owner Author

hebasto commented Apr 30, 2024

cc @m3dwards @theuni @TheCharlatan

cc @fanquake as bitcoin#29890 author.

@hebasto
Copy link
Owner Author

hebasto commented Apr 30, 2024

My Guix builds for macOS:

x86_64
4f4d91eed74695067f8f70ee1fa36fadb937447a6b70a04419567c093c5585b6  guix-build-772769abc1f5/output/arm64-apple-darwin/SHA256SUMS.part
55e65a16bfd341d8289ffa9d53dd2c5279a27a8df42808f62eed2ac9d72eb208  guix-build-772769abc1f5/output/arm64-apple-darwin/bitcoin-772769abc1f5-arm64-apple-darwin-unsigned.tar.gz
cc49cd6c254517e504d6ebe2615f11e3b7d5d446d9943c45a3d628b99a8ea3ca  guix-build-772769abc1f5/output/arm64-apple-darwin/bitcoin-772769abc1f5-arm64-apple-darwin-unsigned.zip
cd84b2dbfc08321d12475e3be923fee9e975b395fcf14e899089b39dec61ecdd  guix-build-772769abc1f5/output/arm64-apple-darwin/bitcoin-772769abc1f5-arm64-apple-darwin.tar.gz
263222c61ead87f1d282b6f0637b465575c4605bf99382dc55ae3d27abb20ada  guix-build-772769abc1f5/output/dist-archive/bitcoin-772769abc1f5.tar.gz
a7afb175b12e51db082ed6e262bc21445b740a93abc239ba9600379abdd975c7  guix-build-772769abc1f5/output/x86_64-apple-darwin/SHA256SUMS.part
ea3d7a12d3b376e19bd647a523ab48751e0cd411c5c13db15d4a414c26dfebfa  guix-build-772769abc1f5/output/x86_64-apple-darwin/bitcoin-772769abc1f5-x86_64-apple-darwin-unsigned.tar.gz
fe69d1006a718c8d490cc03dbc8fa5a844e17fd5fc7bafdc0c303b993d424377  guix-build-772769abc1f5/output/x86_64-apple-darwin/bitcoin-772769abc1f5-x86_64-apple-darwin-unsigned.zip
22bb002bd205c9676fa8bad9ad4f0ae48ca1b981f01f0c0177815e9461c50cc3  guix-build-772769abc1f5/output/x86_64-apple-darwin/bitcoin-772769abc1f5-x86_64-apple-darwin.tar.gz

@hebasto hebasto added the port from autotools Ported from the main repository label Apr 30, 2024
@m3dwards
Copy link

Same Guix output:

4f4d91eed74695067f8f70ee1fa36fadb937447a6b70a04419567c093c5585b6  guix-build-772769abc1f5/output/arm64-apple-darwin/SHA256SUMS.part
55e65a16bfd341d8289ffa9d53dd2c5279a27a8df42808f62eed2ac9d72eb208  guix-build-772769abc1f5/output/arm64-apple-darwin/bitcoin-772769abc1f5-arm64-apple-darwin-unsigned.tar.gz
cc49cd6c254517e504d6ebe2615f11e3b7d5d446d9943c45a3d628b99a8ea3ca  guix-build-772769abc1f5/output/arm64-apple-darwin/bitcoin-772769abc1f5-arm64-apple-darwin-unsigned.zip
cd84b2dbfc08321d12475e3be923fee9e975b395fcf14e899089b39dec61ecdd  guix-build-772769abc1f5/output/arm64-apple-darwin/bitcoin-772769abc1f5-arm64-apple-darwin.tar.gz
263222c61ead87f1d282b6f0637b465575c4605bf99382dc55ae3d27abb20ada  guix-build-772769abc1f5/output/dist-archive/bitcoin-772769abc1f5.tar.gz
a7afb175b12e51db082ed6e262bc21445b740a93abc239ba9600379abdd975c7  guix-build-772769abc1f5/output/x86_64-apple-darwin/SHA256SUMS.part
ea3d7a12d3b376e19bd647a523ab48751e0cd411c5c13db15d4a414c26dfebfa  guix-build-772769abc1f5/output/x86_64-apple-darwin/bitcoin-772769abc1f5-x86_64-apple-darwin-unsigned.tar.gz
fe69d1006a718c8d490cc03dbc8fa5a844e17fd5fc7bafdc0c303b993d424377  guix-build-772769abc1f5/output/x86_64-apple-darwin/bitcoin-772769abc1f5-x86_64-apple-darwin-unsigned.zip
22bb002bd205c9676fa8bad9ad4f0ae48ca1b981f01f0c0177815e9461c50cc3  guix-build-772769abc1f5/output/x86_64-apple-darwin/bitcoin-772769abc1f5-x86_64-apple-darwin.tar.gz

@@ -27,7 +30,7 @@ project(BitcoinCore
VERSION ${CLIENT_VERSION_MAJOR}.${CLIENT_VERSION_MINOR}.${CLIENT_VERSION_BUILD}
DESCRIPTION "Bitcoin client software"
HOMEPAGE_URL "https://bitcoincore.org/"
LANGUAGES CXX
LANGUAGES NONE

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the reason for switching to None to stop cmake from doing a compiler/language check at this point which will fail on a missing install_name_tool?

The check will occur later with call to enable_language?

Copy link
Owner Author

@hebasto hebasto Apr 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two points behind switching to NONE:

  1. Group all C++-specific setting:

    bitcoin/CMakeLists.txt

    Lines 53 to 56 in 772769a

    enable_language(CXX)
    set(CMAKE_CXX_STANDARD 20)
    set(CMAKE_CXX_STANDARD_REQUIRED ON)
    set(CMAKE_CXX_EXTENSIONS OFF)
  2. Unify the approach across the entire code base. For example:
    enable_language(C)
    set(CMAKE_C_STANDARD 90)
    set(CMAKE_C_EXTENSIONS OFF)

At the point of the project() call, CMake is not aware of the platform, therefore, the CMAKE_PLATFORM_HAS_INSTALLNAME remains unset, and no check for the install_name_tool happens.

The check will occur later with call to enable_language?

The check is disabled by setting the CMAKE_PLATFORM_HAS_INSTALLNAME to FALSE (on platforms that might support it).

@m3dwards
Copy link

I've reviewed the code and can see CI is green.

Giving a utACK 772769a

@hebasto hebasto merged commit 6640654 into cmake-staging Apr 30, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
port from autotools Ported from the main repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants