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

x/tools/go/packages: NeedImports doesn't populate Imports field when NeedDeps not set #31752

Closed
dmitshur opened this issue Apr 29, 2019 · 1 comment
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@dmitshur
Copy link
Contributor

Environment

$ go version
go version go1.12.4 darwin/amd64
go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/dmitshur/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/dmitshur/go"
GOPROXY="https://proxy.golang.org"
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/var/folders/3m/rg2zm24d1jg40wb48wr0hdjw00jwcj/T/tmp.JaaD6nLe/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/3m/rg2zm24d1jg40wb48wr0hdjw00jwcj/T/go-build560539665=/tmp/go-build -gno-record-gcc-switches -fno-common"

Issue

packages.NeedImports is documented as:

// NeedImports adds Imports. If NeedDeps is not set, the Imports field will contain
// "placeholder" Packages with only the ID set.
NeedImports

I ran the following command at the root of a module, inside that directory:

package main

import (
	"fmt"
	"log"

	"golang.org/x/tools/go/packages"
)

func main() {
	cfg := &packages.Config{
		//Mode: packages.NeedImports | packages.NeedDeps, // works
		Mode: packages.NeedImports, // doesn't work
	}
	pkgs, err := packages.Load(cfg, ".")
	if err != nil {
		log.Fatalln("packages.Load:", err)
	}
	if len(pkgs) != 1 {
		log.Fatalln("got something other than 1 package")
	}
	p := pkgs[0]
	if len(p.Errors) > 0 {
		log.Fatalln("p.Errors:", p.Errors)
	}
	fmt.Println("Imports:")
	for _, i := range p.Imports {
		fmt.Printf("  • %s\n", i.ID)
	}
	if len(p.Imports) == 0 {
		fmt.Println("  (none)")
	}
}

With this go.mod file:

module m

go 1.12

require golang.org/x/tools v0.0.0-20190429184909-35c670923e21

With packages.Config.Mode set to just packages.NeedImports, the Imports field is not populated at all:

Imports:
  (none)

With it set to packages.NeedImports | packages.NeedDeps, it works as expected:

Imports:
  • golang.org/x/tools/go/packages
  • log
  • fmt

/cc @ianthehat @matloob

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 29, 2019
@dmitshur dmitshur added this to the Unreleased milestone Apr 29, 2019
jirfag added a commit to golangci/tools that referenced this issue Jul 16, 2019
Before separating Load* into Need* we could use LoadSyntax
to get types information by loading inital packages
from source code and loading it's direct dependencies from export data.
It was broken when separation was introduced and before this commit
everything was loading from source code what resulted into slow
loading times. This commit fixes it.

Also, do backwards-incompatible fix of definition of deprecated
LoadImports and LoadAllSyntax.

Improve an internal error message
"internal error: nil Pkg importing x from y": replace it with
"internal error: package x without types was imported from y".

Remove packages.NeedDeps request for loading in tests
TestLoadTypesBits and TestContainsOverlayXTest.

Fixes golang/go#31752, fixes #golang/go#33077
jirfag added a commit to golangci/tools that referenced this issue Jul 16, 2019
Before separating Load* into Need* we could use LoadSyntax
to get types information by loading inital packages
from source code and loading it's direct dependencies from export data.
It was broken when separation was introduced and before this commit
everything was loading from source code what resulted into slow
loading times. This commit fixes it.

Also, do backwards-incompatible fix of definition of deprecated
LoadImports and LoadAllSyntax.

Improve an internal error message
"internal error: nil Pkg importing x from y": replace it with
"internal error: package x without types was imported from y".

Remove packages.NeedDeps request for loading in tests
TestLoadTypesBits and TestContainsOverlayXTest.

Fixes golang/go#31752, fixes golang/go#33077
@gopherbot
Copy link

Change https://golang.org/cl/186337 mentions this issue: go/packages: allow types loading without NeedDeps

jirfag added a commit to golangci/tools that referenced this issue Sep 9, 2019
Before separating Load* into Need* we could use LoadSyntax
to get types information by loading inital packages
from source code and loading it's direct dependencies from export data.
It was broken when separation was introduced and before this commit
everything was loading from source code what resulted into slow
loading times. This commit fixes it.

Also, do backwards-incompatible fix of definition of deprecated
LoadImports and LoadAllSyntax.

Improve an internal error message
"internal error: nil Pkg importing x from y": replace it with
"internal error: package x without types was imported from y".

Remove packages.NeedDeps request for loading in tests
TestLoadTypesBits and TestContainsOverlayXTest.

Fixes golang/go#31752, fixes golang/go#33077, fixes golang/go#32814,
      fixes golang/go#31699, golang/go#31930
jirfag added a commit to golangci/tools that referenced this issue Sep 9, 2019
Before separating Load* into Need* we could use LoadSyntax
to get types information by loading inital packages
from source code and loading it's direct dependencies from export data.
It was broken when separation was introduced and before this commit
everything was loading from source code what resulted into slow
loading times. This commit fixes it.

Also, do backwards-incompatible fix of definition of deprecated
LoadImports and LoadAllSyntax.

Improve an internal error message
"internal error: nil Pkg importing x from y": replace it with
"internal error: package x without types was imported from y".

Remove packages.NeedDeps request for loading in tests
TestLoadTypesBits and TestContainsOverlayXTest.

Fixes golang/go#31752, fixes golang/go#33077, fixes golang/go#32814,
      fixes golang/go#31699, fixes golang/go#31930
jirfag added a commit to golangci/tools that referenced this issue Sep 10, 2019
Before separating Load* into Need* we could use LoadSyntax
to get types information by loading inital packages
from source code and loading it's direct dependencies from export data.
It was broken when separation was introduced and before this commit
everything was loading from source code what resulted into slow
loading times. This commit fixes it.

Also, do backwards-incompatible fix of definition of deprecated
LoadImports and LoadAllSyntax.

Improve an internal error message
"internal error: nil Pkg importing x from y": replace it with
"internal error: package x without types was imported from y".

Remove packages.NeedDeps request for loading in tests
TestLoadTypesBits and TestContainsOverlayXTest.

Fixes golang/go#31752, fixes golang/go#33077, fixes golang/go#32814,
      fixes golang/go#31699, fixes golang/go#31930
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 17, 2019
clintjedwards pushed a commit to clintjedwards/tools that referenced this issue Sep 19, 2019
Before separating Load* into Need* we could use LoadSyntax
to get types information by loading inital packages
from source code and loading it's direct dependencies from export data.
It was broken when separation was introduced and before this commit
everything was loading from source code what resulted into slow
loading times. This commit fixes it.

Also, do backwards-incompatible fix of definition of deprecated
LoadImports and LoadAllSyntax.

Improve an internal error message
"internal error: nil Pkg importing x from y": replace it with
"internal error: package x without types was imported from y".

Remove packages.NeedDeps request for loading in tests
TestLoadTypesBits and TestContainsOverlayXTest.

Fixes golang/go#31752, fixes golang/go#33077, fixes golang/go#32814,
          fixes golang/go#31699, fixes golang/go#31930

Change-Id: I416e3c1035d555d67039e45a4fdd1deb9b2184ef
GitHub-Last-Rev: 2e3a46e
GitHub-Pull-Request: golang#139
Reviewed-on: https://go-review.googlesource.com/c/tools/+/186337
Reviewed-by: Michael Matloob <matloob@golang.org>
@golang golang locked and limited conversation to collaborators Sep 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants