Skip to content

Conversation

@fanquake
Copy link
Member

@fanquake fanquake commented Nov 2, 2023

Drop the workaround of setting optimization flags to -O1 for the arm64-apple-darwin builds. I no-longer see reproducibility issues when building across x86_64 and aarch64:

real	7m21.192s
user	67m41.047s
sys	5m8.596s
fedora-32gb-hel1-1 bitcoin]# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
62373549d2884e8ef8f46a77b9a93f64ebfc88603569e9d33b68fc67beaf2226  guix-build-664c87354f9e/output/arm64-apple-darwin/SHA256SUMS.part
597889f1908fdb67a6419177a98935b7119c637a962f03f47270893c5ba3fd6f  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.tar.gz
289340354532a54a42b7235c831d13fdb28751c643f0fa0fc417ab195e9b5d90  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.zip
74f4ab3819a186d6c34ca0a4b9dda7c38fcb36bd9b22075a5d91df9bdd5df98a  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin.tar.gz
f0d0dad63057c7dddf6d6ccee244e7916ac0ee26b3bef8dd35f8430280043b38  guix-build-664c87354f9e/output/dist-archive/bitcoin-664c87354f9e.tar.gz
fedora-32gb-hel1-1 bitcoin]# uname -m
aarch64
real	18m10.759s
user	108m21.656s
sys	6m12.930s
ubuntu-32gb-hel1-1:~/bitcoin# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
62373549d2884e8ef8f46a77b9a93f64ebfc88603569e9d33b68fc67beaf2226  guix-build-664c87354f9e/output/arm64-apple-darwin/SHA256SUMS.part
597889f1908fdb67a6419177a98935b7119c637a962f03f47270893c5ba3fd6f  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.tar.gz
289340354532a54a42b7235c831d13fdb28751c643f0fa0fc417ab195e9b5d90  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.zip
74f4ab3819a186d6c34ca0a4b9dda7c38fcb36bd9b22075a5d91df9bdd5df98a  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin.tar.gz
f0d0dad63057c7dddf6d6ccee244e7916ac0ee26b3bef8dd35f8430280043b38  guix-build-664c87354f9e/output/dist-archive/bitcoin-664c87354f9e.tar.gz
ubuntu-32gb-hel1-1:~/bitcoin# uname -m
x86_64

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 2, 2023

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
ACK hebasto, TheCharlatan

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

@hebasto
Copy link
Member

hebasto commented Nov 2, 2023

Concept ACK. Nice :)

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 664c873.

My Guix builds:

x86_64
62373549d2884e8ef8f46a77b9a93f64ebfc88603569e9d33b68fc67beaf2226  guix-build-664c87354f9e/output/arm64-apple-darwin/SHA256SUMS.part
597889f1908fdb67a6419177a98935b7119c637a962f03f47270893c5ba3fd6f  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.tar.gz
289340354532a54a42b7235c831d13fdb28751c643f0fa0fc417ab195e9b5d90  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.zip
74f4ab3819a186d6c34ca0a4b9dda7c38fcb36bd9b22075a5d91df9bdd5df98a  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin.tar.gz
f0d0dad63057c7dddf6d6ccee244e7916ac0ee26b3bef8dd35f8430280043b38  guix-build-664c87354f9e/output/dist-archive/bitcoin-664c87354f9e.tar.gz
aarch64
62373549d2884e8ef8f46a77b9a93f64ebfc88603569e9d33b68fc67beaf2226  guix-build-664c87354f9e/output/arm64-apple-darwin/SHA256SUMS.part
597889f1908fdb67a6419177a98935b7119c637a962f03f47270893c5ba3fd6f  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.tar.gz
289340354532a54a42b7235c831d13fdb28751c643f0fa0fc417ab195e9b5d90  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.zip
74f4ab3819a186d6c34ca0a4b9dda7c38fcb36bd9b22075a5d91df9bdd5df98a  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin.tar.gz
f0d0dad63057c7dddf6d6ccee244e7916ac0ee26b3bef8dd35f8430280043b38  guix-build-664c87354f9e/output/dist-archive/bitcoin-664c87354f9e.tar.gz

