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: dependencies that become unused are left over #47363

Closed
andreimatei opened this issue Apr 10, 2020 · 3 comments
Closed

vendor: dependencies that become unused are left over #47363

andreimatei opened this issue Apr 10, 2020 · 3 comments
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) no-issue-activity T-dev-inf

Comments

@andreimatei
Copy link
Contributor

andreimatei commented Apr 10, 2020

If the last import of a vendored dependency goes away, nothing forces the commit in question to remove the dependency from the vendored repo. Worse, the next sucker who runs dep ensure to muck with some other dependency will have the surprise of seeing the useless dependency removed at that point, unexpectedly. Which is very confusing, and a pain to figure out why and how.

We have a check that TC is supposed to do to ensure that the vendored repo is copacetic, but it fails to catch this case. The check in question only runs when dependencies change, not when our imports change:

for _, path := range []string{"Gopkg.lock", "vendor"} {

This has just happened when @otan deleted some imports here. @dt helped me figure it out.

I think we should run a dep ensure nightly to catch this.

Epic CRDB-10447

Jira issue: CRDB-5015

@andreimatei andreimatei added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Apr 10, 2020
andreimatei added a commit to cockroachdb/vendored that referenced this issue Apr 11, 2020
The stopped being needed after
cockroachdb/cockroach#47171 but we didn't catch
it because of cockroachdb/cockroach#47363
andreimatei added a commit to cockroachdb/vendored that referenced this issue Apr 11, 2020
The deps stopped being needed after
cockroachdb/cockroach#47171 but we didn't catch
the leak because of
cockroachdb/cockroach#47363
andreimatei added a commit to cockroachdb/vendored that referenced this issue Apr 11, 2020
The deps stopped being needed after
cockroachdb/cockroach#47171 but we didn't catch
the leak because of
cockroachdb/cockroach#47363
andreimatei added a commit to andreimatei/cockroach that referenced this issue Apr 11, 2020
The deps stopped being needed after
cockroachdb#47171 but we didn't catch
the leak because of
cockroachdb#47363

Release note: None
otan added a commit to otan-cockroach/cockroach that referenced this issue Apr 11, 2020
This solves the "doofus who has to deps/ensure next" issue.

Refs: cockroachdb#47363

Release note: None
craig bot pushed a commit that referenced this issue Apr 11, 2020
47367: geo: restore blank imports r=petermattis a=otan

This solves the "doofus who has to deps/ensure next" issue.

Refs: #47363

Release note: None

Co-authored-by: Oliver Tan <[email protected]>
@bobvawter
Copy link
Member

This would be obviated by switching the build to go modules per #38824

@bobvawter bobvawter removed their assignment May 27, 2020
@github-actions
Copy link

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
10 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

@rickystewart
Copy link
Collaborator

This should be resolved by go mod tidy, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) no-issue-activity T-dev-inf
Projects
None yet
Development

No branches or pull requests

4 participants