[Autorollers] Extract out Skia specific logic from Android repo manager
Extract out and put into preupload step and config files.
Bug: skia:10099
Change-Id: Id78f3059106a0516a5e6f68fef75f0fb8aa9d374
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/290124
Reviewed-by: Eric Boren <borenet@google.com>
Commit-Queue: Ravi Mistry <rmistry@google.com>
diff --git a/autoroll/go/repo_manager/android_repo_manager.go b/autoroll/go/repo_manager/android_repo_manager.go
index 07d074c..55ab7ad 100644
--- a/autoroll/go/repo_manager/android_repo_manager.go
+++ b/autoroll/go/repo_manager/android_repo_manager.go
@@ -21,8 +21,6 @@
"go.skia.org/infra/autoroll/go/repo_manager/parent"
"go.skia.org/infra/autoroll/go/revision"
"go.skia.org/infra/autoroll/go/strategy"
- "go.skia.org/infra/go/android_skia_checkout"
- "go.skia.org/infra/go/common"
"go.skia.org/infra/go/exec"
"go.skia.org/infra/go/gerrit"
"go.skia.org/infra/go/sklog"
@@ -62,12 +60,6 @@
)
var (
- // Files which exist in Skia but do not exist in Android.
- DELETE_MERGE_CONFLICT_FILES = []string{android_skia_checkout.SkUserConfigRelPath}
- // Files which exist in Android but do not exist in Skia.
- IGNORE_MERGE_CONFLICT_FILES = []string{android_skia_checkout.SkUserConfigAndroidRelPath, android_skia_checkout.SkUserConfigLinuxRelPath, android_skia_checkout.SkUserConfigMacRelPath, android_skia_checkout.SkUserConfigWinRelPath}
- FILES_GENERATED_BY_GN_TO_GP = []string{android_skia_checkout.SkUserConfigAndroidRelPath, android_skia_checkout.SkUserConfigLinuxRelPath, android_skia_checkout.SkUserConfigMacRelPath, android_skia_checkout.SkUserConfigWinRelPath, android_skia_checkout.AndroidBpRelPath}
-
AUTHOR_EMAIL_RE = regexp.MustCompile(".* \\((.*)\\)")
)
@@ -93,6 +85,7 @@
type AndroidRepoManagerConfig struct {
CommonRepoManagerConfig
*ProjectMetadataFileConfig `json:"projectMetadataFileConfig,omitempty"`
+ ChildRepoURL string `json:"childRepoURL"`
}
// See documentation for util.Validator interface.
@@ -105,6 +98,9 @@
return err
}
}
+ if c.ChildRepoURL == "" {
+ return errors.New("childRepoURL must be specified")
+ }
return nil
}
@@ -119,8 +115,9 @@
// androidRepoManager is a struct used by Android AutoRoller for managing checkouts.
type androidRepoManager struct {
*commonRepoManager
- repoUrl string
- repoToolPath string
+ childRepoURL string
+ parentRepoURL string
+ repoToolPath string
projectMetadataFileConfig *ProjectMetadataFileConfig
}
@@ -162,9 +159,10 @@
}
r := &androidRepoManager{
commonRepoManager: crm,
- repoUrl: g.GetRepoUrl(),
+ parentRepoURL: g.GetRepoUrl(),
repoToolPath: repoToolPath,
projectMetadataFileConfig: c.ProjectMetadataFileConfig,
+ childRepoURL: c.ChildRepoURL,
}
return r, nil
}
@@ -179,7 +177,7 @@
}
// Run repo init and sync commands.
- if _, err := exec.RunCwd(ctx, r.workdir, r.repoToolPath, "init", "-u", fmt.Sprintf("%s/a/platform/manifest", r.repoUrl), "-g", "all,-notdefault,-darwin", "-b", r.parentBranch.String()); err != nil {
+ if _, err := exec.RunCwd(ctx, r.workdir, r.repoToolPath, "init", "-u", fmt.Sprintf("%s/a/platform/manifest", r.parentRepoURL), "-g", "all,-notdefault,-darwin", "-b", r.parentBranch.String()); err != nil {
return err
}
// Sync only the child path and the repohooks directory (needed to upload changes).
@@ -193,7 +191,7 @@
}
// Fix the review config to a URL which will work outside prod.
- if _, err := r.childRepo.Git(ctx, "config", "remote.goog.review", fmt.Sprintf("%s/", r.repoUrl)); err != nil {
+ if _, err := r.childRepo.Git(ctx, "config", "remote.goog.review", fmt.Sprintf("%s/", r.parentRepoURL)); err != nil {
return err
}
@@ -203,7 +201,7 @@
return err
}
if !strings.Contains(remoteOutput, UPSTREAM_REMOTE_NAME) {
- if _, err := r.childRepo.Git(ctx, "remote", "add", UPSTREAM_REMOTE_NAME, common.REPO_SKIA); err != nil {
+ if _, err := r.childRepo.Git(ctx, "remote", "add", UPSTREAM_REMOTE_NAME, r.childRepoURL); err != nil {
return err
}
}
@@ -303,54 +301,12 @@
// Start the merge.
if _, err := r.childRepo.Git(ctx, "merge", to.Id, "--no-commit"); err != nil {
- // Check to see if this was a merge conflict with IGNORE_MERGE_CONFLICT_FILES.
+ // Check to see if this was a merge conflict with ignoreMergeConflictFiles and deleteMergeConflictFiles.
conflictsOutput, conflictsErr := r.childRepo.Git(ctx, "diff", "--name-only", "--diff-filter=U")
if conflictsErr != nil || conflictsOutput == "" {
util.LogErr(conflictsErr)
return 0, fmt.Errorf("Failed to roll to %s. Needs human investigation: %s", to, err)
}
- for _, conflict := range strings.Split(conflictsOutput, "\n") {
- if conflict == "" {
- continue
- }
- ignoreConflict := false
- for _, ignore := range IGNORE_MERGE_CONFLICT_FILES {
- if conflict == ignore {
- ignoreConflict = true
- sklog.Infof("Ignoring conflict in %s", conflict)
- break
- }
- }
- for _, del := range DELETE_MERGE_CONFLICT_FILES {
- if conflict == del {
- _, resetErr := r.childRepo.Git(ctx, "reset", "--", del)
- util.LogErr(resetErr)
- _, delErr := exec.RunCwd(ctx, r.childDir, "rm", del)
- util.LogErr(delErr)
- ignoreConflict = true
- sklog.Infof("Deleting %s due to merge conflict", conflict)
- break
- }
- }
- if !ignoreConflict {
- util.LogErr(r.abortMerge(ctx))
- return 0, fmt.Errorf("Failed to roll to %s. Conflicts in %s: %s", to, conflictsOutput, err)
- }
- }
- }
-
- if err := android_skia_checkout.RunGnToBp(ctx, r.childDir); err != nil {
- util.LogErr(r.abortMerge(ctx))
- return 0, fmt.Errorf("Error when running gn_to_bp: %s", err)
- }
- for _, genFile := range FILES_GENERATED_BY_GN_TO_GP {
- if _, err := r.childRepo.Git(ctx, "add", genFile); err != nil {
- return 0, err
- }
- }
-
- if _, addGifErr := r.childRepo.Git(ctx, "add", android_skia_checkout.LibGifRelPath); addGifErr != nil {
- return 0, addGifErr
}
if r.projectMetadataFileConfig != nil {
@@ -389,6 +345,7 @@
// Run the pre-upload steps.
for _, s := range r.preUploadSteps {
if err := s(ctx, nil, r.httpClient, r.workdir); err != nil {
+ util.LogErr(r.abortMerge(ctx))
return 0, fmt.Errorf("Failed pre-upload step: %s", err)
}
}
@@ -419,7 +376,7 @@
// Create commit message.
commitMsg, err := r.buildCommitMsg(&parent.CommitMsgVars{
ChildPath: r.childPath,
- ChildRepo: common.REPO_SKIA, // TODO(borenet): Don't hard-code.
+ ChildRepo: r.childRepoURL,
Reviewers: rollEmails,
Revisions: rolling,
RollingFrom: from,
@@ -441,7 +398,7 @@
// Bypass the repo upload prompt by setting autoupload config to true.
// Strip "-review" from the upload URL else autoupload does not work.
- uploadUrl := strings.Replace(r.repoUrl, "-review", "", 1)
+ uploadUrl := strings.Replace(r.parentRepoURL, "-review", "", 1)
if _, configErr := r.childRepo.Git(ctx, "config", fmt.Sprintf("review.%s/.autoupload", uploadUrl), "true"); configErr != nil {
util.LogErr(r.abandonRepoBranch(ctx))
return 0, fmt.Errorf("Could not set autoupload config: %s", configErr)
diff --git a/autoroll/go/repo_manager/android_repo_manager_test.go b/autoroll/go/repo_manager/android_repo_manager_test.go
index 22b623a..0913c91 100644
--- a/autoroll/go/repo_manager/android_repo_manager_test.go
+++ b/autoroll/go/repo_manager/android_repo_manager_test.go
@@ -12,6 +12,7 @@
"go.skia.org/infra/autoroll/go/codereview"
"go.skia.org/infra/autoroll/go/config_vars"
"go.skia.org/infra/autoroll/go/repo_manager/parent"
+ "go.skia.org/infra/go/common"
"go.skia.org/infra/go/exec"
"go.skia.org/infra/go/gerrit"
gerrit_mocks "go.skia.org/infra/go/gerrit/mocks"
@@ -44,13 +45,13 @@
func androidCfg(t *testing.T) *AndroidRepoManagerConfig {
return &AndroidRepoManagerConfig{
- CommonRepoManagerConfig{
+ CommonRepoManagerConfig: CommonRepoManagerConfig{
ChildBranch: masterBranchTmpl(t),
ChildPath: childPath,
ParentBranch: masterBranchTmpl(t),
ParentRepo: "https://my-repo.com",
},
- &ProjectMetadataFileConfig{
+ ProjectMetadataFileConfig: &ProjectMetadataFileConfig{
FilePath: "METADATA",
Name: "skia",
Description: "Skia Graphics Library",
@@ -58,6 +59,7 @@
GitURL: "https://skia.googlesource.com/skia",
LicenseType: "RECIPROCAL",
},
+ ChildRepoURL: common.REPO_SKIA,
}
}
diff --git a/autoroll/go/repo_manager/parent/pre_upload_steps.go b/autoroll/go/repo_manager/parent/pre_upload_steps.go
index 0cba1e8..e33025d 100644
--- a/autoroll/go/repo_manager/parent/pre_upload_steps.go
+++ b/autoroll/go/repo_manager/parent/pre_upload_steps.go
@@ -14,6 +14,7 @@
"strings"
"github.com/google/uuid"
+ "go.skia.org/infra/go/android_skia_checkout"
"go.skia.org/infra/go/cipd"
"go.skia.org/infra/go/exec"
"go.skia.org/infra/go/git"
@@ -41,6 +42,7 @@
"FlutterLicenseScripts": FlutterLicenseScripts,
"FlutterLicenseScriptsForDart": FlutterLicenseScriptsForDart,
"FlutterLicenseScriptsForFuchsia": FlutterLicenseScriptsForFuchsia,
+ "SkiaGnToBp": SkiaGnToBp,
"UpdateFlutterDepsForDart": UpdateFlutterDepsForDart,
}
@@ -277,6 +279,23 @@
return nil
}
+// SkiaGnToBp runs Skia's gn_to_bp.py script to roll into Android.
+func SkiaGnToBp(ctx context.Context, env []string, client *http.Client, parentRepoDir string) error {
+ skiaDir := filepath.Join(parentRepoDir, "external", "skia")
+ if err := android_skia_checkout.RunGnToBp(ctx, skiaDir); err != nil {
+ return fmt.Errorf("Error when running gn_to_bp: %s", err)
+ }
+ for _, genFile := range android_skia_checkout.FilesGeneratedByGnToGp {
+ if _, err := git.GitDir(skiaDir).Git(ctx, "add", genFile); err != nil {
+ return fmt.Errorf("Could not git add %s: %s", genFile, err)
+ }
+ }
+ if _, err := git.GitDir(skiaDir).Git(ctx, "add", android_skia_checkout.LibGifRelPath); err != nil {
+ return fmt.Errorf("Could not git add %s: %s", android_skia_checkout.LibGifRelPath, err)
+ }
+ return nil
+}
+
// Run the ANGLE code generation script.
func ANGLECodeGeneration(ctx context.Context, env []string, client *http.Client, parentRepoDir string) error {
sklog.Info("Running code generation script...")
diff --git a/go/android_skia_checkout/android_skia_checkout.go b/go/android_skia_checkout/android_skia_checkout.go
index c5013fb..807d0f8 100644
--- a/go/android_skia_checkout/android_skia_checkout.go
+++ b/go/android_skia_checkout/android_skia_checkout.go
@@ -21,6 +21,8 @@
SkUserConfigWinRelPath = path.Join("win", "include", "config", "SkUserConfig.h")
AndroidBpRelPath = path.Join("Android.bp")
LibGifRelPath = path.Join("third_party", "libgifcodec")
+
+ FilesGeneratedByGnToGp = []string{SkUserConfigAndroidRelPath, SkUserConfigLinuxRelPath, SkUserConfigMacRelPath, SkUserConfigWinRelPath, AndroidBpRelPath}
)
const (
