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 -m all' and 'go list -m ...' started to produce different results on tip #35728

Closed
dmitshur opened this issue Nov 21, 2019 · 7 comments
Assignees
Milestone

Comments

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Nov 21, 2019

With Go 1.13, both go list -m all and go list -m ... return the same results when executed outside of a module (i.e., when go env GOMOD reports os.DevNull):

$ cd $(mktemp -d)
$ export GO111MODULE=on
$ go version
go version go1.13.4 darwin/amd64
$ go list -m all
warning: pattern "all" matched no module dependencies
$ echo $?
0
$ go list -m ...
warning: pattern "..." matched no module dependencies
$ echo $?
0

On tip, they produce different results:

$ cd $(mktemp -d)
$ export GO111MODULE=on
$ gotip version
go version devel +39a9cb4b Wed Nov 20 22:38:34 2019 +0000 darwin/amd64
$ gotip list -m all
go: cannot match "all": working directory is not part of a module
$ echo $?
1
$ gotip list -m ...
warning: pattern "..." matched no module dependencies
$ echo $?
0

Is this intentional new behavior?

/cc @bcmills @jayconrod

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Nov 21, 2019

Change https://golang.org/cl/208258 mentions this issue: cmd/godoc: don't execute go list -m all when GOMOD is /dev/null

@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Nov 21, 2019

The distinction between the two is intentional. Per https://golang.org/cmd/go/#hdr-Package_lists_and_patterns:

When using modules, "all" expands to all packages in the main module and their dependencies, including dependencies needed by tests of any of those.

In contrast, ... matches all packages found in all active modules, regardless of whether those packages are dependencies of the main module, so it is somewhat more coherent without a “main module”.

@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Nov 21, 2019

Hmm, reading this more closely, I see that you're talking about module patterns rather than package patterns.

The package pattern ... makes sense without a main module, but the module pattern probably does not. We should probably reject wildcard arguments to go list -m uniformly when there is no main module.

@jayconrod jayconrod self-assigned this Nov 21, 2019
@dmitshur

This comment has been minimized.

Copy link
Member Author

@dmitshur dmitshur commented Nov 21, 2019

Yes, I am talking about listing modules specifically (the -m flag), not packages.

Relevant documentation

The arguments to list -m are interpreted as a list of modules, not packages. The main module is the module containing the current directory. The active modules are the main module and its dependencies. With no arguments, list -m shows the main module. With arguments, list -m shows the modules specified by the arguments. Any of the active modules can be specified by its module path. The special pattern "all" specifies all the active modules, first the main module and then dependencies sorted by module path. A pattern containing "..." specifies the active modules whose module paths match the pattern.

(Source: https://golang.org/cmd/go/#hdr-List_packages_or_modules.)

Would you mind elaborating on why go list -m all intentionally reports an error when GOMOD is /dev/null instead of producing empty output and a warning, as before? What's the motivation for making that change?

I'm noticing that doing go list -m and gotip list -m do not produce an error, despite there not being a main module:

# ... same setup as above
$ go list -m
command-line-arguments
$ echo $?
0
$ gotip list -m
command-line-arguments
$ echo $?
0

I'm asking because I'd like to understand this better. Thank you.

@jayconrod

This comment has been minimized.

Copy link
Contributor

@jayconrod jayconrod commented Nov 21, 2019

The main issue for this is #32027.

To summarize, a command run in module mode outside a module completes slowly when it needs to resolve a package path to a module, since it needs to find the latest version of that module. The command also isn't reproducible, since it may find a different version of the module every time. It's not a good user experience, especially for new users trying out modules by setting GO111MODULE=on globally.

A few different remedies were proposed in #32027. The one we went with was simply to make it an error to resolve an import path to a module. It's still possible to list or build packages in std and packages specified as sources on the command line, as long as they don't depend on anything outside std. go get also continues to work as it does in 1.13.

@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Nov 21, 2019

go list -m is somewhat useful outside of a module because it determines visibility rules for file arguments.

If I'm in module mode and I'm about to go build some_file.go, I know that that file can import $(go list -m)/internal, as well as the internal packages of any prefix of $(go list -m), and in that case I don't really need to care whether there is an actual main module or not.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Nov 21, 2019

Change https://golang.org/cl/208222 mentions this issue: cmd/go: report an error for 'go list -m ...' outside a module

@gopherbot gopherbot closed this in 2434869 Nov 21, 2019
gopherbot pushed a commit to golang/tools that referenced this issue Nov 21, 2019
When the GOMOD value is the operating system's null device, there
isn't a main module. Return an empty build list right away, since
running 'go list -m all' in Go 1.14 will cause a "cannot match "all":
working directory is not part of a module" error.

Fixes golang/go#35690
Updates golang/go#35728

Change-Id: I024ca3b7d774835140ce4a1625133aff6554a533
Reviewed-on: https://go-review.googlesource.com/c/tools/+/208258
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.