Google Git
Sign in
go/exp/b4e88ed8e8aab63a9aa9a52276782ebbc547adef^!/.
commit
b4e88ed8e8aab63a9aa9a52276782ebbc547adef
[log]
author
Jean de Klerk <deklerk@google.com>
Thu Aug 12 16:12:15 2021 -0600
committer
Jean de Klerk <deklerk@google.com>
Tue Aug 31 22:17:22 2021 +0000
tree
a344696a560ac9047672957a4beb91f7fafc0f52
parent
b422fde0612a0373cf64829aa70f3fbde47fc90f [diff]
cmd/gorelease: use CreateFromVCS instead of CreateFromDir

This will ignore gitignored files during the zip file creation, which means
gitignored files won't be included in the analysis.

Fixes golang/go#37413

Change-Id: Id5df46408a48e0be53157d95333ef3c2e02765bc
Reviewed-on: https://go-review.googlesource.com/c/exp/+/341930
Trust: Jean de Klerk <deklerk@google.com>
Run-TryBot: Jean de Klerk <deklerk@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>

diff --git a/cmd/gorelease/gorelease.go b/cmd/gorelease/gorelease.go
index 825bf88..340d81f 100644
--- a/cmd/gorelease/gorelease.go
+++ b/cmd/gorelease/gorelease.go

@@ -387,7 +387,7 @@
 	// as if it were published and downloaded. We'll detect any errors that would
 	// occur (for example, invalid file names). We avoid loading it as the
 	// main module.
-	tmpModRoot, err := copyModuleToTempDir(m.modPath, m.modRoot)
+	tmpModRoot, err := copyModuleToTempDir(repoRoot, m.modPath, m.modRoot)
 	if err != nil {
 		return moduleInfo{}, err
 	}
@@ -872,7 +872,7 @@
 // An error is returned if the module contains any files or directories that
 // can't be included in a module zip file (due to special characters,
 // excessive sizes, etc.).
-func copyModuleToTempDir(modPath, modRoot string) (dir string, err error) {
+func copyModuleToTempDir(repoRoot, modPath, modRoot string) (dir string, err error) {
 	// Generate a fake version consistent with modPath. We need a canonical
 	// version to create a zip file.
 	version := "v0.0.0-gorelease"
@@ -902,13 +902,26 @@
 		}
 	}()
 
-	if err := zip.CreateFromDir(zipFile, m, modRoot); err != nil {
-		var e zip.FileErrorList
-		if errors.As(err, &e) {
-			return "", e
+	var fallbackToDir bool
+	if repoRoot != "" {
+		var err error
+		fallbackToDir, err = tryCreateFromVCS(zipFile, m, modRoot, repoRoot)
+		if err != nil {
+			return "", err
 		}
-		return "", err
 	}
+
+	if repoRoot == "" || fallbackToDir {
+		// Not a recognised repo: fall back to creating from dir.
+		if err := zip.CreateFromDir(zipFile, m, modRoot); err != nil {
+			var e zip.FileErrorList
+			if errors.As(err, &e) {
+				return "", e
+			}
+			return "", err
+		}
+	}
+
 	if err := zipFile.Close(); err != nil {
 		return "", err
 	}
@@ -918,6 +931,40 @@
 	return dir, nil
 }
 
+// tryCreateFromVCS tries to create a module zip file from VCS. If it succeeds,
+// it returns fallBackToDir false and a nil err. If it fails in a recoverable
+// way, it returns fallBackToDir true and a nil err. If it fails in an
+// unrecoverable way, it returns a non-nil err.
+func tryCreateFromVCS(zipFile io.Writer, m module.Version, modRoot, repoRoot string) (fallbackToDir bool, _ error) {
+	// We recognised a repo: create from VCS.
+	if !hasFilePathPrefix(modRoot, repoRoot) {
+		panic(fmt.Sprintf("repo root %q is not a prefix of mod root %q", repoRoot, modRoot))
+	}
+	hasUncommitted, err := hasGitUncommittedChanges(repoRoot)
+	if err != nil {
+		// Fallback to CreateFromDir.
+		return true, nil
+	}
+	if hasUncommitted {
+		return false, fmt.Errorf("repo %s has uncommitted changes", repoRoot)
+	}
+	modRel := filepath.ToSlash(trimFilePathPrefix(modRoot, repoRoot))
+	if err := zip.CreateFromVCS(zipFile, m, repoRoot, "HEAD", modRel); err != nil {
+		var fel zip.FileErrorList
+		if errors.As(err, &fel) {
+			return false, fel
+		}
+		var uve *zip.UnrecognizedVCSError
+		if errors.As(err, &uve) {
+			// Fallback to CreateFromDir.
+			return true, nil
+		}
+		return false, err
+	}
+	// Success!
+	return false, nil
+}
+
 // downloadModule downloads a specific version of a module to the
 // module cache using 'go mod download'.
 func downloadModule(ctx context.Context, m module.Version) (modRoot, goModPath string, err error) {
@@ -1455,3 +1502,16 @@
 	// NOTE: the go.mod parser rejects invalid UTF-8, so we don't check that here.
 	return rationale, true
 }
+
+// hasGitUncommittedChanges checks if the given directory has uncommitteed git
+// changes.
+func hasGitUncommittedChanges(dir string) (bool, error) {
+	stdout := &bytes.Buffer{}
+	cmd := exec.Command("git", "status", "--porcelain")
+	cmd.Dir = dir
+	cmd.Stdout = stdout
+	if err := cmd.Run(); err != nil {
+		return false, cleanCmdError(err)
+	}
+	return stdout.Len() != 0, nil
+}

diff --git a/cmd/gorelease/gorelease_test.go b/cmd/gorelease/gorelease_test.go
index 54087b8..96b3f9a 100644
--- a/cmd/gorelease/gorelease_test.go
+++ b/cmd/gorelease/gorelease_test.go

@@ -15,6 +15,7 @@
 	"path/filepath"
 	"strconv"
 	"strings"
+	"sync"
 	"testing"
 
 	"golang.org/x/mod/module"
@@ -26,6 +27,22 @@
 	updateGolden = flag.Bool("u", false, "update expected text in test files instead of failing")
 )
 
+var hasGitCache struct {
+	once  sync.Once
+	found bool
+}
+
+// hasGit reports whether the git executable exists on the PATH.
+func hasGit() bool {
+	hasGitCache.once.Do(func() {
+		if _, err := exec.LookPath("git"); err != nil {
+			return
+		}
+		hasGitCache.found = true
+	})
+	return hasGitCache.found
+}
+
 // prepareProxy creates a proxy dir and returns an associated ctx.
 //
 // proxyVersions must be a map of module version to true. If proxyVersions is
@@ -130,6 +147,10 @@
 	// If it is not empty, each entry must be of the form <modpath>@v<version>
 	// and exist in testdata/mod/.
 	proxyVersions map[module.Version]bool
+
+	// vcs is used to set the VCS that the root of the test should
+	// emulate. Allowed values are git, and hg.
+	vcs string
 }
 
 // readTest reads and parses a .test file with the given name.
@@ -203,6 +224,8 @@
 				proxyVersions[mv] = true
 			}
 			t.proxyVersions = proxyVersions
+		case "vcs":
+			t.vcs = value
 		default:
 			return nil, fmt.Errorf("%s:%d: unknown key: %q", testPath, lineNum, key)
 		}
@@ -277,6 +300,33 @@
 	}
 }
 
+func TestRelease_gitRepo_uncommittedChanges(t *testing.T) {
+	ctx := context.Background()
+	buf := &bytes.Buffer{}
+	releaseDir, err := ioutil.TempDir("", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	goModInit(t, releaseDir)
+	gitInit(t, releaseDir)
+
+	// Create an uncommitted change.
+	bContents := `package b
+const B = "b"`
+	if err := ioutil.WriteFile(filepath.Join(releaseDir, "b.go"), []byte(bContents), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	success, err := runRelease(ctx, buf, releaseDir, nil)
+	if got, want := err.Error(), fmt.Sprintf("repo %s has uncommitted changes", releaseDir); got != want {
+		t.Errorf("runRelease:\ngot error:\n%q\nwant error\n%q", got, want)
+	}
+	if success {
+		t.Errorf("runRelease: expected failure, got success")
+	}
+}
+
 func testRelease(ctx context.Context, tests []*test, test *test) func(t *testing.T) {
 	return func(t *testing.T) {
 		if test.skip != "" {
@@ -325,6 +375,21 @@
 			t.Fatal(err)
 		}
 
+		switch test.vcs {
+		case "git":
+			// Convert testDir to a git repository with a single commit, to
+			// simulate a real user's module-in-a-git-repo.
+			gitInit(t, testDir)
+		case "hg":
+			// Convert testDir to a mercurial repository to simulate a real
+			// user's module-in-a-hg-repo.
+			hgInit(t, testDir)
+		case "":
+			// No VCS.
+		default:
+			t.Fatalf("unknown vcs %q", test.vcs)
+		}
+
 		// Generate the report and compare it against the expected text.
 		var args []string
 		if test.baseVersion != "" {
@@ -373,3 +438,65 @@
 		}
 	}
 }
+
+// hgInit initialises a directory as a mercurial repo.
+func hgInit(t *testing.T, dir string) {
+	t.Helper()
+
+	if err := os.Mkdir(filepath.Join(dir, ".hg"), 0777); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := ioutil.WriteFile(filepath.Join(dir, ".hg", "branch"), []byte("default"), 0777); err != nil {
+		t.Fatal(err)
+	}
+}
+
+// gitInit initialises a directory as a git repo, and adds a simple commit.
+func gitInit(t *testing.T, dir string) {
+	t.Helper()
+
+	if !hasGit() {
+		t.Skip("PATH does not contain git")
+	}
+
+	stdout := &bytes.Buffer{}
+	stderr := &bytes.Buffer{}
+
+	for _, args := range [][]string{
+		{"git", "init"},
+		{"git", "checkout", "-b", "test"},
+		{"git", "add", "-A"},
+		{"git", "commit", "-m", "test"},
+	} {
+		cmd := exec.Command(args[0], args[1:]...)
+		cmd.Dir = dir
+		cmd.Stdout = stdout
+		cmd.Stderr = stderr
+		if err := cmd.Run(); err != nil {
+			cmdArgs := strings.Join(args, " ")
+			t.Fatalf("%s\n%s\nerror running %q on dir %s: %v", stdout.String(), stderr.String(), cmdArgs, dir, err)
+		}
+	}
+}
+
+// goModInit runs `go mod init` in the given directory.
+func goModInit(t *testing.T, dir string) {
+	t.Helper()
+
+	aContents := `package a
+const A = "a"`
+	if err := ioutil.WriteFile(filepath.Join(dir, "a.go"), []byte(aContents), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	stdout := &bytes.Buffer{}
+	stderr := &bytes.Buffer{}
+	cmd := exec.Command("go", "mod", "init", "example.com/uncommitted")
+	cmd.Stdout = stdout
+	cmd.Stderr = stderr
+	cmd.Dir = dir
+	if err := cmd.Run(); err != nil {
+		t.Fatalf("error running `go mod init`: %s, %v", stderr.String(), err)
+	}
+}

diff --git a/cmd/gorelease/testdata/basic/v0_compatible_suggest_git.test b/cmd/gorelease/testdata/basic/v0_compatible_suggest_git.test
new file mode 100644
index 0000000..080cafd
--- /dev/null
+++ b/cmd/gorelease/testdata/basic/v0_compatible_suggest_git.test

@@ -0,0 +1,16 @@
+mod=example.com/basic
+version=v0.1.0
+base=v0.0.1
+proxyVersions=example.com/basic@v0.0.1
+vcs=git
+-- want --
+# example.com/basic/a
+## compatible changes
+A2: added
+
+# example.com/basic/b
+## compatible changes
+package added
+
+# summary
+Suggested version: v0.1.0

diff --git a/cmd/gorelease/testdata/basic/v0_compatible_suggest_hg.test b/cmd/gorelease/testdata/basic/v0_compatible_suggest_hg.test
new file mode 100644
index 0000000..329ea88
--- /dev/null
+++ b/cmd/gorelease/testdata/basic/v0_compatible_suggest_hg.test

@@ -0,0 +1,16 @@
+mod=example.com/basic
+version=v0.1.0
+base=v0.0.1
+proxyVersions=example.com/basic@v0.0.1
+vcs=hg
+-- want --
+# example.com/basic/a
+## compatible changes
+A2: added
+
+# example.com/basic/b
+## compatible changes
+package added
+
+# summary
+Suggested version: v0.1.0

diff --git a/cmd/gorelease/testdata/errors/bad_filenames.test b/cmd/gorelease/testdata/errors/bad_filenames.test
index 279740f..c8bf6e0 100644
--- a/cmd/gorelease/testdata/errors/bad_filenames.test
+++ b/cmd/gorelease/testdata/errors/bad_filenames.test

@@ -2,10 +2,10 @@
 dir=x
 base=none
 error=true
+vcs=git
 -- want --
 testdata/this_file_also_has_a_bad_filename'.txt: malformed file path "testdata/this_file_also_has_a_bad_filename'.txt": invalid char '\''
 testdata/this_file_has_a_bad_filename'.txt: malformed file path "testdata/this_file_has_a_bad_filename'.txt": invalid char '\''
--- .git/HEAD --
 -- x/go.mod --
 module example.com/x
 

diff --git a/cmd/gorelease/testdata/patherrors/dup_roots_branch.test b/cmd/gorelease/testdata/patherrors/dup_roots_branch.test
index 695d744..66f79bb 100644
--- a/cmd/gorelease/testdata/patherrors/dup_roots_branch.test
+++ b/cmd/gorelease/testdata/patherrors/dup_roots_branch.test

@@ -1,8 +1,7 @@
 dir=dup
 base=none
 success=false
--- .git/empty --
-empty file to mark repository root
+vcs=git
 -- dup/go.mod --
 module example.com/dup/v2
 

diff --git a/cmd/gorelease/testdata/patherrors/dup_roots_dir.test b/cmd/gorelease/testdata/patherrors/dup_roots_dir.test
index f369704..3898da4 100644
--- a/cmd/gorelease/testdata/patherrors/dup_roots_dir.test
+++ b/cmd/gorelease/testdata/patherrors/dup_roots_dir.test

@@ -1,8 +1,7 @@
 dir=dup/v2
 base=none
 success=false
--- .git/empty --
-empty file to mark repository root
+vcs=git
 -- dup/go.mod --
 module example.com/dup/v2
 

diff --git a/cmd/gorelease/testdata/patherrors/dup_roots_ok.test b/cmd/gorelease/testdata/patherrors/dup_roots_ok.test
index bc71480..afd157c 100644
--- a/cmd/gorelease/testdata/patherrors/dup_roots_ok.test
+++ b/cmd/gorelease/testdata/patherrors/dup_roots_ok.test

@@ -1,7 +1,6 @@
 dir=dup/v2
 base=none
--- .git/empty --
-empty file to mark repository root
+vcs=git
 -- dup/go.mod --
 module example.com/dup
 

diff --git a/cmd/gorelease/testdata/patherrors/gopkginsub.test b/cmd/gorelease/testdata/patherrors/gopkginsub.test
index 2cfcdd9..2c2fb7c 100644
--- a/cmd/gorelease/testdata/patherrors/gopkginsub.test
+++ b/cmd/gorelease/testdata/patherrors/gopkginsub.test

@@ -2,6 +2,7 @@
 base=none
 dir=yaml
 success=false
+vcs=git
 -- want --
 # diagnostics
 go.mod: go directive is missing
@@ -11,6 +12,5 @@
 Suggested version: v2.0.0
 -- .mod --
 module example.com/patherrors
--- .git/HEAD --
 -- yaml/go.mod --
 module gopkg.in/yaml.v2

diff --git a/cmd/gorelease/testdata/patherrors/pathsub.test b/cmd/gorelease/testdata/patherrors/pathsub.test
index 04dbdd6..000c02a 100644
--- a/cmd/gorelease/testdata/patherrors/pathsub.test
+++ b/cmd/gorelease/testdata/patherrors/pathsub.test

@@ -2,6 +2,7 @@
 dir=x
 base=none
 success=false
+vcs=git
 -- want --
 # diagnostics
 example.com/y: module path must end with "x", since it is in subdirectory "x"
@@ -10,7 +11,6 @@
 Suggested version: v0.1.0
 -- .mod --
 module example.com/patherrors
--- .git/HEAD --
 -- x/go.mod --
 module example.com/y
 

diff --git a/cmd/gorelease/testdata/patherrors/pathsubv2.test b/cmd/gorelease/testdata/patherrors/pathsubv2.test
index 76cc284..8ab2c80 100644
--- a/cmd/gorelease/testdata/patherrors/pathsubv2.test
+++ b/cmd/gorelease/testdata/patherrors/pathsubv2.test

@@ -2,6 +2,7 @@
 base=none
 dir=x
 success=false
+vcs=git
 -- want --
 # diagnostics
 example.com/y/v2: module path must end with "x" or "x/v2", since it is in subdirectory "x"
@@ -10,7 +11,6 @@
 Suggested version: v2.0.0
 -- .mod --
 module example.com/patherrors
--- .git/HEAD --
 -- x/go.mod --
 module example.com/y/v2
 

diff --git a/cmd/gorelease/testdata/sub/nest.test b/cmd/gorelease/testdata/sub/nest.test
index e8a2aae..c2a6436 100644
--- a/cmd/gorelease/testdata/sub/nest.test
+++ b/cmd/gorelease/testdata/sub/nest.test

@@ -1,10 +1,10 @@
 mod=example.com/sub/nest
 dir=nest
 base=v1.0.0
+vcs=git
 -- want --
 # summary
 Suggested version: v1.0.1 (with tag nest/v1.0.1)
--- .git/HEAD --
 -- nest/go.mod --
 module example.com/sub/nest
 

diff --git a/cmd/gorelease/testdata/sub/nest_v2.test b/cmd/gorelease/testdata/sub/nest_v2.test
index 209029e..6d8bf0c 100644
--- a/cmd/gorelease/testdata/sub/nest_v2.test
+++ b/cmd/gorelease/testdata/sub/nest_v2.test

@@ -1,10 +1,10 @@
 mod=example.com/sub/nest/v2
 dir=nest
 base=v2.0.0
+vcs=git
 -- want --
 # summary
 Suggested version: v2.0.1 (with tag nest/v2.0.1)
--- .git/HEAD --
 -- nest/go.mod --
 module example.com/sub/nest/v2
 

diff --git a/cmd/gorelease/testdata/sub/nest_v2_dir.test b/cmd/gorelease/testdata/sub/nest_v2_dir.test
index 8d549e5..7e99c1e 100644
--- a/cmd/gorelease/testdata/sub/nest_v2_dir.test
+++ b/cmd/gorelease/testdata/sub/nest_v2_dir.test

@@ -1,10 +1,10 @@
 mod=example.com/sub/nest/v2
 dir=nest/v2
 base=v2.0.0
+vcs=git
 -- want --
 # summary
 Suggested version: v2.0.1 (with tag nest/v2.0.1)
--- .git/HEAD --
 -- nest/v2/go.mod --
 module example.com/sub/nest/v2
 

diff --git a/cmd/gorelease/testdata/sub/v2_dir.test b/cmd/gorelease/testdata/sub/v2_dir.test
index feb690a..ed07eb9 100644
--- a/cmd/gorelease/testdata/sub/v2_dir.test
+++ b/cmd/gorelease/testdata/sub/v2_dir.test

@@ -1,10 +1,10 @@
 mod=example.com/sub/v2
 dir=v2
 base=v2.0.0
+vcs=git
 -- want --
 # summary
 Suggested version: v2.0.1
--- .git/HEAD --
 -- v2/go.mod --
 module example.com/sub/v2
 

diff --git a/cmd/gorelease/testdata/sub/v2_root.test b/cmd/gorelease/testdata/sub/v2_root.test
index ce6d68f..1a34c67 100644
--- a/cmd/gorelease/testdata/sub/v2_root.test
+++ b/cmd/gorelease/testdata/sub/v2_root.test

@@ -1,9 +1,9 @@
 mod=example.com/sub/v2
 base=v2.0.0
+vcs=git
 -- want --
 # summary
 Suggested version: v2.0.1
--- .git/HEAD --
 -- go.mod --
 module example.com/sub/v2
 

diff --git a/go.mod b/go.mod
index e32b3f4..686f45b 100644
--- a/go.mod
+++ b/go.mod

@@ -21,7 +21,7 @@
 	go.uber.org/zap v1.16.0
 	golang.org/x/image v0.0.0-20190802002840-cff245a6509b
 	golang.org/x/mobile v0.0.0-20201217150744-e6ae53a27f4f
-	golang.org/x/mod v0.4.2
+	golang.org/x/mod v0.5.1-0.20210830214625-1b1db11ec8f4
 	golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4
 	golang.org/x/tools v0.1.0
 	golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1

diff --git a/go.sum b/go.sum
index 5d70f5f..6c1ce5d 100644
--- a/go.sum
+++ b/go.sum

@@ -307,8 +307,8 @@
 golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg=
 golang.org/x/mod v0.1.1-0.20191209134235-331c550502dd/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
-golang.org/x/mod v0.4.2 h1:Gz96sIWK3OalVv/I/qNygP42zyoKp3xptRVCWRFEBvo=
-golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
+golang.org/x/mod v0.5.1-0.20210830214625-1b1db11ec8f4 h1:7Qds88gNaRx0Dz/1wOwXlR7asekh1B1u26wEwN6FcEI=
+golang.org/x/mod v0.5.1-0.20210830214625-1b1db11ec8f4/go.mod h1:5OXOZSfqPIIbmVBIIKWRFfZjPR0E5r58TLhUjH0a2Ro=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=