Bump golang to 1.11.0 by kolyshkin · Pull Request #37358 · moby/moby

kolyshkin · 2018-06-27T22:30:06Z

It's that time of year again! Go 1.11 is released, let's use it.

This PR also:

removes our archive/tar fork (since upstream archive/tar is fixed for static builds, and osusergo build tag is set in PR Add osusergo build tar for static binaries #37500);
removes ENV GO_VERSION from Dockerfile (not needed since PR hack/make.ps1: don't rely on GO_VERSION #37592);
reformats sources with gofmt -s from go 1.11 to avoid validation errors;
Revendor ugorji/go to include a backported fix (kolyshkin/ugorji-go@1cf431c13dec46596)

image credit: https://www.maxpixel.net/Animal-Mammal-Nora-Nature-Gopher-Meadow-Spring-2196633

cpuguy83 · 2018-06-27T23:10:14Z

golang:1.11beta1 not found

kolyshkin · 2018-06-27T23:15:56Z

It worked on my laptop before I pushed it. Perhaps CI uses some old(er) private copy of dockerhub that needs to be updated?

kolyshkin · 2018-07-02T20:26:38Z

PPC CI, looks like I chose a bad moment to re-push:

13:09:05 Could not resolve 'cdn-fastly.deb.debian.org'

Windows, apparently

13:11:05 # github.com/docker/docker/vendor/github.com/google/certificate-transparency/go/x509
13:11:05 ....\vendor\github.com\google\certificate-transparency\go\x509\root_windows.go:112:3: cannot use uintptr(unsafe.Pointer(sslPara)) (type uintptr) as type syscall.Pointer in field value

is caused by golang commit golang/go@4869ec0

thaJeztah · 2018-07-04T00:36:05Z

/cc @cyli for the certificate-transparency issue (FYI)

kolyshkin · 2018-07-04T05:21:25Z

certificate-transparency requires a one-line patch but it will break 1.10 compatibility. I think a backward-compatible fix can be made, too, although it'd be more than one liner.

thaJeztah · 2018-07-04T05:37:45Z

I think it's important we report it (both in the Go issue tracker, and in certificate-transparency); don't think there's been a single version of go that didn't break something

kolyshkin · 2018-07-04T11:04:00Z

I think it's important we report it

windows/go-1.11 compile error google/certificate-transparency-go#284
Fix golang 1.11 compatibility on Windows google/certificate-transparency-go#285 (looks somewhat ugly to me)

Not quite sure yet if I want to report this one to golang/go (as the issue they solved is quite complicated) but I left a comment in golang/go#21376 (comment)

thaJeztah · 2018-07-04T11:44:54Z

Thanks @kolyshkin those look good; at least we reported upstream (so that they're aware of the issue), and we have a possible way forward 👍

thaJeztah · 2018-07-04T12:52:44Z

Opened #37390 to bump certificate-transparency to match what's currently in SwarmKit (so that a later bump will be smaller)

kolyshkin · 2018-07-04T17:42:56Z

Not sure where this is coming from

13:27:55 --- FAIL: TestSessionCreate (0.01s)
13:27:55 session_test.go:23: assertion failed: error is not nil: Post http://%2Fgo%2Fsrc%2Fgithub.com%2Fdocker%2Fdocker%2Fbundles%2Ftest-integration%2Fdocker.sock/session: net/http: HTTP/1.x transport connection broken: malformed HTTP status code "*"

thaJeztah · 2018-07-04T19:30:42Z

ping @tonistiigi could you have a look at that session failure? IIRC, that endpoint does an upgrade?

kolyshkin · 2018-07-04T19:34:24Z

janky fail:

11:29:17 WARNING: failed to resolve symlinks in copy.go: lstat /go/src/github.com/docker/docker/copy.go: no such file or directory
11:29:17 WARNING: failed to make copy.go a relative path: Rel: can't make copy.go relative to /go/src/github.com/docker/docker
11:29:17 WARNING: failed to resolve symlinks in copy.go: lstat /go/src/github.com/docker/docker/copy.go: no such file or directory
11:29:17 WARNING: failed to make copy.go a relative path: Rel: can't make copy.go relative to /go/src/github.com/docker/docker
11:29:17 WARNING: failed to resolve symlinks in copy.go: lstat /go/src/github.com/docker/docker/copy.go: no such file or directory
11:29:17 WARNING: failed to make copy.go a relative path: Rel: can't make copy.go relative to /go/src/github.com/docker/docker
11:29:17 WARNING: failed to resolve symlinks in copy.go: lstat /go/src/github.com/docker/docker/copy.go: no such file or directory
11:29:17 WARNING: failed to make copy.go a relative path: Rel: can't make copy.go relative to /go/src/github.com/docker/docker
11:29:17 copy.go:241::warning: unnecessary conversion (unconvert)
11:29:17 copy.go:240::warning: unnecessary conversion (unconvert)
11:29:17 copy.go:240::warning: unnecessary conversion (unconvert)
11:29:17 copy.go:241::warning: unnecessary conversion (unconvert)
11:29:17 Build step 'Execute shell' marked build as failure

This is something going wrong with gometalinter. The file is daemon/graphdriver/copy/copy.go but somehow gometalinter (or maybe one of linters) fails to figure out its path, and probably because of this it fails to ignore the // nolint: unconvert directive added by commit 21b2c27.

@dnephin @alecthomas ^^^ can you PTAL?

thaJeztah · 2018-07-04T19:53:06Z

Should we try updating gometalinter? Looks like the version hasn't been updated in a while;

moby/hack/dockerfile/install/gometalinter.installer

Line 3 in 49ad3d6

GOMETALINTER_COMMIT=bfcc1d6942136fd86eb6f1a6fb328de8398fbd80

alecthomas/gometalinter@bfcc1d6...master (version v2.0.6 was tagged a few days ago https://github.com/alecthomas/gometalinter/releases/tag/v2.0.6)

kolyshkin · 2018-07-04T20:25:16Z

Should we try updating gometalinter?

Indeed; let's see if it helps

dnephin · 2018-07-04T20:59:57Z

Doesn't look like it helped. I'll try to look into it further soon. I took a quick look and my best guess so far is that something started returning basename instead of a full path to a file. I'm not sure why that is. Maybe a regression in the go beta release?

To get the rest of Ci running maybe add an || true to the validate step for now?

thaJeztah · 2018-07-04T23:57:22Z

@dnephin looks like there's more breaking changes in Go 1.11; I opened alecthomas/gometalinter#499 to test gometalinter on 1.11, and golang/go#26222 upstream (output being printed twice)

quite some changes in this file since go 1.10; https://github.com/golang/go/commits/4ba55273713bebfbfe0bed9ce196e845c0c69567/src/cmd/vet/print.go diff

UPDATE:

my issue was marked as a duplicate of golang/go#26125 (which is labeled "release blocker", so should be fixed before Go 1.11 final)

thaJeztah · 2018-07-05T10:46:03Z

api/types/strslice/strslice_test.go

As a workaround, we can add a blank line between the short and long keys to make it work for both 1.10 and 1.11 (see containerd/containerd#2436) but it's really a workaround

I opened a ticket upstream for the gofmt change; golang/go#26228

Ok, this was marked as a "won't fix", "documentation only" change golang/go#26228 (comment), so if we want to support multiple versions of Go, we need to make some changes to CI and/or the contributing guide;

This change was done on purpose in golang/go@542ea5a.
As to why gofmt changes every release - see @griesemer's comment in golang/go#25161 (comment).
However, I agree that this change should be documented in the release notes. I can't see it in https://tip.golang.org/doc/go1.11.

Yes, this looks better (in most cases), thanks for suggestion!

Discussing possible enhancements on the ticket upstream to ease migrating between Go versions.

In most cases, if I remember correctly, once rebasing to a newer Go version we are losing compatibility with older ones either directly (i.e. some code in moby/moby starts using some feature from the new Go version) or indirectly (i.e. some code in vendored or otherwise used packages relies on a feature from the new Go version). I am about 84% sure current moby can't be compiled with Go 1.9, for example.

Therefore, chances are slim we'll be supporting Go 1.10 when Go 1.11 will become the current stable version of Go. Having said that, I am all for maintaining compatibility with older versions unless it requires some tremendous efforts.

kolyshkin · 2018-07-05T20:51:36Z

Finally something new for a change! On janky, powerpc, and z:

12:05:57 --- FAIL: TestMountInit (0.00s)
12:05:57 mount_test.go:40: operation not supported
12:05:57 --- FAIL: TestMountChanges (0.00s)
12:05:57 mount_test.go:152: operation not supported
12:05:57 FAIL

On Windows:

12:09:14 --- FAIL: TestTarSums (0.05s)
12:09:14 tarsum_test.go:393: expecting [tarsum+sha256:8bf12d7e67c51ee2e8306cba569398b1b9f419969521a12ffb9d8875e8836738], but got [tarsum+sha256:75258b2c5dcd9adfe24ce71eeca5fc5019c7e669912f15703ede92b1a60cb11f]
12:09:14 tarsum_test.go:393: expecting [tarsum+md5:0d7529ec7a8360155b48134b8e599f53], but got [tarsum+md5:3a6cdb475d90459ac0d3280703d17be2]
12:09:14 tarsum_test.go:393: expecting [tarsum+sha1:f1fee39c5925807ff75ef1925e7a23be444ba4df], but got [tarsum+sha1:14b5e0d12a0c50a4281e86e92153fa06d55d00c6]
12:09:14 tarsum_test.go:393: expecting [tarsum+sha224:6319390c0b061d639085d8748b14cd55f697cf9313805218b21cf61c], but got [tarsum+sha224:dd8925b7a4c71b13f3a68a0f9428a757c76b93752c398f272a9062d5]
12:09:14 tarsum_test.go:393: expecting [tarsum+sha384:a578ce3ce29a2ae03b8ed7c26f47d0f75b4fc849557c62454be4b5ffd66ba021e713b48ce71e947b43aab57afd5a7636], but got [tarsum+sha384:e39e82f40005134bed13fb632d1a5f2aa4675c9ddb4a136fbcec202797e68d2f635e1200dee2e3a8d7f69d54d3f2fd27]
12:09:14 tarsum_test.go:393: expecting [tarsum+sha512:e9bfb90ca5a4dfc93c46ee061a5cf9837de6d2fdf82544d6460d3147290aecfabf7b5e415b9b6e72db9b8941f149d5d69fb17a394cbfaf2eac523bd9eae21855], but got [tarsum+sha512:7c56de40b2d1ed3863ff25d83b59cdc8f53e67d1c01c3ee8f201f8e4dec3107da976d0c0ec9109c962a152b32699fe329b2dab13966020e400c32878a0761a7e]
12:09:14 FAIL

kolyshkin · 2018-09-06T00:23:44Z

@thaJeztah your review comments are addressed; PTAL

lowenna · 2018-09-06T00:31:35Z

Running Windows CI locally also revealed a bug: #37770

thaJeztah

LGTM

thaJeztah · 2018-09-06T18:20:09Z

Discussing a bit more in the maintainers meeting; and ideally we'd still have the .0 in the FROM line, but that requires some changes in the make.ps1; @kolyshkin will have a look into changing that.

lowenna · 2018-09-06T18:20:39Z

LGTM (apologies, couldn't make maintainers meeting - on another call)

This should eliminate a bunch of new (go-1.11 related) validation errors telling that the code is not formatted with `gofmt -s`. No functional change, just whitespace (i.e. `git show --ignore-space-change` shows nothing). Patch generated with: > git ls-files | grep -v ^vendor/ | grep .go$ | xargs gofmt -s -w Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

To include the following backported fix: kolyshkin/ugorji-go@1cf431c13dec46596 which should fix this: > 13:40:53 vendor/github.com/ugorji/go/codec/gen-helper.generated.go:1: > possible malformed +build comment%!(EXTRA []interface {}=[]) Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

This is to include the Go 1.11 fix (containerd/continuity#120). Again (see c64a244). Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

Instead of installing golang from sources, it's easier to use golang image which is based on Debian Stretch. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

We would like to use a version with .0 suffix (like 1.11.0) in Dockerfile, so that once a .1 version is out (like 1.11.1) we won't accidentally switch to it. Unfortunately it's not possible to use .0 suffix currently as it breaks the check in make.ps1. This patch fixes that. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

It's that time of year again! Go 1.11 is released, time to use it. This commit also * removes our archive/tar fork, since upstream archive/tar is fixed for static builds, and osusergo build tag is set. * removes ENV GO_VERSION from Dockerfile as it's not needed anymore since PR moby#37592 is merged. [v2: switch to beta2] [v3: switch to beta3] [v4: rc1] [v5: remove ENV GO_VERSION as PR moby#37592 is now merged] [v6: rc2] [v7: final!] [v8: use 1.11.0] [v9: back to 1.11] [v8: use 1.11.0] Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

We can do that now as we're no longer carrying archive/tar. Note that latest vndr removes vendor/ subdir so we don't have to, thus the change in hack/validate/vendor. While at it, re-run a new vndr version to make sure everything that should be there is. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

kolyshkin · 2018-09-06T22:26:30Z

To allow for FROM golang:1.11.0, we need to remove .0 suffix from the version number.

To that effect:

Once the CI PR Is merged, windows CI needs to be restarted, and once green, we're good to go!

lowenna · 2018-09-07T00:44:25Z

CI Script updated merged in my repo
Jenkins Windows RS1 job updated with revised script
rebuild/WindowsRS1
I also restarted experimental failing for an unrelated reason

thaJeztah

LGTM, and all green now!

thaJeztah · 2018-09-07T08:34:12Z

vendor.conf

 github.com/coreos/etcd v3.2.1
 github.com/coreos/go-semver v0.2.0
-github.com/ugorji/go f1f1a805ed361a0e078bb537e4ea78cd37dcf065
+# fix for go vet (https://github.com/kolyshkin/ugorji-go/commit/1cf431c13dec46596)


we should create a tracking issue for this (ie to get rid of the fork)

Sure, #37804

kolyshkin requested a review from tianon as a code owner June 27, 2018 22:30

GordonTheTurtle added the status/0-triage label Jun 27, 2018

kolyshkin force-pushed the go111 branch from 9945f74 to 8462c23 Compare July 2, 2018 20:07

kolyshkin mentioned this pull request Jul 4, 2018

windows/go-1.11 compile error google/certificate-transparency-go#284

Closed

kolyshkin force-pushed the go111 branch from 93bceec to 8cbf4f7 Compare July 4, 2018 18:19

kolyshkin requested a review from vdemeester as a code owner July 4, 2018 18:19

kolyshkin force-pushed the go111 branch from a2db0b2 to 9517c51 Compare July 4, 2018 22:28

This was referenced Jul 5, 2018

Bump to Go 1.11.0 containerd/containerd#2435

Merged

doc: document changes in behavior of gofmt in 1.11 golang/go#26228

Closed

thaJeztah reviewed Jul 5, 2018

View reviewed changes

kolyshkin force-pushed the go111 branch 2 times, most recently from 7f2084c to 298e445 Compare July 5, 2018 18:13

kolyshkin force-pushed the go111 branch from e0fd81b to ac2965a Compare July 5, 2018 21:02

thaJeztah approved these changes Sep 6, 2018

View reviewed changes

lowenna approved these changes Sep 6, 2018

View reviewed changes

kolyshkin force-pushed the go111 branch from 1b2b3ba to ae688d0 Compare September 6, 2018 18:43

kolyshkin added 7 commits September 6, 2018 15:24

vendor: bump containerd/continuity

bac6720

This is to include the Go 1.11 fix (containerd/continuity#120). Again (see c64a244). Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

Dockerfile.simple: simplify by using golang img

32a2331

Instead of installing golang from sources, it's easier to use golang image which is based on Debian Stretch. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

kolyshkin force-pushed the go111 branch from ae688d0 to ce858fe Compare September 6, 2018 22:25

lowenna added the rebuild/windowsRS1 label Sep 7, 2018

GordonTheTurtle removed the rebuild/windowsRS1 label Sep 7, 2018

lowenna added rebuild/windowsRS1 labels Sep 7, 2018

GordonTheTurtle removed rebuild/experimental labels Sep 7, 2018

thaJeztah approved these changes Sep 7, 2018

View reviewed changes

thaJeztah merged commit ec99bd6 into moby:master Sep 7, 2018

bhipple mentioned this pull request Sep 9, 2018

dockerTools: unpin go version NixOS/nixpkgs#46428

Merged

9 tasks

AkihiroSuda mentioned this pull request Sep 14, 2018

update Go to 1.11 moby/buildkit#626

Merged

adrien-f mentioned this pull request Jan 19, 2019

chore: added formatting check to circle-ci thanos-io/thanos#733

Closed

thaJeztah mentioned this pull request Mar 2, 2019

[18.09 backport] Bump Golang to 1.11.11 docker-archive/engine#168

Merged

8 tasks

kolyshkin mentioned this pull request Aug 20, 2019

get rid of ugorji/go fork #37804

Closed

Conversation

kolyshkin commented Jun 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cpuguy83 commented Jun 27, 2018

Uh oh!

kolyshkin commented Jun 27, 2018

Uh oh!

kolyshkin commented Jul 2, 2018

Uh oh!

thaJeztah commented Jul 4, 2018

Uh oh!

kolyshkin commented Jul 4, 2018

Uh oh!

thaJeztah commented Jul 4, 2018

Uh oh!

kolyshkin commented Jul 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Jul 4, 2018

Uh oh!

thaJeztah commented Jul 4, 2018

Uh oh!

kolyshkin commented Jul 4, 2018

Uh oh!

thaJeztah commented Jul 4, 2018

Uh oh!

kolyshkin commented Jul 4, 2018

Uh oh!

thaJeztah commented Jul 4, 2018

Uh oh!

kolyshkin commented Jul 4, 2018

Uh oh!

dnephin commented Jul 4, 2018

Uh oh!

thaJeztah commented Jul 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah Jul 5, 2018

Choose a reason for hiding this comment

Uh oh!

thaJeztah Jul 5, 2018

Choose a reason for hiding this comment

Uh oh!

kolyshkin Jul 5, 2018

Choose a reason for hiding this comment

Uh oh!

thaJeztah Jul 5, 2018

Choose a reason for hiding this comment

Uh oh!

kolyshkin Jul 6, 2018

Choose a reason for hiding this comment

Uh oh!

kolyshkin commented Jul 5, 2018

Uh oh!

kolyshkin commented Sep 6, 2018

Uh oh!

lowenna commented Sep 6, 2018

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Sep 6, 2018

Uh oh!

lowenna commented Sep 6, 2018

Uh oh!

kolyshkin commented Sep 6, 2018

Uh oh!

lowenna commented Sep 7, 2018

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah Sep 7, 2018

Choose a reason for hiding this comment

Uh oh!

kolyshkin Sep 8, 2018

Choose a reason for hiding this comment

Uh oh!

Reviewers

kolyshkin commented Jun 27, 2018 •

edited

Loading

kolyshkin commented Jul 4, 2018 •

edited

Loading

thaJeztah commented Jul 4, 2018 •

edited

Loading