[gitiles] Tweaks needed for GitSync
Change-Id: Ieb548a8571383dd14980f9eadf32ec802a96b0db
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/235537
Reviewed-by: Kevin Lubick <kjlubick@google.com>
Commit-Queue: Eric Boren <borenet@google.com>
diff --git a/autoroll/go/autoroll-google3/google3_test.go b/autoroll/go/autoroll-google3/google3_test.go
index f50d12a..7b1c2c7 100644
--- a/autoroll/go/autoroll-google3/google3_test.go
+++ b/autoroll/go/autoroll-google3/google3_test.go
@@ -100,7 +100,7 @@
commits = append(commits, gb.CommitGen(ctx, "a.txt"))
mockChild.MockGetCommit(ctx, "master")
- mockChild.MockLog(ctx, commits[0], commits[1])
+ mockChild.MockLog(ctx, git.LogFromTo(commits[0], commits[1]))
assert.NoError(t, a.UpdateStatus(ctx, "", true))
status := a.status.Get()
assert.Equal(t, 0, status.NumFailedRolls)
@@ -128,7 +128,7 @@
recent := []*autoroll.AutoRollIssue{issue4, issue3, issue2, issue1}
mockChild.MockGetCommit(ctx, "master")
- mockChild.MockLog(ctx, commits[0], commits[2])
+ mockChild.MockLog(ctx, git.LogFromTo(commits[0], commits[2]))
assert.NoError(t, a.UpdateStatus(ctx, "error message", false))
status = a.status.Get()
assert.Equal(t, 2, status.NumFailedRolls)
@@ -141,7 +141,7 @@
// Test preserving error.
mockChild.MockGetCommit(ctx, "master")
- mockChild.MockLog(ctx, commits[0], commits[2])
+ mockChild.MockLog(ctx, git.LogFromTo(commits[0], commits[2]))
assert.NoError(t, a.UpdateStatus(ctx, "", true))
status = a.status.Get()
assert.Equal(t, "error message", status.Error)
@@ -160,7 +160,7 @@
recent = append([]*autoroll.AutoRollIssue{issueI}, recent...)
}
mockChild.MockGetCommit(ctx, "master")
- mockChild.MockLog(ctx, commits[0], commits[2])
+ mockChild.MockLog(ctx, git.LogFromTo(commits[0], commits[2]))
assert.NoError(t, a.UpdateStatus(ctx, "error message", false))
status = a.status.Get()
assert.Equal(t, recent_rolls.RECENT_ROLLS_LENGTH+1, status.NumFailedRolls)
@@ -188,7 +188,7 @@
closeIssue(issue2, autoroll.ROLL_RESULT_SUCCESS)
assert.NoError(t, a.AddOrUpdateIssue(ctx, issue2, http.MethodPut))
mockChild.MockGetCommit(ctx, "master")
- mockChild.MockLog(ctx, commits[1], commits[2])
+ mockChild.MockLog(ctx, git.LogFromTo(commits[1], commits[2]))
assert.NoError(t, a.UpdateStatus(ctx, "", true))
deepequal.AssertDeepEqual(t, []*autoroll.AutoRollIssue{issue2, issue1}, a.status.Get().Recent)
@@ -198,7 +198,7 @@
issue4 := makeIssue(4, commits[2])
assert.NoError(t, a.AddOrUpdateIssue(ctx, issue4, http.MethodPost))
mockChild.MockGetCommit(ctx, "master")
- mockChild.MockLog(ctx, commits[1], commits[2])
+ mockChild.MockLog(ctx, git.LogFromTo(commits[1], commits[2]))
assert.NoError(t, a.UpdateStatus(ctx, "", true))
issue3.Closed = true
issue3.Result = autoroll.ROLL_RESULT_FAILURE
@@ -209,7 +209,7 @@
closeIssue(issue5, autoroll.ROLL_RESULT_SUCCESS)
assert.NoError(t, a.AddOrUpdateIssue(ctx, issue5, http.MethodPut))
mockChild.MockGetCommit(ctx, "master")
- mockChild.MockLog(ctx, commits[2], commits[2])
+ mockChild.MockLog(ctx, git.LogFromTo(commits[2], commits[2]))
assert.NoError(t, a.UpdateStatus(ctx, "", true))
issue4.Closed = true
issue4.Result = autoroll.ROLL_RESULT_FAILURE
diff --git a/autoroll/go/repo_manager/android_repo_manager.go b/autoroll/go/repo_manager/android_repo_manager.go
index 1937f49..28fc1d2 100644
--- a/autoroll/go/repo_manager/android_repo_manager.go
+++ b/autoroll/go/repo_manager/android_repo_manager.go
@@ -18,6 +18,7 @@
"go.skia.org/infra/go/common"
"go.skia.org/infra/go/exec"
"go.skia.org/infra/go/gerrit"
+ "go.skia.org/infra/go/git"
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/go/util"
"go.skia.org/infra/go/vcsinfo"
@@ -281,7 +282,7 @@
// Create the roll CL.
cr := r.childRepo
- commits, err := cr.RevList(ctx, fmt.Sprintf("%s..%s", from.Id, to.Id))
+ commits, err := cr.RevList(ctx, git.LogFromTo(from.Id, to.Id))
if err != nil {
return 0, fmt.Errorf("Failed to list revisions: %s", err)
}
@@ -508,7 +509,7 @@
return []*revision.Revision{}, nil
}
// Only consider commits on the "main" branch as roll candidates.
- commits, err := r.childRepo.RevList(ctx, "--ancestry-path", "--first-parent", fmt.Sprintf("%s..%s", lastRollRev.Id, head))
+ commits, err := r.childRepo.RevList(ctx, "--ancestry-path", "--first-parent", git.LogFromTo(lastRollRev.Id, head))
if err != nil {
return nil, err
}
diff --git a/autoroll/go/repo_manager/copy_repo_manager.go b/autoroll/go/repo_manager/copy_repo_manager.go
index 174daf2..2cfcf7f 100644
--- a/autoroll/go/repo_manager/copy_repo_manager.go
+++ b/autoroll/go/repo_manager/copy_repo_manager.go
@@ -163,7 +163,7 @@
}()
// List the revisions in the roll.
- commits, err := rm.childRepo.RevList(ctx, fmt.Sprintf("%s..%s", from.Id, to.Id))
+ commits, err := rm.childRepo.RevList(ctx, git.LogFromTo(from.Id, to.Id))
if err != nil {
return 0, fmt.Errorf("Failed to list revisions: %s", err)
}
diff --git a/autoroll/go/repo_manager/deps_repo_manager.go b/autoroll/go/repo_manager/deps_repo_manager.go
index 9fd8a04..9f8d22f 100644
--- a/autoroll/go/repo_manager/deps_repo_manager.go
+++ b/autoroll/go/repo_manager/deps_repo_manager.go
@@ -15,6 +15,7 @@
"go.skia.org/infra/autoroll/go/revision"
"go.skia.org/infra/go/exec"
"go.skia.org/infra/go/gerrit"
+ "go.skia.org/infra/go/git"
"go.skia.org/infra/go/issues"
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/go/util"
@@ -162,7 +163,7 @@
// Create the roll CL.
cr := dr.childRepo
- commits, err := cr.RevList(ctx, fmt.Sprintf("%s..%s", from.Id, to.Id))
+ commits, err := cr.RevList(ctx, git.LogFromTo(from.Id, to.Id))
if err != nil {
return 0, fmt.Errorf("Failed to list revisions: %s", err)
}
diff --git a/autoroll/go/repo_manager/freetype_repo_manager.go b/autoroll/go/repo_manager/freetype_repo_manager.go
index 038f4b5..eb038b1 100644
--- a/autoroll/go/repo_manager/freetype_repo_manager.go
+++ b/autoroll/go/repo_manager/freetype_repo_manager.go
@@ -182,7 +182,7 @@
}
// Check modules.cfg. Give up if it has changed.
- diff, err := rm.localChildRepo.Git(ctx, "diff", "--name-only", fmt.Sprintf("%s..%s", from.Id, to.Id))
+ diff, err := rm.localChildRepo.Git(ctx, "diff", "--name-only", git.LogFromTo(from.Id, to.Id))
if err != nil {
return "", nil, err
}
diff --git a/autoroll/go/repo_manager/freetype_repo_manager_test.go b/autoroll/go/repo_manager/freetype_repo_manager_test.go
index 4fbd3c5..5bb6e67 100644
--- a/autoroll/go/repo_manager/freetype_repo_manager_test.go
+++ b/autoroll/go/repo_manager/freetype_repo_manager_test.go
@@ -110,7 +110,7 @@
parentMaster, err := git.GitDir(parent.Dir()).RevParse(ctx, "HEAD")
assert.NoError(t, err)
mockParent.MockReadFile(ctx, "DEPS", parentMaster)
- mockChild.MockLog(ctx, childCommits[0], "master")
+ mockChild.MockLog(ctx, git.LogFromTo(childCommits[0], "master"))
mockChild.MockGetCommit(ctx, childCommits[0])
rm, err := NewFreeTypeRepoManager(ctx, cfg, wd, g, recipesCfg, "fake.server.com", "", urlmock.Client(), gerritCR(t, g), false)
@@ -135,7 +135,7 @@
parentMaster, err := git.GitDir(parentRepo.Dir()).RevParse(ctx, "HEAD")
assert.NoError(t, err)
mockParent.MockReadFile(ctx, "DEPS", parentMaster)
- mockChild.MockLog(ctx, childCommits[0], "master")
+ mockChild.MockLog(ctx, git.LogFromTo(childCommits[0], "master"))
mockChild.MockGetCommit(ctx, childCommits[0])
nextRollRev := childCommits[len(childCommits)-1]
assert.NoError(t, rm.Update(ctx))
@@ -154,7 +154,7 @@
rev, err := rm.GetRevision(ctx, c)
assert.NoError(t, err)
assert.Equal(t, c, rev.Id)
- mockChild.MockLog(ctx, c, childCommits[0])
+ mockChild.MockLog(ctx, git.LogFromTo(c, childCommits[0]))
rp, err := rm.RolledPast(ctx, rev)
assert.NoError(t, err)
assert.False(t, rp)
@@ -169,7 +169,7 @@
parentMaster, err := git.GitDir(parentRepo.Dir()).RevParse(ctx, "HEAD")
assert.NoError(t, err)
mockParent.MockReadFile(ctx, "DEPS", parentMaster)
- mockChild.MockLog(ctx, childCommits[0], "master")
+ mockChild.MockLog(ctx, git.LogFromTo(childCommits[0], "master"))
mockChild.MockGetCommit(ctx, childCommits[0])
nextRollRev := childCommits[1]
assert.NoError(t, rm.Update(ctx))
@@ -179,7 +179,7 @@
assert.NoError(t, SetStrategy(ctx, rm, strategy.ROLL_STRATEGY_BATCH))
mockParent.MockGetCommit(ctx, "master")
mockParent.MockReadFile(ctx, "DEPS", parentMaster)
- mockChild.MockLog(ctx, childCommits[0], "master")
+ mockChild.MockLog(ctx, git.LogFromTo(childCommits[0], "master"))
mockChild.MockGetCommit(ctx, childCommits[0])
assert.NoError(t, rm.Update(ctx))
assert.Equal(t, childCommits[len(childCommits)-1], rm.NextRollRev().Id)
@@ -187,7 +187,7 @@
assert.NoError(t, SetStrategy(ctx, rm, strategy.ROLL_STRATEGY_SINGLE))
mockParent.MockGetCommit(ctx, "master")
mockParent.MockReadFile(ctx, "DEPS", parentMaster)
- mockChild.MockLog(ctx, childCommits[0], "master")
+ mockChild.MockLog(ctx, git.LogFromTo(childCommits[0], "master"))
mockChild.MockGetCommit(ctx, childCommits[0])
assert.NoError(t, rm.Update(ctx))
assert.Equal(t, childCommits[1], rm.NextRollRev().Id)
@@ -201,7 +201,7 @@
parentMaster, err := git.GitDir(parentRepo.Dir()).RevParse(ctx, "HEAD")
assert.NoError(t, err)
mockParent.MockReadFile(ctx, "DEPS", parentMaster)
- mockChild.MockLog(ctx, childCommits[0], "master")
+ mockChild.MockLog(ctx, git.LogFromTo(childCommits[0], "master"))
mockChild.MockGetCommit(ctx, childCommits[0])
nextRollRev := childCommits[len(childCommits)-1]
assert.NoError(t, rm.Update(ctx))
@@ -224,7 +224,7 @@
// Mock the initial change creation.
logStr := ""
childGitRepo := git.GitDir(childRepo.Dir())
- commitsToRoll, err := childGitRepo.RevList(ctx, fmt.Sprintf("%s..%s", lastRollRev, nextRollRev))
+ commitsToRoll, err := childGitRepo.RevList(ctx, git.LogFromTo(lastRollRev, nextRollRev))
assert.NoError(t, err)
for _, c := range commitsToRoll {
mockChild.MockGetCommit(ctx, c)
diff --git a/autoroll/go/repo_manager/github_deps_repo_manager.go b/autoroll/go/repo_manager/github_deps_repo_manager.go
index 6ada471..ee6c244 100644
--- a/autoroll/go/repo_manager/github_deps_repo_manager.go
+++ b/autoroll/go/repo_manager/github_deps_repo_manager.go
@@ -304,7 +304,7 @@
// Build the commit message.
user, repo := GetUserAndRepo(rm.childRepoUrl)
- commits, err := rm.childRepo.RevList(ctx, "--no-merges", fmt.Sprintf("%s..%s", from.Id, to.Id))
+ commits, err := rm.childRepo.RevList(ctx, "--no-merges", git.LogFromTo(from.Id, to.Id))
if err != nil {
return 0, fmt.Errorf("Failed to list revisions: %s", err)
}
diff --git a/autoroll/go/repo_manager/github_repo_manager.go b/autoroll/go/repo_manager/github_repo_manager.go
index 6db3d39..263b9bf 100644
--- a/autoroll/go/repo_manager/github_repo_manager.go
+++ b/autoroll/go/repo_manager/github_repo_manager.go
@@ -301,7 +301,7 @@
// Build the commit message.
user, repo := GetUserAndRepo(rm.childRepoURL)
- commits, err := rm.childRepo.RevList(ctx, "--no-merges", fmt.Sprintf("%s..%s", from.Id, to.Id))
+ commits, err := rm.childRepo.RevList(ctx, "--no-merges", git.LogFromTo(from.Id, to.Id))
if err != nil {
return 0, fmt.Errorf("Failed to list revisions: %s", err)
}
diff --git a/autoroll/go/repo_manager/no_checkout_deps_repo_manager.go b/autoroll/go/repo_manager/no_checkout_deps_repo_manager.go
index 7b95630..14f4651 100644
--- a/autoroll/go/repo_manager/no_checkout_deps_repo_manager.go
+++ b/autoroll/go/repo_manager/no_checkout_deps_repo_manager.go
@@ -17,6 +17,7 @@
"go.skia.org/infra/go/depot_tools"
"go.skia.org/infra/go/exec"
"go.skia.org/infra/go/gerrit"
+ "go.skia.org/infra/go/git"
"go.skia.org/infra/go/gitiles"
"go.skia.org/infra/go/issues"
"go.skia.org/infra/go/sklog"
@@ -267,7 +268,7 @@
if rev.Id == rm.lastRollRev.Id {
return true, nil
}
- commits, err := rm.childRepo.Log(ctx, rev.Id, rm.lastRollRev.Id)
+ commits, err := rm.childRepo.Log(ctx, git.LogFromTo(rev.Id, rm.lastRollRev.Id))
if err != nil {
return false, err
}
diff --git a/autoroll/go/repo_manager/no_checkout_deps_repo_manager_test.go b/autoroll/go/repo_manager/no_checkout_deps_repo_manager_test.go
index 48ece00..0a18186 100644
--- a/autoroll/go/repo_manager/no_checkout_deps_repo_manager_test.go
+++ b/autoroll/go/repo_manager/no_checkout_deps_repo_manager_test.go
@@ -79,7 +79,7 @@
parentMaster, err := git.GitDir(parent.Dir()).RevParse(ctx, "HEAD")
assert.NoError(t, err)
mockParent.MockReadFile(ctx, "DEPS", parentMaster)
- mockChild.MockLog(ctx, childCommits[0], "master")
+ mockChild.MockLog(ctx, git.LogFromTo(childCommits[0], "master"))
mockChild.MockGetCommit(ctx, childCommits[0])
rm, err := NewNoCheckoutDEPSRepoManager(ctx, cfg, wd, g, recipesCfg, "fake.server.com", "", urlmock.Client(), gerritCR(t, g), false)
@@ -118,7 +118,7 @@
parentMaster, err := git.GitDir(parentRepo.Dir()).RevParse(ctx, "HEAD")
assert.NoError(t, err)
mockParent.MockReadFile(ctx, "DEPS", parentMaster)
- mockChild.MockLog(ctx, childCommits[0], "master")
+ mockChild.MockLog(ctx, git.LogFromTo(childCommits[0], "master"))
mockChild.MockGetCommit(ctx, childCommits[0])
nextRollRev := childCommits[len(childCommits)-1]
assert.NoError(t, rm.Update(ctx))
@@ -139,7 +139,7 @@
rev, err := rm.GetRevision(ctx, c)
assert.NoError(t, err)
assert.Equal(t, c, rev.Id)
- mockChild.MockLog(ctx, c, childCommits[0])
+ mockChild.MockLog(ctx, git.LogFromTo(c, childCommits[0]))
rp, err := rm.RolledPast(ctx, rev)
assert.NoError(t, err)
assert.False(t, rp)
@@ -155,7 +155,7 @@
parentMaster, err := git.GitDir(parentRepo.Dir()).RevParse(ctx, "HEAD")
assert.NoError(t, err)
mockParent.MockReadFile(ctx, "DEPS", parentMaster)
- mockChild.MockLog(ctx, childCommits[0], "master")
+ mockChild.MockLog(ctx, git.LogFromTo(childCommits[0], "master"))
mockChild.MockGetCommit(ctx, childCommits[0])
nextRollRev := childCommits[1]
assert.NoError(t, rm.Update(ctx))
@@ -165,7 +165,7 @@
assert.NoError(t, SetStrategy(ctx, rm, strategy.ROLL_STRATEGY_BATCH))
mockParent.MockGetCommit(ctx, "master")
mockParent.MockReadFile(ctx, "DEPS", parentMaster)
- mockChild.MockLog(ctx, childCommits[0], "master")
+ mockChild.MockLog(ctx, git.LogFromTo(childCommits[0], "master"))
mockChild.MockGetCommit(ctx, childCommits[0])
assert.NoError(t, rm.Update(ctx))
assert.Equal(t, childCommits[len(childCommits)-1], rm.NextRollRev().Id)
@@ -173,7 +173,7 @@
assert.NoError(t, SetStrategy(ctx, rm, strategy.ROLL_STRATEGY_SINGLE))
mockParent.MockGetCommit(ctx, "master")
mockParent.MockReadFile(ctx, "DEPS", parentMaster)
- mockChild.MockLog(ctx, childCommits[0], "master")
+ mockChild.MockLog(ctx, git.LogFromTo(childCommits[0], "master"))
mockChild.MockGetCommit(ctx, childCommits[0])
assert.NoError(t, rm.Update(ctx))
assert.Equal(t, childCommits[1], rm.NextRollRev().Id)
@@ -188,7 +188,7 @@
parentMaster, err := git.GitDir(parentRepo.Dir()).RevParse(ctx, "HEAD")
assert.NoError(t, err)
mockParent.MockReadFile(ctx, "DEPS", parentMaster)
- mockChild.MockLog(ctx, childCommits[0], "master")
+ mockChild.MockLog(ctx, git.LogFromTo(childCommits[0], "master"))
mockChild.MockGetCommit(ctx, childCommits[0])
nextRollRev := childCommits[len(childCommits)-1]
assert.NoError(t, rm.Update(ctx))
@@ -201,7 +201,7 @@
// Mock the initial change creation.
logStr := ""
childGitRepo := git.GitDir(childRepo.Dir())
- commitsToRoll, err := childGitRepo.RevList(ctx, fmt.Sprintf("%s..%s", lastRollRev, nextRollRev))
+ commitsToRoll, err := childGitRepo.RevList(ctx, git.LogFromTo(lastRollRev, nextRollRev))
assert.NoError(t, err)
for _, c := range commitsToRoll {
mockChild.MockGetCommit(ctx, c)
@@ -301,7 +301,7 @@
parentMaster, err := git.GitDir(parentRepo.Dir()).RevParse(ctx, "HEAD")
assert.NoError(t, err)
mockParent.MockReadFile(ctx, "DEPS", parentMaster)
- mockChild.MockLog(ctx, childCommits[0], "master")
+ mockChild.MockLog(ctx, git.LogFromTo(childCommits[0], "master"))
mockChild.MockGetCommit(ctx, childCommits[0])
nextRollRev := childCommits[len(childCommits)-1]
assert.NoError(t, rm.Update(ctx))
@@ -317,7 +317,7 @@
// Mock the initial change creation.
logStr := ""
childGitRepo := git.GitDir(childRepo.Dir())
- commitsToRoll, err := childGitRepo.RevList(ctx, fmt.Sprintf("%s..%s", lastRollRev, nextRollRev))
+ commitsToRoll, err := childGitRepo.RevList(ctx, git.LogFromTo(lastRollRev, nextRollRev))
assert.NoError(t, err)
for _, c := range commitsToRoll {
mockChild.MockGetCommit(ctx, c)
diff --git a/autoroll/go/repo_manager/repo_manager.go b/autoroll/go/repo_manager/repo_manager.go
index 478da33..84b494ea0 100644
--- a/autoroll/go/repo_manager/repo_manager.go
+++ b/autoroll/go/repo_manager/repo_manager.go
@@ -311,7 +311,7 @@
if head == lastRollRev.Id {
return []*revision.Revision{}, nil
}
- commits, err := r.childRepo.RevList(ctx, fmt.Sprintf("%s..%s", lastRollRev.Id, head))
+ commits, err := r.childRepo.RevList(ctx, git.LogFromTo(lastRollRev.Id, head))
if err != nil {
return nil, err
}
diff --git a/gitsync/go/gitsync/watcher.go b/gitsync/go/gitsync/watcher.go
index cb8b5ab..51691f5 100644
--- a/gitsync/go/gitsync/watcher.go
+++ b/gitsync/go/gitsync/watcher.go
@@ -126,7 +126,7 @@
if anc {
// Only get the commits between the old and new head.
- revListStr = fmt.Sprintf("%s..%s", foundBranch.Head, newBranch.Head)
+ revListStr = git.LogFromTo(foundBranch.Head, newBranch.Head)
}
}
diff --git a/go/git/gitinfo/gitinfo.go b/go/git/gitinfo/gitinfo.go
index f1390b2..1b90967 100644
--- a/go/git/gitinfo/gitinfo.go
+++ b/go/git/gitinfo/gitinfo.go
@@ -15,6 +15,7 @@
"go.skia.org/infra/go/depot_tools"
"go.skia.org/infra/go/exec"
+ "go.skia.org/infra/go/git"
"go.skia.org/infra/go/sklog"
"go.skia.org/infra/go/tiling"
"go.skia.org/infra/go/util"
@@ -312,7 +313,7 @@
func (g *GitInfo) IndexOf(ctx context.Context, hash string) (int, error) {
// Count the lines from running:
// git rev-list --count <first-commit>..hash.
- output, err := g.RevList(ctx, "--count", fmt.Sprintf("%s..%s", g.firstCommit, hash))
+ output, err := g.RevList(ctx, "--count", git.LogFromTo(g.firstCommit, hash))
if err != nil {
return 0, fmt.Errorf("git rev-list failed: %s", err)
}
diff --git a/go/git/repograph/shared_tests/shared_tests.go b/go/git/repograph/shared_tests/shared_tests.go
index 2ecc52c..dd150f0 100644
--- a/go/git/repograph/shared_tests/shared_tests.go
+++ b/go/git/repograph/shared_tests/shared_tests.go
@@ -385,7 +385,7 @@
if from == "" {
cmd = append(cmd, to)
} else {
- cmd = append(cmd, "--ancestry-path", fmt.Sprintf("%s..%s", from, to))
+ cmd = append(cmd, "--ancestry-path", git.LogFromTo(from, to))
}
hashes, err := gitdir.RevList(ctx, cmd...)
assert.NoError(t, err)
@@ -702,7 +702,7 @@
check := func(from, to string, expectOrig []string) {
expect := util.CopyStringSlice(expectOrig)
- revs, err := co.RevList(ctx, fmt.Sprintf("%s..%s", from, to))
+ revs, err := co.RevList(ctx, git.LogFromTo(from, to))
assert.NoError(t, err)
// Sanity check; assert that the commits returned from git are
// in reverse topological order.
diff --git a/go/git/util.go b/go/git/util.go
index aa1f3c1..c17178d 100644
--- a/go/git/util.go
+++ b/go/git/util.go
@@ -1,10 +1,24 @@
package git
import (
+ "fmt"
"net/url"
"strings"
)
+// LogFromTo returns a string which is used to log from one commit to another.
+// It is important to note that:
+// - The results will include the second commit but not the first.
+// - The results include all commits reachable from the first commit which are
+// not reachable from the second, ie. if there is a merge in the given
+// range, the results will include that line of history and not just the
+// commits which are descendants of the first commit. If you want only commits
+// which are ancestors of the second commit AND descendants of the first, you
+// should use LogLinear.
+func LogFromTo(from, to string) string {
+ return fmt.Sprintf("%s..%s", from, to)
+}
+
// NormalizeURL strips everything from the URL except for the host and the path.
// A trailing ".git" is also stripped. The purpose is to allow for small
// variations in repo URL to be recognized as the same repo. The URL needs to
diff --git a/go/gitiles/gitiles.go b/go/gitiles/gitiles.go
index 7646a49..88fd638 100644
--- a/go/gitiles/gitiles.go
+++ b/go/gitiles/gitiles.go
@@ -206,14 +206,38 @@
return commitToLongCommit(&c)
}
+// LogOption represents an optional parameter to a Log function.
+type LogOption string
+
+// LogReverse is a LogOption which indicates that the commits in the Log should
+// be returned in reverse order from the typical "git log" ordering, ie. each
+// commit's parents appear before the commit itself.
+func LogReverse() LogOption {
+ return LogOption("reverse=true")
+}
+
+// LogBatchSize is a LogOption which indicates the number of commits which
+// should be included in each batch of commits returned by Log.
+func LogBatchSize(n int) LogOption {
+ return LogOption(fmt.Sprintf("n=%d", n))
+}
+
// logHelper is used to perform requests which are equivalent to "git log".
// Loads commits in batches and calls the given function for each batch of
// commits. If the function returns an error, iteration stops, and the error is
// returned, unless it was ErrStopIteration.
-func (r *Repo) logHelper(ctx context.Context, urlTmpl, commit string, fn func(context.Context, []*vcsinfo.LongCommit) error) error {
+func (r *Repo) logHelper(ctx context.Context, url string, fn func(context.Context, []*vcsinfo.LongCommit) error, opts ...LogOption) error {
+ for _, opt := range opts {
+ url += "&" + string(opt)
+ }
+ start := ""
for {
var l Log
- if err := r.getJson(ctx, fmt.Sprintf(urlTmpl, commit), &l); err != nil {
+ u := url
+ if start != "" {
+ u += "&s=" + start
+ }
+ if err := r.getJson(ctx, u, &l); err != nil {
return err
}
// Convert to vcsinfo.LongCommit.
@@ -233,20 +257,19 @@
if l.Next == "" {
return nil
} else {
- commit = l.Next
+ start = l.Next
}
}
}
-// Log returns Gitiles' equivalent to "git log" for the given start and end
-// commits.
-func (r *Repo) Log(ctx context.Context, from, to string) ([]*vcsinfo.LongCommit, error) {
+// Log returns Gitiles' equivalent to "git log" for the given expression.
+func (r *Repo) Log(ctx context.Context, logExpr string, opts ...LogOption) ([]*vcsinfo.LongCommit, error) {
rv := []*vcsinfo.LongCommit{}
- tmpl := fmt.Sprintf(LOG_URL, r.URL, from+"..%s")
- if err := r.logHelper(ctx, tmpl, to, func(ctx context.Context, commits []*vcsinfo.LongCommit) error {
+ url := fmt.Sprintf(LOG_URL, r.URL, logExpr)
+ if err := r.logHelper(ctx, url, func(ctx context.Context, commits []*vcsinfo.LongCommit) error {
rv = append(rv, commits...)
return nil
- }); err != nil {
+ }, opts...); err != nil {
return nil, err
}
return rv, nil
@@ -256,9 +279,9 @@
// ie. it only returns commits which are on the direct path from A to B, and
// only on the "main" branch. This is as opposed to "git log from..to" which
// returns all commits which are ancestors of 'to' but not 'from'.
-func (r *Repo) LogLinear(ctx context.Context, from, to string) ([]*vcsinfo.LongCommit, error) {
+func (r *Repo) LogLinear(ctx context.Context, from, to string, opts ...LogOption) ([]*vcsinfo.LongCommit, error) {
// Retrieve the normal "git log".
- commits, err := r.Log(ctx, from, to)
+ commits, err := r.Log(ctx, git.LogFromTo(from, to), opts...)
if err != nil {
return nil, err
}
@@ -314,31 +337,37 @@
return rv, nil
}
-// LogFn runs the given function for each commit in the log history reachable
-// from the given commit hash or ref name. It stops when ErrStopIteration is
-// returned.
-func (r *Repo) LogFn(ctx context.Context, to string, fn func(context.Context, *vcsinfo.LongCommit) error) error {
- return r.LogFnBatch(ctx, to, func(ctx context.Context, commits []*vcsinfo.LongCommit) error {
+// LogFn runs the given function for each commit in the log for the given
+// expression. It stops when ErrStopIteration is returned.
+func (r *Repo) LogFn(ctx context.Context, logExpr string, fn func(context.Context, *vcsinfo.LongCommit) error, opts ...LogOption) error {
+ return r.LogFnBatch(ctx, logExpr, func(ctx context.Context, commits []*vcsinfo.LongCommit) error {
for _, c := range commits {
if err := fn(ctx, c); err != nil {
return err
}
}
return nil
- })
+ }, opts...)
}
// LogFnBatch is the same as LogFn but it runs the given function over batches
// of commits.
-func (r *Repo) LogFnBatch(ctx context.Context, to string, fn func(context.Context, []*vcsinfo.LongCommit) error) error {
- return r.logHelper(ctx, fmt.Sprintf(LOG_URL, r.URL, "%s"), to, fn)
+func (r *Repo) LogFnBatch(ctx context.Context, logExpr string, fn func(context.Context, []*vcsinfo.LongCommit) error, opts ...LogOption) error {
+ url := fmt.Sprintf(LOG_URL, r.URL, logExpr)
+ return r.logHelper(ctx, url, fn, opts...)
}
+// Ref represents a single ref, as returned by the API.
+type Ref struct {
+ Value string `json:"value"`
+}
+
+// RefsMap is the result of a request to REFS_URL.
+type RefsMap map[string]Ref
+
// Branches returns the list of branches in the repo.
func (r *Repo) Branches(ctx context.Context) ([]*git.Branch, error) {
- branchMap := map[string]struct {
- Value string `json:"value"`
- }{}
+ branchMap := RefsMap{}
if err := r.getJson(ctx, fmt.Sprintf(REFS_URL, r.URL), &branchMap); err != nil {
return nil, err
}
diff --git a/go/gitiles/gitiles_test.go b/go/gitiles/gitiles_test.go
index 51a6b5d..bdce5ac 100644
--- a/go/gitiles/gitiles_test.go
+++ b/go/gitiles/gitiles_test.go
@@ -110,7 +110,7 @@
}
js := testutils.MarshalJSON(t, results)
js = ")]}'\n" + js
- urlMock.MockOnce(fmt.Sprintf(LOG_URL, gb.RepoUrl(), fmt.Sprintf("%s..%s", from, to)), mockhttpclient.MockGetDialogue([]byte(js)))
+ urlMock.MockOnce(fmt.Sprintf(LOG_URL, gb.RepoUrl(), git.LogFromTo(from, to)), mockhttpclient.MockGetDialogue([]byte(js)))
}
// Return a slice of the hashes for the given commits.
@@ -123,9 +123,9 @@
}
// Verify that we got the correct list of commits from the call to Log.
- checkGitiles := func(fn func(context.Context, string, string) ([]*vcsinfo.LongCommit, error), from, to string, expect []string) {
+ checkGitiles := func(fn func(context.Context, string, ...LogOption) ([]*vcsinfo.LongCommit, error), from, to string, expect []string) {
mockLog(from, to, expect)
- log, err := fn(ctx, from, to)
+ log, err := fn(ctx, git.LogFromTo(from, to))
assert.NoError(t, err)
deepequal.AssertDeepEqual(t, hashes(log), expect)
assert.True(t, urlMock.Empty())
@@ -137,7 +137,7 @@
checkGit := func(from, to string, expect []string, args ...string) {
cmd := []string{"rev-list", "--date-order"}
cmd = append(cmd, args...)
- cmd = append(cmd, fmt.Sprintf("%s..%s", from, to))
+ cmd = append(cmd, string(git.LogFromTo(from, to)))
output, err := repo.Git(ctx, cmd...)
assert.NoError(t, err)
log := strings.Split(strings.TrimSpace(output), "\n")
@@ -161,7 +161,11 @@
// ("git log --date-order --first-parent --ancestry-path).
checkLinear := func(from, to string, expect []string) {
checkGit(from, to, expect, "--first-parent", "--ancestry-path")
- checkGitiles(r.LogLinear, from, to, expect)
+ mockLog(from, to, expect)
+ log, err := r.LogLinear(ctx, from, to)
+ assert.NoError(t, err)
+ deepequal.AssertDeepEqual(t, hashes(log), expect)
+ assert.True(t, urlMock.Empty())
}
// Test cases.
@@ -213,7 +217,7 @@
last = rv
return rv
}
- mock := func(from, to *vcsinfo.LongCommit, commits []*vcsinfo.LongCommit, next string) {
+ mock := func(from, to *vcsinfo.LongCommit, commits []*vcsinfo.LongCommit, start, next string) {
// Create the mock results.
results := &Log{
Log: make([]*Commit, 0, len(commits)),
@@ -239,8 +243,14 @@
}
js := testutils.MarshalJSON(t, results)
js = ")]}'\n" + js
- urlMock.MockOnce(fmt.Sprintf(LOG_URL, repoUrl, fmt.Sprintf("%s..%s", from.Hash, to.Hash)), mockhttpclient.MockGetDialogue([]byte(js)))
- urlMock.MockOnce(fmt.Sprintf(LOG_URL, repoUrl, to.Hash), mockhttpclient.MockGetDialogue([]byte(js)))
+ url1 := fmt.Sprintf(LOG_URL, repoUrl, git.LogFromTo(from.Hash, to.Hash))
+ url2 := fmt.Sprintf(LOG_URL, repoUrl, to.Hash)
+ if start != "" {
+ url1 += "&s=" + start
+ url2 += "&s=" + start
+ }
+ urlMock.MockOnce(url1, mockhttpclient.MockGetDialogue([]byte(js)))
+ urlMock.MockOnce(url2, mockhttpclient.MockGetDialogue([]byte(js)))
}
check := func(from, to *vcsinfo.LongCommit, expectCommits []*vcsinfo.LongCommit) {
// The expectations are in chronological order for convenience
@@ -251,7 +261,7 @@
sort.Sort(vcsinfo.LongCommitSlice(expect))
// Test standard Log(from, to) function.
- log, err := repo.Log(ctx, from.Hash, to.Hash)
+ log, err := repo.Log(ctx, git.LogFromTo(from.Hash, to.Hash))
assert.NoError(t, err)
deepequal.AssertDeepEqual(t, expect, log)
@@ -275,20 +285,20 @@
}
// Most basic test case; no pagination.
- mock(commits[0], commits[5], commits[1:5], "")
+ mock(commits[0], commits[5], commits[1:5], "", "")
check(commits[0], commits[5], commits[1:5])
// Two pages.
split := 5
- mock(commits[0], commits[len(commits)-1], commits[split:], commits[split].Hash)
- mock(commits[0], commits[split], commits[1:split], "")
+ mock(commits[0], commits[len(commits)-1], commits[split:], "", commits[split].Hash)
+ mock(commits[0], commits[len(commits)-1], commits[1:split], commits[split].Hash, "")
check(commits[0], commits[len(commits)-1], commits[1:])
// Three pages.
split1 := 7
split2 := 3
- mock(commits[0], commits[len(commits)-1], commits[split1:], commits[split1].Hash)
- mock(commits[0], commits[split1], commits[split2:split1], commits[split2].Hash)
- mock(commits[0], commits[split2], commits[1:split2], "")
+ mock(commits[0], commits[len(commits)-1], commits[split1:], "", commits[split1].Hash)
+ mock(commits[0], commits[len(commits)-1], commits[split2:split1], commits[split1].Hash, commits[split2].Hash)
+ mock(commits[0], commits[len(commits)-1], commits[1:split2], commits[split2].Hash, "")
check(commits[0], commits[len(commits)-1], commits[1:])
}
diff --git a/go/gitiles/testutils/testutils.go b/go/gitiles/testutils/testutils.go
index 947e1d1..6e5cdd9 100644
--- a/go/gitiles/testutils/testutils.go
+++ b/go/gitiles/testutils/testutils.go
@@ -34,6 +34,10 @@
}
}
+func (mr *MockRepo) Empty() bool {
+ return mr.c.Empty()
+}
+
func (mr *MockRepo) MockReadFile(ctx context.Context, srcPath, ref string) {
contents, err := mr.repo.GetFile(ctx, srcPath, ref)
assert.NoError(mr.t, err)
@@ -79,18 +83,37 @@
mr.c.MockOnce(url, mockhttpclient.MockGetDialogue(b))
}
-func (mr *MockRepo) MockLog(ctx context.Context, from, to string) {
- revlist, err := mr.repo.RevList(ctx, fmt.Sprintf("%s..%s", from, to))
+func (mr *MockRepo) MockBranches(ctx context.Context) {
+ branches, err := mr.repo.Branches(ctx)
+ assert.NoError(mr.t, err)
+ res := make(gitiles.RefsMap, len(branches))
+ for _, branch := range branches {
+ res[branch.Name] = gitiles.Ref{
+ Value: branch.Head,
+ }
+ }
+ b, err := json.Marshal(res)
+ assert.NoError(mr.t, err)
+ b = append([]byte(")]}'\n"), b...)
+ url := fmt.Sprintf(gitiles.REFS_URL, mr.url)
+ mr.c.MockOnce(url, mockhttpclient.MockGetDialogue(b))
+}
+
+func (mr *MockRepo) MockLog(ctx context.Context, logExpr string, opts ...gitiles.LogOption) {
+ revlist, err := mr.repo.RevList(ctx, logExpr)
assert.NoError(mr.t, err)
log := &gitiles.Log{
- Log: []*gitiles.Commit{},
+ Log: make([]*gitiles.Commit, len(revlist)),
}
- for _, hash := range revlist {
- log.Log = append(log.Log, mr.getCommit(ctx, hash))
+ for idx, hash := range revlist {
+ log.Log[idx] = mr.getCommit(ctx, hash)
}
b, err := json.Marshal(log)
assert.NoError(mr.t, err)
b = append([]byte(")]}'\n"), b...)
- url := fmt.Sprintf(gitiles.LOG_URL, mr.url, fmt.Sprintf("%s..%s", from, to))
+ url := fmt.Sprintf(gitiles.LOG_URL, mr.url, logExpr)
+ for _, opt := range opts {
+ url += "&" + string(opt)
+ }
mr.c.MockOnce(url, mockhttpclient.MockGetDialogue(b))
}
