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

fix bitcoin-qt app categorization on apple silicon #418

Merged
merged 1 commit into from
Sep 14, 2021

Conversation

jarolrod
Copy link
Member

@jarolrod jarolrod commented Sep 5, 2021

System Information contains many insights into various aspects of your macOS system; the 'Applications' tab contains info on apps. Starting with macOS 11, the 'kind' column under the 'Applications' tab started displaying the CPU architecture of the application. The options are; apple silicon, intel, universal. Previously, the kind column indicated where the application originated. The change was made to conveniently determine if the app installed was built to run natively on the new M1 CPU or an intel app that will run under rosetta. Of course, there are several other tools to confirm this; the 'kind' column provides a user-friendly way.

We expect that Bitcoin Core compiled, built, and deployed on an intel CPU will be classified as Intel. Similarly, we expect that if this is done on an M1 mac, the resulting app is classified as Apple Silicon. In reality, Bitcoin-qt built and deployed on an M1 mac will be classified as IOS. This behavior is incorrect and should be fixed.

We fix this by setting the CFBundleSupportedPlatforms in our info.plist to the value of MacOSX. In doing this, we are telling macOS, "We do not support IOS; stop it!".

Tested and confirmed that this is a no-op on macOS < 11.

On #22546 Branch #22546 + PR Branch
Screen Shot 2021-09-04 at 6 21 49 PM Screen Shot 2021-09-04 at 6 12 14 PM

To Test:
For testing, our base branch will be #22546. Please perform the following steps on the base branch and then the base branch with the commit from this PR cherry-picked onto it:

  • Have an M1 mac
  • Compile and deploy bitcoin
  • Open up the deployed *.dmg, installed the bundled app
  • Eject the bitcoin dmg that should currently be mounted
  • Navigate to System Information -> Applications
    • Click on the top-left Apple icon
    • Click about this mac
    • Click on the 'Storage' tab
    • Click on the 'Manage...' button
    • On the left, click on 'Applications'
    • Sort by Name
  • Look for the Bitcoin Core application
    • Base Branch: The kind column should state that this application is of type IOS
    • PR Branch: The kind column should state that this application is of type Apple Silicon

Note: Intel users on at least macOS 11 can help test by confirming that the application still shows up as kind=Intel

@jarolrod jarolrod added Bug Something isn't working macOS labels Sep 5, 2021
@hebasto
Copy link
Member

hebasto commented Sep 5, 2021

Concept ACK.

I failed to find CFBundleSupportedPlatforms in Apple's docs (e.g., here and here).

@jarolrod
Copy link
Member Author

jarolrod commented Sep 5, 2021

@hebasto
The usage of CFBundleSupportedPlatforms was done through deductive reasoning. I went through the info.plist of other applications installed on my machine. For the most part, those who were correctly categorized as Apple Silicon included this key. I had one app, besides Bitcoin Core, that was incorrectly categorized as IOS; this other app lacked the CFBundleSupportedPlatforms key.

This key is not documented properly, but a google search and the info.plist of installed applications will show that it is used in production quite ubiquitously. Using a key that is not properly documented can have negative consequences, especially down the line. I'm open to other approaches!

Here's a qt bug that mentions this key: QTBUG-75917
The linked comment states "CFBundleSupportedPlatforms appears to be needed for MacOS deployment as well."

Here we have a qt bug that mentions that QTCreator leaves out the "important" CFBundleSupportedPlatforms key, while it is automatically filled in by Xcode: QTBUG-74872

@jarolrod
Copy link
Member Author

cc @fanquake: can I get your opinion on this?

@fanquake
Copy link
Member

can I get your opinion on this?

Using CFBundleSupportedPlatforms to get the macOS app categorized / recognized correctly by the OS seems ok.

@hebasto
Copy link
Member

hebasto commented Sep 13, 2021

I've checked out multiple non-Apple sources, including stackoverflow.com and other projects on GitHub. And I agree that CFBundleSupportedPlatforms is a valid key, and it is required to recognize an app in a right way.

Why not making this PR independent of bitcoin/bitcoin#22546?

On master, the deployed bitcoin-qt application is categorized as an IOS application.
This is obviously incorrect, and the built executable is not an IOS executable.
To fix this, we set the CFBundleSupportedPlatforms key in our info.plist
@jarolrod jarolrod force-pushed the applesilicon-categorization branch from a03a1a6 to 3765c48 Compare September 13, 2021 09:18
@jarolrod
Copy link
Member Author

Updated from a03a1a6 -> 3765c48 (pr418.01, pr418.02, diff)

Changes: drop commits from #22546 to address @hebasto comment.

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 3765c48

Guix build:

$ env HOSTS='x86_64-apple-darwin18' ./contrib/guix/guix-build
$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
f59636390f1b6c1e51d1c3f1ec5896bfe5ffa65c97f690675e1a1ced9cb6b59e  guix-build-3765c486ef57/output/dist-archive/bitcoin-3765c486ef57.tar.gz
2476e872eefe3d2733689fb841630f647356f84ca3231ad8b5042f06ec01c8ef  guix-build-3765c486ef57/output/x86_64-apple-darwin18/SHA256SUMS.part
eda7eb51866354e5ca2566596fc55fe9ee7b423b3b2ab7af869908751bccfffa  guix-build-3765c486ef57/output/x86_64-apple-darwin18/bitcoin-3765c486ef57-osx-unsigned.dmg
63c4e2c58cb94026dd564e8fae17e5d7a6a4ff7ffa2bc4501239f86cdbc3b849  guix-build-3765c486ef57/output/x86_64-apple-darwin18/bitcoin-3765c486ef57-osx-unsigned.tar.gz
cfda44bb5e56b2bca46024b545cd28b5cc6053a826f9f667da9c732aaf4d8afb  guix-build-3765c486ef57/output/x86_64-apple-darwin18/bitcoin-3765c486ef57-osx64.tar.gz

@jarolrod
Copy link
Member Author

GUIX hashes, mine match @hebasto

$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum

f59636390f1b6c1e51d1c3f1ec5896bfe5ffa65c97f690675e1a1ced9cb6b59e  guix-build-3765c486ef57/output/dist-archive/bitcoin-3765c486ef57.tar.gz
2476e872eefe3d2733689fb841630f647356f84ca3231ad8b5042f06ec01c8ef  guix-build-3765c486ef57/output/x86_64-apple-darwin18/SHA256SUMS.part
eda7eb51866354e5ca2566596fc55fe9ee7b423b3b2ab7af869908751bccfffa  guix-build-3765c486ef57/output/x86_64-apple-darwin18/bitcoin-3765c486ef57-osx-unsigned.dmg
63c4e2c58cb94026dd564e8fae17e5d7a6a4ff7ffa2bc4501239f86cdbc3b849  guix-build-3765c486ef57/output/x86_64-apple-darwin18/bitcoin-3765c486ef57-osx-unsigned.tar.gz
cfda44bb5e56b2bca46024b545cd28b5cc6053a826f9f667da9c732aaf4d8afb  guix-build-3765c486ef57/output/x86_64-apple-darwin18/bitcoin-3765c486ef57-osx64.tar.gz

@hebasto hebasto merged commit b5ede2a into bitcoin-core:master Sep 14, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 15, 2021
…e silicon

3765c48 qt: fix bitcoin-qt app categorization on apple silicon (Jarol Rodriguez)

Pull request description:

  `System Information` contains many insights into various aspects of your macOS system; the 'Applications' tab contains info on apps. Starting with macOS 11, the 'kind' column under the 'Applications' tab started displaying the CPU architecture of the application. The options are; apple silicon, intel, universal. Previously, the `kind` column indicated where the application originated. The change was made to conveniently determine if the app installed was built to run natively on the new M1 CPU or an intel app that will run under rosetta. Of course, there are several other tools to confirm this; the 'kind' column provides a user-friendly way.

  We expect that Bitcoin Core compiled, built, and deployed on an intel CPU will be classified as `Intel`. Similarly, we expect that if this is done on an M1 mac, the resulting app is classified as `Apple Silicon`. In reality, Bitcoin-qt built and deployed on an M1 mac will be classified as `IOS`. This behavior is incorrect and should be fixed.

  We fix this by setting the `CFBundleSupportedPlatforms` in our info.plist to the value of `MacOSX`. In doing this, we are telling macOS, "We do not support IOS; stop it!".

  Tested and confirmed that this is a no-op on macOS < 11.

  | On [bitcoin#22546](bitcoin#22546) Branch | [bitcoin#22546](bitcoin#22546) + PR Branch  |
  | ------------------------------------------------------------------- | ----------- |
  | ![Screen Shot 2021-09-04 at 6 21 49 PM](https://user-images.githubusercontent.com/23396902/132113868-c697d699-12c3-4834-8b8a-003ff475d946.jpeg) | ![Screen Shot 2021-09-04 at 6 12 14 PM](https://user-images.githubusercontent.com/23396902/132113875-6f004f72-4108-41d6-ab03-e90d3e400713.jpeg) |

  **To Test:**
  For testing, our base branch will be [bitcoin#22546](bitcoin#22546). Please perform the following steps on the base branch and then the base branch with the commit from this PR cherry-picked onto it:

  - Have an M1 mac
  - Compile and deploy bitcoin
  - Open up the deployed *.dmg, installed the bundled app
  - Eject the bitcoin dmg that should currently be mounted
  - Navigate to System Information -> Applications
    - Click on the top-left Apple icon
    - Click about this mac
    - Click on the 'Storage' tab
    - Click on the 'Manage...' button
    - On the left, click on 'Applications'
    - Sort by Name
  - Look for the Bitcoin Core application
    - Base Branch: The kind column should state that this application is of type `IOS`
    - PR Branch: The kind column should state that this application is of type `Apple Silicon`

  Note: Intel users on at least macOS 11 can help test by confirming that the application still shows up as kind=`Intel`

ACKs for top commit:
  hebasto:
    ACK 3765c48

Tree-SHA512: 666672025e81e59fe1803859a7f9a4fd3b93a3aba05a163ce223c36081dd579b866d071455608011a19d9ba0c3e9f564cca0c4cb941452f2b51f4ef0dfead1fa
@bitcoin-core bitcoin-core locked as resolved and limited conversation to collaborators Sep 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Something isn't working macOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants