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

strict mode does not fail for "cannot find package" error #62

Open
ijc opened this issue Mar 8, 2018 · 2 comments
Open

strict mode does not fail for "cannot find package" error #62

ijc opened this issue Mar 8, 2018 · 2 comments

Comments

@ijc
Copy link
Contributor

ijc commented Mar 8, 2018

I noticed that a project (moby/buildkit as it happens) was missing a dependency (you can see it here). I patched the check (./hack/validate-vendor and hack/dockerfiles/vendor.Dockerfile) thinking it would cause the check to fail but it did not:

$ git diff
diff --git a/hack/dockerfiles/vendor.Dockerfile b/hack/dockerfiles/vendor.Dockerfile
index 64cd656..ac82dfe 100644
--- a/hack/dockerfiles/vendor.Dockerfile
+++ b/hack/dockerfiles/vendor.Dockerfile
@@ -7,4 +7,4 @@ RUN go get -d github.com/LK4D4/vndr \
        && go install ./
 WORKDIR /go/src/github.com/moby/buildkit
 COPY . .
-RUN vndr --verbose
\ No newline at end of file
+RUN vndr --verbose --strict
[...]
$ ./hack/validate-vendor 
[...]
+ docker build --build-arg VNDR_VERSION=48ac2669d9d1bcacd3163650ef911edca2ec3b42 --iidfile /tmp/docker-iidfile.9obqMoTlAJ -f ./hack/dockerfiles/vendor.Dockerfile --force-rm .
[...]Step 7/7 : RUN vndr --verbose --strict
 ---> Running in 1c252b10149d
2018/03/08 14:26:15 Collecting initial packages
2018/03/08 14:26:15 Download dependencies
[...]
2018/03/08 14:27:11 Dependencies downloaded. Download time: 55.205977209s
2018/03/08 14:27:11 Collecting all dependencies
2018/03/08 14:27:13 	WARNING(verbose) github.com/tonistiigi/llb-gobuild: cannot find package "github.com/tonistiigi/llb-gobuild" in any of:
	/go/src/github.com/moby/buildkit/vendor/github.com/tonistiigi/llb-gobuild (vendor tree)
	/go/src/github.com/moby/buildkit/vendor/github.com/tonistiigi/llb-gobuild
	/usr/local/go/src/github.com/tonistiigi/llb-gobuild (from $GOROOT)
	/go/src/github.com/tonistiigi/llb-gobuild (from $GOPATH)
2018/03/08 14:27:14 Clean vendor dir from unused packages
2018/03/08 14:27:14 Success
2018/03/08 14:27:14 Running time: 59.498826306s
Removing intermediate container 1c252b10149d
 ---> cf2f4a83aad9
Successfully built cf2f4a83aad9
++ cat /tmp/docker-iidfile.9obqMoTlAJ
+ iid=sha256:cf2f4a83aad9c24e1e16e2ff5dc6fde8265d4872dbe1f6f91cc12bb7abded0e1
++ docker run sha256:cf2f4a83aad9c24e1e16e2ff5dc6fde8265d4872dbe1f6f91cc12bb7abded0e1 git status --porcelain -- vendor
+ diffs=
+ '[' '' ']'
+ echo 'Congratulations! All vendoring changes are done the right way.'
Congratulations! All vendoring changes are done the right way.
+ rm -f /tmp/docker-iidfile.9obqMoTlAJ

I think this should have failed.

The above is with 48ac266 but I have also reproduced with b57c579 which is the most recent master.

In fact with the most recent master it seems the WARNING has gone, even though llb-gobuild is still used -- the usage is in a example command (so a main package, but a few levels down in the source tree) so perhaps that got missed?

ijc pushed a commit to ijc/buildkit that referenced this issue Mar 8, 2018
At the moment vendor.conf is missing entries for `github.com/tonistiigi/llb-gobuild`
and `github.com/morikuni/aec` due in part because of a combination of
LK4D4/vndr#62 and LK4D4/vndr#63.

The issue vndr#63 (related to lack of `github.com/morikuni/aec`) can be worked
around by removing the vendor directory before rerunning `vndr`, so do so.

Due to vndr#62 the issue with `github.com/tonistiigi/llb-gobuild` cannot be
detected at the moment, but pass `-strict` to `vndr` in anticipation of a fix
there.

Signed-off-by: Ian Campbell <[email protected]>
@ijc
Copy link
Contributor Author

ijc commented Mar 8, 2018

In fact with the most recent master it seems the WARNING has gone, even though llb-gobuild is still used -- the usage is in a example command (so a main package, but a few levels down in the source tree) so perhaps that got missed?

Ah, the file has // +build ignore so perhaps this is a deliberate fix.

@ijc
Copy link
Contributor Author

ijc commented Mar 8, 2018

Having noticed the // +build ignore I think this is now only relevant as it relates to #63.

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

No branches or pull requests

1 participant