depends: update `LD64_VERSION` to 711 #28630

fanquake · 2023-10-10T10:49:27Z

I forgot to do this in 7d58152.
Add a test so it's impossible to forget.

I forgot to do this in 7d58152.

DrahtBot · 2023-10-10T10:49:29Z

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	TheCharlatan, jarolrod, hebasto, laanwj, achow101

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:

#28622 (build: use macOS 14 SDK (Xcode 15.0) 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.

DrahtBot · 2023-10-10T20:09:57Z

Guix builds (on x86_64)

File	commit `04265ba` (master)	commit `9b18f32` (master and this pull)
SHA256SUMS.part	`5f7b9409967bce91...`	`24ee5cd4de9d0194...`
*-aarch64-linux-gnu-debug.tar.gz	`52e90d34f345ce49...`	`652ec94723bc5fd2...`
*-aarch64-linux-gnu.tar.gz	`4e902429fabc1e11...`	`6a33ab46a38d86eb...`
*-arm-linux-gnueabihf-debug.tar.gz	`9e24365552e7732f...`	`41b6d9756020af3e...`
*-arm-linux-gnueabihf.tar.gz	`996542b0d675b54f...`	`e0a73f94c191e9a2...`
*-arm64-apple-darwin-unsigned.tar.gz	`49bd07b0508b50e4...`	`43f2811db216134b...`
*-arm64-apple-darwin-unsigned.zip	`084ac0129c7c7c6b...`	`504e5c93e98104dc...`
*-arm64-apple-darwin.tar.gz	`b97f14c06c00862b...`	`6a8ec1c14238c63d...`
*-powerpc64-linux-gnu-debug.tar.gz	`35993005b3322e3e...`	`ddfca84061c6a589...`
*-powerpc64-linux-gnu.tar.gz	`489a57c7b9cd3f2b...`	`c235d888d8802bef...`
*-powerpc64le-linux-gnu-debug.tar.gz	`eb29e1ea6cec70c3...`	`620e5891b4842168...`
*-powerpc64le-linux-gnu.tar.gz	`7895006f6318ed62...`	`d421cd812736ae2c...`
*-riscv64-linux-gnu-debug.tar.gz	`f9119fd9fa7d3ed8...`	`56fc636a25f68e39...`
*-riscv64-linux-gnu.tar.gz	`7644ccd5cd789154...`	`cf5596ff8ee83177...`
*-x86_64-apple-darwin-unsigned.tar.gz	`21265c6fc5d444d0...`	`59bd9f4fd30a3dad...`
*-x86_64-apple-darwin-unsigned.zip	`905a80a477bfed07...`	`9cd169c8366a211e...`
*-x86_64-apple-darwin.tar.gz	`6ff350519b9f4c49...`	`ca5298bee8f70fc4...`
*-x86_64-linux-gnu-debug.tar.gz	`36c1e3b815680db6...`	`5560a407c87a1c33...`
*-x86_64-linux-gnu.tar.gz	`c80ffb5eec287559...`	`7b7866d814205171...`
*.tar.gz	`a7aabcdfbf89f7c6...`	`05af7e0c9b374927...`
guix_build.log	`1ac99f60c27ed1bc...`	`7240616b6ec6ecdf...`
guix_build.log.diff		`a1049e9d5b5fdcd6...`

TheCharlatan · 2023-10-11T06:07:52Z

