Comment 1:

Easy doc fix. Should do for 1.3

Labels changed: added release-go1.3, repo-tools.

Owner changed to @robpike.

Status changed to Accepted.

Comment 2:

I think part of this confusion is from me misunderstanding the purpose of srcRoot. I had
been using the Root field from 'go list' directly, but it looks like it should be
filepath.Join(pkg.Root, "src")

Comment 3:

Not part of the release.

Labels changed: removed release-go1.3.

Comment 5:

I have a couple of follow-up questions, to make sure I get this right.  The `importPath`
comment is erroneous as you state, but I'm wondering if you could provide some context
about the other two points you make.  Perhaps an example of a directory layout for a
given repository and a mismatch between what you expect from FromDir and what it returns
instead.
"- the returned root is a filesystem path, but documenting it as an
"import path" suggests it'll always be slash-separated.
Doc bug or code bug?"
Can you clarify this, as well?  What was the import path separated by if not by slashes?

Comment 6:

On Windows, the root parameter contains backslashes.
But an import path is conventionally slash-separated,
not a filepath as implemented here. (It's a slice of dir.)
My middle point above was due to my confusion about
what to provide for srcRoot. For a dir of $GOPATH/src/foo/bar,
I had been providing $GOPATH and getting back src/foo.
Providing $GOPATH/src yields just foo, as documented.
A sentence documenting the expected form of srcRoot would
have saved me a little time.

Comment 7:

Labels changed: added release-none.

I also ran into the invalid root value on Windows bug and was going to file an issue, but I found this existing one.

I think part of this confusion is from me misunderstanding the purpose of srcRoot. I had
been using the Root field from 'go list' directly, but it looks like it should be
filepath.Join(pkg.Root, "src")

It should be pkg.SrcRoot, which is already set to that value. /cc @kr

• importPath isn't part of the function's signature. Copy-paste error
  from somewhere?

You're right, importPath is confusing because it makes it sound like a variable, but there is no such variable anywhere. Perhaps it meant to say "thus root is a prefix of import path", meaning root is a prefix of the import path of the package that is in in dir.

On Windows, the root parameter contains backslashes.
But an import path is conventionally slash-separated,
not a filepath as implemented here. (It's a slice of dir.)

I can confirm, and I think it's a bug. Here's the rationale. The documentation says:

On return, root is the import path corresponding to the root of the repository

If I understand correctly, an import path is defined as a /-separated path (source here).

However, as @kr said, on Windows, there's a bug in that the returned root value uses \ as separators.

Consider this snippet being executed on Windows:

pkg, err := build.Import("example.com/repo/subpackage", "", build.FindOnly)
if err != nil {
    panic(err)
}
_, root, err := FromDir(pkg.Dir, pkg.SrcRoot)
if err != nil {
    panic(err)
}
fmt.Println(root)

On Windows, that snippet will print example.com\repo, which is incorrect (it is not an import path corresponding to repo root, it has a wrong \ character). On OS X and Linux it will print example.com/repo correctly.

The fix is simple:

diff --git a/go/vcs/vcs.go b/go/vcs/vcs.go
index a702496..b615508 100644
--- a/go/vcs/vcs.go
+++ b/go/vcs/vcs.go
@@ -348,7 +348,7 @@ func FromDir(dir, srcRoot string) (vcs *Cmd, root string, err error) {
    for len(dir) > len(srcRoot) {
        for _, vcs := range vcsList {
            if fi, err := os.Stat(filepath.Join(dir, "."+vcs.Cmd)); err == nil && fi.IsDir() {
-               return vcs, dir[len(srcRoot)+1:], nil
+               return vcs, filepath.ToSlash(dir[len(srcRoot)+1:]), nil
            }
        }

I can make a CL that fixes this issue.

I've created https://golang.org/cl/18461 that resolves this issue.

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

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