[Autorollers] When rev is a patch ref use gclient's patch-ref args
Verified that this works: https://github.com/flutter/engine/pull/19747/files
Bug: skia:10477
Change-Id: I769274d86ea047af3a8a401cd26ef9acf9b68da4
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/302502
Commit-Queue: Ravi Mistry <rmistry@google.com>
Reviewed-by: Eric Boren <borenet@google.com>
diff --git a/autoroll/go/repo_manager/deps_repo_manager_test.go b/autoroll/go/repo_manager/deps_repo_manager_test.go
index 74be892..98440e2 100644
--- a/autoroll/go/repo_manager/deps_repo_manager_test.go
+++ b/autoroll/go/repo_manager/deps_repo_manager_test.go
@@ -14,6 +14,7 @@
"github.com/stretchr/testify/require"
"go.skia.org/infra/autoroll/go/codereview"
"go.skia.org/infra/autoroll/go/repo_manager/parent"
+ "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"
@@ -23,6 +24,7 @@
"go.skia.org/infra/go/skerr"
"go.skia.org/infra/go/testutils"
"go.skia.org/infra/go/testutils/unittest"
+ "go.skia.org/infra/go/util"
"go.skia.org/infra/go/vcsinfo"
)
@@ -60,7 +62,7 @@
}
}
-func setupDEPSRepoManager(t *testing.T, cfg *DEPSRepoManagerConfig) (context.Context, *parentChildRepoManager, string, *git_testutils.GitBuilder, []string, *git_testutils.GitBuilder, *exec.CommandCollector, *vcsinfo.LongCommit, *mockhttpclient.URLMock, func()) {
+func setupDEPSRepoManager(t *testing.T, cfg *DEPSRepoManagerConfig) (context.Context, *parentChildRepoManager, string, *git_testutils.GitBuilder, []string, *git_testutils.GitBuilder, *exec.CommandCollector, *vcsinfo.LongCommit, *mockhttpclient.URLMock, *bool, func()) {
wd, err := ioutil.TempDir("", "")
require.NoError(t, err)
@@ -85,6 +87,7 @@
parent.Commit(ctx)
lastUpload := new(vcsinfo.LongCommit)
+ patchRefInSyncCmd := new(bool)
mockRun := &exec.CommandCollector{}
ctx = exec.NewContext(ctx, mockRun.Run)
mockRun.SetDelegateRun(func(ctx context.Context, cmd *exec.Command) error {
@@ -95,6 +98,9 @@
}
*lastUpload = *d
return nil
+ } else if cmd.Name == "python" && strings.Contains(cmd.Args[0], "gclient.py") && cmd.Args[1] == "sync" && util.In("--patch-ref", cmd.Args) && util.In("--no-rebase-patch-ref", cmd.Args) && util.In("--no-reset-patch-ref", cmd.Args) {
+ *patchRefInSyncCmd = true
+ return nil
}
return exec.DefaultRun(ctx, cmd)
})
@@ -129,7 +135,7 @@
parent.Cleanup()
}
- return ctx, rm, wd, child, childCommits, parent, mockRun, lastUpload, urlmock, cleanup
+ return ctx, rm, wd, child, childCommits, parent, mockRun, lastUpload, urlmock, patchRefInSyncCmd, cleanup
}
func setupFakeGerrit(t *testing.T, cfg *codereview.GerritConfig, urlMock *mockhttpclient.URLMock) *gerrit.Gerrit {
@@ -160,7 +166,7 @@
unittest.LargeTest(t)
cfg := depsCfg(t)
- ctx, rm, _, _, childCommits, _, _, _, _, cleanup := setupDEPSRepoManager(t, cfg)
+ ctx, rm, _, _, childCommits, _, _, _, _, _, cleanup := setupDEPSRepoManager(t, cfg)
defer cleanup()
// Test update.
@@ -212,7 +218,7 @@
unittest.LargeTest(t)
cfg := depsCfg(t)
- ctx, rm, _, _, _, _, _, _, urlmock, cleanup := setupDEPSRepoManager(t, cfg)
+ ctx, rm, _, _, _, _, _, _, urlmock, _, cleanup := setupDEPSRepoManager(t, cfg)
defer cleanup()
lastRollRev, tipRev, notRolledRevs, err := rm.Update(ctx)
@@ -227,6 +233,30 @@
require.Equal(t, int64(123), issue)
}
+func TestDEPSRepoManagerCreateNewRollWithPatchRef(t *testing.T) {
+ unittest.LargeTest(t)
+
+ cfg := depsCfg(t)
+ ctx, rm, _, _, _, _, _, _, urlmock, patchRefInSyncCmd, cleanup := setupDEPSRepoManager(t, cfg)
+ defer cleanup()
+
+ lastRollRev, _, notRolledRevs, err := rm.Update(ctx)
+ require.NoError(t, err)
+ require.Equal(t, false, *patchRefInSyncCmd)
+
+ // Mock the request to load the change.
+ mockGerritGetAndPublishChange(t, urlmock, cfg)
+
+ // Use a patch ref and create a roll.
+ unsubmittedRev := &revision.Revision{
+ Id: "refs/changes/11/1111/1",
+ }
+ issue, err := rm.CreateNewRoll(ctx, lastRollRev, unsubmittedRev, notRolledRevs, emails, false, fakeCommitMsg)
+ require.NoError(t, err)
+ require.Equal(t, true, *patchRefInSyncCmd)
+ require.Equal(t, int64(123), issue)
+}
+
// Verify that we ran the PreUploadSteps.
func TestDEPSRepoManagerPreUploadSteps(t *testing.T) {
unittest.LargeTest(t)
@@ -241,7 +271,7 @@
cfg := depsCfg(t)
cfg.PreUploadSteps = []string{stepName}
- ctx, rm, _, _, _, _, _, _, urlmock, cleanup := setupDEPSRepoManager(t, cfg)
+ ctx, rm, _, _, _, _, _, _, urlmock, _, cleanup := setupDEPSRepoManager(t, cfg)
defer cleanup()
lastRollRev, tipRev, notRolledRevs, err := rm.Update(ctx)
@@ -280,7 +310,7 @@
cfg.GClientSpec = gclientSpec
cfg.ParentPath = filepath.Join("alternate", "location", "{{.ParentBase}}")
- ctx, rm, _, _, _, _, mockRun, _, urlmock, cleanup := setupDEPSRepoManager(t, cfg)
+ ctx, rm, _, _, _, _, mockRun, _, urlmock, _, cleanup := setupDEPSRepoManager(t, cfg)
defer cleanup()
lastRollRev, tipRev, notRolledRevs, err := rm.Update(ctx)
diff --git a/autoroll/go/repo_manager/parent/deps_local.go b/autoroll/go/repo_manager/parent/deps_local.go
index 4d1f578..bf467f4 100644
--- a/autoroll/go/repo_manager/parent/deps_local.go
+++ b/autoroll/go/repo_manager/parent/deps_local.go
@@ -15,6 +15,7 @@
"go.skia.org/infra/go/depot_tools"
"go.skia.org/infra/go/depot_tools/deps_parser"
"go.skia.org/infra/go/exec"
+ "go.skia.org/infra/go/gerrit"
"go.skia.org/infra/go/git"
"go.skia.org/infra/go/skerr"
"go.skia.org/infra/go/sklog"
@@ -91,11 +92,14 @@
})
return skerr.Wrap(err)
}
- sync := func(ctx context.Context) error {
+ sync := func(ctx context.Context, extraArgs ...string) error {
args := []string{"sync", "--delete_unversioned_trees", "--force"}
if !c.RunHooks {
args = append(args, "--nohooks")
}
+ if len(extraArgs) > 0 {
+ args = append(args, extraArgs...)
+ }
return skerr.Wrap(gclient(ctx, args...))
}
@@ -136,8 +140,17 @@
return "", skerr.Wrap(err)
}
- // Run "gclient sync" to sync dependencies.
- if err := sync(ctx); err != nil {
+ syncExtraArgs := []string{}
+ if strings.HasPrefix(to.Id, gerrit.CHANGE_REF_PREFIX) {
+ // If the rev is a patch ref then add patch-ref args to gclient sync. Syncing
+ // the child repo will not work without these args.
+ syncExtraArgs = append(syncExtraArgs,
+ "--patch-ref", fmt.Sprintf("%s@%s:%s", c.DependencyConfig.ID, c.Branch, to.Id),
+ "--no-rebase-patch-ref",
+ "--no-reset-patch-ref",
+ )
+ }
+ if err := sync(ctx, syncExtraArgs...); err != nil {
return "", skerr.Wrap(err)
}
