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

Use evaluated project root path #812

Closed
wants to merge 13 commits into from
14 changes: 13 additions & 1 deletion context.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"runtime"
"strings"

"fmt"
Copy link
Collaborator

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.

"github.com/Masterminds/vcs"
"github.com/golang/dep/internal/fs"
"github.com/golang/dep/internal/gps"
Expand Down Expand Up @@ -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)

Copy link
Collaborator

Choose a reason for hiding this comment

The 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())
Copy link
Collaborator

Choose a reason for hiding this comment

The 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 path would be set to the result of EvalSymlinks.

Copy link
Contributor Author

@tkrajina tkrajina Jul 15, 2017

Choose a reason for hiding this comment

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

Yes, you're right. Pushed now a fix (includes moving "fmt" import up).

Copy link
Collaborator

@darkowlzz darkowlzz Jul 16, 2017

Choose a reason for hiding this comment

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

fmt.Errorf -> errors.Errorf

Copy link
Collaborator

Choose a reason for hiding this comment

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

Same nit as the error above.

}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't we just do path = p.ResolvedAbsRoot instead?

If they are the same, we did what line 223 is doing. If they are different, we did the if block logic.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or just use p.ResolvedAbsRoot instead of path...


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")
Expand Down