Skip to content

Commit

Permalink
Changed all uses of path to path/filepath to get cross-platform compa…
Browse files Browse the repository at this point in the history
…tibility. Also fixed tests to work on windows as well.
  • Loading branch information
mktange committed Feb 7, 2018
1 parent 6d7700b commit 5f8a5da
Show file tree
Hide file tree
Showing 14 changed files with 194 additions and 68 deletions.
13 changes: 9 additions & 4 deletions cli/exitcode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import (
"io/ioutil"
"os"
"os/exec"
"path"
"path/filepath"
"runtime"
"syscall"
"testing"

Expand Down Expand Up @@ -66,14 +67,18 @@ func runGoFile(t *testing.T, src string) ([]byte, error) {
}
}()

err = ioutil.WriteFile(path.Join(tmpDir, "test_cli.go"), []byte(src), 0644)
err = ioutil.WriteFile(filepath.Join(tmpDir, "test_cli.go"), []byte(src), 0644)
require.NoError(t, err)

buildCmd := exec.Command("go", "build", "-o", "test-cli", ".")
binName := "test-cli"
if runtime.GOOS == "windows" {
binName += ".exe"
}
buildCmd := exec.Command("go", "build", "-o", binName, ".")
buildCmd.Dir = tmpDir
output, err := buildCmd.CombinedOutput()
require.NoError(t, err, "%v failed: %s", buildCmd.Args, string(output))

testCLICmd := exec.Command(path.Join(tmpDir, "test-cli"))
testCLICmd := exec.Command(filepath.Join(tmpDir, binName))
return testCLICmd.CombinedOutput()
}
10 changes: 7 additions & 3 deletions matcher/files_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ package matcher_test
import (
"io/ioutil"
"os"
"path"
"path/filepath"
"testing"

"github.com/nmiyake/pkg/dirs"
Expand Down Expand Up @@ -67,6 +67,10 @@ func TestListFiles(t *testing.T) {
got, err := matcher.ListFiles(currCaseTmpDir, currCase.include, currCase.exclude)
require.NoError(t, err, "Case %d", i)

for i, want := range currCase.want {
currCase.want[i] = filepath.FromSlash(want)
}

assert.Equal(t, currCase.want, got, "Case %d", i)
}
}
Expand All @@ -76,10 +80,10 @@ func createFiles(t *testing.T, tmpDir string, files map[string]string) string {
require.NoError(t, err)

for currFile, currContent := range files {
err := os.MkdirAll(path.Join(currCaseTmpDir, path.Dir(currFile)), 0755)
err := os.MkdirAll(filepath.Join(currCaseTmpDir, filepath.Dir(currFile)), 0755)
require.NoError(t, err)

err = ioutil.WriteFile(path.Join(currCaseTmpDir, currFile), []byte(currContent), 0644)
err = ioutil.WriteFile(filepath.Join(currCaseTmpDir, currFile), []byte(currContent), 0644)
require.NoError(t, err)
}

Expand Down
24 changes: 17 additions & 7 deletions matcher/matchers.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"path"
"path/filepath"
"regexp"
"runtime"
)

