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

Fix protobuf's inconsistent dependency #2283

Merged
merged 1 commit into from
Jun 22, 2017

Conversation

alyyousuf7
Copy link
Contributor

I started getting "inconsistent dependency" errors in CI: https://circleci.com/gh/docker/swarmkit/7532

I guess some of the files were missed while revendoring in #2229.
The committed differences were found in vendor/github.com/golang/protobuf after running make dep-validate.

Signed-off-by: Ali Yousuf aly.yousuf7@gmail.com

@ijc
Copy link
Contributor

ijc commented Jun 22, 2017

This is strange, CI is complaining that it's vendoring check is seeing the older (current master) version:

diff -r vendor/github.com/golang/protobuf/ptypes/empty/empty.pb.go .vendor.bak/github.com/golang/protobuf/ptypes/empty/empty.pb.go
1c1
< // Code generated by protoc-gen-go. DO NOT EDIT.
---
> // Code generated by protoc-gen-go.
2a3
> // DO NOT EDIT!
+ inconsistent dependencies! what you have in vendor.conf does not match with what you have in vendor

But when I run vndr locally against master I'm (now, I'm sure I wasn't seeing this when I made #2229) seeing the same diff as you.

github.com/golang/protobuf is vendored using a specific hash 7a211bcf3bce0e3f1d74f9894916e6f116ae83b4 so it ought to be getting identical content each time.

@ijc
Copy link
Contributor

ijc commented Jun 22, 2017

@ijc
Copy link
Contributor

ijc commented Jun 22, 2017

I'm not confused, the diff here really is missing that first hunk to vendor/github.com/golang/protobuf/ptypes/empty/empty.pb.go, or so it seems.

@alyyousuf7
Copy link
Contributor Author

I ran vndr again locally and now I see the following diff:

diff --git a/vendor/github.com/golang/protobuf/ptypes/empty/empty.pb.go b/vendor/github.com/golang/protobuf/ptypes/empty/empty.pb.go
index 9779cb3f..ae159414 100644
--- a/vendor/github.com/golang/protobuf/ptypes/empty/empty.pb.go
+++ b/vendor/github.com/golang/protobuf/ptypes/empty/empty.pb.go
@@ -1,6 +1,5 @@
-// Code generated by protoc-gen-go.
+// Code generated by protoc-gen-go. DO NOT EDIT.
 // source: github.com/golang/protobuf/ptypes/empty/empty.proto
-// DO NOT EDIT!
 
 /*
 Package empty is a generated protocol buffer package.

@ijc
Copy link
Contributor

ijc commented Jun 22, 2017

That's exactly what I see too when I run vndr on this PR.

Signed-off-by: Ali Yousuf <aly.yousuf7@gmail.com>
@alyyousuf7
Copy link
Contributor Author

Pushed the diff. And it passed the dep-validate check.

@ijc
Copy link
Contributor

ijc commented Jun 22, 2017

Excellent, LGTM.

Thanks for sorting this out, I'm still rather mystified about the whole thing but I'll learn to live with it.

@codecov
Copy link

codecov bot commented Jun 22, 2017

Codecov Report

Merging #2283 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #2283      +/-   ##
=========================================
+ Coverage    60.4%   60.4%   +<.01%     
=========================================
  Files         125     125              
  Lines       20394   20394              
=========================================
+ Hits        12318   12320       +2     
- Misses       6682    6686       +4     
+ Partials     1394    1388       -6

@aaronlehmann
Copy link
Collaborator

This is interesting. I'd like to understand why CI passed in #2229 (https://circleci.com/gh/docker/swarmkit/7361). I wonder if there's a bug in the dep-validate target.

@aaronlehmann
Copy link
Collaborator

#2229 didn't use this github.com/golang/protobuf/ptypes/empty package, so it wasn't present in the vendor directory on that branch.

The package was added to the vendor directory when containerd dependencies were added. It must have been vendored from the earlier version. Then when this was merged, it made the vendor tree inconsistent.

LGTM

@aaronlehmann aaronlehmann merged commit 4b21888 into moby:master Jun 22, 2017
@ijc
Copy link
Contributor

ijc commented Jun 22, 2017

Thanks for figuring it out, I'll sleep a little better!

andrewhsu pushed a commit to docker-archive/docker-ce that referenced this pull request Jul 14, 2017
- moby/swarmkit#2266 (support for templating Node.Hostname in docker executor)
- moby/swarmkit#2281 (change restore action on objects to be update, not delete/create)
- moby/swarmkit#2285 (extend watch queue with timeout and size limit)
- moby/swarmkit#2253 (version-aware failure tracking in the scheduler)
- moby/swarmkit#2275 (update containerd and port executor to container client library)
- moby/swarmkit#2292 (rename some generic resources)
- moby/swarmkit#2300 (limit the size of the external CA response)
- moby/swarmkit#2301 (delete global tasks when the node running them is deleted)

Minor cleanups, dependency bumps, and vendoring:
- moby/swarmkit#2271
- moby/swarmkit#2279
- moby/swarmkit#2283
- moby/swarmkit#2282
- moby/swarmkit#2274
- moby/swarmkit#2296 (dependency bump of etcd, go-winio)

Signed-off-by: Ying Li <ying.li@docker.com>
Upstream-commit: 4509a00
Component: engine
silvin-lubecki pushed a commit to silvin-lubecki/engine-extract that referenced this pull request Mar 16, 2020
- moby/swarmkit#2266 (support for templating Node.Hostname in docker executor)
- moby/swarmkit#2281 (change restore action on objects to be update, not delete/create)
- moby/swarmkit#2285 (extend watch queue with timeout and size limit)
- moby/swarmkit#2253 (version-aware failure tracking in the scheduler)
- moby/swarmkit#2275 (update containerd and port executor to container client library)
- moby/swarmkit#2292 (rename some generic resources)
- moby/swarmkit#2300 (limit the size of the external CA response)
- moby/swarmkit#2301 (delete global tasks when the node running them is deleted)

Minor cleanups, dependency bumps, and vendoring:
- moby/swarmkit#2271
- moby/swarmkit#2279
- moby/swarmkit#2283
- moby/swarmkit#2282
- moby/swarmkit#2274
- moby/swarmkit#2296 (dependency bump of etcd, go-winio)

Signed-off-by: Ying Li <ying.li@docker.com>
Upstream-commit: 4509a00
Component: engine
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.

3 participants