-
Notifications
You must be signed in to change notification settings - Fork 1k
Use evaluated project root path #812
Changes from 3 commits
849e187
f9b8523
1ad2706
88faf50
c28ab32
45540a7
96a9333
40d9942
3d59125
b124a46
618951f
92886c5
93dfae7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ import ( | |
"runtime" | ||
"strings" | ||
|
||
"fmt" | ||
"github.com/Masterminds/vcs" | ||
"github.com/golang/dep/internal/fs" | ||
"github.com/golang/dep/internal/gps" | ||
|
@@ -222,7 +223,18 @@ func (c *Ctx) detectGOPATH(path string) (string, error) { | |
// ImportForAbs returns the import path for an absolute project path by trimming the | ||
// `$GOPATH/src/` prefix. Returns an error for paths equal to, or without this prefix. | ||
func (c *Ctx) ImportForAbs(path string) (string, error) { | ||
srcprefix := filepath.Join(c.GOPATH, "src") + string(filepath.Separator) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove the extra new line. |
||
gopathEvaluated, err := filepath.EvalSymlinks(c.GOPATH) | ||
if err != nil { | ||
return "", fmt.Errorf("Error evaluating symlinks in GOPATH %s: %s", path, err.Error()) | ||
} | ||
|
||
path, err = filepath.EvalSymlinks(path) | ||
if err != nil { | ||
return "", fmt.Errorf("Error evaluating symlinks in %s: %s", path, err.Error()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the intention to show the original path in the error? This looks like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, you're right. Pushed now a fix (includes moving There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same nit as the error above. |
||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't we just do If they are the same, we did what line 223 is doing. If they are different, we did the if block logic. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or just use |
||
|
||
srcprefix := filepath.Join(gopathEvaluated, "src") + string(filepath.Separator) | ||
if fs.HasFilepathPrefix(path, srcprefix) { | ||
if len(path) <= len(srcprefix) { | ||
return "", errors.New("dep does not currently support using GOPATH/src as the project root") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: could move this import up with the other std lib imports
LGTM besides, unless there should be some new test cases as well.