Skip to content

build, mac: Include arch in codesignature tarball #24506

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Mar 9, 2022
Merged

build, mac: Include arch in codesignature tarball #24506

merged 2 commits into from
Mar 9, 2022

Conversation

achow101
Copy link
Member

@achow101 achow101 commented Mar 8, 2022

Since we have two architectures for Mac binaries, having the architecture in the code signature tarball generated by detached-sig-create.sh allows us to avoid accidentally overwriting an existing code signature tarball during the code signing process.

@achow101 achow101 force-pushed the mac-arch-in-codesig branch from 99e6b29 to 8f8c411 Compare March 8, 2022 17:18
@fanquake fanquake added this to the 23.0 milestone Mar 8, 2022
@fanquake
Copy link
Member

fanquake commented Mar 8, 2022

Concept ACK

@hebasto
Copy link
Member

hebasto commented Mar 8, 2022

Concept ACK.

@gruve-p
Copy link
Contributor

gruve-p commented Mar 8, 2022

Concept ACK

@luke-jr
Copy link
Member

luke-jr commented Mar 8, 2022

I may be out of the loop, but I would have expected macOS binaries to bundle both archs together in the same download? So-called "universal binaries"...

Do those require two separate signatures still?

@achow101
Copy link
Member Author

achow101 commented Mar 8, 2022

I may be out of the loop, but I would have expected macOS binaries to bundle both archs together in the same download? So-called "universal binaries"...

Do those require two separate signatures still?

We don't currently create universal binaries. Even so, there would still need to be two separate detached signatures as the universal binaries are ostensibly just the binaries for each arch cat'd together (with some extra headers). So there will be a signature for each of the internal arch specific binaries.

@hebasto
Copy link
Member

hebasto commented Mar 8, 2022

I think we shouldn't force users to download "fat"/universal binaries, which are appr. 2 times bigger, when actually the only arch is used.

fanquake
fanquake reviewed Mar 9, 2022
View reviewed changes
@laanwj
Copy link
Member

laanwj commented Mar 9, 2022

Concept ACK.

I think we shouldn't force users to download "fat"/universal binaries, which are appr. 2 times bigger, when actually the only arch is used.

Agree. If we can, I'd prefer to stick to one OS/architecture pair per download,. It makes things simpler for us in several ways, and if we're worried users have difficulty picking the right file we could implement detection on the download website. And last but not least it seems Apple is really committed to this ARM thing so any work on fat binaries is wasted in a slightly longer timeframe.

@achow101 achow101 force-pushed the mac-arch-in-codesig branch from 8f8c411 to 00d8ad0 Compare March 9, 2022 14:51
@achow101
Copy link
Member Author

achow101 commented Mar 9, 2022 •
edited
Loading

Apparently file on linux has different output from file on Mac, so I've added a command to signapple that will also give us the arch and we can use that instead of file.

This requires (yet another) signapple update in the guix manifest.

@achow101 achow101 force-pushed the mac-arch-in-codesig branch from 00d8ad0 to 5591c10 Compare March 9, 2022 14:52
@achow101 achow101 force-pushed the mac-arch-in-codesig branch from 5591c10 to 0189df1 Compare March 9, 2022 15:38
@laanwj
Copy link
Member

laanwj commented Mar 9, 2022

Apparently file on linux has different output from file on Mac

Right, you can't generally rely on the output of file having a certain format. It's always a hassle with those external utilities. If we convert the script to Python we could use LIEF to do the identification. But making it part of signapple will work too.

@fanquake
Copy link
Member

fanquake commented Mar 9, 2022

Guix Build:

bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
09db635cb20def6de50c5bf65e7bcd59a0e21264dafc510cd939468ae7fb999f  guix-build-0189df1d3171/output/aarch64-linux-gnu/SHA256SUMS.part
353a261df932f46ec7ae168754487aa1431c15c11e06a544f4dcf9b5ab8c3a05  guix-build-0189df1d3171/output/aarch64-linux-gnu/bitcoin-0189df1d3171-aarch64-linux-gnu-debug.tar.gz
a110a2d2bacd8e60f7b5fcd77c3aa2de67ce85506308d7a51c3c4f5ca3a91aaa  guix-build-0189df1d3171/output/aarch64-linux-gnu/bitcoin-0189df1d3171-aarch64-linux-gnu.tar.gz
4106dc2ec9a341d5e565d03f55a8c74f53343f3502c7446e7076457933b25710  guix-build-0189df1d3171/output/arm-linux-gnueabihf/SHA256SUMS.part
6a8e94e539af46d2499feecf831363a3f7f9b73f3a32f763f86d2972004c1489  guix-build-0189df1d3171/output/arm-linux-gnueabihf/bitcoin-0189df1d3171-arm-linux-gnueabihf-debug.tar.gz
608121622e39bd43289facce3e1bac162a53377912cf632e571badfb6a79f7e1  guix-build-0189df1d3171/output/arm-linux-gnueabihf/bitcoin-0189df1d3171-arm-linux-gnueabihf.tar.gz
ea0e7192934131f1334a74c733a446f8fcf8df40af8c3656e4e9a7354d5cf506  guix-build-0189df1d3171/output/arm64-apple-darwin/SHA256SUMS.part
d3e9af6798f7a90e40c1377b892c1af6d22ce6b8b21ce52693f94e85a41783d8  guix-build-0189df1d3171/output/arm64-apple-darwin/bitcoin-0189df1d3171-arm64-apple-darwin.tar.gz
20d8636b06c5b359d42b49b001d3be45a8e0296cb80e678db93086924acfa283  guix-build-0189df1d3171/output/arm64-apple-darwin/bitcoin-0189df1d3171-osx-unsigned.dmg
cb95db1e635faea3ba4408815ab637221c5716f58fc0f33ccf73db573d515a3b  guix-build-0189df1d3171/output/arm64-apple-darwin/bitcoin-0189df1d3171-osx-unsigned.tar.gz
69ee69895b16bf442aeae9f4db71cea22d5db3cfd83aca4b6ff05b78b13d46f7  guix-build-0189df1d3171/output/dist-archive/bitcoin-0189df1d3171.tar.gz
22bccdd004c2437602c1a32f3e308f34c04c366d83c94291db684e94593773c4  guix-build-0189df1d3171/output/powerpc64-linux-gnu/SHA256SUMS.part
d07a69e5df94e0915e59a4f4214c1b4ed2e63de623cbffdb32c1f160c7da95e9  guix-build-0189df1d3171/output/powerpc64-linux-gnu/bitcoin-0189df1d3171-powerpc64-linux-gnu-debug.tar.gz
407eef789dbd4844def7ed89cc6deae6f134c2a5142505f85419b3ac5b3fa688  guix-build-0189df1d3171/output/powerpc64-linux-gnu/bitcoin-0189df1d3171-powerpc64-linux-gnu.tar.gz
96da23ee5ea47efc9b6a600afd828a19b488fb960820d1a49c6d4da6e79c9ab8  guix-build-0189df1d3171/output/powerpc64le-linux-gnu/SHA256SUMS.part
ed64502878fbc65f1e3fbf106697fae891476ba942e8ffe6e91618a23fc14d7e  guix-build-0189df1d3171/output/powerpc64le-linux-gnu/bitcoin-0189df1d3171-powerpc64le-linux-gnu-debug.tar.gz
ea36305e663e2227b1efcc5e6d6c834e37427b238c5916f4908f2290c5ace393  guix-build-0189df1d3171/output/powerpc64le-linux-gnu/bitcoin-0189df1d3171-powerpc64le-linux-gnu.tar.gz
393f0f2adac52c33f1db4d83e763a12bc18df2930ba53310af792398411db34d  guix-build-0189df1d3171/output/riscv64-linux-gnu/SHA256SUMS.part
c127f8c76798d901cd34ae43020dc100aabeff0f42432dd75047b69e14ce9bae  guix-build-0189df1d3171/output/riscv64-linux-gnu/bitcoin-0189df1d3171-riscv64-linux-gnu-debug.tar.gz
919ff300d0f52bde80435e33289a9212ca993e8eef1f7cab2fc65e46622397da  guix-build-0189df1d3171/output/riscv64-linux-gnu/bitcoin-0189df1d3171-riscv64-linux-gnu.tar.gz
093ce6591f3f8717eb2a1e958923adbf0c0f089cfdbf26bedf080e5413aa8eea  guix-build-0189df1d3171/output/x86_64-apple-darwin/SHA256SUMS.part
600c39d062e5ac91880a06ce545e1de12d1c65f36e6d75752afb9c1800a11ae1  guix-build-0189df1d3171/output/x86_64-apple-darwin/bitcoin-0189df1d3171-osx-unsigned.dmg
780b0e080a1752a041ae3da789cd4e8ebe160b0984930d11d20e9c42245f0070  guix-build-0189df1d3171/output/x86_64-apple-darwin/bitcoin-0189df1d3171-osx-unsigned.tar.gz
a3c476e0622a3250d3411d352698f4e0745d3c801048bf3af3b671edf2311ba3  guix-build-0189df1d3171/output/x86_64-apple-darwin/bitcoin-0189df1d3171-osx64.tar.gz
8e7bce66c933c44e8b9ed4b3677b2aca61b4db82d31b706df6f61162b184dd99  guix-build-0189df1d3171/output/x86_64-linux-gnu/SHA256SUMS.part
094cca2a913e90f9fbad2e1748f85d4c4dbaf27e47f679d28c38bf258c509b73  guix-build-0189df1d3171/output/x86_64-linux-gnu/bitcoin-0189df1d3171-x86_64-linux-gnu-debug.tar.gz
bbe6e61e1774af0010caefb2d8e40947798bc118cb6ea1ec8e4cc9810370afe0  guix-build-0189df1d3171/output/x86_64-linux-gnu/bitcoin-0189df1d3171-x86_64-linux-gnu.tar.gz
06c88caf944ebae2c3cae310c5b5e562d0a8164e09c9273968086638c73f4e68  guix-build-0189df1d3171/output/x86_64-w64-mingw32/SHA256SUMS.part
2d625f68e1bb4157bdd5cc18aca0507a4e9468b56e9041ddcc99d95885bb3a93  guix-build-0189df1d3171/output/x86_64-w64-mingw32/bitcoin-0189df1d3171-win-unsigned.tar.gz
64896ec36395c4c5eb99185a34006346e57c1132a3c8bbd6f2c6c9ff22334654  guix-build-0189df1d3171/output/x86_64-w64-mingw32/bitcoin-0189df1d3171-win64-debug.zip
e2c532d3e7b26381481847597f5cedee712e64831d99b1696bf6be3e1c877bc9  guix-build-0189df1d3171/output/x86_64-w64-mingw32/bitcoin-0189df1d3171-win64-setup-unsigned.exe
4232a4ef3d5e958fd19461503fef29e64b3de60867bb812bc92f498f2ffabf19  guix-build-0189df1d3171/output/x86_64-w64-mingw32/bitcoin-0189df1d3171-win64.zip

fanquake
fanquake approved these changes Mar 9, 2022
View reviewed changes
Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

ACK 0189df1

I've tested that running detached-sig-create.sh now creates signature-osx-x86_64.tar.gz and signature-osx-arm64.tar.gz (with the latest signapple installed), and that those two tarballs contain Bitcoin-Qt.x86_64sign and Bitcoin-Qt.arm64sign.

Also ran a (non-code-sign) Guix build (hashes above).

@fanquake fanquake merged commit 430acb7 into bitcoin:master Mar 9, 2022
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Mar 9, 2022
@achow101 @fanquake
guix: use latest signapple
eaf6715 
Github-Pull: bitcoin#24506
Rebased-From: 6e9308c
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Mar 9, 2022
@achow101 @fanquake
build, mac: Include arch in codesignature tarball
7a614a7 
Github-Pull: bitcoin#24506
Rebased-From: 0189df1
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Mar 9, 2022
@achow101 @fanquake
guix: use latest signapple
eba4658 
Github-Pull: bitcoin#24506
Rebased-From: 6e9308c
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Mar 9, 2022
@achow101 @fanquake
build, mac: Include arch in codesignature tarball
1fd91da 
Github-Pull: bitcoin#24506
Rebased-From: 0189df1
@fanquake fanquake mentioned this pull request Mar 9, 2022
Merged
fanquake added a commit that referenced this pull request Mar 10, 2022
@fanquake
Merge #24514: [23.x] Second round signapple backports (M1)
2bd5fdb 
1fd91da build, mac: Include arch in codesignature tarball (Andrew Chow)
eba4658 guix: use latest signapple (Andrew Chow)

Pull request description:

  This backports additional signapple and macOS codesigning changes from #24506 for 23.x (skipping #24503, which is an interim signapple update, and the certvalidator change, which is not strictly necessary), and should complete the changes required to perform M1 codesigning for the 23.x release.

ACKs for top commit:
  hebasto:
    ACK 1fd91da, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: 55b7e0680058203744dc491da060ef56a880270a01c10e475b7a171457902410806895dbe7332a323ce47c2a776337fa29eb811d2511fce84ede13aeacbe2a02
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Mar 11, 2022
@fanquake
Merge bitcoin#24506: build, mac: Include arch in codesignature tarball
a5c1678
@bitcoin bitcoin locked and limited conversation to collaborators Mar 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@fanquake fanquake fanquake approved these changes

+2 more reviewers

@prusnak prusnak prusnak left review comments

@ClaraBara22 ClaraBara22 ClaraBara22 approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Build system macOS
Projects
None yet
Milestone
23.0
Development

Successfully merging this pull request may close these issues.
8 participants
@achow101 @fanquake @hebasto @gruve-p @luke-jr @laanwj @prusnak @ClaraBara22