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: go:embed errors attributed to the wrong file #43469

Closed
zombiezen opened this issue Jan 2, 2021 · 9 comments
Closed

cmd/go: go:embed errors attributed to the wrong file #43469

zombiezen opened this issue Jan 2, 2021 · 9 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone

Comments

@zombiezen
Copy link
Contributor

What version of Go are you using (go version)?

$ go version
go version go1.16beta1 linux/amd64

Does this issue reproduce with the latest release?

Haven't tried on tip.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/light/.cache/go-build"
GOENV="/home/light/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/light/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/light"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/light/sdk/go1.16beta1"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/light/sdk/go1.16beta1/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16beta1"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/light/src/foo/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build4010338713=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Run go build on:

https://play.golang.org/p/HR-SdE2vE8c

What did you expect to see?

bar/bar.go:5:1: pattern xxx.txt: no matching files found

What did you see instead?

The line number is for the import of the package with the go:embed directive:

foo.go:5:2: pattern xxx.txt: no matching files found
@zombiezen
Copy link
Contributor Author

Further, running go build ./bar outputs no line numbers:

pattern xxx.txt: no matching files found

I would expect to see the same output as what I expected above.

@zombiezen zombiezen changed the title cmd/go: line numbers for go:embed errors are not on the directive cmd/go: line numbers for go:embed errors do not indicate the incorrect directive Jan 2, 2021
@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 5, 2021
@toothrot toothrot added this to the Backlog milestone Jan 5, 2021
@toothrot toothrot changed the title cmd/go: line numbers for go:embed errors do not indicate the incorrect directive cmd/go: line numbers for go:embed errors do not indicate the correct directive Jan 5, 2021
@toothrot
Copy link
Contributor

toothrot commented Jan 5, 2021

/cc @rsc @bcmills @jayconrod @matloob

@bcmills bcmills modified the milestones: Backlog, Go1.16 Jan 5, 2021
@bcmills bcmills changed the title cmd/go: line numbers for go:embed errors do not indicate the correct directive cmd/go: go:embed errors attributed to the wrong file Jan 5, 2021
@bcmills
Copy link
Contributor

bcmills commented Jan 5, 2021

Looks like there are no cmd/go tests for the no matching files found and invalid pattern syntax error messages, and the existing tests for the adjacent pattern …: cannot embed … errors lack anchors — so they aren't testing for the correct file or line numbers. 😞

@bcmills
Copy link
Contributor

bcmills commented Jan 5, 2021

This is a release blocker for 1.16: go:embed is new in 1.16, and misattributing the file may mean that we're also misattributing the module, which could get very confusing for transitive errors in modules deep in the module cache.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/280452 mentions this issue: cmd/go: allow go fmt to complete when embedded file is missing

@jayconrod
Copy link
Contributor

The fix for this looks pretty small. We just need to resolve embeds a little lower in Package.load, after we push the path onto the import stack. That distinguishes errors in the importing and imported packages.

I'll wait a bit to send a fix until we decide how to resolve #43632, since that will affect tests.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/283638 mentions this issue: cmd/go/internal/load: report positions for embed errors

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/283636 mentions this issue: go/build: report positions for go:embed directives

gopherbot pushed a commit that referenced this issue Jan 14, 2021
For #43469
For #43632

Change-Id: I9ac2da690344935da0e1dbe00b134dfcee65ec8a
Reviewed-on: https://go-review.googlesource.com/c/go/+/283636
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/285213 mentions this issue: doc/go1.16: mention go/build changes

gopherbot pushed a commit that referenced this issue Jan 25, 2021
For #40070
For #41191
For #43469
For #43632

Change-Id: I6dc6b6ea0f35876a4c252e4e287a0280aca9d502
Reviewed-on: https://go-review.googlesource.com/c/go/+/285213
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@golang golang locked and limited conversation to collaborators Jan 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
None yet
Development

No branches or pull requests

6 participants
@toothrot @jayconrod @zombiezen @bcmills @gopherbot and others