vendor: migrate to "golang/dep" #9155

gyuho · 2018-01-16T23:11:54Z

codecov-io · 2018-01-17T20:02:36Z

Codecov Report

❗ No coverage uploaded for pull request base (master@6a80e94). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #9155   +/-   ##
=========================================
  Coverage          ?   75.94%           
=========================================
  Files             ?      359           
  Lines             ?    30046           
  Branches          ?        0           
=========================================
  Hits              ?    22817           
  Misses            ?     5634           
  Partials          ?     1595

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 6a80e94...c16473f. Read the comment docs.

xiang90 · 2018-01-17T20:28:10Z

.gitignore

+!vendor/**/License*
+!vendor/**/LICENCE*
+!vendor/**/LICENSE*
+vendor/**/*_test.go


Just added.

gyuho · 2018-01-17T21:30:48Z

Major change would be enforcing etcd dependencies (e.g. grpc-go v1.7.5 on clientv3) on go get commands. Previously, we didn't do it to let users choose their own gRPC version. Now, clientv3 is so sensitive to gRPC versions, so it has to be enforced. Also, Go 1.9 type alias won't complain about context import conflicts.

gyuho · 2018-01-18T20:54:49Z

@sdboyer Could you quickly go over this?

Gopkg.toml 35cae90
scripts/updatedep.sh 9a75a2a

1838ee1 is the diff between glide + glide vc --only-code --no-tests and dep with gitignore.

dep is still missing complete prune feature (e.g. for etcd, github.com/golang/groupcache/... are directive dependencies, but not github.com/golang/groupcache itself. But, dep does not prune github.com/golang/groupcache although it's never used.). However, I think we are ok with it. Other than that, golang/dep works great!

fanminshi · 2018-01-24T22:49:27Z

build

@@ -36,31 +36,14 @@ etcd_build() {
 	# Static compilation is useful when etcd is run in a container. $GO_BUILD_FLAGS is OK

 	# shellcheck disable=SC2086
-	CGO_ENABLED=0 go build $GO_BUILD_FLAGS -installsuffix cgo -ldflags "$GO_LDFLAGS" -o "${out}/etcd" ${REPO_PATH}/cmd/etcd || return
+	CGO_ENABLED=0 go build $GO_BUILD_FLAGS -installsuffix cgo -ldflags "$GO_LDFLAGS" -o "${out}/etcd" . || return


i think having ${REPO_PATH} as build path is more strict.

fanminshi · 2018-01-24T22:50:15Z

build

-	rm -rf "${etcdGOPATH}/src"
-	mkdir -p "${etcdGOPATH}"
-	ln -s "${CDIR}/cmd/vendor" "${etcdGOPATH}/src"
+	CGO_ENABLED=0 go build $GO_BUILD_FLAGS -installsuffix cgo -ldflags "$GO_LDFLAGS" -o "${out}/etcdctl" ./etcdctl || return


maybe have ${REPO_PATH}/etcdctl? i think it is more strict.

fanminshi · 2018-01-24T22:58:57Z

lgtm after few suggestions.

Signed-off-by: Gyuho Lee <[email protected]>

sdboyer · 2018-01-25T01:43:47Z

ack. sorry. i was taken up all this week with the new release of dep. awesome to see you pulled the trigger, though!

the comments i'd make are:

it's redundant, and potentially harmful, to specify source in Gopkg.toml when it's not actually needed (and it's not in any of the cases i saw from skimming).
unless there's a specific reason that you need exactly those revisions for all those projects, and that they NEVER change under ANY circumstances, it's preferable that you instead just specify e.g. branch = "master", and trust Gopkg.lock to give you the version stability you want.
v0.4.1 of dep prunes automatically, and you have more granular controls over it from within Gopkg.toml.
overrides should also be avoided, when possible. they can be useful as a one-time way of getting Gopkg.lock to the right place, but once you've done that, you should ideally be able to remove them.
strongly prefer released versions to go get-ing tip. we should have a curl-able installer soon Install.sh script for dep golang/dep#1533

gyuho · 2018-01-25T01:45:25Z

@sdboyer Awesome. Thanks for the tips! We will address those shortly!

tamird · 2018-01-26T20:49:15Z

There are some really unfortunate side effects as a result of this. For instance, cockroachdb wants to use a newer version of gogo/protobuf than what's specified in your new Gopkg.toml - the result is that dep will refuse to ignore etcd's specified version and won't resolve (in the presence of the two constraints etcd.branch = master and protobuf.branch = master).

As far as I know, there's no workaround here, other than for etcd to be very careful to not over-constrain your dependencies (and currently they appear to all be maximally constrained; your Gopkg.toml is roughly equivalent to your Gopkg.lock).

xiang90 · 2018-01-26T20:53:35Z

@tamird

can you move the discussion to here #9224?

thanks!

gyuho · 2018-01-26T20:55:58Z

@tamird Wasn't aware of it. We did over-constrain the dependencies to minimize difference during migration. But open to any workaround if that works for external projects.

I see https://github.com/cockroachdb/cockroach/blob/master/Gopkg.toml#L92

[[constraint]]
  name = "github.com/gogo/protobuf"
  branch = "master"

Would [[override]] work for cockroachdb/cockroach?

tamird · 2018-01-26T20:59:06Z

@gyuho no, that still doesn't work.

You can, at minimum, remove all [constraint]s from Gopkg.toml now that you have a Gopkg.lock.

gyuho · 2018-01-26T21:49:48Z

Replacing all our constraints with branch = "master" would work?

sdboyer · 2018-01-26T21:51:18Z

why does an override not work? this is what they're actually designed for.

tamird · 2018-01-26T21:58:30Z

@gyuho you shouldn't need any constraints at all, provided you don't run dep ensure -update. The Gopkg.lock will prevent any of the versions from changing.

I'm suggesting you simply delete all the [constraint]s without replacement.

@sdboyer "why" is a bit of an academic question - I can't tell you why. All I know is that using [override] produces the exact same result as using [constraint] in this case. Here's the key line of output, in case it's helpful:

Solving failure: No versions of github.com/coreos/etcd met constraints:
...
master: Could not introduce github.com/coreos/etcd@master, as it depends on github.com/gogo/protobuf from https://github.com/gogo/protobuf, but github.com/gogo/protobuf is already marked as coming from github.com/gogo/protobuf by github.com/cockroachdb/cockroach

sdboyer · 2018-01-26T22:04:09Z

I'm not sure what makes "why" an academic question - that's the info i needed.

the problem is a conflict on source. you can also issue an override on that.

gyuho · 2018-01-26T22:08:49Z

I'm fine with deleting constraints from Gopkg.toml, but that means etcd will need hand-maintain Gopkg.lock, which is also fine with me:

Gopkg.lock is autogenerated; editing it manually is generally an antipattern. If there is a goal you can only achieve by hand-editing Gopkg.lock, it is at least a feature request, and likely a bug.

@sdboyer Is this the valid use case for editing Gopkg.lock? Or I'm missing something.

tamird · 2018-01-26T22:17:34Z

@sdboyer yeah, that seems to work, but requires an override for every overlapping dependency (of which github.com/gogo/protobuf is just one).

@gyuho why do you need to hand-edit Gopkg.lock? You can update single packages with dep ensure -update <pkg>. Are you often updating to a version that is not the latest?

gyuho added WIP and removed WIP labels Jan 16, 2018

gyuho force-pushed the dep branch from 34d84a5 to 4258185 Compare January 17, 2018 17:44

gyuho requested a review from xiang90 January 17, 2018 19:07

xiang90 reviewed Jan 17, 2018

View reviewed changes

gyuho force-pushed the dep branch from 4258185 to c16473f Compare January 17, 2018 20:32

gyuho mentioned this pull request Jan 17, 2018

vendoring clientv3 with glide vendors packages twice #8441

Closed

gyuho added the kind/need-release-note-v3.4 label Jan 22, 2018

fanminshi reviewed Jan 24, 2018

View reviewed changes

gyuho added 17 commits January 24, 2018 15:26

scripts/updatedep.sh: use "golang/dep"

505d24c

Signed-off-by: Gyuho Lee <[email protected]>

glide: remove

fa9f3dd

Signed-off-by: Gyuho Lee <[email protected]>

cmd: move "cmd/vendor" to root, remove "cmd"

b1e87f8

Signed-off-by: Gyuho Lee <[email protected]>

gitignore: ignore test files in "vendor"

4fc2db7

Signed-off-by: Gyuho Lee <[email protected]>

Gopkg.toml: initial commit

65ee3a0

Signed-off-by: Gyuho Lee <[email protected]>

vendor: regenerate

970ab0b

Signed-off-by: Gyuho Lee <[email protected]>

Gopkg.lock: initial commit

b5d5ce4

Signed-off-by: Gyuho Lee <[email protected]>

tools/functional-tester: update "build" script

b596c3c

Signed-off-by: Gyuho Lee <[email protected]>

scripts/updatebom.sh: remove "./cmd/vendor"

698ec7f

Signed-off-by: Gyuho Lee <[email protected]>

Documentation/dl_build: remove "./cmd/etcd"

ab24938

Signed-off-by: Gyuho Lee <[email protected]>

Documentation/op-guide: remove "./cmd/vendor" in monitoring.md

66d6a39

Signed-off-by: Gyuho Lee <[email protected]>

Dockerfile: remove "cmd/vendor"

c8e5311

Signed-off-by: Gyuho Lee <[email protected]>

test: remove "./cmd", "etcd_setup_gopath"

6eefc93

Signed-off-by: Gyuho Lee <[email protected]>

client,clientv3: remove "cmd/vendor" in README.md

97cd2a4

Signed-off-by: Gyuho Lee <[email protected]>

travis.yml: remove "./cmd"

ab7c319

Signed-off-by: Gyuho Lee <[email protected]>

build.ps1: remove "cmd/vendor"

a270a91

Signed-off-by: Gyuho Lee <[email protected]>

build: remove "./cmd/etcd,etcdctl"

4d06d32

Signed-off-by: Gyuho Lee <[email protected]>

gyuho added 2 commits January 24, 2018 15:26

tools/benchmark/cmd: fix fmt warning in import path

fcfbeb3

Signed-off-by: Gyuho Lee <[email protected]>

build: specify target with "${REPO_PATH}"

96a0392

Signed-off-by: Gyuho Lee <[email protected]>

gyuho force-pushed the dep branch from c16473f to 96a0392 Compare January 24, 2018 23:28

gyuho merged commit aadfb2b into etcd-io:master Jan 25, 2018

gyuho deleted the dep branch January 25, 2018 01:34

gyuho mentioned this pull request Jan 25, 2018

vendor: follow golang/dep best practice #9224

Closed

gyuho mentioned this pull request Jan 26, 2018

Gopkg.*: remove directive dependencies from Gopkg.toml #9240

Merged

menghanl mentioned this pull request Feb 22, 2018

Error happens when a program imports grpc more than twice grpc/grpc-go#566

Closed

jsha mentioned this pull request Jul 2, 2018

Tag a release with "dep" vendoring? #9898

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vendor: migrate to "golang/dep" #9155

vendor: migrate to "golang/dep" #9155

gyuho commented Jan 16, 2018

codecov-io commented Jan 17, 2018 •

edited

Loading

xiang90 Jan 17, 2018

gyuho Jan 17, 2018

gyuho commented Jan 17, 2018 •

edited

Loading

gyuho commented Jan 18, 2018 •

edited

Loading

fanminshi Jan 24, 2018

fanminshi Jan 24, 2018

fanminshi commented Jan 24, 2018

sdboyer commented Jan 25, 2018

gyuho commented Jan 25, 2018

tamird commented Jan 26, 2018

xiang90 commented Jan 26, 2018

gyuho commented Jan 26, 2018 •

edited

Loading

tamird commented Jan 26, 2018

gyuho commented Jan 26, 2018

sdboyer commented Jan 26, 2018

tamird commented Jan 26, 2018

sdboyer commented Jan 26, 2018

gyuho commented Jan 26, 2018 •

edited

Loading

tamird commented Jan 26, 2018

vendor: migrate to "golang/dep" #9155

vendor: migrate to "golang/dep" #9155

Conversation

gyuho commented Jan 16, 2018

codecov-io commented Jan 17, 2018 • edited Loading

Codecov Report

xiang90 Jan 17, 2018

Choose a reason for hiding this comment

gyuho Jan 17, 2018

Choose a reason for hiding this comment

gyuho commented Jan 17, 2018 • edited Loading

gyuho commented Jan 18, 2018 • edited Loading

fanminshi Jan 24, 2018

Choose a reason for hiding this comment

fanminshi Jan 24, 2018

Choose a reason for hiding this comment

fanminshi commented Jan 24, 2018

sdboyer commented Jan 25, 2018

gyuho commented Jan 25, 2018

tamird commented Jan 26, 2018

xiang90 commented Jan 26, 2018

gyuho commented Jan 26, 2018 • edited Loading

tamird commented Jan 26, 2018

gyuho commented Jan 26, 2018

sdboyer commented Jan 26, 2018

tamird commented Jan 26, 2018

sdboyer commented Jan 26, 2018

gyuho commented Jan 26, 2018 • edited Loading

tamird commented Jan 26, 2018

codecov-io commented Jan 17, 2018 •

edited

Loading

gyuho commented Jan 17, 2018 •

edited

Loading

gyuho commented Jan 18, 2018 •

edited

Loading

gyuho commented Jan 26, 2018 •

edited

Loading

gyuho commented Jan 26, 2018 •

edited

Loading