Skip to content

Fix protobuf's inconsistent dependency#2283

Merged
aaronlehmann merged 1 commit intomoby:masterfrom
alyyousuf7:protobuf-revendoring
Jun 22, 2017
Merged

Fix protobuf's inconsistent dependency#2283
aaronlehmann merged 1 commit intomoby:masterfrom
alyyousuf7:protobuf-revendoring

Conversation

@alyyousuf7
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

ijc commented Jun 22, 2017

@ijc
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 alyyousuf7 force-pushed the protobuf-revendoring branch from 0313e42 to 7c5e988 Compare June 22, 2017 09:55
@alyyousuf7
Copy link
Copy Markdown
Contributor Author

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

@ijc
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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