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

Support arm64 binary generation #1342

Merged
merged 3 commits into from Jan 5, 2023
Merged

Conversation

pepix
Copy link
Contributor

@pepix pepix commented Dec 19, 2022

What was the problem?

Before fix

error

What I did

  • Separate compile command into two. One is for x86_64 users (Intel Mac) and another one is for arm64 users (ex. M1, M1 Pro, etc...)
  • 🆕 Add a new workflow to build macOS binaries for all architectures on GitHub Actions

After fix

pass

Benefit

  • Apple silicon users can install their suitable danger by only brew install danger/tap/danger-swift command as before without any concerns.

Next actions

  • 🆕 Configure Actions Secret and Deploy Key
    1. Generate SSH key with RSA algorithm using keygen on your terminal. Leave passphrase empty.
    2. Add public key to deploy keys in homebrew-tap repository.
    3. Add secret key to Actions secrets in danger-js repository. Key name must be HOMEBREW_TAP_DEPLOY_SECRET_KEY.
  • Release operator has to run yarn package:x64 and yarn package:arm64 when releasing new version from now on. And publish both.
    • 🆕 Just run $ gh workflow run .github/workflows/release.yml version_digit={major | minor | patch} (select update version type) on your terminal if you have GitHub CLI. Or execute on GitHub Actions dashboard page.

@orta
Copy link
Member

orta commented Dec 19, 2022

Nice thinking!

The deploy process handles the homebrew-tap here: https://github.com/danger/danger-js/blob/main/scripts/create-homebrew-tap-pr.sh

And https://github.com/danger/danger-js/blob/main/.release-it.json#L9 will need to be tweaked to generate both

That should be it though IMO

@pepix
Copy link
Contributor Author

pepix commented Dec 20, 2022

Hello! @orta
Before updating, I realized it seems only arm64 mac can make package for both architectures. However x64 mac cannot.
I'm not very sure current release operation though, are you sure release operator have arm64 mac or release pipeline can build for both?
If no, we have to prepare workaround before merge.
Updating GitHub Actions could be enough I believe.

@pepix
Copy link
Contributor Author

pepix commented Dec 23, 2022

Hi! @orta
I've added a new GitHub workflow to build packages for all macOS architectures on cloud.
Once you setup SSH keys, only one command delivers new release even you don't have arm64 mac.

Any feedbacks and opinions are welcome :)

@orta
Copy link
Member

orta commented Jan 5, 2023

Alright, well this looks great to me - I was a little wary because ~250 people have write access to the org, but they already have write access to the homebrew parts of it anyway and so changing just this part of the process doesn't actually change anything fundementally.

So, I've given it all a once over and it looks good to me - later this evening I'll try ship the latest build of danger with it- thanks!

@orta
Copy link
Member

orta commented Jan 5, 2023

OK, so there's a lot going on here - I'm going to need to tweak this a bit to let the npm parts of the process work locally (because I don't want everyone having access to deploy to npm) on my device, and then the github release + homebrew stuff happen in GH actions

@orta
Copy link
Member

orta commented Jan 5, 2023

That said, thanks a lot - this must have been quite time consuming, and probably required learning a bunch of new things!

@orta orta merged commit 0f48d32 into danger:main Jan 5, 2023
@orta
Copy link
Member

orta commented Jan 5, 2023

Alright, I've got this process up and running:

It looks correct to me!

@orta
Copy link
Member

orta commented Jan 5, 2023

Thanks! I was keeping an intel mac around (I use linux as my main daily machine) specifically for Danger deploys, this means a deploy for me is now trivial and doesn't require pulling out an old computer and doing a bunch of work.

@pepix
Copy link
Contributor Author

pepix commented Jan 6, 2023

I'm glad that I could alleviate your loads by my commit.
Thank you for supporting npm part and others. :)

Now I confirmed brew install danger/tap/danger-swift and danger-swift pr https://github.com/danger/swift/pull/146 works without error on M1 mac.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants