[autoroll] Refactor roll strategy into interface; add LKGR support
Bug: skia:
Change-Id: I5368251c79410d1ea63a26ab472b15f9344e60d1
Reviewed-on: https://skia-review.googlesource.com/25260
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Eric Boren <borenet@google.com>
diff --git a/autoroll/go/autoroll/main.go b/autoroll/go/autoroll/main.go
index c9bb7bd..b941f91 100644
--- a/autoroll/go/autoroll/main.go
+++ b/autoroll/go/autoroll/main.go
@@ -270,12 +270,16 @@
}
// Start the autoroller.
+ strat, err := repo_manager.GetNextRollStrategy(*strategy, *childBranch, "")
+ if err != nil {
+ sklog.Fatal(err)
+ }
if *rollIntoAndroid {
- arb, err = autorollerv2.NewAndroidAutoRoller(*workdir, *parentBranch, *childPath, *childBranch, cqExtraTrybots, emails, g, *strategy, *preUploadSteps)
+ arb, err = autorollerv2.NewAndroidAutoRoller(*workdir, *parentBranch, *childPath, *childBranch, cqExtraTrybots, emails, g, repo_manager.StrategyRemoteHead(*childBranch), *preUploadSteps)
} else if *useManifest {
- arb, err = autorollerv2.NewManifestAutoRoller(*workdir, *parentRepo, *parentBranch, *childPath, *childBranch, cqExtraTrybots, emails, g, depotTools, *strategy, *preUploadSteps)
+ arb, err = autorollerv2.NewManifestAutoRoller(*workdir, *parentRepo, *parentBranch, *childPath, *childBranch, cqExtraTrybots, emails, g, depotTools, strat, *preUploadSteps)
} else {
- arb, err = autorollerv2.NewDEPSAutoRoller(*workdir, *parentRepo, *parentBranch, *childPath, *childBranch, cqExtraTrybots, emails, g, depotTools, *strategy, *preUploadSteps)
+ arb, err = autorollerv2.NewDEPSAutoRoller(*workdir, *parentRepo, *parentBranch, *childPath, *childBranch, cqExtraTrybots, emails, g, depotTools, strat, *preUploadSteps)
}
if err != nil {
sklog.Fatal(err)
diff --git a/autoroll/go/autoroller/roller.go b/autoroll/go/autoroller/roller.go
index f999308..79fcacd 100644
--- a/autoroll/go/autoroller/roller.go
+++ b/autoroll/go/autoroller/roller.go
@@ -50,7 +50,7 @@
}
// NewAutoRoller creates and returns a new AutoRoller which runs at the given frequency.
-func NewAutoRoller(workdir, parentRepo, parentBranch, childPath, childBranch, cqExtraTrybots string, emails []string, gerrit *gerrit.Gerrit, depot_tools string, rollIntoAndroid, useManifest bool, strategy string) (*AutoRoller, error) {
+func NewAutoRoller(workdir, parentRepo, parentBranch, childPath, childBranch, cqExtraTrybots string, emails []string, gerrit *gerrit.Gerrit, depot_tools string, rollIntoAndroid, useManifest bool, strategy repo_manager.NextRollStrategy) (*AutoRoller, error) {
var err error
var rm repo_manager.RepoManager
if rollIntoAndroid {
diff --git a/autoroll/go/autoroller/roller_test.go b/autoroll/go/autoroller/roller_test.go
index 025415f..39b1cdc 100644
--- a/autoroll/go/autoroller/roller_test.go
+++ b/autoroll/go/autoroller/roller_test.go
@@ -76,6 +76,13 @@
return r.updateCount
}
+// ChildRevList returns a list of hashes.
+func (r *mockRepoManager) ChildRevList(args ...string) ([]string, error) {
+ r.mtx.RLock()
+ defer r.mtx.RUnlock()
+ return nil, fmt.Errorf("Not implemented")
+}
+
// FullChildHash returns the full hash of the given short hash or ref in the
// mocked child repo.
func (r *mockRepoManager) FullChildHash(shortHash string) (string, error) {
@@ -408,7 +415,7 @@
// setup initializes a fake AutoRoller for testing. It returns the working
// directory, AutoRoller instance, URLMock for faking HTTP requests, and an
// gerrit.ChangeInfo representing the first CL that was uploaded by the AutoRoller.
-func setup(t *testing.T, strategy string) (string, *AutoRoller, *mockRepoManager, *mockCodereview, *gerrit.ChangeInfo) {
+func setup(t *testing.T, strategy repo_manager.NextRollStrategy) (string, *AutoRoller, *mockRepoManager, *mockCodereview, *gerrit.ChangeInfo) {
testutils.SkipIfShort(t)
// Setup mocks.
@@ -429,7 +436,7 @@
}
rm := &mockRepoManager{t: t}
- repo_manager.NewDEPSRepoManager = func(workdir, parentRepo, parentBranch, childPath, childBranch string, depot_tools string, g *gerrit.Gerrit, strategy string, preUploadSteps []string) (repo_manager.RepoManager, error) {
+ repo_manager.NewDEPSRepoManager = func(workdir, parentRepo, parentBranch, childPath, childBranch string, depot_tools string, g *gerrit.Gerrit, strategy repo_manager.NextRollStrategy, preUploadSteps []string) (repo_manager.RepoManager, error) {
return rm, nil
}
@@ -456,7 +463,9 @@
func TestAutoRollBasic(t *testing.T) {
testutils.LargeTest(t)
// setup will initialize the roller and upload a CL.
- workdir, roller, rm, rv, roll1 := setup(t, repo_manager.ROLL_STRATEGY_BATCH)
+ s, err := repo_manager.GetNextRollStrategy(repo_manager.ROLL_STRATEGY_BATCH, "master", "")
+ assert.NoError(t, err)
+ workdir, roller, rm, rv, roll1 := setup(t, s)
defer func() {
assert.NoError(t, roller.Close())
assert.NoError(t, os.RemoveAll(workdir))
@@ -491,7 +500,9 @@
func TestAutoRollStop(t *testing.T) {
testutils.MediumTest(t)
// setup will initialize the roller and upload a CL.
- workdir, roller, rm, rv, roll1 := setup(t, repo_manager.ROLL_STRATEGY_BATCH)
+ s, err := repo_manager.GetNextRollStrategy(repo_manager.ROLL_STRATEGY_BATCH, "master", "")
+ assert.NoError(t, err)
+ workdir, roller, rm, rv, roll1 := setup(t, s)
defer func() {
assert.NoError(t, roller.Close())
assert.NoError(t, os.RemoveAll(workdir))
@@ -539,7 +550,9 @@
// TestAutoRollDryRun ensures that the Dry Run functionalify works as expected.
func TestAutoRollDryRun(t *testing.T) {
testutils.MediumTest(t)
- workdir, roller, rm, rv, roll1 := setup(t, repo_manager.ROLL_STRATEGY_BATCH)
+ s, err := repo_manager.GetNextRollStrategy(repo_manager.ROLL_STRATEGY_BATCH, "master", "")
+ assert.NoError(t, err)
+ workdir, roller, rm, rv, roll1 := setup(t, s)
defer func() {
assert.NoError(t, roller.Close())
assert.NoError(t, os.RemoveAll(workdir))
@@ -640,7 +653,9 @@
// sync the code, waiting for the commit to show up.
func TestAutoRollCommitLandRace(t *testing.T) {
testutils.LargeTest(t)
- workdir, roller, rm, rv, roll1 := setup(t, repo_manager.ROLL_STRATEGY_BATCH)
+ s, err := repo_manager.GetNextRollStrategy(repo_manager.ROLL_STRATEGY_BATCH, "master", "")
+ assert.NoError(t, err)
+ workdir, roller, rm, rv, roll1 := setup(t, s)
defer func() {
assert.NoError(t, roller.Close())
assert.NoError(t, os.RemoveAll(workdir))
@@ -693,7 +708,9 @@
// doesn't upload new CLs over and over.
func TestAutoRollThrottle(t *testing.T) {
testutils.MediumTest(t)
- workdir, roller, rm, rv, roll1 := setup(t, repo_manager.ROLL_STRATEGY_BATCH)
+ s, err := repo_manager.GetNextRollStrategy(repo_manager.ROLL_STRATEGY_BATCH, "master", "")
+ assert.NoError(t, err)
+ workdir, roller, rm, rv, roll1 := setup(t, s)
defer func() {
assert.NoError(t, roller.Close())
assert.NoError(t, os.RemoveAll(workdir))
@@ -732,7 +749,9 @@
func TestAutoRollSingle(t *testing.T) {
testutils.MediumTest(t)
// setup will initialize the roller and upload a CL.
- workdir, roller, rm, rv, roll1 := setup(t, repo_manager.ROLL_STRATEGY_SINGLE)
+ s, err := repo_manager.GetNextRollStrategy(repo_manager.ROLL_STRATEGY_BATCH, "master", "")
+ assert.NoError(t, err)
+ workdir, roller, rm, rv, roll1 := setup(t, s)
defer func() {
assert.NoError(t, roller.Close())
assert.NoError(t, os.RemoveAll(workdir))
diff --git a/autoroll/go/autorollerv2/autoroller.go b/autoroll/go/autorollerv2/autoroller.go
index 9054caf..f898004 100644
--- a/autoroll/go/autorollerv2/autoroller.go
+++ b/autoroll/go/autorollerv2/autoroller.go
@@ -77,7 +77,7 @@
}
// NewAndroidAutoRoller returns an AutoRoller instance which rolls into Android.
-func NewAndroidAutoRoller(workdir, parentBranch, childPath, childBranch, cqExtraTrybots string, emails []string, gerrit *gerrit.Gerrit, strategy string, preUploadSteps []string) (*AutoRoller, error) {
+func NewAndroidAutoRoller(workdir, parentBranch, childPath, childBranch, cqExtraTrybots string, emails []string, gerrit *gerrit.Gerrit, strategy repo_manager.NextRollStrategy, preUploadSteps []string) (*AutoRoller, error) {
rm, err := repo_manager.NewAndroidRepoManager(workdir, parentBranch, childPath, childBranch, gerrit, strategy, preUploadSteps)
if err != nil {
return nil, err
@@ -89,7 +89,7 @@
}
// NewDEPSAutoRoller returns an AutoRoller instance which rolls using DEPS.
-func NewDEPSAutoRoller(workdir, parentRepo, parentBranch, childPath, childBranch, cqExtraTrybots string, emails []string, gerrit *gerrit.Gerrit, depot_tools string, strategy string, preUploadSteps []string) (*AutoRoller, error) {
+func NewDEPSAutoRoller(workdir, parentRepo, parentBranch, childPath, childBranch, cqExtraTrybots string, emails []string, gerrit *gerrit.Gerrit, depot_tools string, strategy repo_manager.NextRollStrategy, preUploadSteps []string) (*AutoRoller, error) {
rm, err := repo_manager.NewDEPSRepoManager(workdir, parentRepo, parentBranch, childPath, childBranch, depot_tools, gerrit, strategy, preUploadSteps)
if err != nil {
return nil, err
@@ -101,7 +101,7 @@
}
// NewManifestAutoRoller returns an AutoRoller instance which rolls using DEPS.
-func NewManifestAutoRoller(workdir, parentRepo, parentBranch, childPath, childBranch, cqExtraTrybots string, emails []string, gerrit *gerrit.Gerrit, depot_tools string, strategy string, preUploadSteps []string) (*AutoRoller, error) {
+func NewManifestAutoRoller(workdir, parentRepo, parentBranch, childPath, childBranch, cqExtraTrybots string, emails []string, gerrit *gerrit.Gerrit, depot_tools string, strategy repo_manager.NextRollStrategy, preUploadSteps []string) (*AutoRoller, error) {
rm, err := repo_manager.NewManifestRepoManager(workdir, parentRepo, parentBranch, childPath, childBranch, depot_tools, gerrit, strategy, preUploadSteps)
if err != nil {
return nil, err
diff --git a/autoroll/go/repo_manager/android_repo_manager.go b/autoroll/go/repo_manager/android_repo_manager.go
index 5789a34..73a8bef 100644
--- a/autoroll/go/repo_manager/android_repo_manager.go
+++ b/autoroll/go/repo_manager/android_repo_manager.go
@@ -26,7 +26,7 @@
var (
// Use this function to instantiate a NewAndroidRepoManager. This is able to be
// overridden for testing.
- NewAndroidRepoManager func(string, string, string, string, gerrit.GerritInterface, string, []string) (RepoManager, error) = newAndroidRepoManager
+ NewAndroidRepoManager func(string, string, string, string, gerrit.GerritInterface, NextRollStrategy, []string) (RepoManager, error) = newAndroidRepoManager
IGNORE_MERGE_CONFLICT_FILES = []string{"include/config/SkUserConfig.h"}
@@ -44,7 +44,7 @@
authDaemonRunning bool
}
-func newAndroidRepoManager(workdir, parentBranch, childPath, childBranch string, g gerrit.GerritInterface, strategy string, preUploadStepNames []string) (RepoManager, error) {
+func newAndroidRepoManager(workdir, parentBranch, childPath, childBranch string, g gerrit.GerritInterface, strategy NextRollStrategy, preUploadStepNames []string) (RepoManager, error) {
user, err := user.Current()
if err != nil {
return nil, err
@@ -52,6 +52,8 @@
repoToolPath := path.Join(user.HomeDir, "bin", "repo")
gitCookieAuthDaemonPath := path.Join(user.HomeDir, "gcompute-tools", "git-cookie-authdaemon")
wd := path.Join(workdir, "android_repo")
+ childDir := path.Join(wd, childPath)
+ childRepo := &git.Checkout{GitDir: git.GitDir(childDir)}
preUploadSteps, err := GetPreUploadSteps(preUploadStepNames)
if err != nil {
@@ -61,9 +63,9 @@
r := &androidRepoManager{
commonRepoManager: &commonRepoManager{
parentBranch: parentBranch,
- childDir: path.Join(wd, childPath),
+ childDir: childDir,
childPath: childPath,
- childRepo: nil, // This will be filled in on the first update.
+ childRepo: childRepo,
childBranch: childBranch,
preUploadSteps: preUploadSteps,
strategy: strategy,
@@ -115,11 +117,6 @@
return err
}
- // Create the child GitInfo if needed.
- if r.childRepo == nil {
- r.childRepo = &git.Checkout{GitDir: git.GitDir(r.childDir)}
- }
-
// Fix the review config to a URL which will work outside prod.
if _, err := exec.RunCwd(r.childRepo.Dir(), "git", "config", "remote.goog.review", fmt.Sprintf("%s/", r.repoUrl)); err != nil {
return err
@@ -143,24 +140,11 @@
}
// Get the next roll revision.
- childHead, err := r.getChildRepoHead()
+ nextRollRev, err := r.strategy.GetNextRollRev(r.childRepo, lastRollRev)
if err != nil {
return err
}
- var nextRollRev string
- if r.strategy == ROLL_STRATEGY_SINGLE {
- commits, err := r.childRepo.RevList(fmt.Sprintf("%s..%s", lastRollRev, childHead))
- if err != nil {
- return fmt.Errorf("Failed to list revisions: %s", err)
- }
- if len(commits) == 0 {
- nextRollRev = lastRollRev
- } else {
- nextRollRev = commits[len(commits)-1]
- }
- } else {
- nextRollRev = childHead
- }
+
r.infoMtx.Lock()
defer r.infoMtx.Unlock()
r.lastRollRev = lastRollRev
@@ -169,16 +153,6 @@
return nil
}
-// getChildRepoHead returns the commit hash of the latest commit in the child repo.
-func (r *androidRepoManager) getChildRepoHead() (string, error) {
- output, err := exec.RunCwd(r.childRepo.Dir(), "git", "ls-remote", UPSTREAM_REMOTE_NAME, fmt.Sprintf("refs/heads/%s", r.childBranch), "-1")
- if err != nil {
- return "", err
- }
- tokens := strings.Split(output, "\t")
- return tokens[0], nil
-}
-
// getLastRollRev returns the commit hash of the last-completed DEPS roll.
func (r *androidRepoManager) getLastRollRev() (string, error) {
output, err := exec.RunCwd(r.childRepo.Dir(), "git", "merge-base", fmt.Sprintf("refs/remotes/remote/%s", r.childBranch), fmt.Sprintf("refs/remotes/goog/%s", r.parentBranch))
diff --git a/autoroll/go/repo_manager/android_repo_manager_test.go b/autoroll/go/repo_manager/android_repo_manager_test.go
index baf3ae5..637c28c 100644
--- a/autoroll/go/repo_manager/android_repo_manager_test.go
+++ b/autoroll/go/repo_manager/android_repo_manager_test.go
@@ -65,7 +65,7 @@
defer cleanup()
g, err := gerrit.NewGerrit(mockAndroidServer, "", nil)
assert.NoError(t, err)
- rm, err := NewAndroidRepoManager(wd, "master", childPath, "master", g, ROLL_STRATEGY_BATCH, nil)
+ rm, err := NewAndroidRepoManager(wd, "master", childPath, "master", g, StrategyRemoteHead("master"), nil)
assert.NoError(t, err)
assert.Equal(t, fmt.Sprintf("%s/android_repo/%s", wd, childPath), rm.(*androidRepoManager).childDir)
@@ -82,7 +82,7 @@
defer cleanup()
g := &gerrit.MockedGerrit{IssueID: androidIssueNum}
- rm, err := NewAndroidRepoManager(wd, "master", childPath, "master", g, ROLL_STRATEGY_BATCH, nil)
+ rm, err := NewAndroidRepoManager(wd, "master", childPath, "master", g, StrategyRemoteHead("master"), nil)
assert.NoError(t, err)
issue, err := rm.CreateNewRoll(rm.LastRollRev(), rm.NextRollRev(), androidEmails, "", false)
@@ -148,7 +148,7 @@
defer cleanup()
g := &gerrit.MockedGerrit{IssueID: androidIssueNum}
- rm, err := NewAndroidRepoManager(wd, "master", childPath, "master", g, ROLL_STRATEGY_BATCH, nil)
+ rm, err := NewAndroidRepoManager(wd, "master", childPath, "master", g, StrategyRemoteHead("master"), nil)
assert.NoError(t, err)
ran := false
diff --git a/autoroll/go/repo_manager/deps_repo_manager.go b/autoroll/go/repo_manager/deps_repo_manager.go
index 9fb9e98..9d50ed1 100644
--- a/autoroll/go/repo_manager/deps_repo_manager.go
+++ b/autoroll/go/repo_manager/deps_repo_manager.go
@@ -28,7 +28,7 @@
var (
// Use this function to instantiate a RepoManager. This is able to be
// overridden for testing.
- NewDEPSRepoManager func(string, string, string, string, string, string, *gerrit.Gerrit, string, []string) (RepoManager, error) = newDEPSRepoManager
+ NewDEPSRepoManager func(string, string, string, string, string, string, *gerrit.Gerrit, NextRollStrategy, []string) (RepoManager, error) = newDEPSRepoManager
DEPOT_TOOLS_AUTH_USER_REGEX = regexp.MustCompile(fmt.Sprintf("Logged in to %s as ([\\w-]+).", autoroll.RIETVELD_URL))
)
@@ -47,7 +47,7 @@
// newDEPSRepoManager returns a RepoManager instance which operates in the given
// working directory and updates at the given frequency.
-func newDEPSRepoManager(workdir, parentRepo, parentBranch, childPath, childBranch string, depot_tools string, g *gerrit.Gerrit, strategy string, preUploadStepNames []string) (RepoManager, error) {
+func newDEPSRepoManager(workdir, parentRepo, parentBranch, childPath, childBranch string, depot_tools string, g *gerrit.Gerrit, strategy NextRollStrategy, preUploadStepNames []string) (RepoManager, error) {
gclient := GCLIENT
rollDep := ROLL_DEP
if depot_tools != "" {
@@ -58,6 +58,8 @@
wd := path.Join(workdir, "repo_manager")
parentBase := strings.TrimSuffix(path.Base(parentRepo), ".git")
parentDir := path.Join(wd, parentBase)
+ childDir := path.Join(wd, childPath)
+ childRepo := &git.Checkout{GitDir: git.GitDir(childDir)}
user, err := g.GetUserEmail()
if err != nil {
@@ -74,15 +76,15 @@
depotToolsRepoManager: &depotToolsRepoManager{
commonRepoManager: &commonRepoManager{
parentBranch: parentBranch,
- childDir: path.Join(wd, childPath),
+ childDir: childDir,
childPath: childPath,
- childRepo: nil, // This will be filled in on the first update.
+ childRepo: childRepo,
childBranch: childBranch,
+ g: g,
preUploadSteps: preUploadSteps,
strategy: strategy,
user: user,
workdir: wd,
- g: g,
},
depot_tools: depot_tools,
gclient: gclient,
@@ -91,6 +93,7 @@
},
rollDep: rollDep,
}
+
// TODO(borenet): This update can be extremely expensive. Consider
// moving it out of the startup critical path.
return dr, dr.Update()
@@ -106,11 +109,6 @@
return fmt.Errorf("Could not create and sync parent repo: %s", err)
}
- // Create the child GitInfo if needed.
- if dr.childRepo == nil {
- dr.childRepo = &git.Checkout{GitDir: git.GitDir(dr.childDir)}
- }
-
// Get the last roll revision.
lastRollRev, err := dr.getLastRollRev()
if err != nil {
@@ -118,24 +116,10 @@
}
// Get the next roll revision.
- childHead, err := dr.childRepo.FullHash(fmt.Sprintf("origin/%s", dr.childBranch))
+ nextRollRev, err := dr.strategy.GetNextRollRev(dr.childRepo, lastRollRev)
if err != nil {
return err
}
- var nextRollRev string
- if dr.strategy == ROLL_STRATEGY_SINGLE {
- commits, err := dr.childRepo.RevList(fmt.Sprintf("%s..%s", lastRollRev, childHead))
- if err != nil {
- return fmt.Errorf("Failed to list revisions: %s", err)
- }
- if len(commits) == 0 {
- nextRollRev = lastRollRev
- } else {
- nextRollRev = commits[len(commits)-1]
- }
- } else {
- nextRollRev = childHead
- }
dr.infoMtx.Lock()
defer dr.infoMtx.Unlock()
dr.lastRollRev = lastRollRev
diff --git a/autoroll/go/repo_manager/deps_repo_manager_test.go b/autoroll/go/repo_manager/deps_repo_manager_test.go
index cc92b8f..b6c5a93 100644
--- a/autoroll/go/repo_manager/deps_repo_manager_test.go
+++ b/autoroll/go/repo_manager/deps_repo_manager_test.go
@@ -107,7 +107,9 @@
defer cleanup()
g := setupFakeGerrit(t, wd)
- rm, err := NewDEPSRepoManager(wd, parent.RepoUrl(), "master", childPath, "master", depotTools, g, ROLL_STRATEGY_BATCH, nil)
+ s, err := GetNextRollStrategy(ROLL_STRATEGY_BATCH, "master", "")
+ assert.NoError(t, err)
+ rm, err := NewDEPSRepoManager(wd, parent.RepoUrl(), "master", childPath, "master", depotTools, g, s, nil)
assert.NoError(t, err)
assert.Equal(t, childCommits[0], rm.LastRollRev())
assert.Equal(t, childCommits[len(childCommits)-1], rm.NextRollRev())
@@ -143,8 +145,10 @@
wd, child, childCommits, parent, cleanup := setup(t)
defer cleanup()
+ s, err := GetNextRollStrategy(strategy, "master", "")
+ assert.NoError(t, err)
g := setupFakeGerrit(t, wd)
- rm, err := NewDEPSRepoManager(wd, parent.RepoUrl(), "master", childPath, "master", depotTools, g, strategy, nil)
+ rm, err := NewDEPSRepoManager(wd, parent.RepoUrl(), "master", childPath, "master", depotTools, g, s, nil)
assert.NoError(t, err)
// Create a roll, assert that it's at tip of tree.
@@ -178,8 +182,10 @@
wd, _, _, parent, cleanup := setup(t)
defer cleanup()
+ s, err := GetNextRollStrategy(ROLL_STRATEGY_BATCH, "master", "")
+ assert.NoError(t, err)
g := setupFakeGerrit(t, wd)
- rm, err := NewDEPSRepoManager(wd, parent.RepoUrl(), "master", childPath, "master", depotTools, g, ROLL_STRATEGY_BATCH, nil)
+ rm, err := NewDEPSRepoManager(wd, parent.RepoUrl(), "master", childPath, "master", depotTools, g, s, nil)
assert.NoError(t, err)
ran := false
diff --git a/autoroll/go/repo_manager/manifest_repo_manager.go b/autoroll/go/repo_manager/manifest_repo_manager.go
index a039d28..8a58c73 100644
--- a/autoroll/go/repo_manager/manifest_repo_manager.go
+++ b/autoroll/go/repo_manager/manifest_repo_manager.go
@@ -25,7 +25,7 @@
var (
// Use this function to instantiate a RepoManager. This is able to be
// overridden for testing.
- NewManifestRepoManager func(string, string, string, string, string, string, *gerrit.Gerrit, string, []string) (RepoManager, error) = newManifestRepoManager
+ NewManifestRepoManager func(string, string, string, string, string, string, *gerrit.Gerrit, NextRollStrategy, []string) (RepoManager, error) = newManifestRepoManager
)
// manifestRepoManager is a struct used by Manifest AutoRoller for managing checkouts.
@@ -35,10 +35,15 @@
// newManifestRepoManager returns a RepoManager instance which operates in the
// given working directory and updates at the given frequency.
-func newManifestRepoManager(workdir, parentRepo, parentBranch, childPath, childBranch string, depot_tools string, g *gerrit.Gerrit, strategy string, preUploadStepNames []string) (RepoManager, error) {
+func newManifestRepoManager(workdir, parentRepo, parentBranch, childPath, childBranch string, depot_tools string, g *gerrit.Gerrit, strategy NextRollStrategy, preUploadStepNames []string) (RepoManager, error) {
wd := path.Join(workdir, "repo_manager")
parentBase := strings.TrimSuffix(path.Base(parentRepo), ".git")
parentDir := path.Join(wd, parentBase)
+ childDir := path.Join(wd, childPath)
+ childRepo, err := git.NewCheckout(common.REPO_SKIA, wd)
+ if err != nil {
+ return nil, err
+ }
user, err := g.GetUserEmail()
if err != nil {
@@ -55,9 +60,9 @@
depotToolsRepoManager: &depotToolsRepoManager{
commonRepoManager: &commonRepoManager{
parentBranch: parentBranch,
- childDir: path.Join(wd, childPath),
+ childDir: childDir,
childPath: childPath,
- childRepo: nil, // This will be filled in on the first update.
+ childRepo: childRepo,
childBranch: childBranch,
preUploadSteps: preUploadSteps,
strategy: strategy,
@@ -71,6 +76,7 @@
parentRepo: parentRepo,
},
}
+
// TODO(borenet): This update can be extremely expensive. Consider
// moving it out of the startup critical path.
return mr, mr.Update()
@@ -86,14 +92,6 @@
return fmt.Errorf("Could not create and sync parent repo: %s", err)
}
- // Create the child GitInfo if needed.
- var err error
- if mr.childRepo == nil {
- mr.childRepo, err = git.NewCheckout(common.REPO_SKIA, mr.workdir)
- if err != nil {
- return err
- }
- }
if err := mr.childRepo.Update(); err != nil {
return err
}
@@ -105,24 +103,11 @@
}
// Get the next roll revision.
- childHead, err := mr.childRepo.FullHash(fmt.Sprintf("origin/%s", mr.childBranch))
+ nextRollRev, err := mr.strategy.GetNextRollRev(mr.childRepo, lastRollRev)
if err != nil {
return err
}
- var nextRollRev string
- if mr.strategy == ROLL_STRATEGY_SINGLE {
- commits, err := mr.childRepo.RevList(fmt.Sprintf("%s..%s", lastRollRev, childHead))
- if err != nil {
- return fmt.Errorf("Failed to list revisions: %s", err)
- }
- if len(commits) == 0 {
- nextRollRev = lastRollRev
- } else {
- nextRollRev = commits[len(commits)-1]
- }
- } else {
- nextRollRev = childHead
- }
+
mr.infoMtx.Lock()
defer mr.infoMtx.Unlock()
mr.lastRollRev = lastRollRev
diff --git a/autoroll/go/repo_manager/manifest_repo_manager_test.go b/autoroll/go/repo_manager/manifest_repo_manager_test.go
index 7fe9b8a..62fb11d 100644
--- a/autoroll/go/repo_manager/manifest_repo_manager_test.go
+++ b/autoroll/go/repo_manager/manifest_repo_manager_test.go
@@ -131,8 +131,10 @@
wd, child, childCommits, parent, cleanup := setupManifest(t)
defer cleanup()
+ s, err := GetNextRollStrategy(ROLL_STRATEGY_BATCH, "master", "")
+ assert.NoError(t, err)
g := setupManifestFakeGerrit(t, wd)
- rm, err := NewManifestRepoManager(wd, parent.RepoUrl(), "master", childPath, "master", depotTools, g, ROLL_STRATEGY_BATCH, nil)
+ rm, err := NewManifestRepoManager(wd, parent.RepoUrl(), "master", childPath, "master", depotTools, g, s, nil)
assert.NoError(t, err)
assert.Equal(t, childCommits[0], rm.LastRollRev())
assert.Equal(t, childCommits[len(childCommits)-1], rm.NextRollRev())
@@ -154,8 +156,10 @@
wd, _, _, parent, cleanup := setupManifest(t)
defer cleanup()
+ s, err := GetNextRollStrategy(ROLL_STRATEGY_BATCH, "master", "")
+ assert.NoError(t, err)
g := setupManifestFakeGerrit(t, wd)
- rm, err := NewManifestRepoManager(wd, parent.RepoUrl(), "master", childPath, "master", depotTools, g, ROLL_STRATEGY_BATCH, nil)
+ rm, err := NewManifestRepoManager(wd, parent.RepoUrl(), "master", childPath, "master", depotTools, g, s, nil)
assert.NoError(t, err)
// Create a roll, assert that it's at tip of tree.
@@ -173,8 +177,10 @@
wd, _, _, parent, cleanup := setupManifest(t)
defer cleanup()
+ s, err := GetNextRollStrategy(ROLL_STRATEGY_BATCH, "master", "")
+ assert.NoError(t, err)
g := setupManifestFakeGerrit(t, wd)
- rm, err := NewManifestRepoManager(wd, parent.RepoUrl(), "master", childPath, "master", depotTools, g, ROLL_STRATEGY_BATCH, nil)
+ rm, err := NewManifestRepoManager(wd, parent.RepoUrl(), "master", childPath, "master", depotTools, g, s, nil)
assert.NoError(t, err)
ran := false
rm.(*manifestRepoManager).preUploadSteps = []PreUploadStep{
diff --git a/autoroll/go/repo_manager/repo_manager.go b/autoroll/go/repo_manager/repo_manager.go
index 1ded0b4..e33206c 100644
--- a/autoroll/go/repo_manager/repo_manager.go
+++ b/autoroll/go/repo_manager/repo_manager.go
@@ -18,9 +18,7 @@
)
const (
- ROLL_STRATEGY_BATCH = "batch"
- ROLL_STRATEGY_SINGLE = "single"
- ROLL_BRANCH = "roll_branch"
+ ROLL_BRANCH = "roll_branch"
)
// RepoManager is the interface used by different Autoroller implementations
@@ -28,6 +26,7 @@
type RepoManager interface {
Update() error
FullChildHash(string) (string, error)
+ ChildRevList(...string) ([]string, error)
LastRollRev() string
NextRollRev() string
RolledPast(string) (bool, error)
@@ -51,12 +50,19 @@
childRepo *git.Checkout
childBranch string
preUploadSteps []PreUploadStep
- strategy string
+ strategy NextRollStrategy
user string
workdir string
g gerrit.GerritInterface
}
+// ChildRevList returns a slice of commit hashes from the child repo.
+func (r *commonRepoManager) ChildRevList(args ...string) ([]string, error) {
+ r.repoMtx.RLock()
+ defer r.repoMtx.RUnlock()
+ return r.childRepo.RevList(args...)
+}
+
// FullChildHash returns the full hash of the given short hash or ref in the
// child repo.
func (r *commonRepoManager) FullChildHash(shortHash string) (string, error) {
diff --git a/autoroll/go/repo_manager/strategy.go b/autoroll/go/repo_manager/strategy.go
new file mode 100644
index 0000000..66d274e
--- /dev/null
+++ b/autoroll/go/repo_manager/strategy.go
@@ -0,0 +1,150 @@
+package repo_manager
+
+import (
+ "fmt"
+ "io/ioutil"
+ "net/http"
+ "strings"
+
+ "go.skia.org/infra/go/git"
+ "go.skia.org/infra/go/util"
+)
+
+const (
+ ROLL_STRATEGY_BATCH = "batch"
+ ROLL_STRATEGY_LKGR = "lkgr"
+ ROLL_STRATEGY_REMOTE_BATCH = "remote batch"
+ ROLL_STRATEGY_SINGLE = "single"
+)
+
+// NextRollStrategy is an interface for modules which determine what the next roll
+// revision should be.
+type NextRollStrategy interface {
+ // Return the next roll revision, or an error. Parameters are the child
+ // git checkout and the last roll revision.
+ GetNextRollRev(*git.Checkout, string) (string, error)
+}
+
+// Return the NextRollStrategy indicated by the given string.
+func GetNextRollStrategy(strategy string, branch, lkgr string) (NextRollStrategy, error) {
+ switch strategy {
+ case ROLL_STRATEGY_REMOTE_BATCH:
+ return StrategyRemoteHead(branch), nil
+ case ROLL_STRATEGY_BATCH:
+ return StrategyHead(branch), nil
+ case ROLL_STRATEGY_SINGLE:
+ return StrategySingle(branch), nil
+ case ROLL_STRATEGY_LKGR:
+ return StrategyLKGR(lkgr), nil
+ default:
+ return nil, fmt.Errorf("Unknown roll strategy %q", strategy)
+ }
+}
+
+// headStrategy is a NextRollStrategy which always rolls to HEAD of a given branch.
+type headStrategy struct {
+ branch string
+}
+
+// See documentation for NextRollStrategy interface.
+func (s *headStrategy) GetNextRollRev(repo *git.Checkout, _ string) (string, error) {
+ return repo.FullHash(fmt.Sprintf("origin/%s", s.branch))
+}
+
+// StrategyHead returns a NextRollStrategy which always rolls to HEAD of a given branch.
+func StrategyHead(branch string) NextRollStrategy {
+ return &headStrategy{
+ branch: branch,
+ }
+}
+
+// remoteHeadStrategy is a NextRollStrategy which always rolls to HEAD of a
+// given branch, as defined by "git ls-remote".
+type remoteHeadStrategy struct {
+ branch string
+}
+
+// See documentation for NextRollStrategy interface.
+func (s *remoteHeadStrategy) GetNextRollRev(repo *git.Checkout, _ string) (string, error) {
+ output, err := repo.Git("ls-remote", UPSTREAM_REMOTE_NAME, fmt.Sprintf("refs/heads/%s", s.branch), "-1")
+ if err != nil {
+ return "", err
+ }
+ tokens := strings.Split(output, "\t")
+ return tokens[0], nil
+}
+
+// StrategyRemoteHead returns a NextRollStrategy which always rolls to HEAD of a
+// given branch, as defined by "git ls-remote".
+func StrategyRemoteHead(branch string) NextRollStrategy {
+ return &remoteHeadStrategy{
+ branch: branch,
+ }
+}
+
+// singleStrategy is a NextRollStrategy which rolls toward HEAD of a given branch, one
+// commit at a time.
+type singleStrategy struct {
+ *headStrategy
+}
+
+// See documentation for NextRollStrategy interface.
+func (s *singleStrategy) GetNextRollRev(repo *git.Checkout, lastRollRev string) (string, error) {
+ head, err := s.headStrategy.GetNextRollRev(repo, lastRollRev)
+ if err != nil {
+ return "", err
+ }
+ commits, err := repo.RevList(fmt.Sprintf("%s..%s", lastRollRev, head))
+ if err != nil {
+ return "", fmt.Errorf("Failed to list revisions: %s", err)
+ }
+ if len(commits) == 0 {
+ return lastRollRev, nil
+ } else {
+ return commits[len(commits)-1], nil
+ }
+}
+
+// StrategySingle returns a NextRollStrategy which rolls toward HEAD of a given branch,
+// one commit at a time.
+func StrategySingle(branch string) NextRollStrategy {
+ return &singleStrategy{StrategyHead(branch).(*headStrategy)}
+}
+
+// urlStrategy is a NextRollStrategy which rolls to a revision specified by a web
+// server.
+type urlStrategy struct {
+ parse func(string) (string, error)
+ url string
+}
+
+// See documentation for NextRollStrategy interface.
+func (s *urlStrategy) GetNextRollRev(_ *git.Checkout, _ string) (string, error) {
+ resp, err := http.Get(s.url)
+ if err != nil {
+ return "", err
+ }
+ defer util.Close(resp.Body)
+ body, err := ioutil.ReadAll(resp.Body)
+ if err != nil {
+ return "", err
+ }
+ return s.parse(string(body))
+}
+
+// StrategyURL returns a NextRollStrategy which rolls to a revision specified by a web
+// server.
+func StrategyURL(url string, parseFn func(string) (string, error)) NextRollStrategy {
+ return &urlStrategy{
+ parse: parseFn,
+ url: url,
+ }
+}
+
+// StrategyLKGR returns a NextRollStrategy which rolls to a Last Known Good Revision,
+// which is obtainable from a web server.
+func StrategyLKGR(url string) NextRollStrategy {
+ return StrategyURL(url, func(body string) (string, error) {
+ return strings.TrimSpace(body), nil
+ })
+}
