Skip to content
This repository was archived by the owner on Sep 9, 2020. It is now read-only.
22 changes: 17 additions & 5 deletions context.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package dep

import (
"fmt"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Make sure you run gofmt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, not really sure what's the problem here. I did run gofmt (in fact one of the pust-push checks fails when the code is not gofmt'ed).

"log"
"os"
"path/filepath"
Expand Down Expand Up @@ -222,18 +223,29 @@ 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)
if fs.HasFilepathPrefix(path, srcprefix) {
if len(path) <= len(srcprefix) {

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", c.GOPATH, err.Error())
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.

Let's use errors.Errorf instead of fmt.Errorf to be consistent with the rest of the file.

Copy link
Collaborator

Choose a reason for hiding this comment

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

oops! there's error value. Follow @ibrasho 's comment 👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

Errors should start with a lowercase letter.

Also, use errors.Wrap to achieve better results:

errors.Wrapf(err, "failed to evaluate symlink %s", c.GOPATH)

}

pathEvaluated, 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(pathEvaluated, srcprefix) {
if len(pathEvaluated) <= len(srcprefix) {
return "", errors.New("dep does not currently support using GOPATH/src as the project root")
}

// filepath.ToSlash because we're dealing with an import path now,
// not an fs path
return filepath.ToSlash(path[len(srcprefix):]), nil
return filepath.ToSlash(pathEvaluated[len(srcprefix):]), nil
}

return "", errors.Errorf("%s not in GOPATH", path)
return "", errors.Errorf("%s not in GOPATH", pathEvaluated)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this one stay path?

}

// absoluteProjectRoot determines the absolute path to the project root
Expand Down