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

Add arm64 support for M1 Mac, replace golint #46

Merged
merged 28 commits into from Apr 7, 2022

Conversation

physik932
Copy link
Contributor

@physik932 physik932 commented Mar 29, 2022

Background

sopstool currently doesn't have support for Mac M1 laptops (darwin-arm64). sops latest 3.7.2 version adds support for the architecture. I'm adding it as an option to goreleaser but ran into some new things:

gomock 1.6.0 breaking change

gomock added ARM support in 1.6.0 but introduced a breaking change to DoAndReturn. It only takes one argument now instead of supporting any number of them (TIL this is what variadic means). I've removed args ...string from these functions to get tests to pass for now, but I'm new to golang and unsure of the consequences. I'm doing a bit of reading to make sure I understand what this means.

golint deprecated

golint appears deprecated and was not installed on my machine when I built the project using go 1.17. I haven't found a replacement yet but a few popped up:

go-version file

I added a .go-version file for the current version of the project. It is recognized by goenv and asdf-vm.

asdf-vm plugin for sopstool

@elementalvoid owns the asdf-vm plugin for sopstool; we will need to introduce a PR there to work with darwin-arm64 packages and update the error message.

Versioning

0.5.0

Additional Requests to Reviewers

Looking for any help on linters, testing the breaking change, and testing arm64 support.

Tasks

  • Specs written
  • Manual testing

/cc

@physik932 physik932 added help wanted Extra attention is needed in progress This issue is a work in progress labels Mar 29, 2022
@physik932 physik932 self-assigned this Mar 29, 2022
ibottamike
ibottamike previously approved these changes Mar 29, 2022
Copy link
Contributor

@ibottamike ibottamike left a comment

Choose a reason for hiding this comment

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

It looks good, some nice updates with adding arm64

.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
.github/CONTRIBUTING.md Show resolved Hide resolved
.goreleaser.yml Show resolved Hide resolved
elementalvoid added a commit to elementalvoid/asdf-sopstool that referenced this pull request Mar 29, 2022
@elementalvoid
Copy link
Contributor

@physik932 -- I've created elementalvoid/asdf-sopstool#1 to get ready for the asdf plugin update. I am not positive what the release file name will look like but this is the basics. Once we have a release here we can ensure the plugin update works!

elementalvoid/asdf-sopstool#1

elementalvoid added a commit to elementalvoid/asdf-sopstool that referenced this pull request Mar 29, 2022
.golangci.yml Show resolved Hide resolved
.goreleaser.yml Show resolved Hide resolved
.goreleaser.yml Show resolved Hide resolved
@@ -94,7 +97,8 @@ blobs:
folder: "{{ .ProjectName }}"

dockers:
- goos: linux
- id: amd64image
goos: linux
Copy link
Member

Choose a reason for hiding this comment

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

this is in prep for manifest build. However - buildx is only available via docker desktop? so I can't get any further.

Copy link
Member

Choose a reason for hiding this comment

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

we should probably add a note in the readme that we're only building amd64 docker images currently

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used colima locally and it seems like the build works (see this comment). I can add that note too.

Copy link
Member

@onyxraven onyxraven Apr 5, 2022

Choose a reason for hiding this comment

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

sure. its building your local platform. we need to build a manifest-build and specify both platforms for it to work properly for both. That requires buildx.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure. its building your local platform. we need to build a manifest-build and specify both platforms for it to work properly for both. That requires buildx.

To clarify for myself, colima is able to process the docker image goreleaser is buildiong because it uses my local machine's platform, but for a manifest build, we would want both amd64 and arm64 to be produced and that requires buildx?

Copy link
Member

Choose a reason for hiding this comment

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

exactly so

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a note about the OS and arch for the docker build to the README.

.travis.yml Show resolved Hide resolved
sopsinstall.sh Show resolved Hide resolved
@physik932 physik932 added enhancement New feature or request request for review Awaiting review and removed help wanted Extra attention is needed in progress This issue is a work in progress labels Apr 5, 2022
@physik932
Copy link
Contributor Author

Looks like we're good to go!

