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

Remove unsigned apple silicon builds #160619

Closed
wants to merge 1 commit into from

Conversation

badonyx
Copy link
Contributor

@badonyx badonyx commented Nov 20, 2023

Important: Do not tick a checkbox if you haven’t performed its action. Honesty is indispensable for a smooth review process.

In the following questions <cask> is the token of the cask you're submitting.

After making any changes to a cask, existing or new, verify:

The Apple Silicon builds for these apps are not functional so only the Intel build should be used.

Context: https://github.com/orgs/Homebrew/discussions/3088#discussioncomment-7623916

@alexreg
Copy link
Contributor

alexreg commented Nov 20, 2023

This seems like a far from ideal solution, given it renders ARM Mac users "second-class citizens"... though I appreciate why you want to do this nonetheless. Let's see how the maintainers reply to this comment about creating their own certificate authority? Alternatively, we could just add a note to the cask to instruct ARM users to self-sign the installed app?

@razvanazamfirei razvanazamfirei added the awaiting maintainer feedback Issue needs response from a maintainer. label Nov 21, 2023
@razvanazamfirei
Copy link
Member

I think the more obvious solution is for upstream to sign their binaries.

@SMillerDev
Copy link
Member

Yeah, I see two solutions here.

  1. You instead contact upstream about signing all their binaries because it doesn't work well on arm otherwise.
  2. We deprecate these calls with a message they don't work well on modern macs.

I'd say the first option should be done either way.

@krehel
Copy link
Member

krehel commented Nov 21, 2023

Signing would be the most appropriate to get working binaries for Apple Silicon, as others mentioned.

I can agree with removing currently broken builds that can have negative user experience impact. I don't know if it requires a special caveat beyond the normal requires_rosetta though.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Not gonna ✅ because I don't feel I have sufficient authority on this repo but: I think this is what should be done here. To not do so feels like we're implicitly encouraging the disabling of security features.

@p-linnane
Copy link
Member

All new Mac devices are using Apple Silicon. We should prioritize the Homebrew experience for those devices. I'm fine requiring Rosetta here since upstream refuses to sign their binaries.

@alexreg
Copy link
Contributor

alexreg commented Nov 22, 2023

All new Mac devices are using Apple Silicon. We should prioritize the Homebrew experience for those devices. I'm fine requiring Rosetta here since upstream refuses to sign their binaries.

There's a significant efficiency hit however.

@p-linnane
Copy link
Member

There's a significant efficiency hit however.

There is, but that is because of choices made by the actual upstream. By not signing their binaries, they are giving their own users a poor expereince. Homebrew will not recommend circumventing Apple's native security mechanisms to accomodate developers who choose to not fully support macOS.

@MikeMcQuaid
Copy link
Member

There is, but that is because of choices made by the actual upstream. By not signing their binaries, they are giving their own users a poor expereince. Homebrew will not recommend circumventing Apple's native security mechanisms to accomodate developers who choose to not fully support macOS.

Strongly agreed.

@alexreg
Copy link
Contributor

alexreg commented Nov 22, 2023

Strongly agreed.

As stated, their reason is because they don't want to attach their identity to a signing certificate. That said, I've suggested they create their own certificate authority, which users can manually trust. Probably the next best thing. Let's see if they go that way.

@SMillerDev
Copy link
Member

That said, I've suggested they create their own certificate authority, which users can manually trust. Probably the next best thing. Let's see if they go that way.

I'm pretty sure macOS won't

@alexreg
Copy link
Contributor

alexreg commented Nov 22, 2023

I'm pretty sure macOS won't

MacOS won't trust it? It's worked for me before, when building my own apps. You just have to go into Keychain Access and select to manually trust it.

@MikeMcQuaid
Copy link
Member

You just have to go into Keychain Access and select to manually trust it.

Unless this is securely integrated into the brew --cask install flow: this will cause more problems for us than it will solve.

@alexreg
Copy link
Contributor

alexreg commented Nov 22, 2023

You just have to go into Keychain Access and select to manually trust it.

Unless this is securely integrated into the brew --cask install flow: this will cause more problems for us than it will solve.

I disagree, but I'll look into automating it with a script regardless, since that would be nice.

@badonyx
Copy link
Contributor Author

badonyx commented Nov 27, 2023

I will point out that lidarr was already being distributed this way on homebrew, it just was not marked as such. To be consistent either lidarr should have the arm build added, or radarr and prowlarr should have the arm builds removed. (sonarr does not distribute an arm build so I only added the required rosetta caveat.)

If upstream starts signing their builds then the arm builds could be added back, no big deal.

The caveat text feels a little unnecessary, maybe a simple (ruby) comment linking to the discussion would suffice?

@alexreg
Copy link
Contributor

alexreg commented Nov 27, 2023

@badonyx FYI Sonarr will start releasing ARM builds when v4 comes out (which is based on .NET Core). Not sure exactly that will be, but it is already in beta at least...

@MikeMcQuaid
Copy link
Member

I will point out that lidarr was already being distributed this way on homebrew, it just was not marked as such.

@badonyx stupid question, sorry: what is "this way"?

If upstream starts signing their builds then the arm builds could be added back, no big deal.

Agreed 👍🏻

@bevanjkay
Copy link
Member

✅ From me, my only question would be around the wording of the caveat. Should we be more explicit about the issue rather than just saying that the build is "not functional", mention that the app is not signed and ask the developer to sign it?
Also should the caveat go in an on_arm block?

@badonyx
Copy link
Contributor Author

badonyx commented Nov 28, 2023

what is "this way"?

An unsigned arm build is available upstream but only the intel build is available from homebrew-cask.

@SMillerDev
Copy link
Member

I find it much more likely that nobody added it yet than that it was intentionally done like this to avoid codesigning issues.

@MikeMcQuaid
Copy link
Member

An unsigned arm build is available upstream but only the intel build is available from homebrew-cask.

Gotcha, thanks. To agree with @SMillerDev and @bevanjkay: we intentionally do not want to attempt to distribute unsigned ARM builds.

Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale Issue which has not received any feedback for some time. label Dec 21, 2023
@github-actions github-actions bot closed this Dec 29, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automerge-skip awaiting maintainer feedback Issue needs response from a maintainer. outdated stale Issue which has not received any feedback for some time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants