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: produce a .zip for macOS distribution #27099

Closed
wants to merge 4 commits into from

Conversation

fanquake
Copy link
Member

@fanquake fanquake commented Feb 14, 2023

Reviving the discussion around using a .zip for the distributed macOS binaries, as opposed to a .dmg.

Given we only had a single report of the "no finder window" issue (#26176), I wonder if that means macOS users were able to figure it out, they gave up/didn't report, or, we just have very few macOS users.

Related to #18128.

@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 14, 2023

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type Reviewers
Concept ACK prusnak, RandyMcMillan, dergoegge
Stale ACK Sjors

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:

  • #25573 ([POC] guix: produce a fully -static-pie bitcoind by fanquake)
  • #25391 (guix: Use LTO to build releases by fanquake)
  • #24123 (build: Pointer Authentication and Branch Target Identification for aarch64 Linux (Guix) by fanquake)
  • #21778 (build: LLD based macOS toolchain 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.

@prusnak
Copy link
Contributor

prusnak commented Feb 14, 2023

Concept ACK

At present, the user experience with DMG is terrible (as noted in this issue: #26176). If there's no straightforward solution on the horizon, it would be best to switch to using ZIP instead. Using DMG without a Finder window provides no advantages and only leads to a lot of confusion. As evidence, Visual Studio Code has recently started distributing their macOS builds as ZIP files instead of DMGs, as you can see on their download page: https://code.visualstudio.com/Download.

@RandyMcMillan
Copy link
Contributor

RandyMcMillan commented Feb 14, 2023

Concept ACK

tested on macOS 12.6.1 (21G217) Python 3.10.9
tested on macOS 10.15.7 (19H2026) Python 3.10.9

@Sjors
Copy link
Member

Sjors commented Feb 15, 2023

Concept ACK, I second @prusnak.

@dergoegge
Copy link
Member

Concept ACK

@fanquake
Copy link
Member Author

fanquake commented Mar 1, 2023

Rebased past #27172.

@fanquake fanquake marked this pull request as ready for review March 3, 2023 14:33
@fanquake
Copy link
Member Author

fanquake commented Mar 3, 2023

A few Concept ACKs here, so have rebased and undrafted.

@Sjors
Copy link
Member

Sjors commented Mar 30, 2023

This is definitely nicer. When you doubleclick on the zip file it magically reveals the orange icon:

Scherm­afbeelding 2023-03-30 om 14 15 54

Scherm­afbeelding 2023-03-30 om 14 16 10

The only thing we need to remind the user of, is to drag it to their applications folder. Worst case: they forget, sync a node, delete everything in the download folder. This won't delete the blockchain or their wallet, which are stored elsewhere. Simply redownload and it will work again. But it may scare users. It's the same as with other macOS applications.

In a followup we could improve this further by moving everything in the archive into a folder. That folder could contain a README, and we could even include bitcoin-cl, bitcoind and (more important perhaps) bitcoin-wallet somewhere.

@Sjors
Copy link
Member

Sjors commented Mar 30, 2023

The above was done with make deploy on macOS. I'd like to test the Guix build too. @achow101 can you sign it? Or is there a way to self-sign it?

Copy link
Member

@Sjors Sjors left a comment

Choose a reason for hiding this comment

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

Code review ACK c5342f0

contrib/guix/manifest.scm Outdated Show resolved Hide resolved
@RandyMcMillan
Copy link
Contributor

note:
a bundled README should be in README.txt format - we can't assume the user can open .md files by default.

@fanquake
Copy link
Member Author

we can't assume the user can open .md files by default.

I'm pretty sure we can assume that on macOS.

@fanquake
Copy link
Member Author

fanquake commented Apr 5, 2023

Rebased, and added a number of small doc fixups.

@Sjors
Copy link
Member

Sjors commented May 9, 2023

When I do make followed by make deploy on e82e731 on macOS 13.3.1 I end up with three files / folders:

  1. Bitcoin-Qt.app (which Finder displays as "Bitcoin Core")
  2. Bitcoin-Core.zip
  3. dist/Bitcoin-Qt.app

It seems like only the zip file needs to be there?

@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 6, 2023

Guix builds (on x86_64)

File commit 9d3b216
(master)
commit 9867382
(master and this pull)
SHA256SUMS.part 3c57cd8199c723d6... 2eba93b4fbc91ca8...
*-aarch64-linux-gnu-debug.tar.gz b3be5bf8b13acd79... f4d78093d8637e39...
*-aarch64-linux-gnu.tar.gz 9e53fc8bdaae2991... b5b09446e21c2d72...
*-arm-linux-gnueabihf-debug.tar.gz 2da52974294d7aa7... 7a7c0950d04b7acd...
*-arm-linux-gnueabihf.tar.gz 849397fb888189c0... 692e3a011e7c2b11...
*-arm64-apple-darwin-unsigned.dmg 83850133c42db63f...
*-arm64-apple-darwin-unsigned.tar.gz 198c1392640d6281... 53c6d09d1d68cd77...
*-arm64-apple-darwin.tar.gz 577cff9bca290b78... eed69b61e2405f9a...
*-powerpc64-linux-gnu-debug.tar.gz df855e3842293446... 772ca96e576228d5...
*-powerpc64-linux-gnu.tar.gz 6dbc4447804f6cfe... ef8bba9df629e9e1...
*-powerpc64le-linux-gnu-debug.tar.gz 394cf58d49da4576... 0f7b6a6a968878cb...
*-powerpc64le-linux-gnu.tar.gz d4bc7a07b39cffe9... 00a7ac08432057a0...
*-riscv64-linux-gnu-debug.tar.gz 53fe0d40b995b2f3... 3c6aa74c60ab6d6e...
*-riscv64-linux-gnu.tar.gz 6fe76c88c2922c51... ba6077364db593e2...
*-x86_64-apple-darwin-unsigned.dmg 5808e1822110dd7f...
*-x86_64-apple-darwin-unsigned.tar.gz 520cbaa034c71b5b... a4e1ae5c6638fd28...
*-x86_64-apple-darwin.tar.gz 9cf60f219aa2bfba... 4017d074beeb2418...
*-x86_64-linux-gnu-debug.tar.gz ffd9cc7139745a85... 1f70316bf34b83aa...
*-x86_64-linux-gnu.tar.gz 61204c2275003f89... f6d0c0fec9e0284b...
*.tar.gz 99ad0782b2c9f90d... 47b88d1c9d319e5b...
guix_build.log e857c1500e6e0d32... a23762b15136650f...
*-arm64-apple-darwin-unsigned.zip 04ca1745805fd6a1...
*-x86_64-apple-darwin-unsigned.zip d74a427298d13114...
guix_build.log.diff 6d05b983d5624a71...

@hebasto
Copy link
Member

hebasto commented Sep 6, 2023

Tested bitcoin-8a6275ba6ea6-x86_64-apple-darwin-unsigned.zip.

I think a couple of comments still need to be addressed:

@Sjors
Copy link
Member

Sjors commented Sep 6, 2023

tACK 8a6275b

Tested that it produces a working zip / binary for me on Intel macOS 13.5.1, both when using make deploy on the machine itself, and when using Guix on Ubuntu (AMD).

fec40bc404855508466d4fdce4f729770b73bf882d9622dbd3505ae2673bb8f1  guix-build-8a6275ba6ea6/output/arm64-apple-darwin/SHA256SUMS.part
2edaca85d4a5f45f0c6b3c461dc90a878a5c672025a965c2f0452dedd722e1ed  guix-build-8a6275ba6ea6/output/arm64-apple-darwin/bitcoin-8a6275ba6ea6-arm64-apple-darwin-unsigned.tar.gz
d603238e2d5410125058a3810af296fc3d575bf86011feddc2c7ff41336098a7  guix-build-8a6275ba6ea6/output/arm64-apple-darwin/bitcoin-8a6275ba6ea6-arm64-apple-darwin-unsigned.zip
e882484df88c76dec6a0eb000735c7941831a4df98f86a4309839221618f6650  guix-build-8a6275ba6ea6/output/arm64-apple-darwin/bitcoin-8a6275ba6ea6-arm64-apple-darwin.tar.gz
367a361ccd3fa9bc6b8ac99eb23b42a6d4baed6746067f94f6fbb41b1cb5351d  guix-build-8a6275ba6ea6/output/dist-archive/bitcoin-8a6275ba6ea6.tar.gz
25353a797dd0e37d229414ff814692c7a2b6930b16e33d41b8e999f1777c92ab  guix-build-8a6275ba6ea6/output/x86_64-apple-darwin/SHA256SUMS.part
8cf69e85249762e04e9fdb707d9e50d37f9d9f6c7aa239477f5a5c913063201e  guix-build-8a6275ba6ea6/output/x86_64-apple-darwin/bitcoin-8a6275ba6ea6-x86_64-apple-darwin-unsigned.tar.gz
56c042d999726e8179f69b5d354bfcb82fcc69b1926f312e30c396d6752cffa1  guix-build-8a6275ba6ea6/output/x86_64-apple-darwin/bitcoin-8a6275ba6ea6-x86_64-apple-darwin-unsigned.zip
c99a9e1e01db20140855fb2dfe662d750e773bcd3aee2ee94f27905f4bd31b9d  guix-build-8a6275ba6ea6/output/x86_64-apple-darwin/bitcoin-8a6275ba6ea6-x86_64-apple-darwin.tar.gz

@DrahtBot DrahtBot removed the request for review from Sjors September 6, 2023 17:40
@hebasto
Copy link
Member

hebasto commented Sep 7, 2023

Having an error when building Guix:

cd ./dist && find . | sort | zip -X@ ..//distsrc-base/distsrc-11f8b6b2b58e-x86_64-apple-darwin/output/bitcoin-11f8b6b2b58e-x86_64-apple-darwin-unsigned.zip
zip I/O error: No such file or directory
zip error: Could not create output file (..//distsrc-base/distsrc-11f8b6b2b58e-x86_64-apple-darwin/output/bitcoin-11f8b6b2b58e-x86_64-apple-darwin-unsigned.zip)

@fanquake
Copy link
Member Author

fanquake commented Sep 7, 2023

Someone else can take this over if interested.

@Sjors
Copy link
Member

Sjors commented Sep 7, 2023

Difference between my ACK and the last push:

$ git range-diff master 8a6275ba6ea6e8eb5342442221d27d71670e0be8 HEAD
1:  667d5735ce = 1:  bc95abd588 build: add -zip option to macdeployqtplus
2:  85f8152b47 ! 2:  b7e409b7bf build: produce a .zip for macOS distribution
    @@ Makefile.am: osx_volname:
     -  $(XORRISOFS) -D -l -V "$(OSX_VOLNAME)" -no-pad -r -dir-mode 0755 -o $@ $(APP_DIST_DIR) -- $(if $(SOURCE_DATE_EPOCH),-volume_date all_file_dates =$(SOURCE_DATE_EPOCH))
     +$(OSX_ZIP): deploydir
     +  if [ -n "$(SOURCE_DATE_EPOCH)" ]; then find $(APP_DIST_DIR) -exec touch -d @$(SOURCE_DATE_EPOCH) {} +; fi
    -+  find $(APP_DIST_DIR) | sort | zip -X@ $@
    ++  cd $(APP_DIST_DIR) && find . | sort | zip -X@ ../$@

      $(APP_DIST_DIR)/$(OSX_APP)/Contents/MacOS/Bitcoin-Qt: $(OSX_APP_BUILT) $(OSX_PACKAGING)
        INSTALL_NAME_TOOL=$(INSTALL_NAME_TOOL) OTOOL=$(OTOOL) STRIP=$(STRIP) $(PYTHON) $(OSX_DEPLOY_SCRIPT) $(OSX_APP) $(OSX_VOLNAME) -translations-dir=$(QT_TRANSLATION_DIR)
3:  117d01be1d = 3:  e78916bf9d macdeploy: remove DMG generation from deploy script
4:  8a6275ba6e = 4:  11f8b6b2b5 build: remove dmg dependencies

Getting the same (?) error as @hebasto now :-(

if [ -n "1668774980" ]; then find ./dist -exec touch -d @1668774980 {} +; fi
cd ./dist && find . | sort | zip -X@ ..//distsrc-base/distsrc-11f8b6b2b58e-x86_64-apple-darwin/output/bitcoin-11f8b6b2b58e-x86_64-apple-darwin-unsigned.zip
zip I/O error: No such file or directory
zip error: Could not create output file (..//distsrc-base/distsrc-11f8b6b2b58e-x86_64-apple-darwin/output/bitcoin-11f8b6b2b58e-x86_64-apple-darwin-unsigned.zip)
make: *** [Makefile:1304: /distsrc-base/distsrc-11f8b6b2b58e-x86_64-apple-darwin/output/bitcoin-11f8b6b2b58e-x86_64-apple-darwin-unsigned.zip] Error 15

If anyone takes this over, please ping me.

@hebasto
Copy link
Member

hebasto commented Sep 8, 2023

Picked in #28432.

fanquake added a commit that referenced this pull request Sep 20, 2023
b5790c3 build: remove dmg dependencies (fanquake)
33ae0bd macdeploy: remove DMG generation from deploy script (fanquake)
a128111 build: produce a .zip for macOS distribution (Hennadii Stepanov)
c38561d build: add -zip option to macdeployqtplus (fanquake)

Pull request description:

  It is #27099 revived with addressed [comments](#27099 (comment)).

  From #27099 (comment):
  > Reviving the discussion around using a `.zip` for the distributed macOS binaries, as opposed to a `.dmg`.
  >
  > Given we only had a single report of the "no finder window" issue (#26176), I wonder if that means macOS users were able to figure it out, they gave up/didn't report, or, we just have very few macOS users.
  >
  > Related to #18128.

  That's how it looks on macOS:

  ![image](https://github.com/bitcoin/bitcoin/assets/32963518/baa637bb-256b-4b24-8645-8c2754c2ae64)

ACKs for top commit:
  Sjors:
    tACK b5790c3
  jarolrod:
    ACK b5790c3
  TheCharlatan:
    utACK b5790c3

Tree-SHA512: 6e9cb3ab0f60f8a92bfec50577e8d096c5b23ec09ebbb334826415609140ddc96d470aea37379495c1c6bb1beec0d306b09460f62e1543bb0f4396c10a1dfbe2
@fanquake fanquake deleted the macdeploy_use_zip branch March 18, 2024 16:38
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

8 participants