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 list' in module mode reports GOPATH Root and Target fields when the main module is in GOPATH/src #37015

Closed
perillo opened this issue Feb 4, 2020 · 12 comments
Assignees
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@perillo
Copy link
Contributor

perillo commented Feb 4, 2020

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

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

Does this issue reproduce with the latest release?

Yes.

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

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN="/home/manlio/.local/bin"
GOCACHE="/home/manlio/.cache/go-build"
GOENV="/home/manlio/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY="github.com/perillo"
GONOSUMDB="github.com/perillo"
GOOS="linux"
GOPATH="/home/manlio/.local/lib/go:/home/manlio/src/go"
GOPRIVATE="github.com/perillo"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/manlio/sdk/go1.14beta1"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/manlio/sdk/go1.14beta1/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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-build892397541=/tmp/go-build -gno-record-gcc-switches"
GOROOT/bin/go version: go version go1.14beta1 linux/amd64
GOROOT/bin/go tool compile -V: compile version go1.14beta1
uname -sr: Linux 5.5.1-arch1-1
/usr/lib/libc.so.6: GNU C Library (GNU libc) stable release version 2.30.
gdb --version: GNU gdb (GDB) 8.3.1

What did you do?

From inside a module root directory, where pkg is a non main package.

go list -f 'Target: {{ .Target }}, Root: {{ .Root }}' ./pkg

What did you expect to see?

An empty Target, since with GO111MODULE=on the package archives are no more installed in $GOPATH.

What did you see instead?

Target: /home/manlio/src/go/pkg/linux_amd64/github.com/perillo/gocmd/pkglist.a, Root: /home/manlio/src/go

NOTE: if it is decided that Target will be left empty, I hope that Root will remain.

@bcmills
Copy link
Contributor

bcmills commented Feb 4, 2020

Hmm, interesting.

In a module root outside of GOPATH, I get:

example.com$ go1.14beta1 list -f '{{.ImportPath}}: {{.Target}} (from {{.Root}})' ./foo
example.com/foo:  (from /tmp/tmp.x68zurPJpH/example.com)

And go install is a no-op.

However, in a module contained within GOPATH, I get:

~/src/golang.org/x/mod$ go1.14beta1 list -f '{{.ImportPath}}: {{.Target}} {{.Root}}' ./zip
golang.org/x/mod/zip: /usr/local/google/home/bcmills/pkg/linux_amd64/golang.org/x/mod/zip.a /usr/local/google/home/bcmills

~/src/golang.org/x/mod$ ls ~/pkg/linux_amd64/golang.org/x/mod/zip.a
ls: cannot access '/usr/local/google/home/bcmills/pkg/linux_amd64/golang.org/x/mod/zip.a': No such file or directory

~/src/golang.org/x/mod$ go1.14beta1 install ./zip

~/src/golang.org/x/mod$ ls ~/pkg/linux_amd64/golang.org/x/mod/zip.a
/usr/local/google/home/bcmills/pkg/linux_amd64/golang.org/x/mod/zip.a

So it appears that we are still computing (and using) the GOPATH-mode Root and Target even in module mode. I suspect this is related to #34860 and/or #29111.

CC @jayconrod @matloob

@bcmills
Copy link
Contributor

bcmills commented Feb 4, 2020

The relevant branch seems to be here:

