Skip to content
This repository has been archived by the owner on Jun 28, 2023. It is now read-only.

add darwin arm64 build target, improve m1 devx #1480

Conversation

joefitzgerald
Copy link
Contributor

@joefitzgerald joefitzgerald commented Aug 28, 2021

Signed-off-by: Joe Fitzgerald jfitzgerald@vmware.com

What this PR does / why we need it

This PR causes darwin / arm64 binaries to be generated by make release. It also makes improvements to scripts that support building a release locally on an arm64 Mac.

Details for the Release Notes

generate arm64 binaries for darwin

Which issue(s) this PR fixes

N/A

Describe testing done for PR

  • ran make release locally and validated it succeeds

Special notes for your reviewer

  • This PR will be accompanied with a cayman_vmware-tanzu_tce MR to ensure the new binaries are notarized correctly.

@joefitzgerald joefitzgerald requested review from a team as code owners August 28, 2021 22:07
@github-actions github-actions bot added the owner/release-eng Work executed by VMware release engineering team label Aug 28, 2021
Copy link
Contributor

@davidvonthenen davidvonthenen left a comment

Choose a reason for hiding this comment

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

I left a comment... and I'm going to private message you about the signing process.

Copy link
Contributor

@davidvonthenen davidvonthenen left a comment

Choose a reason for hiding this comment

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

This looks good to me!

@davidvonthenen
Copy link
Contributor

The build failure is being fixed right now... we just need to add the platform ENV to tanzu cli as an environment variable that it should pass. we talked about it and it should be happening any second now.

this looks great!!!!!! 🎆

@joefitzgerald joefitzgerald force-pushed the jf-add-darwin-arm64-build-target branch 2 times, most recently from 63095bb to dd2f06d Compare August 31, 2021 17:28
@github-actions github-actions bot added the owner/core-eng Work executed by TCE's core engineering team label Aug 31, 2021
@joefitzgerald joefitzgerald force-pushed the jf-add-darwin-arm64-build-target branch from dd2f06d to 9c75020 Compare August 31, 2021 17:32
@joefitzgerald joefitzgerald force-pushed the jf-add-darwin-arm64-build-target branch from 9c75020 to 4119d7c Compare August 31, 2021 17:57
@github-actions github-actions bot removed the owner/core-eng Work executed by TCE's core engineering team label Aug 31, 2021
@nrb
Copy link
Contributor

nrb commented Aug 31, 2021

#1158 will also touch on this.

@joefitzgerald joefitzgerald force-pushed the jf-add-darwin-arm64-build-target branch from 4119d7c to 2b05a5c Compare September 1, 2021 23:17
@joefitzgerald
Copy link
Contributor Author

Rebased on main, updated the PR to take advantage of changes in #1158.

@davidvonthenen
Copy link
Contributor

davidvonthenen commented Sep 1, 2021

We need to hold merging this PR after it was switched from curl to go install. A number of the pre-baked runner and jenkins OVAs don't have a go env. Will need to add one. I need to see about reprioritizing updating those images to support this.

Copy link
Contributor

@davidvonthenen davidvonthenen left a comment

Choose a reason for hiding this comment

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

Please see comment above. This PR is good but I have work on the build infra side to support this.

EDIT: Going to try to do this week.

@joefitzgerald joefitzgerald force-pushed the jf-add-darwin-arm64-build-target branch from 2b05a5c to 6c80f3a Compare September 2, 2021 03:01
@joefitzgerald
Copy link
Contributor Author

joefitzgerald commented Sep 2, 2021

@dvonthenen I updated the PR to also copy the go install-ed binaries to /usr/local/bin. This should obviate the need for adding $GOPATH/bin to the path on the runners.

The above assumes that go itself is installed, given that these machines build go code. No explicit go environment needs to be set up; there is a default $GOPATH when none is set in the environment, and $GOPATH/bin is automatically created by the go tool if it does not exist when running go install.

Also please note that the use of go install is limited to Darwin. Linux is unmodified and uses curl.

@joefitzgerald joefitzgerald force-pushed the jf-add-darwin-arm64-build-target branch 2 times, most recently from 6a50f08 to b4e1395 Compare September 2, 2021 03:43
Copy link
Contributor

@nrb nrb left a comment

Choose a reason for hiding this comment

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

@joefitzgerald Are you able to build the tanzu binary with this?

I ask because the tanzu-framework Makefile defines a small subset of environments, and it's checked such that it shouldn't compile.

I did a search for PRs on tanzu-framework but couldn't find any.

@stmcginnis
Copy link
Contributor

Tanzu framework will warn on building it since it's not in the "standard" set of target platforms yet, but it will actually build. I tested it a few months ago and it builds and the CLI runs fine. The issues were actually trying to deploy a management cluster where things failed completely.

Signed-off-by: Joe Fitzgerald <jfitzgerald@vmware.com>

Update .github/workflows/release-nonga.yaml

Co-authored-by: Navid Shaikh <shaikhnavid14@gmail.com>

- explicitly set envs for tanzu framework build

Signed-off-by: Joe Fitzgerald <jfitzgerald@vmware.com>
@joefitzgerald joefitzgerald force-pushed the jf-add-darwin-arm64-build-target branch from b4e1395 to be82290 Compare September 2, 2021 18:19
Copy link
Contributor

@davidvonthenen davidvonthenen left a comment

Choose a reason for hiding this comment

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

LGTM. thanks for this awesome contribution!

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
cla-not-required owner/release-eng Work executed by VMware release engineering team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants