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: list includes "C" in the Imports field but not in Deps #60453

Open
mvdan opened this issue May 26, 2023 · 0 comments
Open

cmd/go: list includes "C" in the Imports field but not in Deps #60453

mvdan opened this issue May 26, 2023 · 0 comments
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@mvdan
Copy link
Member

mvdan commented May 26, 2023

$ go version
go version devel go1.21-f90b4cd655 Fri May 26 03:21:41 2023 +0000 linux/amd64
$ go list -f '{{.Imports}}{{"\n"}}{{.Deps}}' runtime/cgo
[C runtime/internal/sys sync sync/atomic unsafe]
[internal/abi internal/bytealg internal/coverage/rtcov internal/cpu internal/goarch internal/godebugs internal/goexperiment internal/goos internal/race runtime runtime/internal/atomic runtime/internal/math runtime/internal/sys runtime/internal/syscall sync sync/atomic unsafe]

Note how runtime/cgo includes C in its Imports field, but not in its Deps field. 1.20 is the same in this regard:

$ go version
go version go1.20.4 linux/amd64
$ go list -f '{{.Imports}}{{"\n"}}{{.Deps}}' runtime/cgo
[C runtime/internal/sys sync sync/atomic unsafe]
[internal/abi internal/bytealg internal/coverage/rtcov internal/cpu internal/goarch internal/goexperiment internal/goos internal/race runtime runtime/internal/atomic runtime/internal/math runtime/internal/sys runtime/internal/syscall sync sync/atomic unsafe]

The docs say:

        Imports      []string          // import paths used by this package
        Deps         []string          // all (recursively) imported dependencies

So I always naively assumed Imports to be a strict subset of Deps, since one is the set of direct imports, and the other is the set of all direct and indirect package dependencies (transitive imports). However, C breaks this rule.

I think Imports should stop including C, and here are my reasons why:

  • Consistency; if Imports is not the list of direct dependencies, compared to the list of all transitive dependencies in Deps, then how do I get the list of direct dependencies? Right now I'm taking Imports and removing C from it, which feels hacky.
  • C is not truly a Go package nor a useful import path; see how go list C fails in a funky way (which arguably could be fixed to give a better error, too).
  • If a user or tool want to know whether a package uses CGo, they can already consult the CgoFiles field, which is clearer and more directly useful.
  • Deps never included it either, so that's some proof that it's not really needed as a package dependency.

The only reason why one could argue that Imports should include C is that there really is an import "C" in the source code. However, Imports is not a direct representation of the import declarations in the source code; they are deduplicated, sorted, and omit any renames like import foo "bar" or import _ "baz".

@seankhliao seankhliao added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels May 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

2 participants