type Matcher interface {
Expand Down Expand Up @@ -87,7 +88,7 @@ type nameMatcher []*regexp.Regexp

func (m nameMatcher) Match(inputRelPath string) bool {
for _, currSubpath := range allSubpaths(inputRelPath) {
currName := path.Base(currSubpath)
currName := filepath.Base(currSubpath)
for _, currRegExp := range []*regexp.Regexp(m) {
matchLoc := currRegExp.FindStringIndex(currName)
if len(matchLoc) > 0 && matchLoc[0] == 0 && matchLoc[1] == len(currName) {
Expand All @@ -104,13 +105,22 @@ func (m nameMatcher) Match(inputRelPath string) bool {
// filepath.Match). However, unlike filepath.Match, subpath matches will match all of the sub-paths of a given match as
// well (for example, the pattern "foo/*/bar" matches "foo/*/bar/baz").
func Path(paths ...string) Matcher {
return &pathMatcher{paths: paths, glob: true}
return newPathMatcher(paths, true)
}

// PathLiteral returns a Matcher that is equivalent to that returned by Paths except that matches are done using string
// equality rather than using glob matching.
func PathLiteral(paths ...string) Matcher {
return &pathMatcher{paths: paths, glob: false}
return newPathMatcher(paths, false)
}

func newPathMatcher(paths []string, glob bool) Matcher {
if runtime.GOOS == "windows" {
for i, p := range paths {
paths[i] = filepath.ToSlash(p)
}
}
return &pathMatcher{paths: paths, glob: glob}
}

type pathMatcher struct {
Expand All @@ -125,7 +135,7 @@ func (m *pathMatcher) Match(inputRelPath string) bool {
var match bool
if m.glob {
var err error
match, err = filepath.Match(currMatcherPath, currSubpath)
match, err = path.Match(currMatcherPath, currSubpath)
if err != nil {
// only possible error is bad pattern
panic(fmt.Sprintf("filepath: Match(%q): %v", currMatcherPath, err))
Expand All @@ -145,12 +155,12 @@ func (m *pathMatcher) Match(inputRelPath string) bool {
// "foo/bar/baz.txt" return [foo/bar/baz.txt foo/bar foo], while "foo.txt" returns [foo.txt]. Returns nil if the
// provided path is an absolute path.
func allSubpaths(relPath string) []string {
if path.IsAbs(relPath) {
if filepath.IsAbs(relPath) {
return nil
}
var subpaths []string
for currRelPath := relPath; currRelPath != "."; currRelPath = path.Dir(currRelPath) {
subpaths = append(subpaths, currRelPath)
for currRelPath := relPath; currRelPath != "." && currRelPath != string(filepath.Separator); currRelPath = filepath.Dir(currRelPath) {
subpaths = append(subpaths, filepath.ToSlash(currRelPath))
}
return subpaths
}
30 changes: 30 additions & 0 deletions osutil/osutil.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package osutil

import (
"path/filepath"
"runtime"
"strings"
)

// MakeValidRegexPath takes a path and makes it into a valid regex string
func MakeValidRegexPath(path string) string {
return strings.Replace(filepath.FromSlash(path), "\\", "\\\\", -1)
}

// GetNotADirErrorMsg returns the error message given if an action is
// performed on a non-existant directory.
func GetNotADirErrorMsg() string {
if runtime.GOOS == "windows" {
return "The system cannot find the path specified."
}
return "not a directory"
}

// GetNoSuchFileOrDirErrorMsg returns the error message given if an action is
// performed on a non-existant file or directory.
func GetNoSuchFileOrDirErrorMsg() string {
if runtime.GOOS == "windows" {
return "The system cannot find the file specified."
}
return "no such file or directory"
}
25 changes: 25 additions & 0 deletions osutil/osutil_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package osutil_test

import (
"fmt"
"path/filepath"
"regexp"
"testing"

"github.com/palantir/pkg/osutil"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestValidRegexPath(t *testing.T) {
currentAbs, err := filepath.Abs(".")
require.NoError(t, err)

fmtString := `here is a path: %s with some text after`
myRegexDef := fmt.Sprintf(
"^"+fmtString+"$",
osutil.MakeValidRegexPath(currentAbs),
)

assert.Regexp(t, regexp.MustCompile(myRegexDef), fmt.Sprintf(fmtString, currentAbs))
}
28 changes: 16 additions & 12 deletions pkgpath/packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"go/parser"
"go/token"
"os"
"path"
"path/filepath"
"sort"
"strings"
Expand Down Expand Up @@ -89,18 +88,22 @@ type pkgPath struct {
func (p *pkgPath) Abs() string {
switch p.pathType {
case Absolute:
return p.path
absPath, err := filepath.Abs(p.path)
if err != nil {
panic(err)
}
return absPath
case GoPathSrcRelative:
return path.Join(os.Getenv("GOPATH"), "src", p.path)
return filepath.Join(os.Getenv("GOPATH"), "src", p.path)
case Relative:
return path.Join(p.baseDir, p.path)
return filepath.Join(p.baseDir, p.path)
default:
panic(fmt.Sprintf("unhandled case: %v", p.path))
}
}

func (p *pkgPath) GoPathSrcRel() (string, error) {
return relPathNoParentDir(p.Abs(), path.Join(os.Getenv("GOPATH"), "src"), "")
return relPathNoParentDir(p.Abs(), filepath.Join(os.Getenv("GOPATH"), "src"), "")
}

func (p *pkgPath) Rel(baseDir string) (string, error) {
Expand All @@ -113,9 +116,10 @@ func relPathNoParentDir(absPath, baseDir, prepend string) (string, error) {
if err != nil {
return "", err
}
if strings.HasPrefix(relPath, parentDirPath) {
if filepath.HasPrefix(relPath, parentDirPath) {
return "", fmt.Errorf("resolving %s against base %s produced relative path starting with %s: %s", absPath, baseDir, parentDirPath, relPath)
}
relPath = filepath.ToSlash(relPath)
return prepend + relPath, nil
}

Expand All @@ -142,7 +146,7 @@ func (p *packages) Filter(exclude matcher.Matcher) (Packages, error) {
filteredAbsPathPkgs := make(map[string]string)
for currPkgRelPath, currPkg := range allPkgsRelPaths {
if exclude == nil || !exclude.Match(currPkgRelPath) {
filteredAbsPathPkgs[path.Join(p.rootDir, currPkgRelPath)] = currPkg
filteredAbsPathPkgs[filepath.Join(p.rootDir, currPkgRelPath)] = currPkg
}
}

Expand Down Expand Up @@ -214,7 +218,7 @@ func PackagesFromPaths(rootDir string, relPaths []string) (Packages, error) {

pkgs := make(map[string]string, len(expandedRelPaths))
for _, currPath := range expandedRelPaths {
currAbsPath := path.Join(absoluteRoot, currPath)
currAbsPath := filepath.Join(absoluteRoot, currPath)
currPkg, err := getPrimaryPkgForDir(currAbsPath, nil)
if err != nil {
return nil, fmt.Errorf("unable to determine package for directory %s: %v", currAbsPath, err)
Expand Down Expand Up @@ -257,7 +261,7 @@ func PackagesInDir(rootDir string, exclude matcher.Matcher) (Packages, error) {
// create a filter for processing package files that only passes if it does not match an exclude
filter := func(info os.FileInfo) bool {
// if exclude exists and matches the file, skip it
if exclude != nil && exclude.Match(path.Join(currRelPath, info.Name())) {
if exclude != nil && exclude.Match(filepath.Join(currRelPath, info.Name())) {
return false
}
// process file if it would be included in build context (handles things like build tags)
Expand All @@ -283,12 +287,12 @@ func PackagesInDir(rootDir string, exclude matcher.Matcher) (Packages, error) {
}

func createPkgsWithValidation(rootDir string, pkgs map[string]string) (*packages, error) {
if !path.IsAbs(rootDir) {
if !filepath.IsAbs(rootDir) {
return nil, fmt.Errorf("rootDir %s is not an absolute path", rootDir)
}

for currAbsPkgPath := range pkgs {
if !path.IsAbs(currAbsPkgPath) {
if !filepath.IsAbs(currAbsPkgPath) {
return nil, fmt.Errorf("package %s in packages %v is not an absolute path", currAbsPkgPath, pkgs)
}
}
Expand All @@ -305,7 +309,7 @@ func expandPaths(rootDir string, relPaths []string) ([]string, error) {
if strings.HasSuffix(currRelPath, "/...") {
// expand splatted paths
splatBaseDir := currRelPath[:len(currRelPath)-len("/...")]
baseDirAbsPath := path.Join(rootDir, splatBaseDir)
baseDirAbsPath := filepath.Join(rootDir, splatBaseDir)
err := filepath.Walk(baseDirAbsPath, func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
Expand Down
26 changes: 18 additions & 8 deletions pkgpath/packages_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ import (
"fmt"
"io/ioutil"
"os"
"path"
"path/filepath"
"regexp"
"runtime"
"strings"
"testing"

"github.com/nmiyake/pkg/dirs"
Expand Down Expand Up @@ -85,7 +86,7 @@ func TestListPackages(t *testing.T) {
wd, err := os.Getwd()
require.NoError(t, err)

testPkgPath, err := filepath.Rel(path.Join(os.Getenv("GOPATH"), "src"), wd)
testPkgPath, err := filepath.Rel(filepath.Join(os.Getenv("GOPATH"), "src"), wd)
require.NoError(t, err)

tmpDir, cleanup, err := dirs.TempDir(wd, "")
Expand Down Expand Up @@ -226,9 +227,9 @@ package different`},
case pkgpath.Relative:
k = "./" + k
case pkgpath.GoPathSrcRelative:
k = path.Join(testPkgPath, currCaseDirRelPath, k)
k = filepath.ToSlash(filepath.Join(testPkgPath, currCaseDirRelPath, k))
case pkgpath.Absolute:
k = path.Join(wd, currCaseDirRelPath, k)
k = filepath.Join(wd, currCaseDirRelPath, k)
default:
require.Fail(t, "Unhandled case: %v", mode)
}
Expand Down Expand Up @@ -299,7 +300,7 @@ func TestListPackagesSetGoPath(t *testing.T) {
err = os.Setenv("GOPATH", tmpDir)
require.NoError(t, err)

projectDir := path.Join(tmpDir, "src", "github.com", "test")
projectDir := filepath.Join(tmpDir, "src", "github.com", "test")
err = os.MkdirAll(projectDir, 0755)
require.NoError(t, err)

Expand All @@ -323,9 +324,18 @@ func TestListPackagesSetGoPath(t *testing.T) {
}

func TestPkgPathOutsideGoPathFails(t *testing.T) {
goPathSrc := path.Join(os.Getenv("GOPATH"), "src")
msg := fmt.Sprintf(`^resolving /foo against base %s produced relative path starting with ../: .+/foo$`, goPathSrc)
goPathSrc := filepath.Join(os.Getenv("GOPATH"), "src")
fooPath := string(filepath.Separator) + "foo"
absPath, err := filepath.Abs(fooPath)
require.NoError(t, err)

msg := fmt.Sprintf(`^resolving %s against base %s produced relative path starting with %s: .+%s$`,
absPath, goPathSrc, ".."+string(filepath.Separator), fooPath)

if runtime.GOOS == "windows" {
msg = strings.Replace(msg, "\\", "\\\\", -1)
}

_, err := pkgpath.NewAbsPkgPath("/foo").GoPathSrcRel()
_, err = pkgpath.NewAbsPkgPath(absPath).GoPathSrcRel()
require.Regexp(t, msg, err.Error())
}
Loading

0 comments on commit 5f8a5da

Please sign in to comment.