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: print import stack when reporting error constructing build list #30661

Closed
jayconrod opened this issue Mar 7, 2019 · 4 comments
Closed
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@jayconrod
Copy link
Contributor

In module mode, every build starts by constructing the build list, the set of modules at specific versions that will be involved in the build. We construct the build list by exploring the import graph rooted at the command line arguments, adding modules to satisfy any imports that can't be resolved.

There are a number of things that can go wrong during this process, not all of which are under the user's direct control. For example, suppose a transitive dependency requires github.com/golang/lint, but the go.mod in that repository declares the module path golang.org/x/lint. The package importing github.com/golang/lint is wrong, but we don't currently report what that package is or why it's part of the build.

When an error like this occurs, we should report the chain of imports from the command line arguments to the problematic package.

@jayconrod jayconrod added NeedsFix The path to resolution is known, but the work has not been done. GoCommand cmd/go modules labels Mar 7, 2019
@jayconrod jayconrod added this to the Go1.13 milestone Mar 7, 2019
@jayconrod jayconrod self-assigned this Mar 7, 2019
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/166984 mentions this issue: cmd/go: print require chains in build list errors

@thepudds
Copy link
Contributor

thepudds commented Apr 8, 2019

@jayconrod I had this question when ran this for the first time.

If you see an error like this:

go get: github.com/Sirupsen/[email protected] ->
        github.com/Sirupsen/[email protected]: parsing go.mod: unexpected module path "github.com/sirupsen/logrus"

First, does the first transition there represent an upgrade? If so, I didn't understand that at first, and at first I thought that -> represented a dependency given that is what it means later in bigger errors. If that does represent an upgrade, maybe it could help to do something like replacing the -> with upgraded to, or some other English phrase?

Second, I think the main module is implied? If so, maybe the "get" could be eliminated and add in the main module as the first line?

I don't think this is quite perfect, but maybe something like:

go:  github.com/my/module ->
     github.com/Sirupsen/[email protected]: upgraded to
     github.com/Sirupsen/[email protected]: parsing go.mod: unexpected module path "github.com/sirupsen/logrus"

CC @mfridman

@jayconrod
Copy link
Contributor Author

@thepudds "upgraded to" is probably the right phrasing. @heschik also asked me about this today, too. I'll change it.

go get: is a prefix of all the error messages for that command, so I think that should be left alone.

About the main module, unfortunately, that's a difficult change to make because we don't know whether requirements are direct or transitive when that error comes up. go get -u constructs the regular build list, adds any named arguments with specific versions, then constructs a new build list from that list where we also upgrade everything. Most of these error come up in that second list, but we've already thrown away the original module graph at that point, so we can't trace a path from the main module to a transitive requirement. All transitive requirements look like direct requirements.

We should refactor this. It will be a complicated change though.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/171150 mentions this issue: cmd/go: describe dependencies in build list error messages

gopherbot pushed a commit that referenced this issue Apr 16, 2019
mvs.BuildList reports errors with a chain of modules to make it
clear why the module where the error occurred was part of the
build. This is a little confusing with "go get -u" since there are
edges in the module graph for requirements and for updates.

With this change, we now print "requires" or "updates to" between
each module version in the chain.

Updates #30661

Change-Id: Ie689500ea86857e715b250b9e0cae0bc6686dc32
Reviewed-on: https://go-review.googlesource.com/c/go/+/171150
Run-TryBot: Jay Conrod <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Bryan C. Mills <[email protected]>
@golang golang locked and limited conversation to collaborators Apr 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants