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

cmd/go: confusing output from go get #37982

Closed
rsc opened this issue Mar 21, 2020 · 14 comments
Closed

cmd/go: confusing output from go get #37982

rsc opened this issue Mar 21, 2020 · 14 comments
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@rsc
Copy link
Contributor

rsc commented Mar 21, 2020

I've never seen the upgrade => output before:

$ go get github.com/markbates/pkger
go: downloading github.com/markbates/pkger v0.15.0
go: github.com/markbates/pkger upgrade => v0.15.0
go: downloading github.com/gobuffalo/here v0.6.0
$ 

I understand why it is there, but it probably should not be mixed into all the other downloading prints. If it's necessary to print the changes being made, print them all at the end. Otherwise it is too easily missed.

@rsc rsc added this to the Go1.15 milestone Mar 21, 2020
@rsc
Copy link
Contributor Author

rsc commented Mar 21, 2020

/cc @jayconrod @bcmills

@dmitshur dmitshur added GoCommand cmd/go modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 21, 2020
@bcmills
Copy link
Contributor

bcmills commented Apr 2, 2020

The upgrade there means “I resolved the version query upgrade to version v0.15.0”, not “I upgraded this module to v0.15.0.”

Perhaps we should omit the version query entirely for the queries upgrade and patch..?

@jayconrod
Copy link
Contributor

I wonder if we should print a list of all the versions that changed and why?

At the moment, each of these message is shown when go get resolves a query. That may be confusing in situations where we resolve a query multiple times to different modules, for example, when a package named on the command line moves to a module that is a prefix of its old module. This also doesn't cover indirect upgrades or downgrades due to MVS.

That feels a bit chatty though. If we print all changes, it should probably be behind a flag.

@bcmills
Copy link
Contributor

bcmills commented Apr 3, 2020

I wonder if we should print a list of all the versions that changed and why?

I would say that we should at least log all of the versions of things actually imported by the main module (see #33284). However, we probably should not bother logging the versions of modules that aren't relevant (see #36460).

@jayconrod
Copy link
Contributor

For Go1.15 (now that the freeze has started), we should at least group these messages as the end of go get output (after "downloading" messages, but before any build-related messages).

I would say that we should at least log all of the versions of things actually imported by the main module (see #33284). However, we probably should not bother logging the versions of modules that aren't relevant (see #36460).

Perhaps this can be simplified to just logging changes to requirements go.mod? Those should generally just be relevant things. Even if they aren't relevant, we should probably log explicitly requested modules (for example tools installed with go get, not imported).

@bcmills
Copy link
Contributor

bcmills commented May 4, 2020

Perhaps this can be simplified to just logging changes to requirements [in] go.mod?

Relevant requirements might not be explicit in the go.mod file.

Maybe we should just put an @ symbol in between the module path and the query, so that it is clear that upgrade is the requested version (rather than a verb).

@jayconrod
Copy link
Contributor

Relevant requirements might not be explicit in the go.mod file.

How about modules providing packages listed on the command line (and their dependencies if -u is given) plus changes to requirements in go.mod? That would be an interesting set without being overwhelming.

Maybe we should just put an @ symbol in between the module path and the query, so that it is clear that upgrade is the requested version (rather than a verb).

I think it would be more useful to list the old version so it's clear whether any change has occurred. Showing upgrade might be confusing unless the query was given explicitly.

$ go get -d example.com/a example.com/b@latest
go get: example.com/a v1.4.0 => v1.5.0
go get: example.com/b v1.2.0 => v1.2.0 (latest)

Another problem here is that we're only printing messages for resolved queries. We don't print anything for removed modules and downgrades.

@bcmills
Copy link
Contributor

bcmills commented May 4, 2020

I think it would be more useful to list the old version so it's clear whether any change has occurred.

Another problem here is that we're only printing messages for resolved queries. We don't print anything for removed modules and downgrades.

Agreed; in general that's #33284.

@bcmills
Copy link
Contributor

bcmills commented May 4, 2020

How about modules providing packages listed on the command line (and their dependencies if -u is given) plus changes to requirements in go.mod? That would be an interesting set without being overwhelming.

That seems fine to do once the requirements in go.mod are relevant, and once all of the relevant dependencies are in go.mod (#36460). Until then, I would prefer to avoid ad-hoc heuristics: part of the point of #33284 is that we should surface “surprising” upgrades implied by the requested upgrades.

For example: if I upgrade A which imports B, and B's requirements upgrade D but B does not use D, and the main module happens to elsewhere import package C which does use D, then the upgrade to D is “interesting” even if I only ran go get -u A, even though A does not import D and D does not appear explicitly in the go.mod file.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/232578 mentions this issue: cmd/go: group 'go get' update messages together near the end of output

@aclements
Copy link
Member

aclements commented May 31, 2020

CL 232578 has been reverted by CL 235857. Reopening.

@aclements aclements reopened this May 31, 2020
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/236165 mentions this issue: cmd/go: group 'go get' update messages together near the end of output

@ianlancetaylor
Copy link
Contributor

@jayconrod Are we going to try to fix this in 1.15 or should we roll it forward to 1.16? Thanks.

@jayconrod jayconrod modified the milestones: Go1.15, Go1.16 Jun 26, 2020
@bcmills
Copy link
Contributor

bcmills commented Nov 5, 2020

This printing was refactored in CL 263267 and the confusing => token was removed. I think this is now fixed.

Separately, we should still consider printing a summary of changes to the versions of relevant modules, grouped together at the end of go get. (That's #33284.)

@bcmills bcmills closed this as completed Nov 5, 2020
@golang golang locked and limited conversation to collaborators Nov 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

7 participants