[autoroll] Push lastRollRev handling into common packages
We already have a concept of attaching pinned dependencies to Revisions,
and we use it on the Child side for transitive deps. This changes the
Parent to do that as well, rather than passing around custom
getLastRollRev functions. This simplifies a number of things, at the
cost of requiring that the lastRollRev is written in a file somewhere.
I don't think that this will be problematic.
Change-Id: I499bfb4a6adbefc5ff4441d8a7b2c380e17a966c
Reviewed-on: https://skia-review.googlesource.com/c/buildbot/+/289488
Reviewed-by: Ravi Mistry <rmistry@google.com>
Commit-Queue: Eric Boren <borenet@google.com>
diff --git a/autoroll/go/repo_manager/common/git_common/git_common.go b/autoroll/go/repo_manager/common/git_common/git_common.go
index a26c2eb..623d830 100644
--- a/autoroll/go/repo_manager/common/git_common/git_common.go
+++ b/autoroll/go/repo_manager/common/git_common/git_common.go
@@ -6,6 +6,7 @@
"path/filepath"
"go.skia.org/infra/autoroll/go/config_vars"
+ "go.skia.org/infra/autoroll/go/repo_manager/common/version_file_common"
"go.skia.org/infra/autoroll/go/revision"
"go.skia.org/infra/go/git"
"go.skia.org/infra/go/skerr"
@@ -13,9 +14,19 @@
// GitCheckoutConfig provides configuration for a Checkout.
type GitCheckoutConfig struct {
- Branch *config_vars.Template `json:"branch"`
- RepoURL string `json:"repoURL"`
- RevLinkTmpl string `json:"revLinkTmpl"`
+ // Branch is the name of the git branch to be tracked by the Checkout.
+ Branch *config_vars.Template `json:"branch"`
+ // RepoURL is the git repository URL.
+ RepoURL string `json:"repoURL"`
+ // RevLinkTmpl is an optional template used for generating links to
+ // Revisions. If not specified, Revisions generated by the Checkout will not
+ // have an associated URL.
+ RevLinkTmpl string `json:"revLinkTmpl"`
+
+ // Dependencies is an optional specification of dependencies to track.
+ // Revisions generated by the Checkout will contain the pinned versions of
+ // these dependencies.
+ Dependencies []*version_file_common.VersionFileConfig `json:"dependencies"`
}
// See documentation for util.Validator interface.
@@ -35,9 +46,10 @@
// Checkout provides common functionality for git checkouts.
type Checkout struct {
*git.Checkout
- Branch *config_vars.Template
- RepoURL string
- RevLinkTmpl string
+ Branch *config_vars.Template
+ Dependencies []*version_file_common.VersionFileConfig
+ RepoURL string
+ RevLinkTmpl string
}
// NewCheckout returns a Checkout instance.
@@ -66,10 +78,11 @@
return nil, skerr.Wrap(err)
}
return &Checkout{
- Checkout: co,
- Branch: c.Branch,
- RepoURL: c.RepoURL,
- RevLinkTmpl: c.RevLinkTmpl,
+ Checkout: co,
+ Branch: c.Branch,
+ Dependencies: c.Dependencies,
+ RepoURL: c.RepoURL,
+ RevLinkTmpl: c.RevLinkTmpl,
}, nil
}
@@ -79,7 +92,17 @@
if err != nil {
return nil, skerr.Wrap(err)
}
- return revision.FromLongCommit(c.RevLinkTmpl, details), nil
+ rev := revision.FromLongCommit(c.RevLinkTmpl, details)
+ if len(c.Dependencies) > 0 {
+ deps, err := version_file_common.GetPinnedRevs(ctx, c.Dependencies, func(ctx context.Context, path string) (string, error) {
+ return c.GetFile(ctx, path, rev.Id)
+ })
+ if err != nil {
+ return nil, skerr.Wrap(err)
+ }
+ rev.Dependencies = deps
+ }
+ return rev, nil
}
// See documentation for child.Child interface.
diff --git a/autoroll/go/repo_manager/common/gitiles_common/gitiles_common.go b/autoroll/go/repo_manager/common/gitiles_common/gitiles_common.go
index 958b50b..17fee2e 100644
--- a/autoroll/go/repo_manager/common/gitiles_common/gitiles_common.go
+++ b/autoroll/go/repo_manager/common/gitiles_common/gitiles_common.go
@@ -16,16 +16,13 @@
// GitilesConfig provides configuration for GitilesRepo.
type GitilesConfig struct {
- // Branch is the name of the git branch to be rolled.
+ // Branch is the name of the git branch to be tracked by the GitilesRepo.
Branch *config_vars.Template `json:"branch"`
// RepoURL is the git repository URL.
RepoURL string `json:"repoURL"`
- // Dependencies is an optional mapping of dependency ID (eg. repo URL)
- // to the location within the repo where its version is pinned (eg. DEPS).
- // Revisions generated by the GitilesRepo will contain the versions of
- // these dependencies. The values must be either files whose entire
- // contents are the version ID of the dependency, or "DEPS" which is a
- // special case.
+ // Dependencies is an optional specification of dependencies to track.
+ // Revisions generated by the GitilesRepo will contain the pinned versions
+ // of these dependencies.
Dependencies []*version_file_common.VersionFileConfig `json:"dependencies,omitempty"`
}
@@ -83,24 +80,13 @@
// Optionally load any dependencies.
if len(r.deps) > 0 {
- rev.Dependencies = make(map[string]string, len(r.deps))
- // Cache files in case multiple dependencies are versioned in
- // the same file, eg. DEPS.
- cache := map[string]string{}
- for _, dep := range r.deps {
- contents, ok := cache[dep.Path]
- if !ok {
- contents, err = r.GetFile(ctx, dep.Path, rev.Id)
- if err != nil {
- return nil, skerr.Wrap(err)
- }
- }
- version, err := version_file_common.GetPinnedRev(*dep, contents)
- if err != nil {
- return nil, skerr.Wrap(err)
- }
- rev.Dependencies[dep.ID] = version
+ deps, err := version_file_common.GetPinnedRevs(ctx, r.deps, func(ctx context.Context, path string) (string, error) {
+ return r.GetFile(ctx, path, rev.Id)
+ })
+ if err != nil {
+ return nil, skerr.Wrap(err)
}
+ rev.Dependencies = deps
}
return rev, nil
}
diff --git a/autoroll/go/repo_manager/common/version_file_common/version_file_common.go b/autoroll/go/repo_manager/common/version_file_common/version_file_common.go
index 780cbb3..6cb400a 100644
--- a/autoroll/go/repo_manager/common/version_file_common/version_file_common.go
+++ b/autoroll/go/repo_manager/common/version_file_common/version_file_common.go
@@ -7,7 +7,6 @@
"go.skia.org/infra/autoroll/go/revision"
"go.skia.org/infra/go/depot_tools/deps_parser"
"go.skia.org/infra/go/skerr"
- "go.skia.org/infra/go/sklog"
)
// VersionFileConfig provides configuration for a file in a git repository which
@@ -74,6 +73,32 @@
}
}
+// GetPinnedRevs reads files using the given GetFileFunc to retrieve the given
+// pinned revisions. File retrievals are cached for efficiency.
+func GetPinnedRevs(ctx context.Context, deps []*VersionFileConfig, getFile GetFileFunc) (map[string]string, error) {
+ rv := make(map[string]string, len(deps))
+ // Cache files in case multiple dependencies are versioned in
+ // the same file, eg. DEPS.
+ cache := map[string]string{}
+ for _, dep := range deps {
+ contents, ok := cache[dep.Path]
+ if !ok {
+ var err error
+ contents, err = getFile(ctx, dep.Path)
+ if err != nil {
+ return nil, skerr.Wrap(err)
+ }
+ cache[dep.Path] = contents
+ }
+ version, err := GetPinnedRev(*dep, contents)
+ if err != nil {
+ return nil, skerr.Wrap(err)
+ }
+ rv[dep.ID] = version
+ }
+ return rv, nil
+}
+
// SetPinnedRev updates the given dependency pin in the given file, returning
// the new contents.
func SetPinnedRev(dep VersionFileConfig, newVersion, oldContents string) (string, error) {
@@ -134,13 +159,11 @@
// Handle transitive dependencies.
var td []*TransitiveDepUpdate
if len(primaryDep.TransitiveDeps) > 0 {
- //td = make([]*TransitiveDep, 0, len(transitiveDeps))
for _, dep := range primaryDep.TransitiveDeps {
// Find the new revision.
newVersion, ok := rev.Dependencies[dep.ID]
if !ok {
- sklog.Errorf("Could not find transitive dependency %q in %+v", dep.ID, rev)
- continue
+ return nil, nil, skerr.Fmt("Could not find transitive dependency %q in %#v", dep.ID, rev)
}
// Update.
oldVersion, err := updateSingleDep(ctx, *dep, newVersion, changes, getFile)
diff --git a/autoroll/go/repo_manager/copy_repo_manager.go b/autoroll/go/repo_manager/copy_repo_manager.go
index 9b44aec..e0d61a89 100644
--- a/autoroll/go/repo_manager/copy_repo_manager.go
+++ b/autoroll/go/repo_manager/copy_repo_manager.go
@@ -9,6 +9,7 @@
"go.skia.org/infra/autoroll/go/repo_manager/child"
"go.skia.org/infra/autoroll/go/repo_manager/common/git_common"
"go.skia.org/infra/autoroll/go/repo_manager/common/gitiles_common"
+ "go.skia.org/infra/autoroll/go/repo_manager/common/version_file_common"
"go.skia.org/infra/autoroll/go/repo_manager/parent"
"go.skia.org/infra/go/gerrit"
"go.skia.org/infra/go/skerr"
@@ -61,11 +62,16 @@
Branch: c.DepotToolsRepoManagerConfig.CommonRepoManagerConfig.ParentBranch,
RepoURL: c.DepotToolsRepoManagerConfig.CommonRepoManagerConfig.ParentRepo,
},
+ DependencyConfig: version_file_common.DependencyConfig{
+ VersionFileConfig: version_file_common.VersionFileConfig{
+ ID: c.ChildRepo,
+ Path: c.VersionFile,
+ },
+ },
},
Gerrit: c.Gerrit,
},
- VersionFile: c.VersionFile,
- Copies: c.Copies,
+ Copies: c.Copies,
}
if err := parentCfg.Validate(); err != nil {
return parent.CopyConfig{}, child.GitilesConfig{}, skerr.Wrapf(err, "generated parent config is invalid")
diff --git a/autoroll/go/repo_manager/deps_repo_manager.go b/autoroll/go/repo_manager/deps_repo_manager.go
index e55a177..493fdef 100644
--- a/autoroll/go/repo_manager/deps_repo_manager.go
+++ b/autoroll/go/repo_manager/deps_repo_manager.go
@@ -61,11 +61,11 @@
Branch: c.DepotToolsRepoManagerConfig.CommonRepoManagerConfig.ParentBranch,
RepoURL: c.DepotToolsRepoManagerConfig.CommonRepoManagerConfig.ParentRepo,
},
- },
- DependencyConfig: version_file_common.DependencyConfig{
- VersionFileConfig: version_file_common.VersionFileConfig{
- ID: c.ChildRepo,
- Path: deps_parser.DepsFileName,
+ DependencyConfig: version_file_common.DependencyConfig{
+ VersionFileConfig: version_file_common.VersionFileConfig{
+ ID: c.ChildRepo,
+ Path: deps_parser.DepsFileName,
+ },
},
},
CheckoutPath: c.ParentPath,
diff --git a/autoroll/go/repo_manager/fuchsia_sdk_android_repo_manager.go b/autoroll/go/repo_manager/fuchsia_sdk_android_repo_manager.go
index 5e85341..d9f52f1 100644
--- a/autoroll/go/repo_manager/fuchsia_sdk_android_repo_manager.go
+++ b/autoroll/go/repo_manager/fuchsia_sdk_android_repo_manager.go
@@ -88,18 +88,20 @@
Branch: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ParentBranch,
RepoURL: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ParentRepo,
},
+ DependencyConfig: version_file_common.DependencyConfig{
+ VersionFileConfig: version_file_common.VersionFileConfig{
+ ID: "FuchsiaSDK",
+ Path: FuchsiaSDKAndroidVersionFile,
+ },
+ },
}
genSdkBpRepo, err := git.NewCheckout(ctx, c.GenSdkBpRepo, workdir)
if err != nil {
return nil, skerr.Wrap(err)
}
- update := parent.VersionFileGetLastRollRevFunc(version_file_common.VersionFileConfig{
- ID: "FuchsiaSDK",
- Path: FuchsiaSDKAndroidVersionFile,
- })
createRoll := fuchsiaSDKAndroidRepoManagerCreateRollFunc(genSdkBpRepo, c.GenSdkBpBranch, workdir)
uploadRoll := parent.GitCheckoutUploadGerritRollFunc(g)
- parentRM, err := parent.NewGitCheckout(ctx, parentCfg, reg, serverURL, workdir, cr.UserName(), cr.UserEmail(), nil, update, createRoll, uploadRoll)
+ parentRM, err := parent.NewGitCheckout(ctx, parentCfg, reg, serverURL, workdir, cr.UserName(), cr.UserEmail(), nil, createRoll, uploadRoll)
if err != nil {
return nil, skerr.Wrap(err)
}
diff --git a/autoroll/go/repo_manager/fuchsia_sdk_repo_manager.go b/autoroll/go/repo_manager/fuchsia_sdk_repo_manager.go
index 56d5b13..b8138eb 100644
--- a/autoroll/go/repo_manager/fuchsia_sdk_repo_manager.go
+++ b/autoroll/go/repo_manager/fuchsia_sdk_repo_manager.go
@@ -56,7 +56,7 @@
// splitParentChild breaks the FuchsiaSDKRepoManagerConfig into parent and child
// configs.
-func (c *FuchsiaSDKRepoManagerConfig) splitParentChild() (parent.GitilesFileConfig, child.FuchsiaSDKConfig, error) {
+func (c *FuchsiaSDKRepoManagerConfig) splitParentChild() (parent.GitilesConfig, child.FuchsiaSDKConfig, error) {
var parentDeps []*version_file_common.VersionFileConfig
if c.IncludeMacSDK {
parentDeps = []*version_file_common.VersionFileConfig{
@@ -70,38 +70,36 @@
if c.CommitMsgTmpl != "" {
commitMsgTmpl = c.CommitMsgTmpl
}
- parentCfg := parent.GitilesFileConfig{
- GitilesConfig: parent.GitilesConfig{
- BaseConfig: parent.BaseConfig{
- ChildPath: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ChildPath,
- ChildRepo: "TODO",
- IncludeBugs: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.IncludeBugs,
- IncludeLog: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.IncludeLog,
- CommitMsgTmpl: commitMsgTmpl,
- MonorailProject: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.BugProject,
- },
- GitilesConfig: gitiles_common.GitilesConfig{
- Branch: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ParentBranch,
- RepoURL: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ParentRepo,
- },
- Gerrit: c.Gerrit,
+ parentCfg := parent.GitilesConfig{
+ BaseConfig: parent.BaseConfig{
+ ChildPath: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ChildPath,
+ ChildRepo: "FuchsiaSDK",
+ IncludeBugs: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.IncludeBugs,
+ IncludeLog: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.IncludeLog,
+ CommitMsgTmpl: commitMsgTmpl,
+ MonorailProject: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.BugProject,
},
DependencyConfig: version_file_common.DependencyConfig{
VersionFileConfig: version_file_common.VersionFileConfig{
- ID: "TODO",
+ ID: "FuchsiaSDK",
Path: FuchsiaSDKVersionFilePathLinux,
},
TransitiveDeps: parentDeps,
},
+ GitilesConfig: gitiles_common.GitilesConfig{
+ Branch: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ParentBranch,
+ RepoURL: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ParentRepo,
+ },
+ Gerrit: c.Gerrit,
}
if err := parentCfg.Validate(); err != nil {
- return parent.GitilesFileConfig{}, child.FuchsiaSDKConfig{}, skerr.Wrapf(err, "generated parent config is invalid")
+ return parent.GitilesConfig{}, child.FuchsiaSDKConfig{}, skerr.Wrapf(err, "generated parent config is invalid")
}
childCfg := child.FuchsiaSDKConfig{
IncludeMacSDK: c.IncludeMacSDK,
}
if err := childCfg.Validate(); err != nil {
- return parent.GitilesFileConfig{}, child.FuchsiaSDKConfig{}, skerr.Wrapf(err, "generated child config is invalid")
+ return parent.GitilesConfig{}, child.FuchsiaSDKConfig{}, skerr.Wrapf(err, "generated child config is invalid")
}
return parentCfg, childCfg, nil
}
diff --git a/autoroll/go/repo_manager/github_cipd_deps_repo_manager.go b/autoroll/go/repo_manager/github_cipd_deps_repo_manager.go
index 880beae..7643f3a 100644
--- a/autoroll/go/repo_manager/github_cipd_deps_repo_manager.go
+++ b/autoroll/go/repo_manager/github_cipd_deps_repo_manager.go
@@ -72,11 +72,11 @@
Branch: c.DepotToolsRepoManagerConfig.CommonRepoManagerConfig.ParentBranch,
RepoURL: c.DepotToolsRepoManagerConfig.CommonRepoManagerConfig.ParentRepo,
},
- },
- DependencyConfig: version_file_common.DependencyConfig{
- VersionFileConfig: version_file_common.VersionFileConfig{
- ID: c.CipdAssetName,
- Path: deps_parser.DepsFileName,
+ DependencyConfig: version_file_common.DependencyConfig{
+ VersionFileConfig: version_file_common.VersionFileConfig{
+ ID: c.CipdAssetName,
+ Path: deps_parser.DepsFileName,
+ },
},
},
CheckoutPath: c.GithubParentPath,
diff --git a/autoroll/go/repo_manager/github_deps_repo_manager.go b/autoroll/go/repo_manager/github_deps_repo_manager.go
index c7beaf7..e8ab66e 100644
--- a/autoroll/go/repo_manager/github_deps_repo_manager.go
+++ b/autoroll/go/repo_manager/github_deps_repo_manager.go
@@ -66,11 +66,11 @@
Branch: c.DepotToolsRepoManagerConfig.CommonRepoManagerConfig.ParentBranch,
RepoURL: c.DepotToolsRepoManagerConfig.CommonRepoManagerConfig.ParentRepo,
},
- },
- DependencyConfig: version_file_common.DependencyConfig{
- VersionFileConfig: version_file_common.VersionFileConfig{
- ID: c.ChildRepo,
- Path: deps_parser.DepsFileName,
+ DependencyConfig: version_file_common.DependencyConfig{
+ VersionFileConfig: version_file_common.VersionFileConfig{
+ ID: c.ChildRepo,
+ Path: deps_parser.DepsFileName,
+ },
},
},
CheckoutPath: c.GithubParentPath,
diff --git a/autoroll/go/repo_manager/github_repo_manager.go b/autoroll/go/repo_manager/github_repo_manager.go
index 3680dd8..9fd42f0 100644
--- a/autoroll/go/repo_manager/github_repo_manager.go
+++ b/autoroll/go/repo_manager/github_repo_manager.go
@@ -72,16 +72,16 @@
RepoURL: c.ParentRepo,
RevLinkTmpl: "", // Not needed.
},
+ DependencyConfig: version_file_common.DependencyConfig{
+ VersionFileConfig: version_file_common.VersionFileConfig{
+ ID: c.ChildRepoURL,
+ Path: c.RevisionFile,
+ },
+ TransitiveDeps: parentDeps,
+ },
},
ForkRepoURL: c.ForkRepoURL,
},
- DependencyConfig: version_file_common.DependencyConfig{
- VersionFileConfig: version_file_common.VersionFileConfig{
- ID: c.ChildRepoURL,
- Path: c.RevisionFile,
- },
- TransitiveDeps: parentDeps,
- },
PreUploadSteps: c.PreUploadSteps,
}
if err := parentCfg.Validate(); err != nil {
@@ -90,9 +90,10 @@
childCfg := child.GitCheckoutGithubConfig{
GitCheckoutConfig: child.GitCheckoutConfig{
GitCheckoutConfig: git_common.GitCheckoutConfig{
- Branch: c.ChildBranch,
- RepoURL: c.ChildRepoURL,
- RevLinkTmpl: c.ChildRevLinkTmpl,
+ Branch: c.ChildBranch,
+ Dependencies: childDeps,
+ RepoURL: c.ChildRepoURL,
+ RevLinkTmpl: c.ChildRevLinkTmpl,
},
},
BuildbucketRevisionFilter: c.BuildbucketRevisionFilter,
diff --git a/autoroll/go/repo_manager/gitiles_cipd_deps_repo_manager.go b/autoroll/go/repo_manager/gitiles_cipd_deps_repo_manager.go
index 7069ae9..946dabb 100644
--- a/autoroll/go/repo_manager/gitiles_cipd_deps_repo_manager.go
+++ b/autoroll/go/repo_manager/gitiles_cipd_deps_repo_manager.go
@@ -8,8 +8,10 @@
"go.skia.org/infra/autoroll/go/config_vars"
"go.skia.org/infra/autoroll/go/repo_manager/child"
"go.skia.org/infra/autoroll/go/repo_manager/common/gitiles_common"
+ "go.skia.org/infra/autoroll/go/repo_manager/common/version_file_common"
"go.skia.org/infra/autoroll/go/repo_manager/parent"
"go.skia.org/infra/autoroll/go/strategy"
+ "go.skia.org/infra/go/depot_tools/deps_parser"
"go.skia.org/infra/go/gerrit"
"go.skia.org/infra/go/skerr"
)
@@ -42,37 +44,40 @@
}
// splitParentChild splits the GitilesCIPDDEPSRepoManagerConfig into a
-// parent.GitilesDEPSConfig and a child.CIPDConfig.
+// parent.GitilesConfig and a child.CIPDConfig.
// TODO(borenet): Update the config format to directly define the parent
// and child. We shouldn't need most of the New.*RepoManager functions.
-func (c GitilesCIPDDEPSRepoManagerConfig) splitParentChild() (parent.GitilesDEPSConfig, child.CIPDConfig, error) {
- parentCfg := parent.GitilesDEPSConfig{
- GitilesConfig: parent.GitilesConfig{
- BaseConfig: parent.BaseConfig{
- ChildPath: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ChildPath,
- ChildRepo: c.CipdAssetName,
- IncludeBugs: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.IncludeBugs,
- IncludeLog: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.IncludeLog,
- CommitMsgTmpl: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.CommitMsgTmpl,
- MonorailProject: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.BugProject,
- },
- GitilesConfig: gitiles_common.GitilesConfig{
- Branch: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ParentBranch,
- RepoURL: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ParentRepo,
- },
- Gerrit: c.Gerrit,
+func (c GitilesCIPDDEPSRepoManagerConfig) splitParentChild() (parent.GitilesConfig, child.CIPDConfig, error) {
+ parentCfg := parent.GitilesConfig{
+ BaseConfig: parent.BaseConfig{
+ ChildPath: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ChildPath,
+ ChildRepo: c.CipdAssetName,
+ IncludeBugs: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.IncludeBugs,
+ IncludeLog: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.IncludeLog,
+ CommitMsgTmpl: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.CommitMsgTmpl,
+ MonorailProject: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.BugProject,
},
- Dep: c.CipdAssetName,
+ DependencyConfig: version_file_common.DependencyConfig{
+ VersionFileConfig: version_file_common.VersionFileConfig{
+ ID: c.CipdAssetName,
+ Path: deps_parser.DepsFileName,
+ },
+ },
+ GitilesConfig: gitiles_common.GitilesConfig{
+ Branch: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ParentBranch,
+ RepoURL: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ParentRepo,
+ },
+ Gerrit: c.Gerrit,
}
if err := parentCfg.Validate(); err != nil {
- return parent.GitilesDEPSConfig{}, child.CIPDConfig{}, skerr.Wrapf(err, "generated parent config is invalid")
+ return parent.GitilesConfig{}, child.CIPDConfig{}, skerr.Wrapf(err, "generated parent config is invalid")
}
childCfg := child.CIPDConfig{
Name: c.CipdAssetName,
Tag: c.CipdAssetTag,
}
if err := childCfg.Validate(); err != nil {
- return parent.GitilesDEPSConfig{}, child.CIPDConfig{}, skerr.Wrapf(err, "generated child config is invalid")
+ return parent.GitilesConfig{}, child.CIPDConfig{}, skerr.Wrapf(err, "generated child config is invalid")
}
return parentCfg, childCfg, nil
}
@@ -87,7 +92,7 @@
if err != nil {
return nil, skerr.Wrap(err)
}
- parentRM, err := parent.NewGitilesDEPS(ctx, parentCfg, reg, client, serverURL)
+ parentRM, err := parent.NewGitilesFile(ctx, parentCfg, reg, client, serverURL)
if err != nil {
return nil, skerr.Wrap(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 88bb048..56c31a3 100644
--- a/autoroll/go/repo_manager/no_checkout_deps_repo_manager.go
+++ b/autoroll/go/repo_manager/no_checkout_deps_repo_manager.go
@@ -12,6 +12,7 @@
"go.skia.org/infra/autoroll/go/repo_manager/common/gitiles_common"
"go.skia.org/infra/autoroll/go/repo_manager/common/version_file_common"
"go.skia.org/infra/autoroll/go/repo_manager/parent"
+ "go.skia.org/infra/go/depot_tools/deps_parser"
"go.skia.org/infra/go/gerrit"
"go.skia.org/infra/go/skerr"
)
@@ -76,10 +77,10 @@
}
// splitParentChild splits the NoCheckoutDEPSRepoManagerConfig into a
-// parent.GitilesDEPSConfig and a child.GitilesConfig.
+// parent.GitilesConfig and a child.GitilesConfig.
// TODO(borenet): Update the config format to directly define the parent
// and child. We shouldn't need most of the New.*RepoManager functions.
-func (c NoCheckoutDEPSRepoManagerConfig) splitParentChild() (parent.GitilesDEPSConfig, child.GitilesConfig, error) {
+func (c NoCheckoutDEPSRepoManagerConfig) splitParentChild() (parent.GitilesConfig, child.GitilesConfig, error) {
var childDeps, parentDeps []*version_file_common.VersionFileConfig
if c.TransitiveDeps != nil {
childDeps = make([]*version_file_common.VersionFileConfig, 0, len(c.TransitiveDeps))
@@ -89,27 +90,30 @@
childDeps = append(childDeps, dep.Child)
}
}
- parentCfg := parent.GitilesDEPSConfig{
- GitilesConfig: parent.GitilesConfig{
- BaseConfig: parent.BaseConfig{
- ChildPath: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ChildPath,
- ChildRepo: c.ChildRepo,
- IncludeBugs: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.IncludeBugs,
- IncludeLog: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.IncludeLog,
- CommitMsgTmpl: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.CommitMsgTmpl,
- MonorailProject: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.BugProject,
- },
- GitilesConfig: gitiles_common.GitilesConfig{
- Branch: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ParentBranch,
- RepoURL: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ParentRepo,
- },
- Gerrit: c.Gerrit,
+ parentCfg := parent.GitilesConfig{
+ BaseConfig: parent.BaseConfig{
+ ChildPath: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ChildPath,
+ ChildRepo: c.ChildRepo,
+ IncludeBugs: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.IncludeBugs,
+ IncludeLog: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.IncludeLog,
+ CommitMsgTmpl: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.CommitMsgTmpl,
+ MonorailProject: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.BugProject,
},
- Dep: c.ChildRepo,
- TransitiveDeps: parentDeps,
+ DependencyConfig: version_file_common.DependencyConfig{
+ VersionFileConfig: version_file_common.VersionFileConfig{
+ ID: c.ChildRepo,
+ Path: deps_parser.DepsFileName,
+ },
+ TransitiveDeps: parentDeps,
+ },
+ GitilesConfig: gitiles_common.GitilesConfig{
+ Branch: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ParentBranch,
+ RepoURL: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ParentRepo,
+ },
+ Gerrit: c.Gerrit,
}
if err := parentCfg.Validate(); err != nil {
- return parent.GitilesDEPSConfig{}, child.GitilesConfig{}, skerr.Wrapf(err, "generated parent config is invalid")
+ return parent.GitilesConfig{}, child.GitilesConfig{}, skerr.Wrapf(err, "generated parent config is invalid")
}
childCfg := child.GitilesConfig{
GitilesConfig: gitiles_common.GitilesConfig{
@@ -119,7 +123,7 @@
},
}
if err := childCfg.Validate(); err != nil {
- return parent.GitilesDEPSConfig{}, child.GitilesConfig{}, skerr.Wrapf(err, "generated child config is invalid")
+ return parent.GitilesConfig{}, child.GitilesConfig{}, skerr.Wrapf(err, "generated child config is invalid")
}
return parentCfg, childCfg, nil
}
@@ -134,7 +138,7 @@
if err != nil {
return nil, skerr.Wrap(err)
}
- parentRM, err := parent.NewGitilesDEPS(ctx, parentCfg, reg, client, serverURL)
+ parentRM, err := parent.NewGitilesFile(ctx, parentCfg, reg, client, serverURL)
if err != nil {
return nil, skerr.Wrap(err)
}
diff --git a/autoroll/go/repo_manager/parent/copy.go b/autoroll/go/repo_manager/parent/copy.go
index 8f4f3e7..1bfd74d 100644
--- a/autoroll/go/repo_manager/parent/copy.go
+++ b/autoroll/go/repo_manager/parent/copy.go
@@ -6,11 +6,9 @@
"net/http"
"os"
"path/filepath"
- "strings"
"go.skia.org/infra/autoroll/go/config_vars"
"go.skia.org/infra/autoroll/go/repo_manager/child"
- "go.skia.org/infra/autoroll/go/repo_manager/common/version_file_common"
"go.skia.org/infra/autoroll/go/revision"
"go.skia.org/infra/go/exec"
"go.skia.org/infra/go/git"
@@ -40,10 +38,6 @@
type CopyConfig struct {
GitCheckoutGerritConfig
- // VersionFile is the path of the file within the repo which contains
- // the current version of the Child.
- VersionFile string `json:"versionFile"`
-
// Copies indicates which files and directories to copy from the
// Child into the Parent.
Copies []CopyEntry `json:"copies,omitempty"`
@@ -54,9 +48,6 @@
if err := c.GitCheckoutGerritConfig.Validate(); err != nil {
return skerr.Wrap(err)
}
- if c.VersionFile == "" {
- return skerr.Fmt("VersionFile is required")
- }
if len(c.Copies) == 0 {
return skerr.Fmt("Copies are required")
}
@@ -71,10 +62,10 @@
// NewCopy returns a Parent implementation which copies the Child into itself.
// It uses a local git checkout and uploads changes to Gerrit.
func NewCopy(ctx context.Context, c CopyConfig, reg *config_vars.Registry, client *http.Client, serverURL, workdir, userName, userEmail string, dep child.Child, uploadRoll GitCheckoutUploadRollFunc) (*GitCheckoutParent, error) {
- getLastRollRev := VersionFileGetLastRollRevFunc(version_file_common.VersionFileConfig{
- ID: c.ChildRepo,
- Path: c.VersionFile,
- })
+ if err := c.Validate(); err != nil {
+ return nil, skerr.Wrap(err)
+ }
+ createRollHelper := gitCheckoutFileCreateRollFunc(c.DependencyConfig)
createRoll := func(ctx context.Context, co *git.Checkout, from *revision.Revision, to *revision.Revision, rolling []*revision.Revision, commitMsg string) (string, error) {
// Create a temporary directory.
tmp, err := ioutil.TempDir("", "")
@@ -109,24 +100,8 @@
}
// Write the new version file.
- versionFile := filepath.Join(co.Dir(), c.VersionFile)
- if err := ioutil.WriteFile(versionFile, []byte(to.Id), os.ModePerm); err != nil {
- return "", skerr.Wrap(err)
- }
- if _, err := co.Git(ctx, "add", c.VersionFile); err != nil {
- return "", skerr.Wrap(err)
- }
-
- // Commit.
- if _, err := co.Git(ctx, "commit", "-m", commitMsg); err != nil {
- return "", skerr.Wrap(err)
- }
- out, err := co.RevParse(ctx, "HEAD")
- if err != nil {
- return "", skerr.Wrap(err)
- }
- return strings.TrimSpace(out), nil
+ return createRollHelper(ctx, co, from, to, rolling, commitMsg)
}
- return NewGitCheckout(ctx, c.GitCheckoutConfig, reg, serverURL, workdir, userName, userEmail, nil, getLastRollRev, createRoll, uploadRoll)
+ return NewGitCheckout(ctx, c.GitCheckoutConfig, reg, serverURL, workdir, userName, userEmail, nil, createRoll, uploadRoll)
}
diff --git a/autoroll/go/repo_manager/parent/deps_local.go b/autoroll/go/repo_manager/parent/deps_local.go
index 4df1a43..129b02c 100644
--- a/autoroll/go/repo_manager/parent/deps_local.go
+++ b/autoroll/go/repo_manager/parent/deps_local.go
@@ -27,7 +27,6 @@
// checkout and DEPS to manage dependencies.
type DEPSLocalConfig struct {
GitCheckoutConfig
- version_file_common.DependencyConfig
// Optional fields.
@@ -120,19 +119,6 @@
return nil, skerr.Wrap(err)
}
- // See documentation for GitCheckoutGetLastRollRevFunc. This
- // implementation also performs a "gclient sync".
- getLastRollRevHelper := VersionFileGetLastRollRevFunc(version_file_common.VersionFileConfig{
- ID: c.ID,
- Path: deps_parser.DepsFileName,
- })
- getLastRollRev := func(ctx context.Context, co *git.Checkout) (string, error) {
- if err := sync(ctx); err != nil {
- return "", skerr.Wrap(err)
- }
- return getLastRollRevHelper(ctx, co)
- }
-
// See documentation for GitCheckoutCreateRollFunc.
createRollHelper := gitCheckoutFileCreateRollFunc(version_file_common.DependencyConfig{
VersionFileConfig: version_file_common.VersionFileConfig{
@@ -177,7 +163,7 @@
return nil, skerr.Wrap(err)
}
co := &git.Checkout{GitDir: git.GitDir(checkoutPath)}
- return NewGitCheckout(ctx, c.GitCheckoutConfig, reg, serverURL, workdir, userName, userEmail, co, getLastRollRev, createRoll, uploadRoll)
+ return NewGitCheckout(ctx, c.GitCheckoutConfig, reg, serverURL, workdir, userName, userEmail, co, createRoll, uploadRoll)
}
// GetDEPSCheckoutPath returns the path to the checkout within the workdir,
diff --git a/autoroll/go/repo_manager/parent/freetype.go b/autoroll/go/repo_manager/parent/freetype.go
index 286867d..d921f38 100644
--- a/autoroll/go/repo_manager/parent/freetype.go
+++ b/autoroll/go/repo_manager/parent/freetype.go
@@ -16,7 +16,6 @@
"go.skia.org/infra/autoroll/go/repo_manager/common/gitiles_common"
"go.skia.org/infra/autoroll/go/repo_manager/common/version_file_common"
"go.skia.org/infra/autoroll/go/revision"
- "go.skia.org/infra/go/depot_tools/deps_parser"
"go.skia.org/infra/go/git"
"go.skia.org/infra/go/skerr"
"go.skia.org/infra/go/util"
@@ -41,23 +40,12 @@
}
)
-func NewFreeTypeParent(ctx context.Context, c GitilesDEPSConfig, reg *config_vars.Registry, workdir string, client *http.Client, serverURL string) (*gitilesParent, error) {
- getLastRollRev := gitilesFileGetLastRollRevFunc(version_file_common.VersionFileConfig{
- ID: c.Dep,
- Path: deps_parser.DepsFileName,
- })
-
+func NewFreeTypeParent(ctx context.Context, c GitilesConfig, reg *config_vars.Registry, workdir string, client *http.Client, serverURL string) (*gitilesParent, error) {
localChildRepo, err := git.NewRepo(ctx, c.ChildRepo, workdir)
if err != nil {
return nil, err
}
- getChangesHelper := gitilesFileGetChangesForRollFunc(version_file_common.DependencyConfig{
- VersionFileConfig: version_file_common.VersionFileConfig{
- ID: c.Dep,
- Path: deps_parser.DepsFileName,
- },
- TransitiveDeps: c.TransitiveDeps,
- })
+ getChangesHelper := gitilesFileGetChangesForRollFunc(c.DependencyConfig)
getChangesForRoll := func(ctx context.Context, parentRepo *gitiles_common.GitilesRepo, baseCommit string, from, to *revision.Revision, rolling []*revision.Revision) (map[string]string, []*version_file_common.TransitiveDepUpdate, error) {
// Get the DEPS changes via gitilesDEPSGetChangesForRollFunc.
changes, transitiveDeps, err := getChangesHelper(ctx, parentRepo, baseCommit, from, to, rolling)
@@ -102,7 +90,7 @@
}
return changes, transitiveDeps, nil
}
- return newGitiles(ctx, c.GitilesConfig, reg, client, serverURL, getLastRollRev, getChangesForRoll)
+ return newGitiles(ctx, c, reg, client, serverURL, getChangesForRoll)
}
// Perform a three-way merge for this header file in a temporary dir. Adds the
diff --git a/autoroll/go/repo_manager/parent/git_checkout.go b/autoroll/go/repo_manager/parent/git_checkout.go
index 8f7b515..8b151cc 100644
--- a/autoroll/go/repo_manager/parent/git_checkout.go
+++ b/autoroll/go/repo_manager/parent/git_checkout.go
@@ -30,6 +30,7 @@
type GitCheckoutConfig struct {
BaseConfig
git_common.GitCheckoutConfig
+ version_file_common.DependencyConfig
}
// See documentation for util.Validator interface.
@@ -40,6 +41,12 @@
if err := c.GitCheckoutConfig.Validate(); err != nil {
return skerr.Wrap(err)
}
+ if err := c.DependencyConfig.Validate(); err != nil {
+ return skerr.Wrap(err)
+ }
+ if len(c.GitCheckoutConfig.Dependencies) != 0 {
+ return skerr.Fmt("Dependencies are inherited from the DependencyConfig and should not be set on the GitCheckoutConfig.")
+ }
return nil
}
@@ -48,9 +55,9 @@
type GitCheckoutParent struct {
*baseParent
*git_common.Checkout
- createRoll GitCheckoutCreateRollFunc
- getLastRollRev GitCheckoutGetLastRollRevFunc
- uploadRoll GitCheckoutUploadRollFunc
+ childID string
+ createRoll GitCheckoutCreateRollFunc
+ uploadRoll GitCheckoutUploadRollFunc
}
// GitCheckoutCreateRollFunc generates commit(s) in the local Git checkout to
@@ -62,38 +69,46 @@
// returns its ID.
type GitCheckoutUploadRollFunc func(context.Context, *git.Checkout, string, string, []string, bool, string) (int64, error)
-// GitCheckoutGetLastRollRevFunc retrieves the last-rolled revision ID from the
-// local Git checkout. GitCheckoutParent handles updating the checkout itself.
-type GitCheckoutGetLastRollRevFunc func(context.Context, *git.Checkout) (string, error)
-
// NewGitCheckout returns a base for implementations of Parent which use
// a local checkout to create changes.
-func NewGitCheckout(ctx context.Context, c GitCheckoutConfig, reg *config_vars.Registry, serverURL, workdir, userName, userEmail string, co *git.Checkout, getLastRollRev GitCheckoutGetLastRollRevFunc, createRoll GitCheckoutCreateRollFunc, uploadRoll GitCheckoutUploadRollFunc) (*GitCheckoutParent, error) {
+func NewGitCheckout(ctx context.Context, c GitCheckoutConfig, reg *config_vars.Registry, serverURL, workdir, userName, userEmail string, co *git.Checkout, createRoll GitCheckoutCreateRollFunc, uploadRoll GitCheckoutUploadRollFunc) (*GitCheckoutParent, error) {
+ if err := c.Validate(); err != nil {
+ return nil, skerr.Wrap(err)
+ }
// Create a baseParent.
base, err := newBaseParent(ctx, c.BaseConfig, serverURL)
if err != nil {
return nil, skerr.Wrap(err)
}
// Create the local checkout.
+ deps := make([]*version_file_common.VersionFileConfig, 0, len(c.DependencyConfig.TransitiveDeps)+1)
+ deps = append(deps, &c.DependencyConfig.VersionFileConfig)
+ deps = append(deps, c.DependencyConfig.TransitiveDeps...)
+ c.GitCheckoutConfig.Dependencies = deps
checkout, err := git_common.NewCheckout(ctx, c.GitCheckoutConfig, reg, workdir, userName, userEmail, co)
if err != nil {
return nil, skerr.Wrap(err)
}
return &GitCheckoutParent{
- baseParent: base,
- Checkout: checkout,
- createRoll: createRoll,
- getLastRollRev: getLastRollRev,
- uploadRoll: uploadRoll,
+ baseParent: base,
+ Checkout: checkout,
+ childID: c.DependencyConfig.ID,
+ createRoll: createRoll,
+ uploadRoll: uploadRoll,
}, nil
}
// See documentation for Parent interface.
func (p *GitCheckoutParent) Update(ctx context.Context) (string, error) {
- if _, _, err := p.Checkout.Update(ctx); err != nil {
+ rev, _, err := p.Checkout.Update(ctx)
+ if err != nil {
return "", skerr.Wrap(err)
}
- return p.getLastRollRev(ctx, p.Checkout.Checkout)
+ lastRollRev, ok := rev.Dependencies[p.childID]
+ if !ok {
+ return "", skerr.Fmt("Unable to find dependency %q in %#v", p.childID, rev)
+ }
+ return lastRollRev, nil
}
// See documentation for Parent interface.
@@ -148,34 +163,6 @@
return p.uploadRoll(ctx, p.Checkout.Checkout, upstreamBranch, hash, emails, dryRun, commitMsg)
}
-// VersionFileGetLastRollRevFunc returns a GitCheckoutGetLastRollRevFunc which
-// reads the given file path from the repo and returns its full contents as the
-// last-rolled revision ID.
-func VersionFileGetLastRollRevFunc(dep version_file_common.VersionFileConfig) GitCheckoutGetLastRollRevFunc {
- return func(ctx context.Context, co *git.Checkout) (string, error) {
- contents, err := ioutil.ReadFile(filepath.Join(co.Dir(), dep.Path))
- if err != nil {
- return "", skerr.Wrap(err)
- }
- // TODO(borenet): It's kind of weird that we're using a gitiles
- // helper function here.
- return version_file_common.GetPinnedRev(dep, string(contents))
- }
-}
-
-// gitCheckoutFileGetLastRollRevFunc returns a GitCheckoutGetLastRollRevFunc
-// which uses a local Git checkout and pins dependencies using a file checked
-// into the repo.
-func gitCheckoutFileGetLastRollRevFunc(dep version_file_common.VersionFileConfig) GitCheckoutGetLastRollRevFunc {
- return func(ctx context.Context, co *git.Checkout) (string, error) {
- contents, err := co.GetFile(ctx, dep.Path, "HEAD")
- if err != nil {
- return "", skerr.Wrap(err)
- }
- return version_file_common.GetPinnedRev(dep, contents)
- }
-}
-
// gitCheckoutFileCreateRollFunc returns a GitCheckoutCreateRollFunc which uses
// a local Git checkout and pins dependencies using a file checked into the
// repo.
diff --git a/autoroll/go/repo_manager/parent/git_checkout_github.go b/autoroll/go/repo_manager/parent/git_checkout_github.go
index 96dd765..0cc2e1e 100644
--- a/autoroll/go/repo_manager/parent/git_checkout_github.go
+++ b/autoroll/go/repo_manager/parent/git_checkout_github.go
@@ -106,7 +106,7 @@
// NewGitCheckoutGithub returns an implementation of Parent which uses a local
// git checkout and uploads pull requests to Github.
-func NewGitCheckoutGithub(ctx context.Context, c GitCheckoutGithubConfig, reg *config_vars.Registry, githubClient *github.GitHub, serverURL, workdir, userName, userEmail string, co *git.Checkout, getLastRollRev GitCheckoutGetLastRollRevFunc, createRoll GitCheckoutCreateRollFunc) (*GitCheckoutParent, error) {
+func NewGitCheckoutGithub(ctx context.Context, c GitCheckoutGithubConfig, reg *config_vars.Registry, githubClient *github.GitHub, serverURL, workdir, userName, userEmail string, co *git.Checkout, createRoll GitCheckoutCreateRollFunc) (*GitCheckoutParent, error) {
if err := c.Validate(); err != nil {
return nil, skerr.Wrap(err)
}
@@ -118,7 +118,7 @@
uploadRoll := GitCheckoutUploadGithubRollFunc(githubClient, userName, c.ForkBranchName)
// Create the GitCheckout Parent.
- p, err := NewGitCheckout(ctx, c.GitCheckoutConfig, reg, serverURL, workdir, userName, userEmail, co, getLastRollRev, createRoll, uploadRoll)
+ p, err := NewGitCheckout(ctx, c.GitCheckoutConfig, reg, serverURL, workdir, userName, userEmail, co, createRoll, uploadRoll)
if err != nil {
return nil, skerr.Wrap(err)
}
diff --git a/autoroll/go/repo_manager/parent/git_checkout_github_file.go b/autoroll/go/repo_manager/parent/git_checkout_github_file.go
index 4787198..8c29cce 100644
--- a/autoroll/go/repo_manager/parent/git_checkout_github_file.go
+++ b/autoroll/go/repo_manager/parent/git_checkout_github_file.go
@@ -7,7 +7,6 @@
"strings"
"go.skia.org/infra/autoroll/go/config_vars"
- "go.skia.org/infra/autoroll/go/repo_manager/common/version_file_common"
"go.skia.org/infra/autoroll/go/revision"
"go.skia.org/infra/go/git"
"go.skia.org/infra/go/github"
@@ -22,7 +21,6 @@
// special case.
type GitCheckoutGithubFileConfig struct {
GitCheckoutGithubConfig
- version_file_common.DependencyConfig
// Named steps to run before uploading roll CLs.
PreUploadSteps []string `json:"preUploadSteps,omitempty"`
@@ -47,8 +45,6 @@
// NewGitCheckoutGithubFile returns a Parent which uses a local checkout and a
// version file (eg. DEPS) to manage dependencies.
func NewGitCheckoutGithubFile(ctx context.Context, c GitCheckoutGithubFileConfig, reg *config_vars.Registry, client *http.Client, githubClient *github.GitHub, serverURL, workdir, userName, userEmail string, co *git.Checkout) (*GitCheckoutParent, error) {
- getLastRollRev := gitCheckoutFileGetLastRollRevFunc(c.VersionFileConfig)
-
// Pre-upload steps are run after setting the new dependency version and
// syncing, but before committing and uploading.
preUploadSteps, err := GetPreUploadSteps(c.PreUploadSteps)
@@ -88,5 +84,5 @@
}
return strings.TrimSpace(out), nil
}
- return NewGitCheckoutGithub(ctx, c.GitCheckoutGithubConfig, reg, githubClient, serverURL, workdir, userName, userEmail, co, getLastRollRev, createRoll)
+ return NewGitCheckoutGithub(ctx, c.GitCheckoutGithubConfig, reg, githubClient, serverURL, workdir, userName, userEmail, co, createRoll)
}
diff --git a/autoroll/go/repo_manager/parent/gitiles.go b/autoroll/go/repo_manager/parent/gitiles.go
index 3aef101..267371e 100644
--- a/autoroll/go/repo_manager/parent/gitiles.go
+++ b/autoroll/go/repo_manager/parent/gitiles.go
@@ -24,6 +24,7 @@
type GitilesConfig struct {
BaseConfig
gitiles_common.GitilesConfig
+ version_file_common.DependencyConfig
// Gerrit provides configuration for the Gerrit instance used for
// uploading rolls.
Gerrit *codereview.GerritConfig `json:"gerrit"`
@@ -43,6 +44,12 @@
if err := c.GitilesConfig.Validate(); err != nil {
return skerr.Wrap(err)
}
+ if err := c.DependencyConfig.Validate(); err != nil {
+ return skerr.Wrap(err)
+ }
+ if len(c.GitilesConfig.Dependencies) != 0 {
+ return skerr.Fmt("Dependencies are inherited from the DependencyConfig and should not be set on the GitilesConfig.")
+ }
return nil
}
@@ -52,20 +59,16 @@
// returns any dependencies which are being transitively rolled.
type gitilesGetChangesForRollFunc func(context.Context, *gitiles_common.GitilesRepo, string, *revision.Revision, *revision.Revision, []*revision.Revision) (map[string]string, []*version_file_common.TransitiveDepUpdate, error)
-// gitilesGetLastRollRevFunc finds the last-rolled child revision ID from the
-// repo at the given base commit.
-type gitilesGetLastRollRevFunc func(context.Context, *gitiles_common.GitilesRepo, string) (string, error)
-
// gitilesParent is a base for implementations of Parent which use Gitiles.
type gitilesParent struct {
*baseParent
*gitiles_common.GitilesRepo
+ childID string
gerrit gerrit.GerritInterface
gerritConfig *codereview.GerritConfig
serverURL string
getChangesForRoll gitilesGetChangesForRollFunc
- update gitilesGetLastRollRevFunc
// TODO(borenet): We could make this "stateless" by having Parent.Update
// also return the tip revision of the Parent and passing it through to
@@ -75,10 +78,14 @@
}
// newGitiles returns a base for implementations of Parent which use Gitiles.
-func newGitiles(ctx context.Context, c GitilesConfig, reg *config_vars.Registry, client *http.Client, serverURL string, update gitilesGetLastRollRevFunc, getChangesForRoll gitilesGetChangesForRollFunc) (*gitilesParent, error) {
+func newGitiles(ctx context.Context, c GitilesConfig, reg *config_vars.Registry, client *http.Client, serverURL string, getChangesForRoll gitilesGetChangesForRollFunc) (*gitilesParent, error) {
if err := c.Validate(); err != nil {
return nil, skerr.Wrap(err)
}
+ deps := make([]*version_file_common.VersionFileConfig, 0, len(c.DependencyConfig.TransitiveDeps)+1)
+ deps = append(deps, &c.DependencyConfig.VersionFileConfig)
+ deps = append(deps, c.DependencyConfig.TransitiveDeps...)
+ c.GitilesConfig.Dependencies = deps
gr, err := gitiles_common.NewGitilesRepo(ctx, c.GitilesConfig, reg, client)
if err != nil {
return nil, skerr.Wrap(err)
@@ -97,11 +104,11 @@
}
return &gitilesParent{
baseParent: base,
+ childID: c.DependencyConfig.ID,
GitilesRepo: gr,
gerrit: g,
gerritConfig: c.Gerrit,
getChangesForRoll: getChangesForRoll,
- update: update,
}, nil
}
@@ -112,11 +119,9 @@
if err != nil {
return "", err
}
-
- // Call the provided gitilesGetLastRollRevFunc.
- lastRollRev, err := p.update(ctx, p.GitilesRepo, baseCommit.Id)
- if err != nil {
- return "", err
+ lastRollRev, ok := baseCommit.Dependencies[p.childID]
+ if !ok {
+ return "", skerr.Fmt("Unable to find dependency %q in %#v", p.childID, lastRollRev)
}
// Save the data.
diff --git a/autoroll/go/repo_manager/parent/gitiles_file.go b/autoroll/go/repo_manager/parent/gitiles_file.go
index 9ef1c11..d78ac63 100644
--- a/autoroll/go/repo_manager/parent/gitiles_file.go
+++ b/autoroll/go/repo_manager/parent/gitiles_file.go
@@ -8,69 +8,9 @@
"go.skia.org/infra/autoroll/go/repo_manager/common/gitiles_common"
"go.skia.org/infra/autoroll/go/repo_manager/common/version_file_common"
"go.skia.org/infra/autoroll/go/revision"
- "go.skia.org/infra/go/depot_tools/deps_parser"
"go.skia.org/infra/go/skerr"
)
-// GitilesFileConfig provides configuration for a Parent which pins dependencies
-// using a file checked into the repo. The revision ID of the dependency makes
-// up the full contents of the file, unless the file is "DEPS", which is a
-// special case.
-type GitilesFileConfig struct {
- GitilesConfig
- version_file_common.DependencyConfig
-}
-
-// See documentation for util.Validator interface.
-func (c GitilesFileConfig) Validate() error {
- if err := c.GitilesConfig.Validate(); err != nil {
- return skerr.Wrap(err)
- }
- if err := c.DependencyConfig.Validate(); err != nil {
- return skerr.Wrap(err)
- }
- return nil
-}
-
-// GitilesDEPSConfig provides configuration for a Parent which pins dependencies
-// using DEPS.
-type GitilesDEPSConfig struct {
- GitilesConfig
-
- // Dep is the ID of the dependency to be rolled, eg. a repo URL.
- Dep string `json:"dep"`
-
- // TransitiveDeps is an optional mapping of dependency ID (eg. repo URL)
- // to the location within the repo where its version is pinned (eg. DEPS).
- // The Child must have a dependencies with the same IDs. When rolling
- // the Child to a new revision, these will be updated to match the
- // versions which are pinned by the Child at the target revision.
- TransitiveDeps []*version_file_common.VersionFileConfig `json:"transitiveDeps,omitempty"`
-}
-
-// See documentation for util.Validator interface.
-func (c GitilesDEPSConfig) Validate() error {
- if err := c.GitilesConfig.Validate(); err != nil {
- return skerr.Wrap(err)
- }
- if c.Dep == "" {
- return skerr.Fmt("Dep is required")
- }
- return nil
-}
-
-// gitilesFileGetLastRollRevFunc returns a gitilesGetLastRollRevFunc which reads
-// the given file to find the last-rolled revision.
-func gitilesFileGetLastRollRevFunc(dep version_file_common.VersionFileConfig) gitilesGetLastRollRevFunc {
- return func(ctx context.Context, repo *gitiles_common.GitilesRepo, baseCommit string) (string, error) {
- contents, err := repo.GetFile(ctx, dep.Path, baseCommit)
- if err != nil {
- return "", skerr.Wrap(err)
- }
- return version_file_common.GetPinnedRev(dep, contents)
- }
-}
-
// gitilesFileGetChangesForRollFunc returns a gitilesGetChangesForRollFunc which
// update the given file.
func gitilesFileGetChangesForRollFunc(dep version_file_common.DependencyConfig) gitilesGetChangesForRollFunc {
@@ -84,26 +24,10 @@
// NewGitilesFile returns a Parent implementation which uses Gitiles to roll
// a dependency.
-func NewGitilesFile(ctx context.Context, c GitilesFileConfig, reg *config_vars.Registry, client *http.Client, serverURL string) (*gitilesParent, error) {
+func NewGitilesFile(ctx context.Context, c GitilesConfig, reg *config_vars.Registry, client *http.Client, serverURL string) (*gitilesParent, error) {
if err := c.Validate(); err != nil {
return nil, skerr.Wrap(err)
}
- getLastRollRev := gitilesFileGetLastRollRevFunc(c.VersionFileConfig)
getChangesForRoll := gitilesFileGetChangesForRollFunc(c.DependencyConfig)
- return newGitiles(ctx, c.GitilesConfig, reg, client, serverURL, getLastRollRev, getChangesForRoll)
-}
-
-// NewGitilesDEPS returns a Parent implementation which uses Gitiles to roll
-// DEPS.
-func NewGitilesDEPS(ctx context.Context, c GitilesDEPSConfig, reg *config_vars.Registry, client *http.Client, serverURL string) (*gitilesParent, error) {
- return NewGitilesFile(ctx, GitilesFileConfig{
- GitilesConfig: c.GitilesConfig,
- DependencyConfig: version_file_common.DependencyConfig{
- VersionFileConfig: version_file_common.VersionFileConfig{
- ID: c.Dep,
- Path: deps_parser.DepsFileName,
- },
- TransitiveDeps: c.TransitiveDeps,
- },
- }, reg, client, serverURL)
+ return newGitiles(ctx, c, reg, client, serverURL, getChangesForRoll)
}
diff --git a/autoroll/go/repo_manager/semver_gcs_repo_manager.go b/autoroll/go/repo_manager/semver_gcs_repo_manager.go
index 03c8938..8cae925 100644
--- a/autoroll/go/repo_manager/semver_gcs_repo_manager.go
+++ b/autoroll/go/repo_manager/semver_gcs_repo_manager.go
@@ -58,26 +58,24 @@
}
// splitParentChild splits the SemVerGCSRepoManagerConfig into a
-// parent.GitilesFileConfig and a child.SemVerGCSConfig.
+// parent.GitilesConfig and a child.SemVerGCSConfig.
// TODO(borenet): Update the config format to directly define the parent
// and child. We shouldn't need most of the New.*RepoManager functions.
-func (c SemVerGCSRepoManagerConfig) splitParentChild() (parent.GitilesFileConfig, child.SemVerGCSConfig, error) {
- parentCfg := parent.GitilesFileConfig{
- GitilesConfig: parent.GitilesConfig{
- BaseConfig: parent.BaseConfig{
- ChildPath: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ChildPath,
- ChildRepo: c.GCSPath, // TODO
- IncludeBugs: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.IncludeBugs,
- IncludeLog: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.IncludeLog,
- CommitMsgTmpl: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.CommitMsgTmpl,
- MonorailProject: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.BugProject,
- },
- GitilesConfig: gitiles_common.GitilesConfig{
- Branch: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ParentBranch,
- RepoURL: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ParentRepo,
- },
- Gerrit: c.Gerrit,
+func (c SemVerGCSRepoManagerConfig) splitParentChild() (parent.GitilesConfig, child.SemVerGCSConfig, error) {
+ parentCfg := parent.GitilesConfig{
+ BaseConfig: parent.BaseConfig{
+ ChildPath: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ChildPath,
+ ChildRepo: c.GCSPath, // TODO
+ IncludeBugs: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.IncludeBugs,
+ IncludeLog: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.IncludeLog,
+ CommitMsgTmpl: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.CommitMsgTmpl,
+ MonorailProject: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.BugProject,
},
+ GitilesConfig: gitiles_common.GitilesConfig{
+ Branch: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ParentBranch,
+ RepoURL: c.NoCheckoutRepoManagerConfig.CommonRepoManagerConfig.ParentRepo,
+ },
+ Gerrit: c.Gerrit,
DependencyConfig: version_file_common.DependencyConfig{
VersionFileConfig: version_file_common.VersionFileConfig{
ID: c.GCSPath, // TODO
@@ -86,7 +84,7 @@
},
}
if err := parentCfg.Validate(); err != nil {
- return parent.GitilesFileConfig{}, child.SemVerGCSConfig{}, skerr.Wrapf(err, "generated parent config is invalid")
+ return parent.GitilesConfig{}, child.SemVerGCSConfig{}, skerr.Wrapf(err, "generated parent config is invalid")
}
childCfg := child.SemVerGCSConfig{
GCSConfig: child.GCSConfig{
@@ -97,7 +95,7 @@
VersionRegex: c.VersionRegex,
}
if err := childCfg.Validate(); err != nil {
- return parent.GitilesFileConfig{}, child.SemVerGCSConfig{}, skerr.Wrapf(err, "generated child config is invalid")
+ return parent.GitilesConfig{}, child.SemVerGCSConfig{}, skerr.Wrapf(err, "generated child config is invalid")
}
return parentCfg, childCfg, nil
}