❯ go build
❯ go fmt ./...
❯ go test ./...
?       github.com/Ibotta/sopstool      [no test files]
?       github.com/Ibotta/sopstool/cmd  [no test files]
ok      github.com/Ibotta/sopstool/filecrypt    (cached)
ok      github.com/Ibotta/sopstool/fileutil     (cached)
ok      github.com/Ibotta/sopstool/oswrap       (cached)
?       github.com/Ibotta/sopstool/oswrap/mock  [no test files]
ok      github.com/Ibotta/sopstool/sopsyaml     (cached)
?       github.com/Ibotta/sopstool/testhelpers  [no test files]
❯ golangci-lint run
❯ goreleaser release --snapshot --rm-dist --skip-publish
   • releasing...     
   • loading config file       file=.goreleaser.yml
   • loading environment variables
   • getting and validating git state
      • building...               commit=a616f92238e93190772106e7106f6196ddad6dc7 latest tag=v0.4.4
      • pipe skipped              error=disabled during snapshot mode
   • parsing tag      
   • setting defaults 
   • snapshotting     
      • building snapshot...      version=0.4.4-SNAPSHOT-a616f92
   • checking distribution directory
      • --rm-dist is set, cleaning it up
   • loading go mod information
   • build prerequisites
   • writing effective config file
      • writing                   config=dist/config.yaml
   • building binaries
      • building                  binary=dist/sopstool_linux_arm64/sopstool
      • building                  binary=dist/sopstool_darwin_arm64/sopstool
      • building                  binary=dist/sopstool_linux_amd64/sopstool
      • building                  binary=dist/sopstool_darwin_amd64/sopstool
   • archives         
      • creating                  archive=dist/sopstool_darwin_amd64.tar.gz
      • creating                  archive=dist/sopstool_darwin_arm64.tar.gz
      • creating                  archive=dist/sopstool_linux_amd64.tar.gz
      • creating                  archive=dist/sopstool_linux_arm64.tar.gz
   • linux packages   
      • creating                  arch=amd64 file=dist/sopstool_linux_amd64.rpm format=rpm package=sopstool
      • creating                  arch=amd64 file=dist/sopstool_linux_amd64.deb format=deb package=sopstool
      • creating                  arch=arm64 file=dist/sopstool_linux_arm64.rpm format=rpm package=sopstool
      • creating                  arch=arm64 file=dist/sopstool_linux_arm64.deb format=deb package=sopstool
   • homebrew tap formula
      • writing                   formula=dist/sopstool.rb
   • calculating checksums
   • docker images    
      • building docker image     image=ibotta/sopstool:latest
      • pipe skipped              error=publishing is disabled
   • storing release metadata
      • writing                   file=dist/artifacts.json
      • writing                   file=dist/metadata.json
   • release succeeded after 7.82s

.goreleaser.yml Show resolved Hide resolved
@@ -94,7 +97,8 @@ blobs:
folder: "{{ .ProjectName }}"

dockers:
- goos: linux
- id: amd64image
goos: linux
Copy link
Member

Choose a reason for hiding this comment

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

we should probably add a note in the readme that we're only building amd64 docker images currently

.travis.yml Show resolved Hide resolved
sopstoolinstall.sh Show resolved Hide resolved
elementalvoid
elementalvoid previously approved these changes Apr 5, 2022
Copy link
Contributor

@elementalvoid elementalvoid left a comment

Choose a reason for hiding this comment

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

Looks good from my side. I'll defer to y'all to decide when the packaging is all correct.

README.md Outdated Show resolved Hide resolved
physik932 and others added 2 commits April 6, 2022 14:37
Co-authored-by: Justin Hart <onyxraven@users.noreply.github.com>
@physik932
Copy link
Contributor Author

Copy link
Member

@onyxraven onyxraven left a comment

Choose a reason for hiding this comment

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

lets ship it

@physik932 physik932 merged commit 7c1722a into master Apr 7, 2022
@physik932 physik932 deleted the rishi/add-arm-support branch April 7, 2022 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request request for review Awaiting review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants