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

fix: allow building with go1.21 or higher via GOTOOLCHAIN=auto #927

Closed

Conversation

jakobmoellerdev
Copy link
Contributor

Since Go 1.21 one can supply a toolchain version string for the minimum required version of Go in their go.mod file (e.g. go 1.21.4). From practical point of view it makes no difference whether you declare your requirement as go 1.21 or go 1.21.0 in the go.mod file because in both cases any Go 1.21+ family toolchain will be able to build your project successfully. However, the former makes it impossible for 1.21 toolchains to automatically download a toolchain from e.g. 1.22 language family (via the means of the GOTOOLCHAIN variable), because "1.22" as a string literal doesn't denote a toolchain (while 1.22.0 does).

To reproduce a build environment like this, one can use the following adjustment to the existing Dockerfile:

# Build topolvm
FROM --platform=$BUILDPLATFORM golang:1.21-bullseye AS build-topolvm

ENV GOTOOLCHAIN=auto

# Get argument
ARG TOPOLVM_VERSION
ARG TARGETARCH

COPY . /workdir
WORKDIR /workdir

RUN touch pkg/lvmd/proto/*.go
RUN make build-topolvm TOPOLVM_VERSION=${TOPOLVM_VERSION} GOARCH=${TARGETARCH}

Examples in other projects:
grafana
syft
kubernetes

Signed-off-by: Jakob Möller <jmoller@redhat.com>
@jakobmoellerdev jakobmoellerdev requested a review from a team as a code owner June 25, 2024 09:58
@jakobmoellerdev jakobmoellerdev changed the title fix: allow building with go1.21 or lower fix: allow building with go1.21 or higher via GOTOOLCHAIN=auto Jun 25, 2024
@toshipp
Copy link
Contributor

toshipp commented Jun 26, 2024

I want to clarify the motivation.

First, as far as I know, go 1.21.11 can accept the language version in the go directive. (ref. golang/go#67235), So, we can build topolvm by the latest golang:1.21-bullseye.
I tested your reproducer, but could not get the compile error. I also tested with golang:1.21.10-bullseye, it failed, but with golang:1.21.11-bullseye it succeeded. I doubt your local image refers to some old 1.21 series.

Second, why do you want to use golang 1.21? Since we intended to use 1.22, we specified the version in Dockerfile and go.mod.

@jakobmoellerdev
Copy link
Contributor Author

jakobmoellerdev commented Jun 26, 2024

To be perfectly transparent, we have build systems in our org that have a decoupled lifecycle from our topolvm compilation and its impossibly hard to get them to upgrade to latest. Thats why I want to add this version despite this as our build systems will not be able to build topolvm for k8s 1.30 in the next few weeks otherwise. I am aware this is an edge case, but considering that topolvm is the only project I know that doesnt specify the z-stream in go1.22 in my ecosystem I thought pulling this in should be fine. If this is absolutely non acceptable to you, I will have to move the same in our midstream

@toshipp
Copy link
Contributor

toshipp commented Jun 26, 2024

I understand your situation. We have just started to support k8s 1.30 by #928. In the PR, the go directive will be written as 1.22.0. If you can wait for the PR, I'd like to leave this to it.

@jakobmoellerdev
Copy link
Contributor Author

LGTM, thanks for your feedback. Ill wait for 1.30 rebase

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants