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

Apple m1 opt homebrew support (Fixes #153) #154

Merged
merged 13 commits into from
Dec 21, 2021

Conversation

trinitronx
Copy link
Contributor

Description

  • Adds support for arm64 installation of Homebrew

Issues Resolved

#153 - Support arm64 install of Homebrew

Check List

  • A summary of changes made is included in the CHANGELOG under ## Unreleased
  • New functionality includes testing.
  • [N/A] New functionality has been documented in the README if applicable.

@trinitronx trinitronx requested a review from a team as a code owner December 19, 2021 20:53
@@ -24,7 +24,7 @@
property :cask_name, String, regex: %r{^[\w/-]+$}, name_property: true
property :options, String
property :install_cask, [true, false], default: true
property :homebrew_path, String, default: '/usr/local/bin/brew'
property :homebrew_path, String, default: lazy { "#{HomebrewWrapper.new.install_path}/bin/brew" }
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this cookbook requires Infra Client 15.3 or later the cask and tap resources should really just get deleted. These ship in 14.0 and later already.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I saw that being mentioned in the README. I figured that it was not yet done for some reason, given that it says this should have happened in 2019 and it's still here.

Also given that there's an open issue #154, I'm targeting that hotfix plus the brew tap --full deprecation with this PR. So, I figured other cookbook LWRP changes were out of scope.

Quick question:

  • Would the brew tap --full change alone require a major version bump à la SemVer?

Copy link
Contributor Author

@trinitronx trinitronx Dec 20, 2021

Choose a reason for hiding this comment

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

Infra Client 15.3 or later

@tas50 Hmm, maybe I'm missing something? I only could find this line in metadata.rb:

chef_version '>= 12.15'

FWIW, all my macOS machines are still using chef gem 13.10.0. I pinned down dependencies a few years back thanks to the ever-present macOS Xcode + CLT issues with gem compilation / bootstrap. The LWRPs from this cookbook work on Chef 13.10.0.

I should probably update at some point. I remember that I tried doing so about a year ago but there was an issue with test-kitchen + InSpec testing in Chef 16.1. The bundled versions of test-kitchen + Inspec in Chef Workstation at the time were testing the wrong cookbook version. The test fixture cookbook dependencies were pulling the homebrew cookbook from supermarket.chef.io rather than testing the currently in-development local git repo version that was supposed to be the code under test. I was able to verify this again with the public vagrant box kd76/macos-with-chef, and compare this with the working results from Chef Infra 17.7.29 and 17.8.25 on both Big Sur and Catalina VMs. It's also great to see that the GitHub actions test-kitchen + InSpec runs were successful too! So maybe the "storm has passed" so to speak, and it's all-clear for me to finally upgrade to Chef Infra 17.

That being said, there's probably still some value in cutting a hotfix release of this cookbook for earlier versions of Chef that don't have the new Homebrew resource providers.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, i think we should cut this release then fix this cookbook up. i.e. remove those resources, and update our test matrix.

Copy link
Contributor

Choose a reason for hiding this comment

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

I missed that this one works on older client releases. This makes sense then. What we should make sure we do is gate these resources so they don't override the built in resources when those are available.

@damacus damacus added the Release: Minor Release to Chef Supermarket as a minor release when merged label Dec 21, 2021
@damacus damacus merged commit 2980305 into sous-chefs:main Dec 21, 2021
@kitchen-porter
Copy link
Contributor

Released as: 5.3.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release: Minor Release to Chef Supermarket as a minor release when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants