Skip to content

[release/2.8] bump up golang version #3812

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[release/2.8] bump up golang version #3812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,21 @@ jobs:
- name: Set up Go
uses: actions/setup-go@v2
with:
go-version: 1.16.*
go-version: 1.18.8

- name: Dependencies
run: |
sudo apt-get -q update
sudo -E apt-get -yq --no-install-suggests --no-install-recommends install python2-minimal
cd /tmp && go get -u github.com/vbatts/git-validation
cd /tmp && go install github.com/vbatts/git-validation@latest

- name: Build
working-directory: ./src/github.com/docker/distribution
run: |
DCO_VERBOSITY=-q script/validate/dco
GO111MODULE=on script/setup/install-dev-tools
script/validate/vendor
go build -i .
go build .
make check
make build
make binaries
Expand Down
78 changes: 44 additions & 34 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,49 +1,59 @@
# syntax=docker/dockerfile:1.3
# syntax=docker/dockerfile:1

ARG GO_VERSION=1.16.15
ARG GORELEASER_XX_VERSION=1.2.5
ARG GO_VERSION=1.18.8
ARG ALPINE_VERSION=3.16
ARG XX_VERSION=1.1.1

FROM --platform=$BUILDPLATFORM crazymax/goreleaser-xx:${GORELEASER_XX_VERSION} AS goreleaser-xx
FROM --platform=$BUILDPLATFORM golang:${GO_VERSION}-alpine AS base
COPY --from=goreleaser-xx / /
RUN apk add --no-cache file git
FROM --platform=$BUILDPLATFORM tonistiigi/xx:${XX_VERSION} AS xx
FROM --platform=$BUILDPLATFORM golang:${GO_VERSION}-alpine${ALPINE_VERSION} AS base
COPY --from=xx / /
RUN apk add --no-cache bash coreutils file git
ENV GO111MODULE=auto
ENV CGO_ENABLED=0
WORKDIR /go/src/github.com/docker/distribution

FROM base AS version
ARG PKG="github.com/distribution/distribution"
RUN --mount=target=. \
VERSION=$(git describe --match 'v[0-9]*' --dirty='.m' --always --tags) REVISION=$(git rev-parse HEAD)$(if ! git diff --no-ext-diff --quiet --exit-code; then echo .m; fi); \
echo "-X ${PKG}/version.Version=${VERSION#v} -X ${PKG}/version.Revision=${REVISION} -X ${PKG}/version.Package=${PKG}" | tee /tmp/.ldflags; \
echo -n "${VERSION}" | tee /tmp/.version;

FROM base AS build
ENV GO111MODULE=auto
ENV CGO_ENABLED=0
# GIT_REF is used by goreleaser-xx to handle the proper git ref when available.
# It will fallback to the working tree info if empty and use "git tag --points-at"
# or "git describe" to define the version info.
ARG GIT_REF
ARG TARGETPLATFORM
ARG PKG="github.com/distribution/distribution"
ARG LDFLAGS="-s -w"
ARG BUILDTAGS="include_oss include_gcs"
RUN --mount=type=bind,rw \
--mount=type=cache,target=/root/.cache/go-build \
--mount=target=/go/pkg/mod,type=cache \
goreleaser-xx --debug \
--name="registry" \
--dist="/out" \
--main="./cmd/registry" \
--flags="-v" \
--ldflags="-s -w -X '$PKG/version.Version={{.Version}}' -X '$PKG/version.Revision={{.Commit}}' -X '$PKG/version.Package=$PKG'" \
--tags="$BUILDTAGS" \
--files="LICENSE" \
--files="README.md"

