Skip to content

Go Mod Support#3182

Closed
ehazlett wants to merge 2 commits intocontainerd:masterfrom
ehazlett:go-mod-support
Closed

Go Mod Support#3182
ehazlett wants to merge 2 commits intocontainerd:masterfrom
ehazlett:go-mod-support

Conversation

@ehazlett
Copy link
Copy Markdown
Member

@ehazlett ehazlett commented Apr 5, 2019

This adds Go module support.

What I did:

  • go mod init (imported from vendor)
  • go mod tidy ensure indirect imports
  • go mod vendor vendor copy of all imports for non-module support (pre 1.11, etc)
  • Update CI config to force use of Go modules (GO111MODULE=on)
  • Tested with a fresh clone to ensure build

@ehazlett ehazlett mentioned this pull request Apr 5, 2019
Closed
1 task
@cpuguy83
Copy link
Copy Markdown
Member

cpuguy83 commented Apr 5, 2019 •
edited
Loading

Should the build target use -mod=vendor?

@AkihiroSuda
Copy link
Copy Markdown
Member

AkihiroSuda commented May 14, 2019

ping

@fuweid
Copy link
Copy Markdown
Member

fuweid commented Jun 4, 2019

ping @ehazlett ^^

@ehazlett ehazlett force-pushed the go-mod-support branch 2 times, most recently from 4e11698 to 4f8fda2 Compare June 11, 2019 17:54
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Jun 11, 2019

Build succeeded.

@ehazlett ehazlett marked this pull request as ready for review June 11, 2019 19:30
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Jun 11, 2019

Build succeeded.

@ehazlett
Go module support
f79e7b0 
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Jun 17, 2019

Build succeeded.

@ehazlett
use modified vendor validation script
9e8223a 
Signed-off-by: Evan Hazlett <ejhazlett@gmail.com>
@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Jun 18, 2019

Build succeeded.

@stevvooe
Copy link
Copy Markdown
Member

stevvooe commented Jul 15, 2019

@ehazlett How did protobuild fare in this scenario?

@ehazlett
Copy link
Copy Markdown
Member Author

ehazlett commented Jul 16, 2019

@stevvooe I get the following warns:

/<path>/go/containerd/src/github.com/gogo/protobuf: warning: directory does not exist.
/<path>/go/containerd/src/github.com/gogo/googleapis: warning: directory does not exist.

I'm also vendoring everything for backwards compat to help.

But since I have them global I think it works. I'm going to try to get this rebased and updated this week to try some more.

@CpuID
Copy link
Copy Markdown

CpuID commented Aug 14, 2019

@ehazlett some merge conflicts still...?

@dmcgowan
Copy link
Copy Markdown
Member

dmcgowan commented Aug 19, 2019

I'm going to close this one for now since it is not in a reviewable state. My suggestion is that we first update script/validate/vendor in the project repository which can switch based on GO111MODULE=on then first validate go mod working in the sub projects.

@dmcgowan dmcgowan closed this Aug 19, 2019
kzys pushed a commit to kzys/project that referenced this pull request Sep 9, 2019
Use "go mod" instead of vndr when GO111MODULE is "on"
cebc28e 
As Derek suggested at
containerd/containerd#3182,
this change adds Go modules support for the script first.
kzys pushed a commit to kzys/project that referenced this pull request Sep 9, 2019
Use "go mod" instead of vndr when GO111MODULE is "on"
0235f1d 
As Derek suggested at
containerd/containerd#3182,
this change adds Go modules support for the script first.

Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
@kzys kzys mentioned this pull request Sep 9, 2019
Closed
@AkihiroSuda
Copy link
Copy Markdown
Member

AkihiroSuda commented Sep 12, 2019

containerd/project#25 got merged, can we reopen and get this before 1.3?

@kzys
Copy link
Copy Markdown
Member

kzys commented Sep 12, 2019 •
edited
Loading

I think we could start from the dependencies of containerd such as https://github.com/containerd/go-cni or https://github.com/containerd/continuity.

https://github.com/containerd/cri is a bit trickier than others, due to the way Kubernetes manages dependencies. You may need a bunch of replace directives there like firecracker-microvm/firecracker-containerd#255

@kzys kzys mentioned this pull request Nov 21, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@ehazlett @cpuguy83 @AkihiroSuda @fuweid @stevvooe @CpuID @dmcgowan @kzys