gocsaf / csaf

Tools to download or provide CSAF (Common Security Advisory Framework) documents.
https://csaf.io
41 stars 24 forks source link

Lower required Go version to 1.20 #509

Closed s-l-teichmann closed 1 year ago

s-l-teichmann commented 1 year ago

This makes it easier to be used in other projects as a library.

tschmidtb51 commented 1 year ago

We also need to change the workflows to make sure that we are not accidentally using 1.21 functions...

s-l-teichmann commented 1 year ago

We also need to change the workflows to make sure that we are not accidentally using 1.21 functions...

@tschmidtb51 No, we should not do this. We should always use the latest stable to build our stuff. The go.mod file ensures that we don't use language features of a higher version. The binaries we deliver should benefit from the newer compilers.

tschmidtb51 commented 1 year ago

@tschmidtb51 No, we should not do this. We should always use the latest stable to build our stuff. The go.mod file ensures that we don't use language features of a higher version. The binaries we deliver should benefit from the newer compilers.

@s-l-teichmann Thanks for the clarification. Processwise: As I did the changes via the web, it might be easiest if you create a new PR with just the commits it originally had...

juan131 commented 1 year ago

@tschmidtb51 I think slog was already in Go 1.20 as experimental, see https://pkg.go.dev/golang.org/x/exp/slog but I'll give it a try and let you know

juan131 commented 1 year ago

I could reproduce the issue with this simple Dockerfile @tschmidtb51:

FROM bitnami/golang:1.20.11 as builder
COPY . /csaf_distribution
WORKDIR /csaf_distribution
RUN mkdir /output && \
    go build -o /output -ldflags "-X github.com/csaf-poc/csaf_distribution/v3/util.SemVersion=v3.0.0-rc.1-5-gf572a3b" -v ./cmd/...

FROM bitnami/bitnami-shell
COPY --from=builder /output /app
WORKDIR /app
ENTRYPOINT ["/app/csaf_checker"]
$ docker build . -t csaf-distro-image
(...)
11.09 internal/options/log.go:12:2: package log/slog is not in GOROOT (/opt/bitnami/go/src/log/slog)

I guess this overcomplicates maintaining compatibility with 1.20 significantly. Should we at least create a go-1.20 branch as suggested by @mpermar and replace slog there?

s-l-teichmann commented 1 year ago

Did someone test that successfully?

I just tried to run in on a machine with Go 1.20.4 and got the following result:

make build_linux 
mkdir -p bin-linux-amd64/ 
env GOARCH=amd64 GOOS=linux go build -o bin-linux-amd64/  -ldflags "-X github.com/csaf-poc/csaf_distribution/v3/util.SemVersion=v3.0.0-rc.1-5-gf572a3b" -v ./cmd/...
internal/options/log.go:12:2: package log/slog is not in GOROOT (/usr/local/go/src/log/slog)
make: *** [Makefile:78: build_linux] Error 1

If I remember correctly, the slog was also a reason why we choose Go 1.21... Or is that an error on my side?

@tschmidt Yes, you are right. I totally forgot this. My bad.

tschmidtb51 commented 1 year ago

We also need to change the workflows to make sure that we are not accidentally using 1.21 functions...

@tschmidtb51 No, we should not do this. We should always use the latest stable to build our stuff. The go.mod file ensures that we don't use language features of a higher version. The binaries we deliver should benefit from the newer compilers.

@s-l-teichmann: You mentioned that we shouldn't build with go 1.20 but those file changes (in the Github Actions) seem still to be part of the PR. Was that intentional?

juan131 commented 1 year ago

any update on this? Thanks in advance

tschmidtb51 commented 1 year ago

We have discussed the path forward:

s-l-teichmann commented 1 year ago

Replaced by PR #514