git-codereview: guard against ambiguous revision parameters
Change-Id: I3afcf06e1eab28bfdb2c6ace7aaf774070084900
Reviewed-on: https://go-review.googlesource.com/1783
Reviewed-by: Russ Cox <rsc@golang.org>
diff --git a/git-codereview/branch.go b/git-codereview/branch.go
index a16de5e..128a577 100644
--- a/git-codereview/branch.go
+++ b/git-codereview/branch.go
@@ -117,7 +117,7 @@
}
const numField = 5
- all := getOutput("git", "log", "--format=format:%H%x00%h%x00%P%x00%s%x00%B%x00", b.OriginBranch()+".."+b.Name)
+ all := getOutput("git", "log", "--format=format:%H%x00%h%x00%P%x00%s%x00%B%x00", b.OriginBranch()+".."+b.Name, "--")
fields := strings.Split(all, "\x00")
if len(fields) < numField {
return // nothing pending
@@ -147,7 +147,7 @@
b.commitsAhead++
}
b.commitsAhead = len(fields) / numField
- b.commitsBehind = len(getOutput("git", "log", "--format=format:x", b.Name+".."+b.OriginBranch()))
+ b.commitsBehind = len(getOutput("git", "log", "--format=format:x", b.Name+".."+b.OriginBranch(), "--"))
}
// Submitted reports whether some form of b's pending commit
@@ -157,7 +157,7 @@
return false
}
line := "Change-Id: " + id
- out := getOutput("git", "log", "-n", "1", "-F", "--grep", line, b.Name+".."+b.OriginBranch())
+ out := getOutput("git", "log", "-n", "1", "-F", "--grep", line, b.Name+".."+b.OriginBranch(), "--")
return strings.Contains(out, line)
}
diff --git a/git-codereview/branch_test.go b/git-codereview/branch_test.go
index 1c18cfa..6bc52f3 100644
--- a/git-codereview/branch_test.go
+++ b/git-codereview/branch_test.go
@@ -91,3 +91,18 @@
t.Errorf("LocalBranches() = %v, want %v", names, want)
}
}
+
+func TestAmbiguousRevision(t *testing.T) {
+ gt := newGitTest(t)
+ defer gt.done()
+ gt.work(t)
+
+ t.Logf("creating file paths that conflict with revision parameters")
+ mkdir(t, gt.client+"/origin")
+ write(t, gt.client+"/origin/master..work", "Uh-Oh! SpaghettiOs")
+ mkdir(t, gt.client+"/work..origin")
+ write(t, gt.client+"/work..origin/master", "Be sure to drink your Ovaltine")
+
+ b := CurrentBranch()
+ b.Submitted("I123456789")
+}
diff --git a/git-codereview/gofmt.go b/git-codereview/gofmt.go
index 648b795..529364b 100644
--- a/git-codereview/gofmt.go
+++ b/git-codereview/gofmt.go
@@ -119,7 +119,7 @@
}
// Find files modified in the index compared to the branchpoint.
- indexFiles := addRoot(repo, filter(gofmtRequired, getLines("git", "diff", "--name-only", "--diff-filter=ACM", "--cached", b.Branchpoint())))
+ indexFiles := addRoot(repo, filter(gofmtRequired, getLines("git", "diff", "--name-only", "--diff-filter=ACM", "--cached", b.Branchpoint(), "--")))
localFiles := addRoot(repo, filter(gofmtRequired, getLines("git", "diff", "--name-only", "--diff-filter=ACM")))
localFilesMap := stringMap(localFiles)
isUnstaged := func(file string) bool {
diff --git a/git-codereview/gofmt_test.go b/git-codereview/gofmt_test.go
index 8cd3d81..94fd132 100644
--- a/git-codereview/gofmt_test.go
+++ b/git-codereview/gofmt_test.go
@@ -169,3 +169,13 @@
testNoStdout(t)
testPrintedStderr(t, wantErr...)
}
+
+func TestGofmtAmbiguousRevision(t *testing.T) {
+ gt := newGitTest(t)
+ defer gt.done()
+
+ t.Logf("creating file that conflicts with revision parameter")
+ write(t, gt.client+"/HEAD", "foo")
+
+ testMain(t, "gofmt")
+}
diff --git a/git-codereview/mail.go b/git-codereview/mail.go
index c06a784..9a44b18 100644
--- a/git-codereview/mail.go
+++ b/git-codereview/mail.go
@@ -36,7 +36,7 @@
}
if *diff {
- run("git", "diff", b.Branchpoint()+"..HEAD")
+ run("git", "diff", b.Branchpoint()+"..HEAD", "--")
return
}
diff --git a/git-codereview/mail_test.go b/git-codereview/mail_test.go
index 8b80f9f..e3543df 100644
--- a/git-codereview/mail_test.go
+++ b/git-codereview/mail_test.go
@@ -35,3 +35,16 @@
testMainDied(t, "mail")
testPrintedStderr(t, "git origin must be a Gerrit host, not GitHub: https://github.com/golang/go")
}
+
+func TestMailAmbiguousRevision(t *testing.T) {
+ gt := newGitTest(t)
+ defer gt.done()
+ gt.work(t)
+
+ t.Logf("creating file that conflicts with revision parameter")
+ b := CurrentBranch()
+ mkdir(t, gt.client+"/origin")
+ write(t, gt.client+"/"+b.Branchpoint()+"..HEAD", "foo")
+
+ testMain(t, "mail", "-diff")
+}
diff --git a/git-codereview/pending.go b/git-codereview/pending.go
index a90f6ce..de9e4f5 100644
--- a/git-codereview/pending.go
+++ b/git-codereview/pending.go
@@ -41,7 +41,7 @@
b.staged, b.unstaged, b.untracked = LocalChanges()
}
if b.parentHash != "" && b.commitHash != "" {
- b.committed = getLines("git", "diff", "--name-only", b.parentHash, b.commitHash)
+ b.committed = getLines("git", "diff", "--name-only", b.parentHash, b.commitHash, "--")
}
if !pendingLocal {
b.g, b.gerr = b.GerritChange("DETAILED_LABELS", "CURRENT_REVISION", "MESSAGES", "DETAILED_ACCOUNTS")
