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

build: Consider SOURCE_DATE_EPOCH in Guix environment only #29761

Closed
wants to merge 1 commit into from

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Mar 28, 2024

The SOURCE_DATE_EPOCH environment variable is supposed to be used to achieve build reproducibility.

However, using it outside Guix scripts for local builds on macOS is not meaningful.

This PR makes usage of SOURCE_DATE_EPOCH for macOS consistent with other platforms and simplifies upcoming CMake-based build system scripts.

@DrahtBot
Copy link
Contributor

DrahtBot commented Mar 28, 2024

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.

Type Reviewers
Concept NACK fanquake

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #29790 ([DO NOT MERGE] cmake: Migrate CI scripts to CMake-based build system -- WIP by hebasto)

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.

@DrahtBot
Copy link
Contributor

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the
documentation.

Possibly this is 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.

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

Debug: https://github.com/bitcoin/bitcoin/runs/23219591618

@hebasto hebasto marked this pull request as draft March 29, 2024 09:08
@hebasto
Copy link
Member Author

hebasto commented Mar 29, 2024

Guix builds:

x86_64
ef12d060b4e4d74030586f9b49e8152e50365d46487bba4f2a659634ef7ee5f6  guix-build-b78b7f2ee77f/output/arm64-apple-darwin/SHA256SUMS.part
4e06e4a4943f9495311b5f4934bd40145a5b80b64254027f97d92425b4669413  guix-build-b78b7f2ee77f/output/arm64-apple-darwin/bitcoin-b78b7f2ee77f-arm64-apple-darwin-unsigned.tar.gz
5f0e1f2f4dcf7948870d11375a1acdc64a0bb2d921666474f4fdbe11914158dd  guix-build-b78b7f2ee77f/output/arm64-apple-darwin/bitcoin-b78b7f2ee77f-arm64-apple-darwin-unsigned.zip
5a0358db31223bcac44b3fd3b9058f12df0bf8b79ff0cdb047ae723303385f09  guix-build-b78b7f2ee77f/output/arm64-apple-darwin/bitcoin-b78b7f2ee77f-arm64-apple-darwin.tar.gz
c268e0729f45d442b1a767af4bb63d7940561fe331bda22c431a30f1ad03103b  guix-build-b78b7f2ee77f/output/dist-archive/bitcoin-b78b7f2ee77f.tar.gz
b4c7adbec6383abe7077f527b01648bb3b43173ba241b647d8a50ba1fede64a7  guix-build-b78b7f2ee77f/output/x86_64-apple-darwin/SHA256SUMS.part
4020b84e34860bf65373d9723d046a991a32c723f1f1a3dece6e7609b4fb3988  guix-build-b78b7f2ee77f/output/x86_64-apple-darwin/bitcoin-b78b7f2ee77f-x86_64-apple-darwin-unsigned.tar.gz
bb2f73676c6fadc814a97a3c0b06aaeeb5cce6456f0a395dc03102f76797e7cb  guix-build-b78b7f2ee77f/output/x86_64-apple-darwin/bitcoin-b78b7f2ee77f-x86_64-apple-darwin-unsigned.zip
b6f9d1e6e6cd7959f43f44d03dc8b958c7e648245452be2112b35a4e0c6af597  guix-build-b78b7f2ee77f/output/x86_64-apple-darwin/bitcoin-b78b7f2ee77f-x86_64-apple-darwin.tar.gz

@hebasto hebasto marked this pull request as ready for review March 29, 2024 11:01
hebasto added a commit to hebasto/bitcoin that referenced this pull request Mar 31, 2024
984c548 fixup! cmake: Migrate Guix build scripts to CMake (Hennadii Stepanov)
fb23c32 fixup! cmake: Add `Maintenance` module (Hennadii Stepanov)
6ac45b7 refactor! cmake: Redefine/adjust per-configuration flags (Hennadii Stepanov)
bce7bea fixup! build: Generate `share/toolchain.cmake` in depends (Hennadii Stepanov)

Pull request description:

  What to test:
  1. `cmake --build build && cmake --build build -t deploydir` and the following `cmake --build build -t deploy`  when cross-compiling for macOS and on macOS natively.
  2. Guix builds.

  This PR includes changes from bitcoin#29733.

  UPD. Also related: bitcoin#29761.

ACKs for top commit:
  TheCharlatan:
    ACK 984c548

Tree-SHA512: be056492ee7f5f210afaef7574bdcc56f01be80e7db888b7571e36a4be50f90337d0f9f1a623588f8c91898665ff06b22300dec9292016f05f2d2913a823d2eb
@fanquake
Copy link
Member

However, using it outside Guix scripts for local builds on macOS is not meaningful.

Why? Using Guix should not be a requirement for SOURCE_DATE_EPOCH to be useful. I would think that given the pinned compilers, and provisioned std lib, the macOS cross build would actually pretty be close to deterministic outside of Guix.

@fanquake
Copy link
Member

What is the status of this?

@hebasto
Copy link
Member Author

hebasto commented Apr 24, 2024

However, using it outside Guix scripts for local builds on macOS is not meaningful.

Why? Using Guix should not be a requirement for SOURCE_DATE_EPOCH to be useful.

Not a requirement. Both Guix and SOURCE_DATE_EPOCH are tools of the same domain, which is build reproducibility.

I would think that given the pinned compilers, and provisioned std lib, the macOS cross build would actually pretty be close to deterministic outside of Guix.

Is anyone interesting / working on that?

What is the status of this?

Under peer reviewing, I guess :)

@fanquake
Copy link
Member

Is anyone interesting / working on that?

I'm saying it's likely already the case, without further changes.

Under peer reviewing, I guess :)

Ok. Concept NACK I guess. I don't see the motivation.

@hebasto hebasto closed this Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants