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

vendoring: try to catch more vendoring issues & fixup an existing one #303

Merged
merged 2 commits into from
Mar 8, 2018

Conversation

ijc
Copy link
Collaborator

@ijc ijc commented 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.

This will make the CI fail on this PR (I hope). Once it has done so I will push an extra patch which fixes things by adding the github.com/morikuni/aec entry to vendor.conf.

I will not add the github.com/tonistiigi/llb-gobuild since the user examples/gobuild/main.go is marked // +build ignore, with current vndr that produces a warning (harmless due too LK4D4/vndr#62 even we add -strict here) but with newer vndr it does not.

Ian Campbell added 2 commits March 8, 2018 14:46
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]>
Accidentally removed in faf258d ("vendor: add go-units").

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

ijc commented Mar 8, 2018

CI failed as desired: https://travis-ci.org/moby/buildkit/builds/350858589#L2293

Will now push the fix.

@tonistiigi
Copy link
Member

Yes, not vendoring llb-gobuild was intentional as it is only used for examples. Although I think we should still try to build it in tests.

LGTM

@tonistiigi tonistiigi merged commit abe6627 into moby:master Mar 8, 2018
@ijc ijc deleted the vndr-fixes branch March 9, 2018 09:19
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.

2 participants