|
|
Created:
4 years, 1 month ago by Dirk Pranke Modified:
3 years, 6 months ago CC:
chromium-reviews, dpranke+depot_tools_chromium.org, iannucci+depot_tools_chromium.org Target Ref:
refs/heads/master Project:
depot_tools Visibility:
Public. |
DescriptionImplement a first pass of `gclient flatten`.
Currently when we do official Chrome releases we take a .gclient and
corresponding set of DEPS files and hand-parse them to produce a
single "flattened" DEPS file (called a buildspec) that contains no
recursion and that can be used to recreate a branch or a release "exactly".
This CL adds that functionality to gclient so that we can reuse the
parsing logic, which will be needed as we implement support for conditionals
and other such things in crbug.com/570091.
R=mmoss@chromium.org, dimu@chromium.org, agable@chromium.org, iannucci@chromium.org
BUG=661382
Patch Set 1 #
Total comments: 5
Messages
Total messages: 7 (1 generated)
https://codereview.chromium.org/2474543002/diff/1/gclient.py File gclient.py (right): https://codereview.chromium.org/2474543002/diff/1/gclient.py#newcode1664 gclient.py:1664: 'specific revision.') And that the .gclient_entries file matches what's actually on the filesystem, maybe? https://codereview.chromium.org/2474543002/diff/1/gclient.py#newcode1735 gclient.py:1735: s += " {\n" Make this a triple-quoted multi-line template string rather than doing lots of string concatenation.
https://codereview.chromium.org/2474543002/diff/1/gclient.py File gclient.py (right): https://codereview.chromium.org/2474543002/diff/1/gclient.py#newcode1664 gclient.py:1664: 'specific revision.') On 2016/11/02 17:53:26, agable wrote: > And that the .gclient_entries file matches what's actually on the filesystem, > maybe? I'm not sure how useful that is, but I'm not actually sure what all .gclient_entries is even used for? https://codereview.chromium.org/2474543002/diff/1/gclient.py#newcode1735 gclient.py:1735: s += " {\n" On 2016/11/02 17:53:26, agable wrote: > Make this a triple-quoted multi-line template string rather than doing lots of > string concatenation. I originally didn't do this because I thought handling the conditionals would make this harder to read. But I'll code up something and we can compare.
I'm pretty sure the only purpose of .gclient_entries these days is to speed up execution of "gclient status". On Wed, Nov 2, 2016 at 12:01 PM <dpranke@chromium.org> wrote: > Reviewers: agable, dimu1, iannucci, Michael Moss > CL: https://codereview.chromium.org/2474543002/ > > > > https://codereview.chromium.org/2474543002/diff/1/gclient.py > File gclient.py (right): > > https://codereview.chromium.org/2474543002/diff/1/gclient.py#newcode1664 > gclient.py:1664: 'specific revision.') > On 2016/11/02 17:53:26, agable wrote: > > And that the .gclient_entries file matches what's actually on the > filesystem, > > maybe? > > I'm not sure how useful that is, but I'm not actually sure what all > .gclient_entries is even used for? > > > https://codereview.chromium.org/2474543002/diff/1/gclient.py#newcode1735 > gclient.py:1735: s += " {\n" > On 2016/11/02 17:53:26, agable wrote: > > Make this a triple-quoted multi-line template string rather than doing > lots of > > string concatenation. > > I originally didn't do this because I thought handling the conditionals > would make this harder to read. But I'll code up something and we can > compare. > > Description: > Implement a first pass of `gclient flatten`. > > Currently when we do official Chrome releases we take a .gclient and > corresponding set of DEPS files and hand-parse them to produce a > single "flattened" DEPS file (called a buildspec) that contains no > recursion and that can be used to recreate a branch or a release "exactly". > > This CL adds that functionality to gclient so that we can reuse the > parsing logic, which will be needed as we implement support for > conditionals > and other such things in crbug.com/570091. > > R=mmoss@chromium.org, dimu@chromium.org, agable@chromium.org, > iannucci@chromium.org > BUG=661382 > > Affected files (+93, -1 lines): > M gclient.py > > > Index: gclient.py > diff --git a/gclient.py b/gclient.py > index > 3e8a7962290a3b63832adc2ad3469ae7ad2d6a53..6676a46d3d2cedc14de3fbffc9aa60cfcb1a57ec > 100755 > --- a/gclient.py > +++ b/gclient.py > @@ -761,7 +761,7 @@ class Dependency(gclient_utils.WorkItem, > DependencySettings): > # When running runhooks, there's no need to consult the SCM. > # All known hooks are expected to run unconditionally regardless of working > # copy state, so skip the SCM status check. > - run_scm = command not in ('runhooks', 'recurse', None) > + run_scm = command not in ('flatten', 'runhooks', 'recurse', None) > parsed_url = self.LateOverride(self.url) > file_list = [] if not options.nohooks else None > revision_override = revision_overrides.pop(self.name, None) > @@ -1655,6 +1655,98 @@ def CMDrecurse(parser, args): > progress=not options.no_progress) > > > +@subcommand.usage('[command] [--verify] [args ...]') > +def CMDflatten(parser, args): > + """Flattens the solutions into a single DEPS file.""" > + # Stop parsing at the first non-arg so that these go through to the > command > + parser.add_option('--verify', action='store_true', > + help='Verifies that each dependencies is pinned at a ' > + 'specific revision.') > + options, args = parser.parse_args(args) > + root_and_entries = gclient_utils.GetGClientRootAndEntries() > + if not root_and_entries: > + print( > + 'You need to run gclient sync at least once to use \'flatten\'.\n' > + 'This is because .gclient_entries needs to exist and be up to date.', > + file=sys.stderr) > + return 1 > + > + options.nohooks = True > + client = GClient.LoadCurrentConfig(options) > + res = client.RunOnDeps('flatten', args, ignore_requirements=True, > + progress=False) > + if res: > + return res > + > + deps = {} > + hooks = [] > + unpinned_deps = {} > + > + for solution in client.dependencies: > + _FlattenSolution(solution, deps, hooks, unpinned_deps) > + > + if options.verify and unpinned_deps: > + for name in unpinned_deps: > + print("'%s' is not pinned to a Git revision" % name) > + return 1 > + > + flattened_deps = ('%s\n%s\n' % > + (_DepsToVarString(deps), _HooksToVarString(hooks))) > + print(flattened_deps) > + return 0 > + > + > +def _FlattenSolution(solution, deps, hooks, unpinned_deps): > + _AddDep(solution, deps, unpinned_deps) > + for dep in solution.dependencies: > + _FlattenDep(dep, deps, hooks, unpinned_deps) > + hooks.extend(solution.deps_hooks) > + > + > +def _FlattenDep(dep, deps, hooks, unpinned_deps): > + _AddDep(dep, deps, unpinned_deps) > + for recurse_dep_name in sorted(dep.recursedeps or []): > + for sub_dep in dep.dependencies: > + if recurse_dep_name == dep.name: > + _FlattenDep(sub_dep, deps, hooks, unpinned_deps) > + break > + hooks.extend(dep.deps_hooks) > + > + > +def _AddDep(dep, deps, unpinned_deps): > + assert dep.name not in deps > + deps[dep.name] = dep.url > + _, revision = gclient_utils.SplitUrlRevision(dep.url) > + if not revision or not gclient_utils.IsGitSha(revision): > + unpinned_deps[dep.name] = dep.url > + > + > +def _DepsToVarString(deps): > + s = "deps = {\n" > + for name in sorted(deps): > + s += " '%s': '%s',\n" % (name, deps[name]) > + s += "}\n" > + return s > + > + > +def _HooksToVarString(hooks): > + s = "hooks = [\n" > + for hook in hooks: > + s += " {\n" > + if 'name' in hook: > + s += " 'name': '%s',\n" % hook['name'] > + if 'pattern' in hook: > + s += " 'pattern': '%s',\n" % hook['pattern'] > + s += " 'action': [\n" > + s += " " > + s += ",\n ".join("'%s'" % arg for arg in hook['action']) > + s += ",\n" > + s += " ],\n" > + s += " },\n" > + s += "}" > + return s > + > + > @subcommand.usage('[args ...]') > def CMDfetch(parser, args): > """Fetches upstream commits for all modules. > > > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
https://codereview.chromium.org/2474543002/diff/1/gclient.py File gclient.py (right): https://codereview.chromium.org/2474543002/diff/1/gclient.py#newcode1664 gclient.py:1664: 'specific revision.') On 2016/11/02 17:53:26, agable wrote: > And that the .gclient_entries file matches what's actually on the filesystem, > maybe? Assuming --verify ends up using gitiles requests, it wouldn't necessarily work as the same command to verify a checkout, but we could have two types of verification. Maybe --validate (syntax and hashes) and --fsck (with it's implied meaning of checking stuff on disk), or something like that.
On 2016/11/02 20:50:30, Michael Moss wrote: > https://codereview.chromium.org/2474543002/diff/1/gclient.py > File gclient.py (right): > > https://codereview.chromium.org/2474543002/diff/1/gclient.py#newcode1664 > gclient.py:1664: 'specific revision.') > On 2016/11/02 17:53:26, agable wrote: > > And that the .gclient_entries file matches what's actually on the filesystem, > > maybe? > > Assuming --verify ends up using gitiles requests, it wouldn't necessarily work > as the same command to verify a checkout, but we could have two types of > verification. Maybe --validate (syntax and hashes) and --fsck (with it's implied > meaning of checking stuff on disk), or something like that. I'm not sure if --fsck is really all that useful (or, at least, different from `gclient status` (or, if it is, we should probably fix status so that it isn't).
On 2016/11/03 00:20:08, Dirk Pranke wrote: > On 2016/11/02 20:50:30, Michael Moss wrote: > > https://codereview.chromium.org/2474543002/diff/1/gclient.py > > File gclient.py (right): > > > > https://codereview.chromium.org/2474543002/diff/1/gclient.py#newcode1664 > > gclient.py:1664: 'specific revision.') > > On 2016/11/02 17:53:26, agable wrote: > > > And that the .gclient_entries file matches what's actually on the > filesystem, > > > maybe? > > > > Assuming --verify ends up using gitiles requests, it wouldn't necessarily work > > as the same command to verify a checkout, but we could have two types of > > verification. Maybe --validate (syntax and hashes) and --fsck (with it's > implied > > meaning of checking stuff on disk), or something like that. > > I'm not sure if --fsck is really all that useful (or, at least, different from > `gclient status` > (or, if it is, we should probably fix status so that it isn't). Yeah, I was just thinking in the context of the previous comments. I agree 'status' would be a better place to handle that.
Description was changed from ========== Implement a first pass of `gclient flatten`. Currently when we do official Chrome releases we take a .gclient and corresponding set of DEPS files and hand-parse them to produce a single "flattened" DEPS file (called a buildspec) that contains no recursion and that can be used to recreate a branch or a release "exactly". This CL adds that functionality to gclient so that we can reuse the parsing logic, which will be needed as we implement support for conditionals and other such things in crbug.com/570091. R=mmoss@chromium.org, dimu@chromium.org, agable@chromium.org, iannucci@chromium.org BUG=661382 ========== to ========== Implement a first pass of `gclient flatten`. Currently when we do official Chrome releases we take a .gclient and corresponding set of DEPS files and hand-parse them to produce a single "flattened" DEPS file (called a buildspec) that contains no recursion and that can be used to recreate a branch or a release "exactly". This CL adds that functionality to gclient so that we can reuse the parsing logic, which will be needed as we implement support for conditionals and other such things in crbug.com/570091. R=mmoss@chromium.org, dimu@chromium.org, agable@chromium.org, iannucci@chromium.org BUG=661382 ========== |