Damn, should have caught this :(
utACK 092daa2

hebasto · 2023-10-11T09:38:52Z

Add a test so it's impossible to forget.

Why not derive the LD64_VERSION value directly from the linker:

$ ./depends/x86_64-apple-darwin/native/bin/x86_64-apple-darwin-ld -v
@(#)PROGRAM:ld  PROJECT:ld64-711
BUILD 09:08:42 Oct 11 2023
configured to support archs: armv6 armv7 armv7s arm64 arm64e arm64_32 i386 x86_64 x86_64h armv6m armv7k armv7m armv7em (tvOS)
LTO support using: LLVM version 15.0.7 (static support for 29, runtime is 29)
TAPI support using: TAPI version 1300.6.5 (1300.6.5)

jarolrod

ACK 092daa2

fanquake · 2023-10-11T10:01:32Z

Why not derive the LD64_VERSION value directly from the linker:

That would prevent us from being able to set it to anything other than the exact value output by -v.

Also unsure if whatever parsing code you write would also be compatible with lld.

hebasto · 2023-10-11T10:17:44Z

Add a test so it's impossible to forget.

The test passes with the first commit been reverted.

fanquake · 2023-10-11T10:18:34Z

The test passes with the first commit been reverted.

The because if unset, the value is the default linker version, iirc.

hebasto · 2023-10-11T10:19:49Z

The test passes with the first commit been reverted.

The because if unset, the value is the default linker version, iirc.

How is it unset?

fanquake · 2023-10-11T10:32:38Z

How is it unset?

If the test is still passing, clearly it's not being set to 609. Otherwise it would fail.

hebasto · 2023-10-11T10:50:48Z

How is it unset?

If the test is still passing, clearly it's not being set to 609. Otherwise it would fail.

Please consider https://github.com/hebasto/bitcoin/tree/pr28630/1011.

the LD64_VERSION is still set to the wrong value::

LD64_VERSION=609

but the Guix build succeeds:

8fdacd899e61f2314c1645c787b409197a9783a33994fc2707cd654ae10760fc  guix-build-4e68bd5325ea/output/dist-archive/bitcoin-4e68bd5325ea.tar.gz
830188b61fa795e496ccf38c80900b5ee778ba2bea30f27552db046d738a5ca2  guix-build-4e68bd5325ea/output/x86_64-apple-darwin/SHA256SUMS.part
ca3cf7f40f4ea85c7cf72abba47f25a509243977a587c2c1d9e6d2c6135099f7  guix-build-4e68bd5325ea/output/x86_64-apple-darwin/bitcoin-4e68bd5325ea-x86_64-apple-darwin-unsigned.tar.gz
0517f0faf2abc3b07d3909f5f4e4e588159105f5d481065b36902d27b043f59c  guix-build-4e68bd5325ea/output/x86_64-apple-darwin/bitcoin-4e68bd5325ea-x86_64-apple-darwin-unsigned.zip
51ada364ee1f37ad792d3b8d03230ac3896f5b2dbff469c1c2b411ad9f94fe1f  guix-build-4e68bd5325ea/output/x86_64-apple-darwin/bitcoin-4e68bd5325ea-x86_64-apple-darwin.tar.gz

fanquake · 2023-10-11T10:52:55Z

We don't need another branch. I'm saying the same thing. If the test is passing, clearly the value in the binary is not being set to 609 (in which case, it's being set to the default linker value (711)). (which is why the test is passing with the 2nd commit reverted).

hebasto · 2023-10-11T10:56:22Z

We don't need another branch. I'm saying the same thing. If the test is passing, clearly the value in the binary is not being set to 609 (in which case, it's being set to the default linker value (711)).

If code has LD64_VERSION=609 and the test passes, what is the purpose of the test then?

fanquake · 2023-10-11T10:59:05Z

what is the purpose of the test then?

To check for the version being used/embedded? Clearly it's exposed a bug here, potentially in our understanding of -mlinker-version.

hebasto

ACK 092daa2.

Suggesting to drop "Add a test so it's impossible to forget." from the PR description before merging.

Clearly it's exposed a bug here, potentially in our understanding of -mlinker-version.

Agree.

laanwj

ACK 092daa2
i dont know about mac linker specifics but reviewed symbol-check.py change and lgtm

jarolrod · 2023-10-12T23:06:05Z

GUIX hashes

1451e9cdec787f51604257ce8810be020a3abc8e307474643f79fbb55e403584  guix-build-092daa2f9524/output/arm64-apple-darwin/SHA256SUMS.part
4ffd62b263db859059840dbb2c8602c187d39ba98b8241135a09f54dba0ec4ea  guix-build-092daa2f9524/output/arm64-apple-darwin/bitcoin-092daa2f9524-arm64-apple-darwin-unsigned.tar.gz
b479787d21ac0defabaccc804e4400d13ea51d73557c1fe9483aa2fd3b536b96  guix-build-092daa2f9524/output/arm64-apple-darwin/bitcoin-092daa2f9524-arm64-apple-darwin-unsigned.zip
fe4e5d623e8ddee89414f5d6c5b370280769f8aadc495af4ed3c464499ec9cb0  guix-build-092daa2f9524/output/arm64-apple-darwin/bitcoin-092daa2f9524-arm64-apple-darwin.tar.gz
714f9980e48a01f6ef8655d7f89a208201d2fb96038905de0e997c0c1f12fe68  guix-build-092daa2f9524/output/dist-archive/bitcoin-092daa2f9524.tar.gz
23588af64e1b7019b237810c68df6750517b41df8c48b061147cafdb40408f5a  guix-build-092daa2f9524/output/x86_64-apple-darwin/SHA256SUMS.part
1634732934ee01803adc84e5ba9f5e56579bff237b8aac8ac1882b02e2f64920  guix-build-092daa2f9524/output/x86_64-apple-darwin/bitcoin-092daa2f9524-x86_64-apple-darwin-unsigned.tar.gz
c4dddc491901fe8ea112eb146a4bb9351797c5d8dfc0b8e84e2b548d01ae938e  guix-build-092daa2f9524/output/x86_64-apple-darwin/bitcoin-092daa2f9524-x86_64-apple-darwin-unsigned.zip
2d5876eb31f2cfa32228029fb20851c1ad43c2c36c3870254db94b1da86e9d58  guix-build-092daa2f9524/output/x86_64-apple-darwin/bitcoin-092daa2f9524-x86_64-apple-darwin.tar.gz

achow101 · 2023-10-16T17:28:53Z

ACK 092daa2

092daa2 contrib: add test for macOS linker version to symbol-check (fanquake) cefbf0b depends: update LD64_VERSION to 711 (fanquake) Pull request description: I forgot to do this in bitcoin@7d58152. Add a test so it's impossible to forget. ACKs for top commit: TheCharlatan: utACK 092daa2 achow101: ACK 092daa2 jarolrod: ACK 092daa2 hebasto: ACK 092daa2. laanwj: ACK 092daa2 Tree-SHA512: 37f0bdfd6607a7760eabe5efe279532ba0c59c0915161e08d5e3b9a0b7705839d62537d6e17406062f6a0a1db5407575da7cd671e9cb916e422e77f5649c6e2b

fanquake added 2 commits October 10, 2023 11:12

depends: update LD64_VERSION to 711

cefbf0b

I forgot to do this in 7d58152.

contrib: add test for macOS linker version to symbol-check

092daa2

DrahtBot changed the title ~~depends: update LD64_VERSION to 711~~ depends: update LD64_VERSION to 711 Oct 10, 2023

DrahtBot added the Build system label Oct 10, 2023

fanquake added the DrahtBot Guix build requested label Oct 10, 2023

DrahtBot mentioned this pull request Oct 10, 2023

build: use macOS 14 SDK (Xcode 15.0) #28622

Merged

DrahtBot removed the DrahtBot Guix build requested label Oct 10, 2023

jarolrod reviewed Oct 11, 2023

View reviewed changes

hebasto approved these changes Oct 11, 2023

View reviewed changes

laanwj approved these changes Oct 11, 2023

View reviewed changes

achow101 merged commit 76d8957 into bitcoin:master Oct 16, 2023

fanquake deleted the forgot_to_bump_ld64 branch October 17, 2023 08:51

bitcoin locked and limited conversation to collaborators Oct 16, 2024

depends: update LD64_VERSION to 711 #28630

depends: update LD64_VERSION to 711 #28630

Uh oh!

Conversation

fanquake commented Oct 10, 2023

Uh oh!

DrahtBot commented Oct 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage

Reviews

Conflicts

Uh oh!

DrahtBot commented Oct 10, 2023

Guix builds (on x86_64)

Uh oh!

TheCharlatan commented Oct 11, 2023

Uh oh!

hebasto commented Oct 11, 2023

Uh oh!

jarolrod left a comment

Choose a reason for hiding this comment

Uh oh!

fanquake commented Oct 11, 2023

Uh oh!

hebasto commented Oct 11, 2023

Uh oh!

fanquake commented Oct 11, 2023

Uh oh!

hebasto commented Oct 11, 2023

Uh oh!

fanquake commented Oct 11, 2023

Uh oh!

hebasto commented Oct 11, 2023

Uh oh!

fanquake commented Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Oct 11, 2023

Uh oh!

fanquake commented Oct 11, 2023

Uh oh!

hebasto left a comment

Choose a reason for hiding this comment

Uh oh!

laanwj left a comment

Choose a reason for hiding this comment

Uh oh!

jarolrod commented Oct 12, 2023

Uh oh!

achow101 commented Oct 16, 2023

Uh oh!

Uh oh!

depends: update `LD64_VERSION` to 711 #28630

depends: update `LD64_VERSION` to 711 #28630

DrahtBot commented Oct 10, 2023 •

edited

Loading

fanquake commented Oct 11, 2023 •

edited

Loading