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: Replace genbuild.sh with GenerateBuildInfo.cmake #54

Merged
merged 3 commits into from
Nov 26, 2023

Conversation

hebasto
Copy link
Owner

@hebasto hebasto commented Nov 22, 2023

This PR replaces shell share/genbuild.sh script with CMake GenerateBuildInfo.cmake script, which works on all platforms, including Windows.

Fixes build on Windows after #51.

Please note, that in the master branch, MSVC builds lack build info entirely.

Added a CI test to ensure equivalence of the script replacement.

No need to test non-CMake stuff in this branch.
Add CI test for `obj/build.h` generation.
@hebasto
Copy link
Owner Author

hebasto commented Nov 22, 2023

Friendly ping @fanquake @theuni @TheCharlatan @vasild @pablomartin4btc :)

Copy link

@pablomartin4btc pablomartin4btc left a comment

Choose a reason for hiding this comment

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

tACK 2e2a7b5

Built on Ubuntu 22.04, ran ctest, functionals and executed bitcoind -signet up to full headers sync.

@vasild
Copy link

vasild commented Nov 24, 2023

Light ACK, why not remove share/genbuild.sh?

@hebasto
Copy link
Owner Author

hebasto commented Nov 24, 2023

Light ACK, why not remove share/genbuild.sh?

Sure. After transition to CMake is completed?

Right now it seems useful to keep it to ensure that share/genbuild.sh and the new CMake script produce the same headers.

Copy link

@pablomartin4btc pablomartin4btc left a comment

Choose a reason for hiding this comment

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

tACK 2e2a7b5

Built cross-compiled for Windows on WSL Ubuntu 22.04 (instructions), binaries are fine and ran on Windows 11 Pro but 2 tests are failing running ctest, I've verified that's also happening on cmake-staging branch, so I've opened #55.

@hebasto
Copy link
Owner Author

hebasto commented Nov 26, 2023

Fixes build on Windows after #51.

Going to merge this PR shortly to unlock Windows builds.

@hebasto hebasto merged commit dddef8c into cmake-staging Nov 26, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants