Merge pull request #162 from hashicorp/fix_url_parse_windows

Fix tests for windows

.travis.yml

@@ -9,6 +9,10 @@ addons:
 
 language: go
 
+os:
+  - linux
+  - osx
+
 go:
   - "1.11.x"
 

appveyor.yml

@@ -13,4 +13,4 @@ install:
 
     go get -d -v -t ./...
 build_script:
-- cmd: go test -v ./...
+- cmd: go test ./...

checksum.go

@@ -197,8 +197,15 @@ func (c *Client) checksumFromFile(checksumFile string, src *url.URL) (*fileCheck
 	if err != nil {
 		return nil, err
 	}
-	relpath, err := filepath.Rel(filepath.Dir(checksumFileURL.Path), absPath)
-	if err != nil {
+	checksumFileDir := filepath.Dir(checksumFileURL.Path)
+	relpath, err := filepath.Rel(checksumFileDir, absPath)
+	switch {
+	case err == nil ||
+		err.Error() == "Rel: can't make "+absPath+" relative to "+checksumFileDir:
+		// ex: on windows C:\gopath\...\content.txt cannot be relative to \
+		// which is okay, may be another expected path will work.
+		break
+	default:
 		return nil, err
 	}
 
@@ -238,7 +245,7 @@ func (c *Client) checksumFromFile(checksumFile string, src *url.URL) (*fileCheck
 		}
 		// make sure the checksum is for the right file
 		for _, option := range options {
-			if checksum.Filename == option {
+			if option != "" && checksum.Filename == option {
 				// any checksum will work so we return the first one
 				return checksum, nil
 			}

detect_file_test.go

@@ -55,9 +55,6 @@ func TestFileDetector(t *testing.T) {
 	for i, tc := range fileTests {
 		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
 			pwd := tc.pwd
-			if !filepath.IsAbs(pwd) {
-				pwd = filepath.Join(pwdRoot, pwd)
-			}
 
 			out, ok, err := f.Detect(tc.in, pwd)
 			if err != nil {

get_git.go

@@ -9,6 +9,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"runtime"
 	"strings"
 
 	urlhelper "github.com/hashicorp/go-getter/helper/url"
@@ -211,7 +212,7 @@ func setupGitEnv(cmd *exec.Cmd, sshKeyFile string) {
 	// with versions of Go < 1.9.
 	env := os.Environ()
 	for i, v := range env {
-		if strings.HasPrefix(v, gitSSHCommand) {
+		if strings.HasPrefix(v, gitSSHCommand) && len(v) > len(gitSSHCommand) {
 			sshCmd = []string{v}
 
 			env[i], env[len(env)-1] = env[len(env)-1], env[i]
@@ -226,6 +227,9 @@ func setupGitEnv(cmd *exec.Cmd, sshKeyFile string) {
 
 	if sshKeyFile != "" {
 		// We have an SSH key temp file configured, tell ssh about this.
+		if runtime.GOOS == "windows" {
+			sshKeyFile = strings.Replace(sshKeyFile, `\`, `/`, -1)
+		}
 		sshCmd = append(sshCmd, "-i", sshKeyFile)
 	}
 
@@ -251,8 +255,17 @@ func checkGitVersion(min string) error {
 	if len(fields) < 3 {
 		return fmt.Errorf("Unexpected 'git version' output: %q", string(out))
 	}
+	v := fields[2]
+	if runtime.GOOS == "windows" && strings.Contains(v, ".windows.") {
+		// on windows, git version will return for example:
+		// git version 2.20.1.windows.1
+		// Which does not follow the semantic versionning specs
+		// https://semver.org. We remove that part in order for
+		// go-version to not error.
+		v = v[:strings.Index(v, ".windows.")]
+	}
 
-	have, err := version.NewVersion(fields[2])
+	have, err := version.NewVersion(v)
 	if err != nil {
 		return err
 	}

get_git_test.go

@@ -1,6 +1,7 @@
 package getter
 
 import (
+	"bytes"
 	"encoding/base64"
 	"io/ioutil"
 	"net/url"
@@ -10,6 +11,8 @@ import (
 	"runtime"
 	"strings"
 	"testing"
+
+	urlhelper "github.com/hashicorp/go-getter/helper/url"
 )
 
 var testHasGit bool
@@ -26,8 +29,7 @@ func TestGitGetter_impl(t *testing.T) {
 
 func TestGitGetter(t *testing.T) {
 	if !testHasGit {
-		t.Log("git not found, skipping")
-		t.Skip()
+		t.Skip("git not found, skipping")
 	}
 
 	g := new(GitGetter)
@@ -50,8 +52,7 @@ func TestGitGetter(t *testing.T) {
 
 func TestGitGetter_branch(t *testing.T) {
 	if !testHasGit {
-		t.Log("git not found, skipping")
-		t.Skip()
+		t.Skip("git not found, skipping")
 	}
 
 	g := new(GitGetter)
@@ -89,8 +90,7 @@ func TestGitGetter_branch(t *testing.T) {
 
 func TestGitGetter_branchUpdate(t *testing.T) {
 	if !testHasGit {
-		t.Log("git not found, skipping")
-		t.Skip()
+		t.Skip("git not found, skipping")
 	}
 
 	g := new(GitGetter)
@@ -132,8 +132,7 @@ func TestGitGetter_branchUpdate(t *testing.T) {
 
 func TestGitGetter_tag(t *testing.T) {
 	if !testHasGit {
-		t.Log("git not found, skipping")
-		t.Skip()
+		t.Skip("git not found, skipping")
 	}
 
 	g := new(GitGetter)
@@ -171,8 +170,7 @@ func TestGitGetter_tag(t *testing.T) {
 
 func TestGitGetter_GetFile(t *testing.T) {
 	if !testHasGit {
-		t.Log("git not found, skipping")
-		t.Skip()
+		t.Skip("git not found, skipping")
 	}
 
 	g := new(GitGetter)
@@ -196,6 +194,12 @@ func TestGitGetter_GetFile(t *testing.T) {
 }
 
 func TestGitGetter_gitVersion(t *testing.T) {
+	if !testHasGit {
+		t.Skip("git not found, skipping")
+	}
+	if runtime.GOOS == "windows" {
+		t.Skip("skipping on windows since the test requires sh")
+	}
 	dir, err := ioutil.TempDir("", "go-getter")
 	if err != nil {
 		t.Fatal(err)
@@ -230,16 +234,19 @@ func TestGitGetter_gitVersion(t *testing.T) {
 
 func TestGitGetter_sshKey(t *testing.T) {
 	if !testHasGit {
-		t.Log("git not found, skipping")
-		t.Skip()
+		t.Skip("git not found, skipping")
 	}
 
 	g := new(GitGetter)
 	dst := tempDir(t)
 
 	encodedKey := base64.StdEncoding.EncodeToString([]byte(testGitToken))
 
-	u, err := url.Parse("ssh://[email protected]/hashicorp/test-private-repo" +
+	// avoid getting locked by a github authenticity validation prompt
+	os.Setenv("GIT_SSH_COMMAND", "ssh -o StrictHostKeyChecking=no")
+	defer os.Setenv("GIT_SSH_COMMAND", "")
+
+	u, err := urlhelper.Parse("ssh://[email protected]/hashicorp/test-private-repo" +
 		"?sshkey=" + encodedKey)
 	if err != nil {
 		t.Fatal(err)
@@ -257,27 +264,36 @@ func TestGitGetter_sshKey(t *testing.T) {
 
 func TestGitGetter_submodule(t *testing.T) {
 	if !testHasGit {
-		t.Log("git not found, skipping")
-		t.Skip()
+		t.Skip("git not found, skipping")
 	}
 
 	g := new(GitGetter)
 	dst := tempDir(t)
 
+	relpath := func(basepath, targpath string) string {
+		relpath, err := filepath.Rel(basepath, targpath)
+		if err != nil {
+			t.Fatal(err)
+		}
+		return strings.Replace(relpath, `\`, `/`, -1)
+		// on windows git still prefers relatives paths
+		// containing `/` for submodules
+	}
+
 	// Set up the grandchild
 	gc := testGitRepo(t, "grandchild")
 	gc.commitFile("grandchild.txt", "grandchild")
 
 	// Set up the child
 	c := testGitRepo(t, "child")
 	c.commitFile("child.txt", "child")
-	c.git("submodule", "add", gc.dir)
+	c.git("submodule", "add", "-f", relpath(c.dir, gc.dir))
 	c.git("commit", "-m", "Add grandchild submodule")
 
 	// Set up the parent
 	p := testGitRepo(t, "parent")
 	p.commitFile("parent.txt", "parent")
-	p.git("submodule", "add", c.dir)
+	p.git("submodule", "add", "-f", relpath(p.dir, c.dir))
 	p.git("commit", "-m", "Add child submodule")
 
 	// Clone the root repository
@@ -299,8 +315,7 @@ func TestGitGetter_submodule(t *testing.T) {
 
 func TestGitGetter_setupGitEnv_sshKey(t *testing.T) {
 	if runtime.GOOS == "windows" {
-		t.Skipf("skipping on windows since the test requires sh")
-		return
+		t.Skip("skipping on windows since the test requires sh")
 	}
 
 	cmd := exec.Command("/bin/sh", "-c", "echo $GIT_SSH_COMMAND")
@@ -362,12 +377,13 @@ func testGitRepo(t *testing.T, name string) *gitRepo {
 		dir: dir,
 	}
 
-	url, err := url.Parse("file://" + r.dir)
+	url, err := urlhelper.Parse("file://" + r.dir)
 	if err != nil {
 		t.Fatal(err)
 	}
 	r.url = url
 
+	t.Logf("initializing git repo in %s", dir)
 	r.git("init")
 	r.git("config", "user.name", "go-getter")
 	r.git("config", "user.email", "[email protected]")
@@ -379,8 +395,10 @@ func testGitRepo(t *testing.T, name string) *gitRepo {
 func (r *gitRepo) git(args ...string) {
 	cmd := exec.Command("git", args...)
 	cmd.Dir = r.dir
+	bfr := bytes.NewBuffer(nil)
+	cmd.Stderr = bfr
 	if err := cmd.Run(); err != nil {
-		r.t.Fatal(err)
+		r.t.Fatal(err, bfr.String())
 	}
 }
 

go.mod

@@ -8,7 +8,7 @@ require (
 	github.com/fatih/color v1.7.0 // indirect
 	github.com/hashicorp/go-cleanhttp v0.5.0
 	github.com/hashicorp/go-safetemp v1.0.0
-	github.com/hashicorp/go-version v1.0.0
+	github.com/hashicorp/go-version v1.1.0
 	github.com/mattn/go-colorable v0.0.9 // indirect
 	github.com/mattn/go-isatty v0.0.4 // indirect
 	github.com/mattn/go-runewidth v0.0.4 // indirect

go.sum

@@ -14,6 +14,8 @@ github.com/hashicorp/go-safetemp v1.0.0 h1:2HR189eFNrjHQyENnQMMpCiBAsRxzbTMIgBhE
 github.com/hashicorp/go-safetemp v1.0.0/go.mod h1:oaerMy3BhqiTbVye6QuFhFtIceqFoDHxNAB65b+Rj1I=
 github.com/hashicorp/go-version v1.0.0 h1:21MVWPKDphxa7ineQQTrCU5brh7OuVVAzGOCnnCPtE8=
 github.com/hashicorp/go-version v1.0.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
+github.com/hashicorp/go-version v1.1.0 h1:bPIoEKD27tNdebFGGxxYwcL4nepeY4j1QP23PFRGzg0=
+github.com/hashicorp/go-version v1.1.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
 github.com/jmespath/go-jmespath v0.0.0-20160202185014-0b12d6b521d8 h1:12VvqtR6Aowv3l/EQUlocDHW2Cp4G9WJVH7uyH8QFJE=
 github.com/jmespath/go-jmespath v0.0.0-20160202185014-0b12d6b521d8/go.mod h1:Nht3zPeWKUH0NzdCt2Blrr5ys8VGpn0CEB0cQHVjt7k=
 github.com/mattn/go-colorable v0.0.9 h1:UVL0vNpWh04HeJXV0KLcaT7r06gOH2l4OW6ddYRUIY4=

helper/url/url_test.go

@@ -36,10 +36,10 @@ var parseTests = []parseTest{
 var winParseTests = []parseTest{
 	{
 		rawURL: `C:\`,
-		scheme: ``,
+		scheme: `file`,
 		host:   ``,
 		path:   `C:/`,
-		str:    `C:/`,
+		str:    `file://C:/`,
 		err:    false,
 	},
 	{

helper/url/url_windows.go

@@ -11,19 +11,18 @@ func parse(rawURL string) (*url.URL, error) {
 	// Make sure we're using "/" since URLs are "/"-based.
 	rawURL = filepath.ToSlash(rawURL)
 
+	if len(rawURL) > 1 && rawURL[1] == ':' {
+		// Assume we're dealing with a drive letter. In which case we
+		// force the 'file' scheme to avoid "net/url" URL.String() prepending
+		// our url with "./".
+		rawURL = "file://" + rawURL
+	}
+
 	u, err := url.Parse(rawURL)
 	if err != nil {
 		return nil, err
 	}
 
-	if len(rawURL) > 1 && rawURL[1] == ':' {
-		// Assume we're dealing with a drive letter file path where the drive
-		// letter has been parsed into the URL Scheme, and the rest of the path
-		// has been parsed into the URL Path without the leading ':' character.
-		u.Path = fmt.Sprintf("%s:%s", string(rawURL[0]), u.Path)
-		u.Scheme = ""
-	}
-
 	if len(u.Host) > 1 && u.Host[1] == ':' && strings.HasPrefix(rawURL, "file://") {
 		// Assume we're dealing with a drive letter file path where the drive
 		// letter has been parsed into the URL Host.