Migrate to go mod #800

tiborvass · 2019-01-26T06:23:54Z

What I did:

$ go get -u github.com/tiborvass/gomod
$ GO111MODULE=on gomod init
$ GO111MODULE=on go mod tidy     # no need for forked gomod anymore
$ GO111MODULE=on go mod vendor

Because of golang/go#25556 I created https://github.com/tiborvass/gomod to convert vendor.conf to go.mod. It's only needed for conversion. Regular go mod can be used once go.mod/go.sum are created.

Related docker/cli#1642 :
Right now, the docker cli dependency is that of Tonis's fork which reduces the dependencies. I submitted a hopefully mergeable version upstream so that we no longer need to depend on a fork.
Until then, the reason we need to update the docker cli dependency is because go mod tidy errors out on logrus's Sirupsen/sirupsen mismatch, due to the docker cli fork not containing the import path update for logrus.
The PR got merged, I removed the fork!

Since go mod uses $GOPATH/pkg/mod to store its cache, I'm mounting it as type=cache in a new
vendor.buildkit.Dockerfile. Legacy vendor.Dockerfile redownloads everything every time like vndr used to.

In order for go build to honor the vendor/ folder when go modules are enabled, I had to add an -mod vendor argument.

I verified that the validate-vendor script works, but please verify as well.

It would be good to try updating some dependencies as a way to test this.

TODO:

Change scripts to run go build with -mod vendor
Update vendor-related scripts
Verify go1.11 vs go1.12 in go.mod

tiborvass · 2019-01-26T06:32:20Z

go.mod

@@ -0,0 +1,83 @@
+module github.com/moby/buildkit
+
+go 1.12


TODO: verify that this doesn't cause issues with go1.11

AkihiroSuda · 2019-01-26T13:55:05Z

Related docker/cli#1642

Could you be more specific?

Are we also going to migrate moby/moby (and containerd? runc?) to go mod soon?

tonistiigi · 2019-01-26T18:32:19Z

@tiborvass Also need to update the vendor validation/update scripts. And CI does not pass.

build flag -mod=vendor only valid when using modules

Signed-off-by: Tibor Vass <tibor@docker.com>

tiborvass · 2019-01-26T23:28:01Z

@AkihiroSuda Updated the description. Ideally I'd like to convert everything to go mod, but I wanted to start with buildkit to see how it goes.

tiborvass · 2019-01-28T21:14:27Z

go.mod

+	github.com/docker/go-events v0.0.0-20170721190031-9461782956ad // indirect
+	github.com/docker/go-units v0.3.1 // indirect
+	github.com/docker/libnetwork v0.0.0-20180913200009-36d3bed0e9f4
+	github.com/godbus/dbus v4.1.0+incompatible // indirect


The +incompatible just means that even though the repo is using semver v2+, it doesn't have a go.mod, so go is precautious and treats it as v0 (aka things can break when it updates).

tiborvass · 2019-01-28T23:17:50Z

The go version in go.mod is simply metadata and is not yet used to inform any decisions. Here is the commit golang/go@16962fa

tonistiigi · 2019-01-28T23:55:41Z

hack/update-vendor

 iidfile=$(mktemp -t docker-iidfile.XXXXXXXXXX)
-docker build --build-arg VNDR_VERSION=1fc68ee0c852556a9ed53cbde16247033f104111 --iidfile $iidfile -f ./hack/dockerfiles/vendor.Dockerfile --force-rm .
+dockerfile=vendor.Dockerfile
+if [ "$buildmode" == "docker-buildkit" ]; then


still support buildkit mode even if it runs docker

tonistiigi · 2019-01-29T00:03:41Z

frontend/dockerfile/cmd/dockerfile-frontend/Dockerfile

 RUN --mount=target=. --mount=type=cache,target=/root/.cache \
  --mount=source=/tmp/.ldflags,target=/tmp/.ldflags,from=version \
-  CGO_ENABLED=0 go build -o /dockerfile-frontend -ldflags "-d $(cat /tmp/.ldflags)" -tags "$BUILDTAGS netgo static_build osusergo" ./frontend/dockerfile/cmd/dockerfile-frontend && \
+  CGO_ENABLED=0 GO111MODULE=on go build -mod vendor -o /dockerfile-frontend -ldflags "-d $(cat /tmp/.ldflags)" -tags "$BUILDTAGS netgo static_build osusergo" ./frontend/dockerfile/cmd/dockerfile-frontend && \


Will this change in go1.12 ? If the dockerfiles don't use go mod anyway atm we don't need -mod vendor. We could just add comments with a reminder that we need to enable it when go mod is enabled.

tonistiigi · 2019-01-29T17:52:22Z

hack/dockerfiles/test.Dockerfile

@@ -1,3 +1,5 @@
+# syntax = docker/dockerfile:1.0-experimental


tonistiigi · 2019-01-29T17:54:12Z

hack/dockerfiles/test.Dockerfile

 FROM buildkit-base AS buildctl
 ENV CGO_ENABLED=0
-RUN go build -ldflags "$(cat .tmp/ldflags) -d" -o /usr/bin/buildctl ./cmd/buildctl
+RUN go build -mod vendor -ldflags "$(cat .tmp/ldflags) -d" -o /usr/bin/buildctl ./cmd/buildctl


I think I prefer GOFLAGS=-mod=vendor although that is easier to break.

tonistiigi · 2019-01-29T17:56:48Z

hack/dockerfiles/vendor.buildkit.Dockerfile

+COPY --from=vendored /out /out
+
+FROM vendored AS validate
+RUN --mount=target=.,rw \


rm -rf vendor

$ touch vendor/toto $ ./hack/validate-vendor

It should fail in that case. If I add rm -rf vendor it will not fail.

removing the file should cause the status --porcelain to fail then.

It doesn't because status --porcelain is run after we "cleaned" vendor.

Fine, you win. It doesn't validate correctly if I commit vendor/toto. I need to add git add -A like in generated-files' dockerfile to cover my less useful usecase.

tonistiigi · 2019-01-29T17:57:56Z

hack/update-vendor

+    --frontend-opt target=update \
+    --frontend-opt filename=./hack/dockerfiles/vendor.buildkit.Dockerfile \
+    --exporter=local --exporter-opt output=$output
+  cp -R "$output/out/" .


rm -rf vendor

tonistiigi · 2019-01-29T17:58:52Z

hack/dockerfiles/vendor.Dockerfile

-# Remove vendor first to workaround  https://github.com/LK4D4/vndr/issues/63.
-RUN rm -rf vendor
-RUN vndr --verbose --strict
+RUN go mod tidy && go mod vendor && rm -rf /go/pkg/mod


How does this work? Doesn't hack script expect files in /out ?

hack/dockerfiles/test.Dockerfile

tonistiigi · 2019-01-31T01:45:29Z

ping @AkihiroSuda

tiborvass · 2019-01-31T02:04:46Z

I'm exploring an issue when upgrading fsutil with go get -u github.com/tonistiigi/fsutil on top of this PR: go.mod removes indirect dependencies that are still in vendor/, even after go mod vendor.

tiborvass · 2019-01-31T03:12:30Z

From go help modules:

Indirect requirements are
automatically removed from the go.mod file once they are implied by other
direct requirements. Indirect requirements only arise when using modules
that fail to state some of their own dependencies or when explicitly
upgrading a module's dependencies ahead of its own stated requirements.

tonistiigi · 2019-01-31T18:43:03Z

Found some issues where validate vendor passes with a dirty go.sum file.

tiborvass · 2019-01-31T21:44:41Z

@tonistiigi fixed!

go get -u github.com/tiborvass/gomod GO111MODULE=on gomod init GO111MODULE=on go mod tidy GO111MODULE=on go mod vendor Signed-off-by: Tibor Vass <tibor@docker.com>

Signed-off-by: Tibor Vass <tibor@docker.com>

tiborvass · 2019-01-31T22:47:22Z

Updated to upstream docker/cli

The AuthConfig type was forked from the docker/docker API types in commit 27b2797 to reduce the dependency on the docker API types in BuildKit and Buildx (see [buildkit#800]). Now that the API is a separate module with minimal dependencies, this should no longer be a big concern, so this patch un-forks the type. [buildkit#800]: moby/buildkit#800 Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

tiborvass force-pushed the gomod branch from 11085b1 to 406e956 Compare January 26, 2019 06:30

tiborvass commented Jan 26, 2019

View reviewed changes

Update docker/cli dependency

6018171

Signed-off-by: Tibor Vass <tibor@docker.com>

tiborvass force-pushed the gomod branch from 406e956 to 770758d Compare January 26, 2019 23:21

tiborvass force-pushed the gomod branch 5 times, most recently from 4527e79 to b12a6f4 Compare January 28, 2019 20:58

tiborvass commented Jan 28, 2019

View reviewed changes

tiborvass force-pushed the gomod branch from b12a6f4 to da6a027 Compare January 28, 2019 23:09

tonistiigi reviewed Jan 29, 2019

View reviewed changes

tiborvass force-pushed the gomod branch 3 times, most recently from ed84a89 to 9c3d761 Compare January 29, 2019 17:07

tonistiigi reviewed Jan 29, 2019

View reviewed changes

tiborvass force-pushed the gomod branch 4 times, most recently from a34695d to e3f0da2 Compare January 30, 2019 01:37

tiborvass changed the title ~~[WIP] Migrate to go mod~~ Migrate to go mod Jan 30, 2019

tonistiigi reviewed Jan 30, 2019

View reviewed changes

hack/dockerfiles/test.Dockerfile Outdated Show resolved Hide resolved

tiborvass force-pushed the gomod branch from e3f0da2 to 76c04ce Compare January 30, 2019 19:09

tonistiigi approved these changes Jan 31, 2019

View reviewed changes

AkihiroSuda approved these changes Jan 31, 2019

View reviewed changes

tonistiigi added the status/do-not-merge label Jan 31, 2019

tiborvass force-pushed the gomod branch from 76c04ce to 9b7e22e Compare January 31, 2019 21:40

tonistiigi removed the status/do-not-merge label Jan 31, 2019

tiborvass force-pushed the gomod branch from 9b7e22e to cf601b2 Compare January 31, 2019 22:31

Tibor Vass added 2 commits January 31, 2019 22:44

Migrate from vndr to go mod

ad8b35e

go get -u github.com/tiborvass/gomod GO111MODULE=on gomod init GO111MODULE=on go mod tidy GO111MODULE=on go mod vendor Signed-off-by: Tibor Vass <tibor@docker.com>

hack: update scripts to be go mod compatible

175506f

Signed-off-by: Tibor Vass <tibor@docker.com>

tiborvass force-pushed the gomod branch from cf601b2 to 175506f Compare January 31, 2019 22:45

tonistiigi approved these changes Feb 1, 2019

View reviewed changes

tonistiigi merged commit ac64f29 into moby:master Feb 1, 2019

tonistiigi mentioned this pull request Feb 1, 2019

Go modules support #798

Closed

thaJeztah mentioned this pull request Dec 4, 2025

cli/config/types: make AuthConfig an alias for registry.AuthConfig docker/cli#6693

Draft

		@@ -1,3 +1,5 @@
		# syntax = docker/dockerfile:1.0-experimental

Migrate to go mod #800

Migrate to go mod #800

Uh oh!

Conversation

tiborvass commented Jan 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda commented Jan 26, 2019

Uh oh!

tonistiigi commented Jan 26, 2019

Uh oh!

tiborvass commented Jan 26, 2019

Uh oh!

tiborvass Jan 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tiborvass commented Jan 28, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tonistiigi commented Jan 31, 2019

Uh oh!

tiborvass commented Jan 31, 2019

Uh oh!

tiborvass commented Jan 31, 2019

Uh oh!

tonistiigi commented Jan 31, 2019

Uh oh!

tiborvass commented Jan 31, 2019

Uh oh!

tiborvass commented Jan 31, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tiborvass commented Jan 26, 2019 •

edited

Loading

tiborvass Jan 28, 2019 •

edited

Loading