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

arm64 docker builds #348

Merged
merged 3 commits into from
Jun 6, 2022

Conversation

kevcube
Copy link
Contributor

@kevcube kevcube commented May 31, 2022

Supersedes #343

  • arm64 docker builds for chamber

@kevcube kevcube requested a review from a team as a code owner May 31, 2022 17:50
@kevcube
Copy link
Contributor Author

kevcube commented May 31, 2022

includes @knorby's work

@@ -3,11 +3,12 @@ FROM golang:1.13-alpine AS build
WORKDIR /go/src/github.com/segmentio/chamber
COPY . .

ARG TARGETARCH
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a default arg of buildkit-based builds

@kevcube
Copy link
Contributor Author

kevcube commented May 31, 2022

@emmy-byrne-segment this should address this comment

@@ -8,6 +8,10 @@ ifndef VERSION
VERSION := $(shell git describe --tags --always --dirty="-dev")
endif

ifndef TARGETARCH
TARGETARCH := $(shell arch)
Copy link
Contributor

Choose a reason for hiding this comment

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

An issue we might have here is that OS X reports arm64 but linux reports aarch64 when running this command on the same machine and arch.

since *nix varieties cannot agree here i think we need to make sure this is going to output exactly what we expect it to.

@kevcube
Copy link
Contributor Author

kevcube commented Jun 2, 2022

@emmy-byrne-segment I just made an extra name for the linux target so that it'll catch both arm64/aarch64; is this an acceptable solution?

@knorby
Copy link
Contributor

knorby commented Jun 6, 2022

Thanks for picking this up, @kevcube!

@emmy-byrne-segment
Copy link
Contributor

@kevcube that should work unless i am mistaken, thank you!

Just checking on this dockerhub failure before we merge this

@emmy-byrne-segment
Copy link
Contributor

Turns out thats working as expected because i forgot this was a fork and that test wont work on branches. Thank you again for the PR! Merging.

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.

3 participants