Fix go.mod to be compatible with go1.13

[zachbadgett]

There was some concern related to using the replace directive in c6641fb; not sure I follow it completely.

Signed-off-by: Zach Badgett [email protected]

[zachbadgett]

This commit had module related errors when resolving golang.org/x/crypto; updating fixes it, and the changes from 3d2716dd0a4d to 0f039a052ca1 seem to be fairly minor.

[thaJeztah]

/cc @SamWhited

[SamWhited]

There was some concern related to using the replace directive in c6641fb; not sure I follow it completely.

The issue with replace is that it's not transitive. Anything importing a package in the buildkit module would not see the replace directive, and would select a different version of containerd than the one required. Put another way: replace is for main packages, not for libraries.

[AkihiroSuda]

can we follow master?
#1293

[zachbadgett]

@SamWhited Thanks! That makes sense.

@AkihiroSuda I've made the same changes you made in yours, still works with go1.13 (tested with version 1.13.4). Travis failed at the "Cross" step, but I haven't been able to replicate any failures locally.

[GordonTheTurtle]

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "fix-go-mod" [email protected]:zachbadgett/buildkit.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354378440
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

[zachbadgett]

@AkihiroSuda Tracked down the issue, it was coming from the docker version you wanted to use. There were a couple dependency issues.

Issue 1:
From this commit: moby/moby@6b91cef docker moved to using osversion from github.com/Microsoft/hcsshim to get the windows OS version, however, osversion.Build() was not added until v0.8.7 in this commit: microsoft/hcsshim@92cb5b6 and buildkit was on v0.8.5.

Issue 2:
From this commit:
moby/moby@c3a0a37#diff-d56bbc14384133b2bc2cf0677bb3561bR105
Using windows.SecurityDescriptorFromString which was not added to sys until golang/sys@14da1ac#diff-c4ffa695b270239c949ef5b31be6c4f5R1317 and buildkit was on v0.0.0-20190812073006-9eafafc0a87e which did not include that change.

[thaJeztah]

however, osversion.Build() was not added until v0.8.7 in this commit: microsoft/hcsshim@92cb5b6 and buildkit was on v0.8.5.

Be aware that tags should no longer be used from that repository. See microsoft/hcsshim#700

This is a code-only repository and does not have a stable release. 0.8.6 is the last tagged release, and it is out of date for ContainerD and Kubernetes. Downstream projects should vendor in this codebase using commit ids instead of tags, then validate fixes and do a full test pass of the whole project.

Unfortunately, go mod has no option for a repository to indicate "don't use tags" 😞

[zachbadgett]

@thaJeztah buildkit does not have a direct dependency on microsoft/hcsshi. Only indirectly through docker and another library called fsutil (which uses v0.8.5); so using the tag that satisfies docker as well seems appropriate. Let me know if anyone disagrees or if anyone has a commit they would like me to switch to and I'll be happy to update it :)

[ghost]

I have the same error when I want to import buildkit via go.mod

go: github.com/moby/[email protected] requires
	github.com/containerd/[email protected]: invalid pseudo-version: version before v1.3.0 would have negative patch number

Any plan to merge the fix ?

[AkihiroSuda]

@tonistiigi @tiborvass @thaJeztah PTAL?

[AkihiroSuda]

@zachbadgett could you rebase and update the PR with replace directives?

[AkihiroSuda]

sorry needs rebase again

@tonistiigi PTAL at replace directives?

[TBBle]

The versions used in these replace lines doesn't match the commit-hash used in the require list above.

[TBBle]

On the pseudo-version issues, is there any chance of the relevant upstream projects (containerd and docker, in this case) putting tags like v1.4.0-alpha.0 (in the case of containerd) on their master branch, on the commit after the previous release branched off?

(Without testing...) then go would pick those up, generate useful pseudo-versions, and we could avoid all the issues around replace in go.mod and having a higher-tagged-but-older version be selected for module resolution.

[thaJeztah]

For containerd there's containerd/containerd#3031 and containerd/containerd#3554 tracking go mod compatibility.

[AkihiroSuda]

needs rebase again

[AkihiroSuda]

#24 [buildkitd 1/1] RUN --mount=target=. --mount=target=/root/.cache,type=ca...
#24 40.32 # github.com/moby/buildkit/solver/llbsolver/file
#24 40.32 solver/llbsolver/file/backend.go:29:29: undefined: "github.com/tonistiigi/fsutil/copy".User
#24 40.32 solver/llbsolver/file/backend.go:29:73: undefined: "github.com/tonistiigi/fsutil/copy".Chowner
#24 40.32 solver/llbsolver/file/backend.go:31:20: undefined: "github.com/tonistiigi/fsutil/copy".User
#24 40.32 solver/llbsolver/file/backend.go:36:12: undefined: "github.com/tonistiigi/fsutil/copy".User
#24 40.32 solver/llbsolver/file/backend.go:46:13: undefined: "github.com/tonistiigi/fsutil/copy".User
#24 40.32 solver/llbsolver/file/backend.go:68:76: undefined: "github.com/tonistiigi/fsutil/copy".User
#24 40.32 solver/llbsolver/file/backend.go:101:78: undefined: "github.com/tonistiigi/fsutil/copy".User
#24 40.32 solver/llbsolver/file/backend.go:163:81: undefined: "github.com/tonistiigi/fsutil/copy".User
#24 40.32 solver/llbsolver/file/unpack.go:15:95: undefined: "github.com/tonistiigi/fsutil/copy".Chowner
#24 40.32 solver/llbsolver/file/user_linux.go:15:63: undefined: "github.com/tonistiigi/fsutil/copy".User
#24 40.32 solver/llbsolver/file/backend.go:46:13: too many errors
#24 ERROR: executor failed running [/bin/sh -c go build -ldflags "$(cat /tmp/.ldflags) -w -extldflags -static" -tags "osusergo netgo static_build seccomp ${BUILDKITD_TAGS}" -o /usr/bin/buildkitd ./cmd/buildkitd &&   file /usr/bin/buildkitd | egrep "statically linked|Windows"]: buildkit-runc did not terminate sucessfully

[zachbadgett]

My mistake, I thought I ran make test before committing.

[tonistiigi]

why is this go1.11 ?

[zachbadgett]

The change to go 1.12 must've been recent; I went over all the changes in the dependencies during rebase, however I missed the version metadata. I've updated it.

[tonistiigi]

Where are these commits coming from? If I compare with containerd I don't see same commits. For some of them it isn't a big deal as if we have direct dependency we could bump it (although would be nice to mark it as upgrade in another commit then). But many in here seem to be just containerd dependencies. Or is it really that the old versions didn't build with go1.13?

[AkihiroSuda]

@tonistiigi : Can we merge this as-is and address other issues in follow-up PRs?

[tonistiigi]

@AkihiroSuda I'm fine with the replace rules but rolling back deps if they are already too new is going to be a pain. Feel free to carry this with extra commits to adjust containerd dependencies to the correct version to get this merged. And while you are at it, update to Dockerfile as well so we see this(and go.mod validation) in action.

[tonistiigi]

carried in #1376