containerd / continuity

A transport-agnostic, filesystem metadata manifest system
https://containerd.io
Apache License 2.0
138 stars 66 forks source link

Support Go Modules #144

Closed SamWhited closed 4 years ago

SamWhited commented 4 years ago

Please also consider adding a semver compatible tag such as v0.0.1, thank you for your consideration.

EDIT apologies for the re-vendor, I can remove that change but thought it would be useful to go ahead and have the vendor tree created by go mod vendor. In general I would say it's best not to vendor in libraries and to rely on the binaries that depend on this library to vendor if they need it, but not knowing much about this project I wanted to keep the changes small (scope wise, obviously it's a lot of files still).

EDIT: CI will depend on containerd/project#25

kzys commented 4 years ago

https://github.com/containerd/project/pull/25 has been merged!

So we need to remove vendor.conf in this PR to actually switch from vndr to Go modules, since the existence of the configuration files changes the behavior of script/validate/vendor.

SamWhited commented 4 years ago

@kzys vendor.conf is removed by this PR.

I didn't think about this before, but this repo is tested against multiple versions of Go. While this is great, sadly the Go team can't stop breaking vendoring and changes to the go.mod file in every version of Go, so Go 1.11, 1.12, and 1.13 may all result in different go.mod files or vendor trees. How would you like to proceed?

EDIT: for now I've tried creating a separate validation stage in CI with a fixed version of Go. Hopefully that works.

kzys commented 4 years ago

Oh, I thought it was still there. Thanks.

Regarding the remaining failures, you would need to have env GO111MODULE=off before go get, like https://github.com/containerd/go-cni/pull/50.

SamWhited commented 4 years ago

Ah yes, I forgot that Go 1.11 didn't have the behavior where it does different things depending on whether it finds a go.mod file or not. That should fix it. I left the pushd/popd in since they'll keep it working even if GO111MODULE is removed in a future version of Go.

SamWhited commented 4 years ago

Fixed conflict. Gentle ping.