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

Comments

@dmitshur
Copy link
Member

commented Apr 29, 2019

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 this to the Unreleased milestone Apr 29, 2019

jirfag added a commit to golangci/tools that referenced this issue Jul 16, 2019
go/packages: allow types loading without NeedDeps
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
go/packages: allow types loading without NeedDeps
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

This comment has been minimized.

Copy link

commented Jul 16, 2019

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
go/packages: allow types loading without NeedDeps
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
go/packages: allow types loading without NeedDeps
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
go/packages: allow types loading without NeedDeps
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.