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

vendor: regenerate glide config for glide 0.12.3 #11218

Merged
merged 1 commit into from
Nov 24, 2016
Merged

vendor: regenerate glide config for glide 0.12.3 #11218

merged 1 commit into from
Nov 24, 2016

Conversation

tamird
Copy link
Contributor

@tamird tamird commented Nov 24, 2016

Performed with:

rm glide.yaml && glide init && cat GLOCKFILE | cut -f2 -d' ' | grep -F vendor | sed -e s,./vendor/,, | xargs glide get --non-interactive

Manually pins risky packages:

  • github.com/cockroachdb/c-jemalloc
  • github.com/cockroachdb/c-rocksdb
  • google.golang.org/grpc & github.com/gogo/protobuf

Manually pins broken updates:

  • githu.com/docker/docker

This reveals serious problems with the dependency update flow under
glide. glide update completely removes the vendor directory, which
means that the .git directory and all non-source content (README.md)
are lost. The workflow needed to achieve a reasonable result is thus:

mv vendor/.git vendorgit && \
glide up && \
mv vendorgit vendor/.git && \
git -C vendor checkout README.md

This script is checked in as scripts/glide-up.sh.

Running git clean -dxn in vendor produces:

Would remove github.com/cockroachdb/c-snappy/internal/testdata/
Would remove github.com/docker/distribution/contrib/docker-integration/malevolent-certs/ca.pem
Would remove github.com/docker/distribution/contrib/docker-integration/nginx/ssl/
Would remove github.com/docker/distribution/contrib/docker-integration/tokenserver-oauth/certs/ca.pem
Would remove github.com/docker/distribution/contrib/docker-integration/tokenserver/certs/ca.pem
Would remove github.com/lightstep/lightstep-tracer-go/lightstep-tracer-common/

github.com/cockroachdb/c-snappy and github.com/docker/distribution
report ignored files because the repositories track files which are
ostensibly ignored by various .gitignores. This PR picks up
cockroachdb/c-snappy#10 which fixes this for
that package.

However, github.com/lightstep/lightstep-tracer-go reporting
github.com/lightstep/lightstep-tracer-go/lightstep-tracer-common as
ignored revealed a serious problem: this directory is a submodule, and
in our vendor checkout, it is empty. This suggests that glide did not
check out the submodule, despite that being the behaviour of go get
since https://golang.org/cl/9815.

glide update is also terribly slow, clocking in around 1m30s.

Finally, the move to glide of course breaks glock-diff-parser, which
means the list of upstream changes must again be compiled manually. The
changes are:

Note that glide also "updated" github.com/grpc-ecosystem/grpc-gateway
to an older version, losing an earlier fix
(grpc-ecosystem/grpc-gateway#256). My best
guess is that one of our dependencies uses this earlier version, likely
github.com/coreos/etcd. There seems to be no way to avoid this
behaviour.


This change is Reviewable

@dt
Copy link
Member

dt commented Nov 24, 2016 via email

@tamird
Copy link
Contributor Author

tamird commented Nov 24, 2016

We can, but I would prefer we did that only after we had an accepted upstream bug or fix in glide.

Performed with:
```
rm glide.yaml && glide init && cat GLOCKFILE | cut -f2 -d' ' | grep -F vendor | sed -e s,./vendor/,, | xargs glide get --non-interactive
```

Manually pins risky packages:
- github.com/cockroachdb/c-jemalloc
- github.com/cockroachdb/c-rocksdb
- google.golang.org/grpc & github.com/gogo/protobuf

Manually pins broken updates:
- githu.com/docker/docker

This reveals serious problems with the dependency update flow under
glide. `glide update` completely removes the vendor directory, which
means that the `.git` directory and all non-source content (README.md)
are lost. The workflow needed to achieve a reasonable result is thus:
```
mv vendor/.git vendorgit && \
glide up && \
mv vendorgit vendor/.git && \
git -C vendor checkout README.md
```

This script is checked in as `scripts/glide-up.sh`.

Running `git clean -dxn` in `vendor` produces:
```
Would remove github.com/cockroachdb/c-snappy/internal/testdata/
Would remove github.com/docker/distribution/contrib/docker-integration/malevolent-certs/ca.pem
Would remove github.com/docker/distribution/contrib/docker-integration/nginx/ssl/
Would remove github.com/docker/distribution/contrib/docker-integration/tokenserver-oauth/certs/ca.pem
Would remove github.com/docker/distribution/contrib/docker-integration/tokenserver/certs/ca.pem
Would remove github.com/lightstep/lightstep-tracer-go/lightstep-tracer-common/
```

`github.com/cockroachdb/c-snappy` and `github.com/docker/distribution`
report ignored files because the repositories track files which are
ostensibly ignored by various `.gitignore`s. This PR picks up
cockroachdb/c-snappy#10 which fixes this for
that package.

However, `github.com/lightstep/lightstep-tracer-go` reporting
`github.com/lightstep/lightstep-tracer-go/lightstep-tracer-common` as
ignored revealed a serious problem: this directory is a submodule, and
in our vendor checkout, it is empty. This suggests that `glide` did not
check out the submodule, despite that being the behaviour of `go get`
since https://golang.org/cl/9815.

`glide update` is also terribly slow, clocking in around 1m30s.

Finally, the move to glide of course breaks `glock-diff-parser`, which
means the list of upstream changes must again be compiled manually. The
changes are:
- github.com/tebeka/go2xunit:
  - tests ending in os.Exit() are now parsed: tebeka/go2xunit#42

Note that glide also "updated" `github.com/grpc-ecosystem/grpc-gateway`
to an older version, losing an earlier fix
(grpc-ecosystem/grpc-gateway#256). My best
guess is that one of our dependencies uses this earlier version, likely
`github.com/coreos/etcd`. There seems to be no way to avoid this
behaviour.
- package: github.com/cockroachdb/pq
- package: github.com/cockroachdb/stress
- package: github.com/codahale/hdrhistogram
- package: github.com/coreos/etcd
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like another one we should pin?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if there was a separate repo for raft, i'd agree, but this one is usually harmless to update since it rarely contains updates to raft.

@@ -45,45 +47,45 @@ imports:
- name: github.com/codahale/hdrhistogram
version: 3a0bb77429bd3a61596f5e8a3172445844342120
- name: github.com/coreos/etcd
version: aea9c6668fdc95fecacfabdc46e9c3f9b840ac94
version: c31b1ab8d18ff7990a43bd258ca54f80c5a3c952
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any cause for concern here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nah, nothing of interest changed.

@tamird tamird merged commit 6e4a9c2 into cockroachdb:master Nov 24, 2016
@tamird tamird deleted the update-glide branch November 24, 2016 13:55
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