Navigation Menu

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

Jump to bottom

x/tools/go/loader: working directory affects resolution of vendored imports #16580

Closed
rhysh opened this issue Aug 2, 2016 · 6 comments
Closed

x/tools/go/loader: working directory affects resolution of vendored imports #16580

rhysh opened this issue Aug 2, 2016 · 6 comments
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@rhysh
Copy link
Contributor

rhysh commented Aug 2, 2016

Please answer these questions before submitting your issue. Thanks!

  1. What version of Go are you using (go version)?

    go1.7rc4, with golang/tools@9e74590

  2. What operating system and processor architecture are you using (go env)?

    darwin/amd64

  3. What did you do?

    I have an eg template that replaces calls to a function in one package ("dep1" below) with calls to a similar function in a second package ("dep2"). I ran eg with the template in a repo with a vendor directory that includes only the first package.

  4. What did you expect to see?

    I expected eg to successfully run, updating the function call if present.

  5. What did you see instead?

    The eg invocation fails when my current working directory is in the directory containing the vendor directory, or a subdirectory of it. The invocation succeeds if I cd to anywhere else.

$ go env GOPATH
/tmp/eg
$ cd "$GOPATH/src/prog" && eg -w -t "$GOPATH/src/prog/tmpl.go" -- "$GOPATH/src/prog/main.go"
/tmp/eg/src/prog/tmpl.go:15:9: cannot use t (variable of type prog/vendor/dep1.T) as dep1.T value in argument to dep2.F
eg: couldn't load packages due to errors: template
$ cd "$GOPATH" && eg -w -t "$GOPATH/src/prog/tmpl.go" -- "$GOPATH/src/prog/main.go"
=== /tmp/eg/src/prog/main.go (1 matches)

Here are the contents of the GOPATH where I've reproduced the issue:

./src/dep1/dep1.go:

package dep1

type T int

var A T

func F(t T) {}

./src/dep2/dep2.go:

package dep2

import "dep1"

func F(t dep1.T) {}

./src/prog/vendor/dep1/dep1.go:

package dep1

type T int

var A T

func F(t T) {}

./src/prog/main.go:

package main

import "dep1"

func main() {
    dep1.F(dep1.A)
}

./src/prog/tmpl.go:

// +build ignore

package eg

import (
    "dep1"
    "dep2"
)

func before(t dep1.T) {
    dep1.F(t)
}

func after(t dep1.T) {
    dep2.F(t)
}
@rhysh
Copy link
Contributor Author

rhysh commented Aug 3, 2016

I've been able to fix this by setting Cwd to "." in the loader.Config, which seems to work by disabling some parts of the loader package which I don't entirely understand. I traced usage of loader.Config.Cwd through the golang.org/x/tools/go/loader package and found a somewhat relevant TODO, though it still doesn't explain what's going on.

It looks like this issue is less about the eg command and more about golang.org/x/tools/go/loader, which doesn't yet handle all the implications of the go15 vendor pattern.

/cc @adonovan

@quentinmit quentinmit added this to the Unreleased milestone Aug 4, 2016
@gopherbot
Copy link

CL https://golang.org/cl/29446 mentions this issue.

@rhysh rhysh mentioned this issue Sep 27, 2016
Closed
@rhysh
Copy link
Contributor Author

rhysh commented Nov 28, 2016

While this is annoying, I'm not sure that it's really a bug. Half of the behavior (now fixed) is due to #17247, and the other half is due to the loader drawing a distinction between package "bar" and package "foo/vendor/bar". That's an annoying distinction when an eg rewrite causes a project to depend on a package for the first time, but it's not an incorrect one.

@rhysh rhysh closed this as completed Nov 28, 2016
@alandonovan
Copy link
Contributor

The meaning of that TODO is that the package's directory (for vendoring purposes) should be the parent directory of its files, not the current directory. This logic is only invoked for xtest packages (of which there are none in your example) and for "created" (not "imported") packages such as the one containing template.go. I would expect that the loader resolves template.go's imports relative to the working directory, so if you run eg beneath vendor, you'll get a different result. That's clearly a vendoring bug in go/loader that I need to fix.

I am surprised that using "." instead of Cwd changes the behavior at all, though.

@alandonovan alandonovan reopened this Nov 28, 2016
@alandonovan alandonovan changed the title x/tools/cmd/eg: vendor and PWD lead to type mismatch x/tools/go/loader: working directory affects resolution of vendored imports Nov 28, 2016
@gopherbot
Copy link

CL https://golang.org/cl/33589 mentions this issue.

@gopherbot
Copy link

CL https://golang.org/cl/33673 mentions this issue.

@golang golang locked and limited conversation to collaborators Nov 30, 2017
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
4 participants
@quentinmit @rhysh @alandonovan @gopherbot