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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/go: 'go get -d' does not report line numbers in errors for bad import statements #41688

Open
bcmills opened this issue Sep 29, 2020 · 8 comments
Open

cmd/go: 'go get -d' does not report line numbers in errors for bad import statements #41688

bcmills opened this issue Sep 29, 2020 · 8 comments
Labels
GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@bcmills
Copy link
Contributor

bcmills commented Sep 29, 2020

The fix for #41315 has revealed another variant of issue #34393.

Adding the following test case to mod_build_info_err.txt

+go get -d ./main
+stderr 'bad[/\\]bad.go:3:8: malformed module path "馃惂.example.com/string": invalid char ''馃惂'''

gives the failure mode of #41315 at release-branch.go1.15. At head, it gives a different, but still not quite right, failure mode:

+ go test cmd/go '-run=TestScript/^mod_build_info_err$'
go test proxy running at GOPROXY=http://127.0.0.1:44957/mod
--- FAIL: TestScript (0.01s)
    --- FAIL: TestScript/mod_build_info_err (0.05s)
        script_test.go:211:
            # This test verifies that line numbers are included in module import errors.
            # Verifies golang.org/issue/34393. (0.036s)
            > go list -e -deps -f '{{with .Error}}{{.Pos}}: {{.Err}}{{end}}' ./main
            [stdout]
            bad/bad.go:3:8: malformed import path "馃惂.example.com/string": invalid char '馃惂'
            > stdout 'bad[/\\]bad.go:3:8: malformed import path "馃惂.example.com/string": invalid char ''馃惂'''
            > go get -d ./main
            [stderr]
            m/main imports
                m/bad imports
                馃惂.example.com/string: import missing
            [exit status 1]
            FAIL: testdata/script/mod_build_info_err.txt:7: unexpected command failure

FAIL

Notably, the import stack for this error message is again bereft of line numbers (as in #34393).

CC @jayconrod @matloob @rogpeppe
@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Sep 29, 2020
@bcmills bcmills added this to the Backlog milestone Sep 29, 2020
@bcmills bcmills changed the title cmd/go: "go get" does not report line numbers in errors for bad import statements cmd/go: 'go get -d' does not report line numbers in errors for bad import statements Sep 29, 2020
@gopherbot
Copy link

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

@bcmills
Copy link
Contributor Author

bcmills commented Sep 29, 2020

I think this needs to be a blocker for 1.16: with -mod=readonly we're going to end up encouraging wider use of go get.

@bcmills bcmills modified the milestones: Backlog, Go1.16 Sep 29, 2020
@bcmills
Copy link
Contributor Author

bcmills commented Sep 29, 2020

(The import missing error message in the above trace is a separate bug in go get, which I plan to fix as part of #37438.)

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>
@toothrot
Copy link
Contributor

@bcmills What else is left to do on this before the release?

@cagedmantis
Copy link
Contributor

@bcmills Is there anything left to do on this before the release?

@bcmills
Copy link
Contributor Author

bcmills commented Oct 22, 2020

I'm not sure. I'm still in the process of overhauling the go get implementation for #36460.

@dmitshur dmitshur added the okay-after-beta1 Used by release team to mark a release-blocker issue as okay to resolve either before or after beta1 label Nov 19, 2020
@toothrot toothrot removed the okay-after-beta1 Used by release team to mark a release-blocker issue as okay to resolve either before or after beta1 label Dec 17, 2020
@bcmills
Copy link
Contributor Author

bcmills commented Jan 5, 2021

We should decide whether this is feasible to fix for the release, but if it turns out to be too invasive we can punt it to 1.17.

@jayconrod
Copy link
Contributor

Looked into this a bit. In go get, we invoke modload.LoadPackages with AllowErrors = true and SilenceErrors = false. This means the stacks are currently formatted by modload.loadPkg.stackText. I see two solutions:

  1. modload.loadPkg doesn't currently track locations of import declarations. We could modify imports.ScanDir to return that information.
  2. We could use load.PackagesAndErrors instead. That would give us error messages identical to those printed by go build, but it would apply build constraints, which we probably don't want.

I think it's viable to implement (1) before 1.16, but there are a lot of open release blockers that should fix first. This shouldn't actually block the release, and it's probably too big to go in after the RC.

@jayconrod jayconrod modified the milestones: Go1.16, Go1.17 Jan 5, 2021
@jayconrod jayconrod added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Jan 5, 2021
@bcmills bcmills self-assigned this Jan 6, 2021
@bcmills bcmills modified the milestones: Go1.17, Backlog Apr 28, 2021
@rsc rsc unassigned bcmills Jun 23, 2022
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
6 participants
@toothrot @cagedmantis @jayconrod @dmitshur @bcmills @gopherbot