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

Set GOARM=7 explicitly for arm32v7 images #498

Merged
merged 1 commit into from
Dec 9, 2023

Conversation

tianon
Copy link
Member

@tianon tianon commented Dec 9, 2023

This fixes the disparity between the "armv6" download of Go from upstream and the image being v7 otherwise (and thus the default value of GOARM being 6 instead of 7, which is unexpected for users of the image).

This is set in such a way that environment variables (or go env -w) will override it, but without requiring us to recompile Go (which I'm not convinced gives us much gain -- happy to reconsider if someone comes up with a good way to benchmark the compiler).

Closes #494

@tianon
Copy link
Member Author

tianon commented Dec 9, 2023

Arguably, we should probably add a "verification" block that validates that for a given architecture, all the values in our env block are correct from go env, but I think that's probably best left as a future enhancement.

This fixes the disparity between the "armv6" download of Go from upstream and the image being v7 otherwise (and thus the default value of `GOARM` being `6` instead of `7`, which is unexpected for users of the image).

This is set in such a way that environment variables (or `go env -w`) will override it, but without requiring us to recompile Go (which I'm not convinced gives us much gain -- happy to reconsider if someone comes up with a good way to benchmark the compiler).
@@ -193,6 +193,17 @@ RUN set -eux; \
/usr/local/go/src/cmd/dist/dist \
"$GOCACHE" \
; \
{{ if [ "1.20" ] | index(env.version) then "" elif .arches["arm32v7"].url // "" | contains("armv6") then ( -}}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is set only on 1.21+ because that's the first version that ships with go.env (I'm not sure if it was supported earlier, but it definitely didn't have any default contents before 1.21 and supported versions are short-lived enough that I'm not too worried about this only being fixed in 1.21+).

@tianon tianon merged commit acf6e86 into docker-library:master Dec 9, 2023
26 checks passed
@tianon tianon deleted the GOARM=7 branch December 9, 2023 01:04
docker-library-bot added a commit to docker-library-bot/official-images that referenced this pull request Dec 9, 2023
Changes:

- docker-library/golang@acf6e86: Merge pull request docker-library/golang#498 from infosiftr/GOARM=7
- docker-library/golang@40db26d: Set GOARM=7 explicitly for arm32v7 images
- docker-library/golang@9f8bfe7: Merge pull request docker-library/golang#497 from jnoordsij/add-alpine-3.19
- docker-library/golang@84f47a9: Update templates to add Alpine 3.19 and drop 3.17
- docker-library/golang@5f181d7: Add Alpine 3.19 and drop 3.17 to versions.sh
@tianon tianon mentioned this pull request Feb 6, 2024
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.

Something is not right with GOARM
2 participants