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.

Sign up for GitHub

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

Jump to bottom

x/tools/go/loader: when loading >1 initial package in ImportBinary mode, some dependencies are loaded twice #9955

Closed
rsc opened this issue Feb 21, 2015 · 7 comments
Closed

x/tools/go/loader: when loading >1 initial package in ImportBinary mode, some dependencies are loaded twice #9955

rsc opened this issue Feb 21, 2015 · 7 comments
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@rsc
Copy link
Contributor

rsc commented Feb 21, 2015

This is breaking a code cleanup tool I am building.

g% cat /tmp/x.go
package main

import (
    "fmt"
    "log"

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

func main() {
    var conf loader.Config
    _, err := conf.FromArgs([]string{"cmd/nm", "cmd/pprof", "debug/gosym"}, false)
    if err != nil {
        log.Fatal(err)
    }
    _, err = conf.Load()
    if err != nil {
        log.Fatal(err)
    }
    fmt.Printf("OK\n")
}

g% go run /tmp/x.go
/Users/rsc/g/go/src/cmd/pprof/pprof.go:196:12: cannot assign pcln (variable of type *debug/gosym.Table) to f.pcln (variable of type *debug/gosym.Table)
2015/02/21 13:25:20 couldn't load packages due to errors: cmd/pprof
exit status 1
g%
@rsc
Copy link
Contributor Author

rsc commented Feb 21, 2015

@alandonovan @griesemer Not sure if this is go/loader or go/types. Seems to be confused and thinks there are (at least) two debug/gosym packages.

@alandonovan alandonovan self-assigned this Feb 21, 2015
@alandonovan
Copy link
Contributor

I can reproduce it although I'm not certain what the cause is yet.

You almost certainly want this:
conf := loader.Config{SourceImports: true}
which is an effective workaround, and should probably be the default but is not for historical reasons; otherwise it will attempt to load gc's export data for all imported packages, which may or may not be up-to-date. I suspect because these packages are mains not libraries that export data is rarely installed.

@mikioh mikioh changed the title golang.org/x/tools/go/loader: cannot load cmd/nm cmd/pprof debug/gosym go/loader: cannot load cmd/nm cmd/pprof debug/gosym Feb 21, 2015
@alandonovan
Copy link
Contributor

When two or more packages are specified on the command line, and !SourceImports is indicated (i.e. load dependencies from gc export data), and the transitive closure of one initial package A (e.g. cmd/pprof) includes another initial package B (e.g. debug/gosym), then B is created twice: once when loading it from binary to satisfy A, and again from source for B.

This isn't going to be a simple fix and the design is already so complex, and this case wasn't even the intended use, that I'm not certain it's worth fixing. We could just restrict to a single import when using !SourceImports. (The only tool I know of that ever wants to load gc export data is llgo, and it doesn't load multiple packages at once.)

@alandonovan alandonovan changed the title go/loader: cannot load cmd/nm cmd/pprof debug/gosym go/loader: when loading >1 initial package in ImportBinary mode, some dependencies are loaded twice Mar 17, 2015
@rsc rsc added this to the Unplanned milestone Apr 10, 2015
@rsc rsc changed the title go/loader: when loading >1 initial package in ImportBinary mode, some dependencies are loaded twice x/tools/go/loader: when loading >1 initial package in ImportBinary mode, some dependencies are loaded twice Apr 14, 2015
@rsc rsc modified the milestones: Unreleased, Unplanned Apr 14, 2015
@rsc rsc removed the repo-tools label Apr 14, 2015
@dmitshur
Copy link
Contributor

This should be closed now. It was fixed in golang/tools@9c57c19, but the commit message mistakenly said Fixes #9955 rather than Fixes golang/go#9955, which prevented this issue from being closed automatically.

@alandonovan
Copy link
Contributor

Oops, thanks.

@josharian
Copy link
Contributor

FYI, @alandonovan, if you update to the latest version of git-codereview, it'll fix issue references for you when you save the commit message.

@alandonovan
Copy link
Contributor

On 17 April 2015 at 15:46, Josh Bleecher Snyder notifications@github.com
wrote:

FYI, @alandonovan https://github.com/alandonovan, if you update to the
latest version of git-codereview, it'll fix issue references for you when
you save the commit message.

Ah, that explains it. Thanks.

@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
6 participants
@josharian @mikioh @rsc @dmitshur @alandonovan @gopherbot