Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update deps to get rid of k8s.io/kubernetes #32

Merged

Conversation

dims
Copy link
Member

@dims dims commented Mar 1, 2021

containerd/containerd depends on this repo and the go.mod/sum ends up pulling in k8s.io/kubernetes transitively to many other projects that vendor containerd including hcsshim and kubernetes.

Signed-off-by: Davanum Srinivas davanum@gmail.com

dims added 2 commits March 1, 2021 08:52
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
Signed-off-by: Davanum Srinivas <davanum@gmail.com>
@dims dims force-pushed the update-dependencies-to-remove-dependency-on-k8s branch from 3e2aa52 to 5a63297 Compare March 1, 2021 14:44
@estesp
Copy link
Member

estesp commented Mar 1, 2021

I just opened #34 as I didn't realize we still had one or more subprojects on Travis! Of course I also had to do some vendor mucking to make my actions pass a run. So--how to coordinate your fixes with #34.. we might just have to ignore Travis output for a few PR runs!

Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines 158 to 159
gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo=
gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we know where this old version is still used?

edit; ah; hcsshim, looks like it;

go mod graph | grep ' gotest.tools@v2'

github.com/Microsoft/hcsshim@v0.8.14 gotest.tools@v2.2.0+incompatible

google.golang.org/grpc v1.26.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably check where this one is still used

go mod graph | grep ' gopkg.in/yaml.v2@v2.2.2'

github.com/urfave/cli@v1.22.2 gopkg.in/yaml.v2@v2.2.2
github.com/stretchr/testify@v1.4.0 gopkg.in/yaml.v2@v2.2.2

Copy link
Member

@estesp estesp Mar 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main containerd repo is still on urfave/cli@v1.22.2 so not sure this can be solved in this repo?

EDIT: Missed the fact that we actually do a replace down to v1.22.1 in containerd due to some regression

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For urfave, I had a PR to fix it, but it's somewhat complex, and have to revisit that one

Dependencies here definitely doesn't look urgent, but whenever I see a go.sum contain multiple (older) versions, I'm always curious "who is depending on this?"

@estesp
Copy link
Member

estesp commented Mar 1, 2021

Let's finish fixing up any vendoring changes in the GH actions PR (#34) since it is based on Go 1.16

@estesp estesp merged commit 11e8f17 into containerd:master Mar 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants