Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

Resolve symlinks if project root has them. #247

Merged
merged 15 commits into from Apr 11, 2017
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 44 additions & 0 deletions context.go
Expand Up @@ -27,6 +27,7 @@ func NewContext() (*Ctx, error) {
// this way we get the default GOPATH that was added in 1.8
buildContext := build.Default
wd, err := os.Getwd()

if err != nil {
return nil, errors.Wrap(err, "getting work directory")
}
Expand Down Expand Up @@ -75,6 +76,13 @@ func (c *Ctx) LoadProject(path string) (*Project, error) {
return nil, err
}

// the path may lie within a symlinked directory, resolve that
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I'm learning that go/stdlib coding standards generally look for complete sentences w/punctuation and capitalization in comments. It's also OK to have a just a couple explanatory words - but this sorta thing (a complete sentence, but lacking proper in these details) is a no-no.

(Something like that, @rakyll ? 😄 )

// before moving forward
p.AbsRoot, err = c.resolveProjectRoot(p.AbsRoot)
if err != nil {
return nil, errors.Wrapf(err, "resolve project root")
}

ip, err := c.SplitAbsoluteProjectRoot(p.AbsRoot)
if err != nil {
return nil, errors.Wrap(err, "split absolute project root")
Expand Down Expand Up @@ -118,6 +126,42 @@ func (c *Ctx) LoadProject(path string) (*Project, error) {
return p, nil
}

// resolveProjectRoot evaluates the root directory and does the following:
//
// If the passed path is a symlink outside GOPATH to a directory within a
// GOPATH, the resolved full real path is returned.
//
// If the passed path is a symlink within a GOPATH, we return an error.
//
// If the passed path isn't a symlink at all, we just pass through.
func (c *Ctx) resolveProjectRoot(path string) (string, error) {
// Determine if this path is a Symlink
l, err := os.Lstat(path)
if err != nil {
return "", errors.Wrap(err, "resolveProjectRoot")
}

// Pass through if not
if l.Mode()&os.ModeSymlink == 0 {
return path, nil
}

// Resolve path
resolved, err := filepath.EvalSymlinks(path)
if err != nil {
return "", errors.Wrap(err, "resolveProjectRoot")
}

// Determine if the symlink is within the GOPATH, in which case we're not
// sure how to resolve it.
if filepath.HasPrefix(path, c.GOPATH) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This search needs to be more expansive - it should check all other GOPATHs, not just the selected one (see newContext() for how they're selected).

To minimize our exposure to other globals, let's update newContext to store all the GOPATHs (in addition to and separately from the selected one), then do the comparison here against what's stored locally on the struct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. 👍 On it.

return "", fmt.Errorf("''%s' is linked to another path within GOPATH", path)
}

// Return the resolved path
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: i don't think this comment adds much

return resolved, nil
}

// SplitAbsoluteProjectRoot takes an absolute path and compares it against declared
// GOPATH(s) to determine what portion of the input path should be treated as an
// import path - as a project root.
Expand Down
47 changes: 47 additions & 0 deletions context_test.go
Expand Up @@ -38,6 +38,7 @@ func TestSplitAbsoluteProjectRoot(t *testing.T) {
defer h.Cleanup()

h.TempDir("src")

h.Setenv("GOPATH", h.Path("."))
depCtx := &Ctx{GOPATH: h.Path(".")}

Expand Down Expand Up @@ -346,3 +347,49 @@ func TestCaseInsentitiveGOPATH(t *testing.T) {
t.Fatalf("expected %s, got %s", ip, pr)
}
}

func TestResolveProjectRoot(t *testing.T) {
tg := test.NewHelper(t)
defer tg.Cleanup()

tg.TempDir("go")
tg.TempDir("go/src")
tg.TempDir("sym")
tg.TempDir("go/src/real")
tg.TempDir("go/src/real/path")
tg.TempDir("go/src/sym")

tg.Setenv("GOPATH", tg.Path(filepath.Join(".", "go")))

ctx := &Ctx{GOPATH: tg.Path(filepath.Join(".", "go"))}

realPath := filepath.Join(ctx.GOPATH, "src/real/path")
symlinkedPath := filepath.Join(tg.Path("."), "sym", "symlink")
symlinkedInGoPath := filepath.Join(ctx.GOPATH, "src/sym/path")
os.Symlink(realPath, symlinkedPath)
os.Symlink(realPath, symlinkedInGoPath)

// Real path should be returned, no symlinks to deal with
p, err := ctx.resolveProjectRoot(realPath)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: as a general naming pattern, we prefer "want" to "expect" - #152. That could be applied to both the var name here, and the wording of the error message below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

np

if err != nil {
t.Fatalf("Error resolving project root: %s", err)
}
if p != realPath {
t.Fatalf("Want path to be %s, got %s", realPath, p)
}

// Real path should be returned, symlink is outside GOPATH
p, err = ctx.resolveProjectRoot(symlinkedPath)
if err != nil {
t.Fatalf("Error resolving project root: %s", err)
}
if p != realPath {
t.Fatalf("Want path to be %s, got %s", realPath, p)
}

// Symlinked path is inside GOPATH, should return error
_, err = ctx.resolveProjectRoot(symlinkedInGoPath)
if err == nil {
t.Fatalf("Expected an error")
}
}