FROM scratch AS artifact
COPY --from=build /out/*.tar.gz /
COPY --from=build /out/*.zip /
COPY --from=build /out/*.sha256 /
RUN --mount=type=bind,target=/go/src/github.com/docker/distribution,rw \
--mount=type=cache,target=/root/.cache/go-build \
--mount=target=/go/pkg/mod,type=cache \
--mount=type=bind,source=/tmp/.ldflags,target=/tmp/.ldflags,from=version \
set -x ; xx-go build -trimpath -ldflags "$(cat /tmp/.ldflags) ${LDFLAGS}" -o /usr/bin/registry ./cmd/registry \
&& xx-verify --static /usr/bin/registry

FROM scratch AS binary
COPY --from=build /usr/local/bin/registry* /
COPY --from=build /usr/bin/registry /

FROM base AS releaser
ARG TARGETOS
ARG TARGETARCH
ARG TARGETVARIANT
WORKDIR /work
RUN --mount=from=binary,target=/build \
--mount=type=bind,target=/src \
--mount=type=bind,source=/tmp/.version,target=/tmp/.version,from=version \
VERSION=$(cat /tmp/.version) \
&& mkdir -p /out \
&& cp /build/registry /src/README.md /src/LICENSE . \
&& tar -czvf "/out/registry_${VERSION#v}_${TARGETOS}_${TARGETARCH}${TARGETVARIANT}.tar.tgz" * \
&& sha256sum -z "/out/registry_${VERSION#v}_${TARGETOS}_${TARGETARCH}${TARGETVARIANT}.tar.tgz" | awk '{ print $1 }' > "/out/registry_${VERSION#v}_${TARGETOS}_${TARGETARCH}${TARGETVARIANT}.tar.tgz.sha256"

FROM scratch AS artifact
COPY --from=releaser /out /

FROM alpine:3.16
FROM alpine:${ALPINE_VERSION}
RUN apk add --no-cache ca-certificates
COPY cmd/registry/config-dev.yml /etc/docker/registry/config.yml
COPY --from=build /usr/local/bin/registry /bin/registry
COPY --from=binary /registry /bin/registry
VOLUME ["/var/lib/registry"]
EXPOSE 5000
ENTRYPOINT ["registry"]
Expand Down
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ version/version.go:

check: ## run all linters (TODO: enable "unused", "varcheck", "ineffassign", "unconvert", "staticheck", "goimports", "structcheck")
@echo "$(WHALE) $@"
golangci-lint run
@GO111MODULE=off golangci-lint run

test: ## run tests, except integration test with test.short
@echo "$(WHALE) $@"
Expand Down
6 changes: 1 addition & 5 deletions context/http.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,11 +246,7 @@ func (ctx *muxVarsContext) Value(key interface{}) interface{} {
return ctx.vars
}

if strings.HasPrefix(keyStr, "vars.") {
keyStr = strings.TrimPrefix(keyStr, "vars.")
}

if v, ok := ctx.vars[keyStr]; ok {
if v, ok := ctx.vars[strings.TrimPrefix(keyStr, "vars.")]; ok {
Copy link
Member

Choose a reason for hiding this comment

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

This change is unrelated, and looks to be part of 3b391d3;

I suggest reverting this; if we want the other changes, we should backport it as a whole and having this partial change will complicate that (result in merge conflicts)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

again, it's to fix a goci-lint failure.

return v
}
}
Expand Down
2 changes: 1 addition & 1 deletion project/dev-image/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ RUN wget https://golang.org/dl/go$GOLANG_VERSION.linux-amd64.tar.gz --quiet && \
tar -C /usr/local -xzf go$GOLANG_VERSION.linux-amd64.tar.gz && \
rm go${GOLANG_VERSION}.linux-amd64.tar.gz

RUN go get github.com/axw/gocov/gocov github.com/mattn/goveralls github.com/golang/lint/golint
RUN go install github.com/axw/gocov/gocov@latest github.com/mattn/goveralls@latest github.com/golang/lint/golint@latest
4 changes: 1 addition & 3 deletions registry/client/repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,7 @@ func (r *registry) Repositories(ctx context.Context, entries []string, last stri
return 0, err
}

for cnt := range ctlg.Repositories {
entries[cnt] = ctlg.Repositories[cnt]
}
copy(entries, ctlg.Repositories)
Copy link
Member

Choose a reason for hiding this comment

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

This looks unrelated as well; looks like it was part of fbdfd1a; which is

Not sure if we want to backport that as a whole (it seems rather large), but if we want, should be separate.

Copy link
Collaborator Author

@wy65701436 wy65701436 Dec 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

copy(entries, ctlg.Repositories) is to fix a goci-lint failure, it must be in.

Copy link
Member

Choose a reason for hiding this comment

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

My approach for these is usually to update golangci-lint first, and any changes related to updated linters together with that update (same PR, depending on the number of fixes needed, same commit). This makes it transparent that updates were needed due to stricter linters.

Golang updates (besides the go install / @version changes, and in some cases replacing deprecated functions) should not require changes to the code; if they do, Go broke their Go 1 compatibility contract, which should be reported. Keeping the changes related to the Go update itself together, shows "this is what was needed to update go" (which should be "only update the version")

Copy link
Collaborator Author

@wy65701436 wy65701436 Dec 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

So, in general, you would want us to fix all the prerequisites for the go upgrade, and then have this pr focus only on the go version upgrade.

i understand, but, it's not easy to decouple the fix for golangci-lint from the version upgrade of go, because there is some correlation between the versions of go and golangci-lint. Put all these changes together because they are what is needed for the upgraded version from the point of view of code changes.

numFilled = len(ctlg.Repositories)

link := resp.Header.Get("Link")
Expand Down
1 change: 1 addition & 0 deletions registry/handlers/basicauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build go1.4
// +build go1.4

package handlers
Expand Down
1 change: 1 addition & 0 deletions registry/storage/blobwriter_resumable.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build !noresumabledigest
// +build !noresumabledigest

package storage
Expand Down
3 changes: 3 additions & 0 deletions registry/storage/driver/inmemory/mfs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,9 @@ func (f *file) sectionReader(offset int64) io.Reader {
}

func (f *file) ReadAt(p []byte, offset int64) (n int, err error) {
if offset >= int64(len(f.data)) {
return 0, io.EOF
}
Comment on lines +282 to +284
Copy link
Member

Choose a reason for hiding this comment

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

This looks like it's not directly related to the Go update, or if it's needed, should be in it's own commit (cherry-pick); coming from

Backporting that fix is good, but should not be stuffed in together with a go update (if it's a separate PR, we can also point to it in the release notes)

Copy link
Collaborator Author

@wy65701436 wy65701436 Dec 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

this must be in, otherwise one UT will panic as mentioned in #3614

Copy link
Member

Choose a reason for hiding this comment

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

For sure, the fix is good, but it's not directly related to go1.18? My issue is mainly that stuffing bug fixes into a commit that only mentions "update go version" hides them.

This makes it hard to find back fixes;

Copy link
Collaborator Author

@wy65701436 wy65701436 Dec 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This is not directly related to Go 1.18, but without this change, UT would have failed.

If I first try to pick a fix, like #3815, there are some faults on Go 1.16 that I don't think are worth investigating.

Copy link
Member

Choose a reason for hiding this comment

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

@wy65701436 do you want to rebase now that we've merged #3815

return copy(p, f.data[offset:]), nil
}

Expand Down
6 changes: 3 additions & 3 deletions script/setup/install-dev-tools
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
#!/usr/bin/env bash

GOLANGCI_LINT_VERSION="v1.27.0"
GOLANGCI_LINT_VERSION="v1.50.1"

#
# Install developer tools to $GOBIN (or $GOPATH/bin if unset)
#
set -eu -o pipefail

cd /tmp
go get -u github.com/LK4D4/vndr
go get "github.com/golangci/golangci-lint/cmd/golangci-lint@${GOLANGCI_LINT_VERSION}"
go install github.com/LK4D4/vndr@latest
go install "github.com/golangci/golangci-lint/cmd/golangci-lint@${GOLANGCI_LINT_VERSION}"
2 changes: 1 addition & 1 deletion script/validate/dco
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ set -eu -o pipefail

if ! command -v git-validation; then
>&2 echo "ERROR: git-validation not found. Install with:"
>&2 echo " go get -u github.com/vbatts/git-validation"
>&2 echo " go install github.com/vbatts/git-validation@latest"
exit 1
fi

Expand Down
2 changes: 1 addition & 1 deletion version/version.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ var Package = "$(go list)"
// Version indicates which version of the binary is running. This is set to
// the latest release tag by hand, always suffixed by "+unknown". During
// build, it will be replaced by the actual version. The value here will be
// used if the registry is run after a go get based install.
// used if the registry is run after a go install based install.
Copy link
Member

Choose a reason for hiding this comment

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

Looks like we didn't update this in the main branch; we should do it there as well;
https://github.com/distribution/distribution/blob/92d136e113cf34fcfd5f0c21fadf6eac2f39e803/version/version.sh#LL20

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will raise pr on main later.

var Version = "$(git describe --match 'v[0-9]*' --dirty='.m' --always)+unknown"

// Revision is filled with the VCS (e.g. git) revision being used to build
Expand Down