vendor: call vndr to remove useless pkgs and update vendor by fuweid · Pull Request #3759 · containerd/containerd

fuweid · 2019-10-18T10:01:33Z

Signed-off-by: Wei Fu <[email protected]>

estesp · 2019-10-18T10:04:59Z

Hmm, there may be other ways to provide these scripts, but vndr is removing a couple files that we refer to in our README.md: https://github.com/containerd/containerd#enabling-command-auto-completion

fuweid · 2019-10-18T10:38:12Z

Hmm, there may be other ways to provide these scripts, but vndr is removing a couple files that we refer to in our README.md: https://github.com/containerd/containerd#enabling-command-auto-completion

Thanks! But why CI pass without any error? 😂

codecov-io · 2019-10-18T10:49:57Z

Codecov Report

Merging #3759 into master will increase coverage by 3.42%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3759      +/-   ##
==========================================
+ Coverage   41.98%   45.41%   +3.42%     
==========================================
  Files         131      118      -13     
  Lines       14536    11644    -2892     
==========================================
- Hits         6103     5288     -815     
+ Misses       7525     5449    -2076     
+ Partials      908      907       -1

Flag	Coverage Δ
#linux	`45.41% <ø> (ø)`	⬆️
#windows	`?`

Impacted Files	Coverage Δ
remotes/docker/fetcher.go	`42.5% <0%> (-7.5%)`	⬇️
remotes/docker/auth.go	`63.82% <0%> (-3.97%)`	⬇️
remotes/docker/status.go	`21.42% <0%> (-3.58%)`	⬇️
remotes/docker/errdesc.go	`28.12% <0%> (-2.65%)`	⬇️
remotes/docker/errcode.go	`47.61% <0%> (-1.6%)`	⬇️
platforms/cpuinfo.go	`3.77% <0%> (-0.85%)`	⬇️
log/context.go	`36.84% <0%> (-0.66%)`	⬇️
remotes/docker/handler.go	`22.38% <0%> (-0.09%)`	⬇️
errdefs/grpc.go	`77.46% <0%> (-0.04%)`	⬇️
remotes/docker/pusher.go	`0% <0%> (ø)`	⬆️
... and 75 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c86b8f...074b453. Read the comment docs.

estesp · 2019-10-18T11:09:14Z

@fuweid they are not used for build; we just need to probably copy them somewhere so packagers and users can have ctr command completion support if they want to. I cheated in our README.md docs and just referenced their existing path in ./vendor/github.com... because for some reason vndr hadn't removed them when they were initially added.

thaJeztah · 2019-10-18T12:29:05Z

Could be a different version of vndr was used; do we pin the version of vndr?

thaJeztah · 2019-10-18T12:30:04Z

🤔 last commit in vndr master was 2 months ago 🤷‍♂️

jterry75 · 2019-10-18T17:25:54Z

@thaJeztah - Can we not use modules?

thaJeztah · 2019-10-18T20:41:25Z

Lots of problems still with go modules

estesp · 2019-10-19T20:09:31Z

I've proposed to move the autocomplete files to contrib/in #3766

estesp

LGTM

thaJeztah

Think I found why vndr now removes these go files (see my comment)

LGTM

thaJeztah · 2019-10-20T15:04:36Z

vendor/github.com/docker/distribution/registry/api/errcode/errors.go

@@ -1,267 +0,0 @@
-package errcode


I think these became no longer necessary after commit 901bcb2 (#3728)

Could it be that the vendor validation doesn't run unless vendor.conf was modified? If so, that would explain why CI didn't catch the problem (ran into a similar issue in moby/moby, and have a PR there to force the vendor check, even if vendor.conf wasn't updated); moby/moby#40050

fuweid · 2019-10-21T02:26:19Z

Thanks for @estesp and @thaJeztah ! sorry for late reply.

Interesting thing is that the travis CI will remove the vendor, call vndr to fetch all the packages and check diff. https://github.com/containerd/project/blob/master/script/validate/vendor.

It should be fail...

fuweid · 2019-10-21T05:41:55Z

I think the travis_wait doesn't return error code

https://travis-ci.org/containerd/containerd/jobs/600526882

/home/travis/.travis/functions: line 553: 20897 Terminated              travis_jigger "${!}" "${timeout}" "${cmd[@]}"
The command "travis_wait ../project/script/validate/vendor" exited with 0.
0.39s$ git --no-pager diff
diff --git a/vendor/github.com/docker/distribution/registry/api/errcode/errors.go b/vendor/github.com/docker/distribution/registry/api/errcode/errors.go
deleted file mode 100644
index 4c35b87..0000000
--- a/vendor/github.com/docker/distribution/registry/api/errcode/errors.go
+++ /dev/null
@@ -1,267 +0,0 @@
-package errcode
-
-import (
-	"encoding/json"
-	"fmt"
-	"strings"
-)
-
-// ErrorCoder is the base interface for ErrorCode and Error allowing
-// users of each to just call ErrorCode to get the real ID of each
-type ErrorCoder interface {
-	ErrorCode() ErrorCode
-}
-
-// ErrorCode represents the error type. The errors are serialized via strings
-// and the integer format may change and should *never* be exported.
-type ErrorCode int

==> "travis_wait ../project/script/validate/vendor" exited with 0.

thaJeztah · 2019-10-21T05:58:52Z

Ouch! That ain't good; good catch!

fuweid · 2019-10-21T06:23:37Z

@thaJeztah ignore my last comment. That travis_wait can return the error code.

The root cause is about shell quote

root@ubuntu-xenial ~/g/s/g/c/containerd# git status --porcelain vendor go.mod go.sum                                                                                                                                                          129 me-for-vendor-check-testing-!
 D vendor/github.com/docker/distribution/registry/api/errcode/errors.go
 D vendor/github.com/docker/distribution/registry/api/errcode/handler.go
 D vendor/github.com/docker/distribution/registry/api/errcode/register.go
 M vendor/github.com/opencontainers/runc/README.md
 D vendor/github.com/urfave/cli/autocomplete/bash_autocomplete
 D vendor/github.com/urfave/cli/autocomplete/zsh_autocomplete

root@ubuntu-xenial ~/g/s/g/c/containerd# git status --porcelain "vendor go.mod go.sum"                                                                                                                                                            me-for-vendor-check-testing-!
root@ubuntu-xenial ~/g/s/g/c/containerd#

In project, we should not add quote to check diff.

But it should be fter this one pr and #3766 merged.

thaJeztah · 2019-10-21T06:27:22Z

ha! of course; it's looking for a file named vendor go.mod go.sum

fuweid · 2019-10-21T06:32:04Z

pin containerd/project#38

cc @estesp @thaJeztah

estesp · 2019-10-21T13:07:57Z

Merging now that we've solved the problem with the git diff invocation--otherwise I think we will have lots of broken CI on the vndr mismatch!

vendor: call vndr to remove useless pkgs and update vendor

074b453

Signed-off-by: Wei Fu <[email protected]>

fuweid force-pushed the me-update-vendor branch from 0e2f362 to 074b453 Compare October 18, 2019 10:03

fuweid changed the title ~~vendor: call vndr to remove and update vendor~~ vendor: call vndr to remove useless pkgs and update vendor Oct 18, 2019

estesp mentioned this pull request Oct 19, 2019

Move autocomplete files to contrib/ #3766

Merged

estesp approved these changes Oct 19, 2019

View reviewed changes

thaJeztah approved these changes Oct 20, 2019

View reviewed changes

This was referenced Oct 20, 2019

Add distribution subpkgs to core #3728

Merged

bump google.golang.org/grpc v1.23.1 #3767

Merged

estesp merged commit 3e3c5fe into containerd:master Oct 21, 2019

fuweid deleted the me-update-vendor branch April 22, 2020 11:34

Conversation

fuweid commented Oct 18, 2019

Uh oh!

estesp commented Oct 18, 2019

Uh oh!

fuweid commented Oct 18, 2019

Uh oh!

codecov-io commented Oct 18, 2019

Codecov Report

Uh oh!

estesp commented Oct 18, 2019

Uh oh!

thaJeztah commented Oct 18, 2019

Uh oh!

thaJeztah commented Oct 18, 2019

Uh oh!

jterry75 commented Oct 18, 2019

Uh oh!

thaJeztah commented Oct 18, 2019

Uh oh!

estesp commented Oct 19, 2019

Uh oh!

estesp left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah Oct 20, 2019

Choose a reason for hiding this comment

Uh oh!

fuweid Oct 21, 2019

Choose a reason for hiding this comment

Uh oh!

fuweid commented Oct 21, 2019

Uh oh!

fuweid commented Oct 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Oct 21, 2019

Uh oh!

fuweid commented Oct 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Oct 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fuweid commented Oct 21, 2019

Uh oh!

estesp commented Oct 21, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

fuweid commented Oct 21, 2019 •

edited

Loading

fuweid commented Oct 21, 2019 •

edited

Loading

thaJeztah commented Oct 21, 2019 •

edited

Loading