Better support for ARM/M1/Apple Silicon

improbable-eng / ansible-buildkite-agent

An Ansible role to install the Buildkite Agent.

Apache License 2.0

12 stars 14 forks source link

Better support for ARM/M1/Apple Silicon #63

Closed bombsimon closed 2 years ago

bombsimon commented 3 years ago

Changes

Support more than one installation of brew - don't rely on $PATH or defaults, use buildkite_agent_brew_dir.

This use case is probably not that uncommon since a lot of things still needs to be emulated/run with Rosetta 2 making you install two versions of brew.

Verification

Run the playbook on a M1 Mac with brew installed both in /opt/homebrew for arm and /usr/local (to support Rosetta 2/Intel installations).

Sorry for the messy PR. I was trying to address a bunch of issues but let's fix the linting in #65 and have this PR only address which brew to invoke when multiple is installed. I also created #66 to resolve the group issues I had that made this PR messy.

bombsimon commented 3 years ago

I'm once again stuck in the workflow. Running yamlfmt (with ci.sh) will add an indentation for the whole file (since it's a list I think?) but after doing so, yamllint will complain about bad indentation. Didn't change any .yamllint configuration since I wanted to hear what your suggested solution would be.

~~Also, this is a really minor thing, haven't added anything to the CHANGELOG yet, please let me know if you want me to do this.~~

EDIT: Seems to be addressed in #65, will wait for that and rebase.

PaulSonOfLars commented 3 years ago

As you've already notced, I've re-linted the repo, which should hopefully make your PR easier and avoid the extra friction.

I'm of the opinion that any new feature should go into changelog, especially given that this is a helpful addition :)

bombsimon commented 3 years ago

I rebased and squashed my mess, updated the CHANGELOG.

This partly fixes my issue of running multiple instances of brew. The problem is that there's still no support for this with the homebrew_tap target, see this issue (closed today but I just added a comment).

I currently solve this by tapping the repository manually with my preferred brew. However, one could argue that this PR itself is useless since with only one installation of brew, this is not an issue and with multiple versions like I have, the user needs to add the tap manually. What do you think, should I add a manual tap step instead of the current homebrew_tap target, should I just close this PR or should we leave this as is with some kind of semi support and wait for upstream?

Helcaraxan commented 3 years ago

Hello @bombsimon. I agree with your analysis as to the usefulness of this PR relative to the upstream support for two brew installations. It seems that the upstream asked you to create a new issue for supporting two separate installations. I'd reckon it's worth following up on that request and see what the reaction is. In the meantime we can hold this PR in its current state. It's unlikely to go stale anytime soon. :)

bombsimon commented 3 years ago

@Helcaraxan Thanks for your response!

I'll create a new issue and maybe a PR upstream regarding specifying path for brew_tap as well. Let's see how that pans out but when we support multiple brews natively we can take a new look at this issue.