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

Added a build for Apple Silicon #55

Merged
merged 1 commit into from
Feb 24, 2022
Merged

Added a build for Apple Silicon #55

merged 1 commit into from
Feb 24, 2022

Conversation

fangel
Copy link
Contributor

@fangel fangel commented Feb 14, 2022

Based on the changes to the main summon .goreleaser.yml file, I've added the same changes to this configuration.

I was able to do a ./bin/build.sh locally, and could manually copy over the result to /usr/local/lib/summon which seems to work.

Note, I've never done any work with either Go or GoReleaser, so this might be the wrong approach. So feel free to suggest a completely different approach to solve the issue of adding support for Apple Silicon.

Desired Outcome

A release for Apple Silicon powered macOS computers to run is created along side all of the existing releases.

Implemented Changes

Changelog

  • The CHANGELOG has been updated, or
  • This PR does not include user-facing changes and doesn't require a
    CHANGELOG update

Test coverage

  • This PR includes new unit and integration tests to go with the code
    changes, or
  • The changes in this PR do not require tests

Documentation

  • Docs (e.g. READMEs) were updated in this PR
  • A follow-up issue to update official docs has been filed here: insert issue ID
  • This PR does not require updating any documentation

Behavior

  • This PR changes product behavior and has been reviewed by a PO, or
  • These changes are part of a larger initiative that will be reviewed later, or
  • No behavior was changed with this PR

Security

  • Security architect has reviewed the changes in this PR,
  • These changes are part of a larger initiative with a separate security review, or
  • There are no security aspects to these changes

@fangel fangel requested a review from a team as a code owner February 14, 2022 15:18
@rpothier
Copy link
Contributor

Hi @fangel , thanks for submitting this PR!
Can you sign off your commit ? I will run Jenkins to verify the image is built.

@fangel
Copy link
Contributor Author

fangel commented Feb 17, 2022

I've amended the commit with --signoff and force-pushed now. I think that should fo the trick wrt sign-off.

There's a sister issue at cyberark/homebrew-tools#41 - I can create a PR for the summons-aws-secrets-formulae, but there's still an issue with the way that other non-Apple Silicon formulae in the cask is formulated that makes the whole cask fail.

@rpothier
Copy link
Contributor

rpothier commented Feb 18, 2022

Hi @fangel
After running the CI I found that the version of Goreleaser that is being used by summon-aws-secrets does not support
the new Apple HW. I posted a PR for that . I kept the update as a separate issue in case it breaks any of the current images. That PR should go in first.

rpothier added a commit that referenced this pull request Feb 18, 2022
@fangel
Copy link
Contributor Author

fangel commented Feb 20, 2022

Do you want me to rebase off of master, so you can try to re-run the CI builds?

@rpothier
Copy link
Contributor

Hi @fangel , Yes please rebase and can you also add a comment in the changelog?

Added

@rpothier rpothier self-requested a review February 22, 2022 17:17
@fangel fangel force-pushed the patch-1 branch 2 times, most recently from 4e5be3c to dcd1360 Compare February 23, 2022 07:46
Based on the changes to the main summon `.goreleaser.yml` file, I've added the same changes to this configuration.

Signed-off-by: Morten Fangel <fangel@sevengoslings.net>
@fangel
Copy link
Contributor Author

fangel commented Feb 23, 2022

I've rebased off of main and added the changelog modifications now.

Copy link
Contributor

@rpothier rpothier left a comment

Choose a reason for hiding this comment

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

LGTM!

@fangel
Copy link
Contributor Author

fangel commented Feb 24, 2022

Perfect! What's the procedure for actually merging? Will you do that, or?

@rpothier rpothier merged commit a3e4456 into cyberark:main Feb 24, 2022
@rpothier
Copy link
Contributor

Hi @fangel , I merged it. Thanks for submitting this PR!

@fangel
Copy link
Contributor Author

fangel commented Feb 24, 2022

Wonderful - then the next step for full Apple Silicon support is cyberark/homebrew-tools#41 (and possibly also adding the ARM-build url to the summon-aws-secrets formulae.

@fangel fangel deleted the patch-1 branch February 24, 2022 14:56
@szh szh mentioned this pull request Mar 2, 2022
14 tasks
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.

2 participants