[autoroll] Use "git log --first-parent" without "--ancestry-path"
This allows the roller to self-recover when the lastRollRev is not
reachable from the configured branch head but gives us the same set of
roll candidates otherwise.
Bug: skia:9669
Change-Id: I0ba9c1058aa6f655eb0cbae0911638b0584c5c6e
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/256021
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Eric Boren <borenet@google.com>
diff --git a/autoroll/go/autoroll-google3/google3.go b/autoroll/go/autoroll-google3/google3.go
index 9524302..5f8b450 100644
--- a/autoroll/go/autoroll-google3/google3.go
+++ b/autoroll/go/autoroll-google3/google3.go
@@ -117,7 +117,7 @@
if err != nil {
return err
}
- revs, err := a.childRepo.LogLinear(ctx, lastSuccessRev, headRev.Hash)
+ revs, err := a.childRepo.LogFirstParent(ctx, lastSuccessRev, headRev.Hash)
if err != nil {
return 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 dd37215..b61eb65 100644
--- a/autoroll/go/repo_manager/no_checkout_deps_repo_manager.go
+++ b/autoroll/go/repo_manager/no_checkout_deps_repo_manager.go
@@ -318,7 +318,7 @@
// Find the not-yet-rolled child repo commits.
// Only consider commits on the "main" branch as roll candidates.
- notRolled, err := rm.childRepo.LogLinear(ctx, lastRollRev.Id, tipRev.Id)
+ notRolled, err := rm.childRepo.LogFirstParent(ctx, lastRollRev.Id, tipRev.Id)
if err != nil {
return nil, nil, nil, err
}
diff --git a/autoroll/go/repo_manager/repo_manager.go b/autoroll/go/repo_manager/repo_manager.go
index 89579ee..f96cb6a 100644
--- a/autoroll/go/repo_manager/repo_manager.go
+++ b/autoroll/go/repo_manager/repo_manager.go
@@ -224,7 +224,7 @@
if tipRev.Id == lastRollRev.Id {
return []*revision.Revision{}, nil
}
- commits, err := r.childRepo.RevList(ctx, "--ancestry-path", "--first-parent", git.LogFromTo(lastRollRev.Id, tipRev.Id))
+ commits, err := r.childRepo.RevList(ctx, "--first-parent", git.LogFromTo(lastRollRev.Id, tipRev.Id))
if err != nil {
return nil, err
}
diff --git a/go/gitiles/gitiles.go b/go/gitiles/gitiles.go
index 76eeb53..ab70236 100644
--- a/go/gitiles/gitiles.go
+++ b/go/gitiles/gitiles.go
@@ -356,6 +356,37 @@
return rv, nil
}
+// LogFirstParent is equivalent to "git log --first-parent A..B", ie. it
+// only returns commits which are reachable from A by following the first parent
+// (the "main" branch) but not from B.
+func (r *Repo) LogFirstParent(ctx context.Context, from, to string, opts ...LogOption) ([]*vcsinfo.LongCommit, error) {
+ // Retrieve the normal "git log".
+ commits, err := r.Log(ctx, git.LogFromTo(from, to), opts...)
+ if err != nil {
+ return nil, err
+ }
+ if len(commits) == 0 {
+ return commits, nil
+ }
+
+ // Now filter to only those commits which are on the first-parent path.
+ commitsMap := make(map[string]*vcsinfo.LongCommit, len(commits))
+ for _, commit := range commits {
+ commitsMap[commit.Hash] = commit
+ }
+ rv := make([]*vcsinfo.LongCommit, 0, len(commits))
+ c := commitsMap[to]
+ for c != nil {
+ rv = append(rv, c)
+ if len(c.Parents) > 0 {
+ c = commitsMap[c.Parents[0]]
+ } else {
+ c = nil
+ }
+ }
+ return rv, nil
+}
+
// LogLinear is equivalent to "git log --first-parent --ancestry-path from..to",
// 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
@@ -402,9 +433,11 @@
}
// If the first parent of this commit is on the direct line,
// then this commit is as well.
- if search(commit.Parents[0]) {
- isDescendant[hash] = true
- return true
+ if len(commit.Parents) > 0 {
+ if search(commit.Parents[0]) {
+ isDescendant[hash] = true
+ return true
+ }
}
return false
}
diff --git a/go/gitiles/gitiles_test.go b/go/gitiles/gitiles_test.go
index 1b71400..e8f76f6 100644
--- a/go/gitiles/gitiles_test.go
+++ b/go/gitiles/gitiles_test.go
@@ -158,6 +158,17 @@
}
// Verify that we get the expected list of commits from both Gitiles
+ // (Repo.LogFirstParent) and Git ("git log --date-order --first-parent").
+ checkFirstParent := func(from, to string, expect []string) {
+ checkGit(from, to, expect, "--first-parent")
+ mockLog(from, to, expect)
+ log, err := r.LogFirstParent(ctx, from, to)
+ require.NoError(t, err)
+ deepequal.AssertDeepEqual(t, hashes(log), expect)
+ require.True(t, urlMock.Empty())
+ }
+
+ // Verify that we get the expected list of commits from both Gitiles
// (Repo.LogLinear) and Git
// ("git log --date-order --first-parent --ancestry-path).
checkLinear := func(from, to string, expect []string) {
@@ -171,18 +182,25 @@
// Test cases.
checkBasic(c0, c8, []string{c8, c7, c6, c5, c4, c3, c2, c1})
+ checkFirstParent(c0, c8, []string{c8, c7, c5, c4, c1})
checkLinear(c0, c8, []string{c8, c7, c5, c4, c1})
checkBasic(c0, c1, []string{c1})
+ checkFirstParent(c0, c1, []string{c1})
checkLinear(c0, c1, []string{c1})
checkBasic(c2, c4, []string{c4})
+ checkFirstParent(c2, c4, []string{c4})
checkLinear(c2, c4, []string{})
checkBasic(c1, c2, []string{c2})
+ checkFirstParent(c1, c2, []string{c2})
checkLinear(c1, c2, []string{c2})
checkBasic(c1, c4, []string{c4})
+ checkFirstParent(c1, c4, []string{c4})
checkLinear(c1, c4, []string{c4})
checkBasic(c5, c7, []string{c7, c6, c3, c2})
+ checkFirstParent(c5, c7, []string{c7})
checkLinear(c5, c7, []string{c7})
checkBasic(c2, c7, []string{c7, c6, c5, c4, c3})
+ checkFirstParent(c2, c7, []string{c7, c5, c4})
checkLinear(c2, c7, []string{})
}
