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

Jump to bottom

cmd/go: go list -e does not assign missing import errors to correct package #26909

Open
rsc opened this issue Aug 10, 2018 · 12 comments
Open

cmd/go: go list -e does not assign missing import errors to correct package #26909

rsc opened this issue Aug 10, 2018 · 12 comments
Labels
GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@rsc
Copy link
Contributor

rsc commented Aug 10, 2018

See TODO in testdata/script/mod_lists_bad_import.txt.
@rsc rsc added this to the Go1.11 milestone Aug 10, 2018
@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 10, 2018
@rsc rsc mentioned this issue Aug 10, 2018
Open
@rsc
Copy link
Contributor Author

rsc commented Aug 17, 2018

It's complicated. The current behavior might even be almost correct. Leaving for Go 1.12.

@rsc rsc modified the milestones: Go1.11, Go1.12 Aug 17, 2018
@rsc rsc added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. and removed NeedsFix The path to resolution is known, but the work has not been done. labels Aug 17, 2018
@bcmills bcmills added the GoCommand cmd/go label Nov 14, 2018
@rsc rsc modified the milestones: Go1.12, Go1.13 Nov 20, 2018
@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@bcmills bcmills mentioned this issue Sep 13, 2019
Closed
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@jayconrod
Copy link
Contributor

@bcmills, @matloob, and I discussed this last week in the context of #36188. There's currently some ambiguity about whether import errors should be attached to the importing package or the imported package in go list -e -json output.

Here's what we came up with:

  • If the package being imported cannot be built, for example, because it doesn't exist or has no .go files, the error should be attached to the imported package. The error may still have a position in a file in the importing package. If multiple packages import an unbuildable package with the same path, multiple packages with errors should be listed.
  • If the package can be built but the importing package is not allowed to import it, for example because it's an internal package, a main package, or has a different canonical path, the error should be attached to the importing package, and the imported package should be listed normally without error. If the importing package has multiple import errors, we'll only include the first one in go list -e -json output.

@jayconrod jayconrod modified the milestones: Backlog, Go1.15 Feb 4, 2020
@bcmills
Copy link
Contributor

bcmills commented Feb 13, 2020 •
edited

If the package being imported cannot be built, … the error may still have a position in a file in the importing package. If multiple packages import an unbuildable package with the same path, multiple packages with errors should be listed.

I don't think that's quite right: if the error for a given package is not dependent on the importer, the Error field of the go list -e -json entry for that package should not itself include information about importers. (However, the information in the DepsErrors field of each of those importers should populate the Pos field with the position of the corresponding import line.)

@bcmills
Copy link
Contributor

bcmills commented Feb 13, 2020

If the package can be built but the importing package is not allowed to import it, … the error should be attached to the importing package, and the imported package should be listed normally without error.

The details of this part are a bit subtle. I think in that case the error should be encoded in the Error field for the importing package (rather than the DepsErrors field), because the error did not strictly occur in the course of “loading” the dependency.

@bcmills
Copy link
Contributor

bcmills commented May 7, 2020

@matloob, is this fixed by your recent import-stack CL?

I think the relevant TODO is here:

go/src/cmd/go/testdata/script/mod_list_bad_import.txt

Lines 55 to 58 in 6f52790

# TODO: go list creates a dummy package with the import-not-found
# but really the Error belongs on example.com/direct, and this package
# should not be printed.
# ! stdout example.com/notfound

@matloob
Copy link
Contributor

matloob commented May 26, 2020

I just checked, and this isn't fixed at head. I'll take a more in-depth look tomorrow.

@gopherbot
Copy link

Change https://golang.org/cl/235925 mentions this issue: cmd/go: assign missing import errors to importing package

@ianlancetaylor
Copy link
Contributor

There is a CL out for this issue but nobody has reviewed it. Is this still expected for 1.15, or should we push it to 1.16? Thanks.

@bcmills
Copy link
Contributor

bcmills commented Jun 16, 2020

Michael tagged the CL as wait-release shortly after it was mailed, so looks like 1.16.

@bcmills bcmills modified the milestones: Go1.15, Go1.16 Jun 16, 2020
@jayconrod jayconrod added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Aug 6, 2020
@gopherbot
Copy link

Change https://golang.org/cl/251445 mentions this issue: cmd/go/internal/modload: rework import resolution

gopherbot pushed a commit that referenced this issue Sep 9, 2020
cmd/go/internal/modload: rework import resolution
015a5a5 
modload.Import previously performed two otherwise-separable tasks:

1. Identify which module in the build list contains the requested
   package.

2. If no such module exists, search available modules to try to find
   the missing package.

This change splits those two tasks into two separate unexported
functions, and reports import-resolution errors by attaching them to
the package rather than emitting them directly to stderr. That allows
'list' to report the errors, but 'list -e' to ignore them.

With the two tasks now separate, it will be easier to avoid the
overhead of resolving missing packages during lazy loading if we
discover that some existing dependency needs to be promoted to the top
level (potentially altering the main module's selected versions, and
thus suppling packages that were previously missing).

For #36460
Updates #26909

Change-Id: I32bd853b266d7cd231d1f45f92b0650d95c4bcbd
Reviewed-on: https://go-review.googlesource.com/c/go/+/251445
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/258219 mentions this issue: cmd/go: test more commands in mod_build_info_error

gopherbot pushed a commit that referenced this issue Sep 30, 2020
cmd/go: test more commands in mod_build_info_error
f811663 
For #26909
For #41688

Change-Id: I22f28d426ce499fce6f0f1295dbde425998042aa
Reviewed-on: https://go-review.googlesource.com/c/go/+/258219
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
@jayconrod jayconrod modified the milestones: Go1.16, Go1.17 Jan 7, 2021
@ianlancetaylor
Copy link
Contributor

Moving to Backlog.

@ianlancetaylor ianlancetaylor modified the milestones: Go1.17, Backlog Apr 19, 2021
@bcmills bcmills mentioned this issue Jan 30, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
7 participants
@rsc @jayconrod @andybons @ianlancetaylor @bcmills @gopherbot @matloob