if IsLocalImport(path) {

p.Root = root

We probably should not be following the IsLocalImport branch in module mode at all.

@bcmills bcmills added modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 4, 2020
@bcmills bcmills added this to the Backlog milestone Feb 4, 2020
@bcmills bcmills changed the title cmd/go: go list still reports a non empty Target for a non main package in module mode cmd/go: 'go list' reports GOPATH Root and Target fields in module mode when the main module is in GOPATH/src Feb 4, 2020
@bcmills bcmills changed the title cmd/go: 'go list' reports GOPATH Root and Target fields in module mode when the main module is in GOPATH/src cmd/go: 'go list' in module mode reports GOPATH Root and Target fields when the main module is in GOPATH/src Feb 4, 2020
@gopherbot
Copy link

Change https://go.dev/cl/410822 mentions this issue: cmd/go: set Root and target fields for packages in GOPATH

@matloob
Copy link
Contributor

matloob commented Jun 7, 2022

I think we should follow the IsLocalImport branch in module mode, but skip the branches that check if the package exists in of the GOPATH src directories.

gopherbot pushed a commit that referenced this issue Jun 8, 2022
This change replicates the behavior filed in issue #37015 for packages
imported from the module index. That behavior is that packages that
happen to exist in a GOPATH src directory have p.Root and p.Target set
even when the packages are loaded from modules. This is likely
unintentional behavior because in module mode, packages shouldn't behave
differently depending on whether their directories exist in GOPATH. But
for uniformity, (and because two of our tests depend on this behavior),
this CL will implement this behavior. We can remove it from the module
index when we remove it from the go/build logic.

Change-Id: I3f501c92fbb76eaf86b6b9275539f2129b67f884
Reviewed-on: https://go-review.googlesource.com/c/go/+/410822
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
@gopherbot
Copy link

Change https://go.dev/cl/414055 mentions this issue: cmd/internal/archive: don't rely on an erroneous install target in tests

gopherbot pushed a commit that referenced this issue Jun 24, 2022
Non-main packages in module mode should not be installed to
GOPATH/pkg, but due to #37015 they were installed there anyway.
This change switches the 'go install' command to instead use
'go build -buildmode=archive' with an explicit archive path.

For #37015.

Change-Id: Ib0c8f213100b6473a7657af96f31395703e28493
Reviewed-on: https://go-review.googlesource.com/c/go/+/414055
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/416954 mentions this issue: misc/cgo/testcarchive: don't rely on an erroneous install target in tests

@gopherbot
Copy link

Change https://go.dev/cl/416955 mentions this issue: cmd/nm: don't rely on an erroneous install target in tests

gopherbot pushed a commit that referenced this issue Jul 11, 2022
…ests

Non-main packages in module mode should not be installed to
GOPATH/pkg, but due to #37015 they were installed there anyway.

This change switches the 'go install' command in TestPIE to instead
use 'go build', and switches TestInstall and TestCachedInstall
(which appear to be explicitly testing 'go install') to explicitly
request GOPATH mode (which does have a well-defined install target).

For #37015.

Change-Id: Ifb24657d2781d1e35cf40078e8e3ebf56aab9cc8
Reviewed-on: https://go-review.googlesource.com/c/go/+/416954
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
gopherbot pushed a commit that referenced this issue Jul 11, 2022
Non-main packages in module mode should not be installed to
GOPATH/pkg, but due to #37015 they were installed there anyway.
This change switches the 'go install' command in testGoLib to instead
use 'go build -buildmode=archive' with an explicit output file.

For #37015.

Change-Id: I15781aa33d1b2adc6a4437a58622276f4e20b889
Reviewed-on: https://go-review.googlesource.com/c/go/+/416955
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/417095 mentions this issue: cmd/go: in script tests, avoid checking non-main packages for staleness

@gopherbot
Copy link

Change https://go.dev/cl/417096 mentions this issue: misc/cgo/testcshared: don't rely on an erroneous install target in tests

@gopherbot
Copy link

Change https://go.dev/cl/417194 mentions this issue: misc/cgo/testshared: run tests only in GOPATH mode

gopherbot pushed a commit that referenced this issue Jul 12, 2022
Non-main packages in module mode should not be installed to
GOPATH/pkg, but due to #37015 they were installed there anyway.
Lacking a proper install location, 'go install' becomes a no-op
for non-main packages in module mode.

This change switches the 'go install' commands in the test_fuzz_cache
and build_overlay tests to instead use 'go build', using the '-x' flag
to check for compile commands instead of querying 'go list' about
staleness.

For #37015.

Change-Id: I56d80cf2a43efb6163c62082c86cd3e4f0ff73c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/417095
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
@bcmills bcmills modified the milestones: Backlog, Go1.20 Jul 12, 2022
@bcmills bcmills added early-in-cycle A change that should be done early in the 3 month dev cycle. NeedsFix The path to resolution is known, but the work has not been done. labels Jul 12, 2022
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 12, 2022
gopherbot pushed a commit that referenced this issue Jul 13, 2022
Non-main packages in module mode should not be installed to
GOPATH/pkg, but due to #37015 they were installed there anyway.

This change switches the 'go install' command in createHeaders to
instead use 'go build' (with an extension determined by the install
target for 'runtime/cgo', which is well-defined at least for the
moment), and switches TestCachedInstall (which appears to be
explicitly testing 'go install') to explicitly request GOPATH mode
(which provides a well-defined install target for the library).

This change follows a similar structure to CL 416954.

For #37015.

Change-Id: I22ae4af0f0d4c50adc9e0f0dc279859d1f258cc8
Reviewed-on: https://go-review.googlesource.com/c/go/+/417096
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
gopherbot pushed a commit that referenced this issue Jul 13, 2022
-buildmode=shared installs shared libraries into GOROOT
and expects to reuse them across builds.
Builds in module mode, however, each have their own set of
dependencies (determined by the module's requirements), so in general
cannot share dependencies with a single GOROOT.

Ideally in the long term we would like to eliminate -buildmode=shared
entirely (see #47788), but first we need a replacement for the subset
of use-cases where it still works today.

In the meantime, we should run these tests only in GOPATH mode.
Non-main packages in module mode should not be installed to
GOPATH/pkg, but due to #37015 they were installed there anyway,
and this test heavily relies on installing non-main packages.

For #37015.

Change-Id: I7c5d90b4075d6f33e3505d6a8f12752309ae5c03
Reviewed-on: https://go-review.googlesource.com/c/go/+/417194
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
Non-main packages in module mode should not be installed to
GOPATH/pkg, but due to golang#37015 they were installed there anyway.
This change switches the 'go install' command to instead use
'go build -buildmode=archive' with an explicit archive path.

For golang#37015.

Change-Id: Ib0c8f213100b6473a7657af96f31395703e28493
Reviewed-on: https://go-review.googlesource.com/c/go/+/414055
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
…ests

Non-main packages in module mode should not be installed to
GOPATH/pkg, but due to golang#37015 they were installed there anyway.

This change switches the 'go install' command in TestPIE to instead
use 'go build', and switches TestInstall and TestCachedInstall
(which appear to be explicitly testing 'go install') to explicitly
request GOPATH mode (which does have a well-defined install target).

For golang#37015.

Change-Id: Ifb24657d2781d1e35cf40078e8e3ebf56aab9cc8
Reviewed-on: https://go-review.googlesource.com/c/go/+/416954
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
Non-main packages in module mode should not be installed to
GOPATH/pkg, but due to golang#37015 they were installed there anyway.
This change switches the 'go install' command in testGoLib to instead
use 'go build -buildmode=archive' with an explicit output file.

For golang#37015.

Change-Id: I15781aa33d1b2adc6a4437a58622276f4e20b889
Reviewed-on: https://go-review.googlesource.com/c/go/+/416955
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
Non-main packages in module mode should not be installed to
GOPATH/pkg, but due to golang#37015 they were installed there anyway.
Lacking a proper install location, 'go install' becomes a no-op
for non-main packages in module mode.

This change switches the 'go install' commands in the test_fuzz_cache
and build_overlay tests to instead use 'go build', using the '-x' flag
to check for compile commands instead of querying 'go list' about
staleness.

For golang#37015.

Change-Id: I56d80cf2a43efb6163c62082c86cd3e4f0ff73c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/417095
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
Non-main packages in module mode should not be installed to
GOPATH/pkg, but due to golang#37015 they were installed there anyway.

This change switches the 'go install' command in createHeaders to
instead use 'go build' (with an extension determined by the install
target for 'runtime/cgo', which is well-defined at least for the
moment), and switches TestCachedInstall (which appears to be
explicitly testing 'go install') to explicitly request GOPATH mode
(which provides a well-defined install target for the library).

This change follows a similar structure to CL 416954.

For golang#37015.

Change-Id: I22ae4af0f0d4c50adc9e0f0dc279859d1f258cc8
Reviewed-on: https://go-review.googlesource.com/c/go/+/417096
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
-buildmode=shared installs shared libraries into GOROOT
and expects to reuse them across builds.
Builds in module mode, however, each have their own set of
dependencies (determined by the module's requirements), so in general
cannot share dependencies with a single GOROOT.

Ideally in the long term we would like to eliminate -buildmode=shared
entirely (see golang#47788), but first we need a replacement for the subset
of use-cases where it still works today.

In the meantime, we should run these tests only in GOPATH mode.
Non-main packages in module mode should not be installed to
GOPATH/pkg, but due to golang#37015 they were installed there anyway,
and this test heavily relies on installing non-main packages.

For golang#37015.

Change-Id: I7c5d90b4075d6f33e3505d6a8f12752309ae5c03
Reviewed-on: https://go-review.googlesource.com/c/go/+/417194
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
@gopherbot
Copy link

This issue is currently labeled as early-in-cycle for Go 1.20.
That time is now, so a friendly reminder to look at it again.

@gopherbot
Copy link

Change https://go.dev/cl/412476 mentions this issue: cmd/go: clear GOPATH from build context when importing from module

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants