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

go/build: reduce redundancy in error messages from Import in module mode #34752

Closed
jayconrod opened this issue Oct 7, 2019 · 3 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

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

$ go version
go version devel +a0894ea5b5 Mon Oct 7 18:50:14 2019 +0000 darwin/amd64

Does this issue reproduce with the latest release?

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

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/jayconrod/Library/Caches/go-build"
GOENV="/Users/jayconrod/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/jayconrod/go"
GOPRIVATE=""
GOPROXY="off"
GOROOT="/Users/jayconrod/Code/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/jayconrod/Code/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/jayconrod/Code/test/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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/rq/x0692kqj6ml8cvrhcqh5bswc008xj1/T/go-build844872233=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

$ go mod init m
$ cat >hello.go <<EOF
package main

import (
	"fmt"
	"go/build"
	"os"
)

func main() {
	os.Setenv("GOPROXY", "off")
	_, err := build.Import("example.com/hello", ".", build.FindOnly)
	fmt.Println(err)
}
EOF
$ go run hello.go

What did you expect to see?

cannot find module providing package example.com/hello: module lookup disabled by GOPROXY=off

What did you see instead?

Package name is repeated three times, and some irrelevant implementation details are exposed.

go/build: importGo example.com/hello: exit status 1
can't load package: package example.com/hello: cannot find module providing package example.com/hello: module lookup disabled by GOPROXY=off


@jayconrod jayconrod added NeedsFix The path to resolution is known, but the work has not been done. GoCommand cmd/go modules labels Oct 7, 2019
@jayconrod jayconrod added this to the Go1.14 milestone Oct 7, 2019
@jayconrod jayconrod self-assigned this Oct 7, 2019
@jayconrod jayconrod changed the title go/build: reduce redundancy in error messages from Import go/build: reduce redundancy in error messages from Import in module mode Oct 7, 2019
@bcmills
Copy link
Member

bcmills commented Oct 8, 2019

I wrote up a related experience report on the general strategy for reducing redundancy. That strategy seemed to work out pretty well for the error messages we tried last cycle.

@gopherbot
Copy link

Change https://golang.org/cl/199840 mentions this issue: cmd/go: eliminate redundancy in import error messages

@gopherbot
Copy link

Change https://golang.org/cl/218817 mentions this issue: go/build: populate partial package information in importGo

gopherbot pushed a commit that referenced this issue Feb 12, 2020
This is a followup to CL 199840 and CL 203820. Cumulatively, they caused
a previously known bug to trigger more often while also nearly fixing it.

This change is a small fixup to CL 199840 that resolves the known bug
and prevents it from causing an additional regression in Go 1.14.

Part 1

The intention in CL 199840 was to return the same error that 'go list'
reported when the package wasn't located, so an early return was added.
However, to determine whether the package was located or not, p.Dir was
unintentionally checked instead of dir.

p is initialized to &Package{ImportPath: path} at top of Context.Import,
and its Dir field is never set before that line in importGo is reached.
So return errors.New(errStr) was always executed whenever errStr != "".

Originally, in CL 125296, the "go list" invocation did not include an
'-e' flag, so it would return a non-zero exit code on packages where
build constraints exclude all Go files, and importGo would return an
error like "go/build: importGo import/path: unexpected output: ...".

CL 199840 added an '-e' flag to the "go list" invocation, but checking
the wrong dir variable caused partial package information to never get
populated, and thus issue #31603 continued to occur, although with a
different error message (which ironically included the location of the
package that was supposedly "not found").

Now that the right dir is checked, issue #31603 is fixed.

Part 2

importGo checks whether it can use the go command to find the directory
of a package. In Go 1.13.x and earlier, one of the conditions to use the
go command was that the source directory must be provided.

CL 203820 made a change such that knowing the source directory was
no longer required:

	 // To invoke the go command,
	-// we must know the source directory,
	 // ...

That meant build.Import invocations where srcDir is the empty string:

	build.Import(path, "", build.FindOnly)

Started using the go command to find the directory of the package, and
started to run into issue #31603 as well. That's the #37153 regression.

Since this change fixes issue #31603, it also fixes issue #37153.

Part 3

There is one more thing. Delete the debugImportGo constant, it's unused.

Updates #26504 (CL 125296)
Updates #34752 (CL 199840)
Updates #34860 (CL 203820)
Fixes #31603
Fixes #37153

Change-Id: Iaa7dcc45ba0f708a978950c75fa4c836b87006f4
Reviewed-on: https://go-review.googlesource.com/c/go/+/218817
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@golang golang locked and limited conversation to collaborators Feb 9, 2021
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

5 participants