Copy link
Contributor

@sedited sedited left a comment

Choose a reason for hiding this comment

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

ACK 664c873

I get the same hashes on both platforms.

@fanquake fanquake merged commit 0fd7ca4 into bitcoin:master Nov 3, 2023
@fanquake fanquake deleted the drop_O1_workaround_qt branch November 3, 2023 14:43
@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 3, 2023

Guix builds (on x86_64)

File commit f23ac10
(master)
commit 1669f4fff6b964ea0c284d13c4745825ee135618
(master and this pull)
SHA256SUMS.part 92659357fc97fc8e... 211b6d4fa72bf6e2...
*-aarch64-linux-gnu-debug.tar.gz 8e85af4277097e63... 0eb731d6627e9455...
*-aarch64-linux-gnu.tar.gz d29c219e45146ed7... e8a600f9707db751...
*-arm-linux-gnueabihf-debug.tar.gz e9564bc7151e514d... f6051729915f9e91...
*-arm-linux-gnueabihf.tar.gz 0dea490de2900729... d2e5032aa674ce4a...
*-arm64-apple-darwin-unsigned.tar.gz ed905a38a6985922... 8e73315081b1c4b6...
*-arm64-apple-darwin-unsigned.zip 5ff300abccc86e16... 6089e2ed3e96c816...
*-arm64-apple-darwin.tar.gz a46b5b97c3daaad9... 9ee04855e6e23393...
*-powerpc64-linux-gnu-debug.tar.gz ed88e835f99cd5cc... 33e99cd311c31db7...
*-powerpc64-linux-gnu.tar.gz 0be7a7cb50a4dd4a... 2b4783713489565d...
*-powerpc64le-linux-gnu-debug.tar.gz 9565b3c366b349e0... 50fc16ba05e8d06b...
*-powerpc64le-linux-gnu.tar.gz 01c7d06acaa2bda9... 9f2fd46a4c2d7043...
*-riscv64-linux-gnu-debug.tar.gz bcc048b2f5be12e1... 6184b27b7c8551cb...
*-riscv64-linux-gnu.tar.gz e9b064d98292bf76... c1c36cdcbddcf43a...
*-x86_64-apple-darwin-unsigned.tar.gz a0715a07c1dcf9d3... f8e7c176b8c33b38...
*-x86_64-apple-darwin-unsigned.zip 31b6d7106c930a54... a36e63bb6d60a591...
*-x86_64-apple-darwin.tar.gz 5ab5018cc8162e08... c6d723ef3e6aec67...
*-x86_64-linux-gnu-debug.tar.gz 250b3ad75c897648... aef211de3fd3cf84...
*-x86_64-linux-gnu.tar.gz 2011c32866473392... 6b33623054f2c9d2...
*.tar.gz 2ea368703376018d... 1348c8b681769fda...
guix_build.log cb9055f69265de9e... 4f5592bb6b68920b...
guix_build.log.diff d2de79cb1dc40b85...

PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Oct 24, 2024
… build

664c873 depends: drop -O1 workaround from arm64 apple Qt build (fanquake)

Pull request description:

  Drop the workaround of setting optimization flags to -O1 for the `arm64-apple-darwin` builds. I no-longer see reproducibility issues when building across `x86_64` and `aarch64`:
  ```bash
  real7m21.192s
  user67m41.047s
  sys5m8.596s
  fedora-32gb-hel1-1 bitcoin]# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
  62373549d2884e8ef8f46a77b9a93f64ebfc88603569e9d33b68fc67beaf2226  guix-build-664c87354f9e/output/arm64-apple-darwin/SHA256SUMS.part
  597889f1908fdb67a6419177a98935b7119c637a962f03f47270893c5ba3fd6f  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.tar.gz
  289340354532a54a42b7235c831d13fdb28751c643f0fa0fc417ab195e9b5d90  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.zip
  74f4ab3819a186d6c34ca0a4b9dda7c38fcb36bd9b22075a5d91df9bdd5df98a  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin.tar.gz
  f0d0dad63057c7dddf6d6ccee244e7916ac0ee26b3bef8dd35f8430280043b38  guix-build-664c87354f9e/output/dist-archive/bitcoin-664c87354f9e.tar.gz
  fedora-32gb-hel1-1 bitcoin]# uname -m
  aarch64
  ```

  ```bash
  real18m10.759s
  user108m21.656s
  sys6m12.930s
  ubuntu-32gb-hel1-1:~/bitcoin# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
  62373549d2884e8ef8f46a77b9a93f64ebfc88603569e9d33b68fc67beaf2226  guix-build-664c87354f9e/output/arm64-apple-darwin/SHA256SUMS.part
  597889f1908fdb67a6419177a98935b7119c637a962f03f47270893c5ba3fd6f  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.tar.gz
  289340354532a54a42b7235c831d13fdb28751c643f0fa0fc417ab195e9b5d90  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin-unsigned.zip
  74f4ab3819a186d6c34ca0a4b9dda7c38fcb36bd9b22075a5d91df9bdd5df98a  guix-build-664c87354f9e/output/arm64-apple-darwin/bitcoin-664c87354f9e-arm64-apple-darwin.tar.gz
  f0d0dad63057c7dddf6d6ccee244e7916ac0ee26b3bef8dd35f8430280043b38  guix-build-664c87354f9e/output/dist-archive/bitcoin-664c87354f9e.tar.gz
  ubuntu-32gb-hel1-1:~/bitcoin# uname -m
  x86_64
  ```

ACKs for top commit:
  hebasto:
    ACK 664c873.
  TheCharlatan:
    ACK 664c873

Tree-SHA512: 79527df4181eb0a0c42fe526581479abcdeba8fb09e1faf52265d697d39a8f3a3532ee3c573579b9af00b7330a401e4b6f1686636f9bac6bf9839be8381a2033
PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Oct 24, 2024
c75a0d4 Merge bitcoin#29177: build: Fix check whether `-latomic` needed (fanquake)
f670118 Merge bitcoin#28851: build: Patch Qt to handle minimum macOS version properly (fanquake)
685ee8a Merge bitcoin#28884: doc: remove x86_64 build assumption from depends doc (fanquake)
47f6126 Merge bitcoin#28881: doc: remove mention of missing bdb being a configure error (fanquake)
a9021db Merge bitcoin#28777: doc: update docs for `CHECK_ATOMIC` macro (fanquake)
d5e15df Merge bitcoin#26839: Add support for RNDR/RNDRRS for AArch64 on Linux (Andrew Chow)
5aedcbf Merge bitcoin#28778: depends: drop -O1 workaround from arm64 apple Qt build (fanquake)
95a8d8c Merge bitcoin#21161: Fee estimation: extend bucket ranges consistently (glozow)
f4ea48e Merge bitcoin#28693: build: Include `config/bitcoin-config.h` explicitly in `util/trace.h` (fanquake)
f160e0d Merge bitcoin#28691: refactor: Remove CBlockFileInfo::SetNull (fanquake)
0278163 Merge bitcoin#28697: fuzz: Increase merge -rss_limit_mb (fanquake)
90a1fb0 Merge bitcoin#28650: fuzz: Merge with -set_cover_merge=1 (fanquake)
f007abd Merge bitcoin#28459: build: add `-mbranch-protection=bti` (aarch64) to hardening flags (fanquake)
af8d124 Merge bitcoin#28624: docs: fix typo (fanquake)
c740264 Merge bitcoin#28532: qt: enable` -ltcg` for windows under LTO (fanquake)
ccd3920 Merge bitcoin#28556: doc: fix link to developer-notes.md file in multiprocess.md (fanquake)

Pull request description:

  ## Issue being fixed or feature implemented
  Batch of trivial backports

  ## What was done?
  See commits

  ## How Has This Been Tested?
  built locally; large combined merge passed tests locally

  ## Breaking Changes
  Should be none

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK c75a0d4

Tree-SHA512: 035dc3fa9812c7f381946ae4798b8e729a58b38a090d94502a8d992e9cfaab3307173c602d7b782c637a79c5c41b62570dc73bb4bb367e4505a039964926181b
@bitcoin bitcoin locked and limited conversation to collaborators Nov 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants