From 0b1247f8506739b0c6d6adc1f8145cb4d0d3cda8 Mon Sep 17 00:00:00 2001 From: iannucci Date: Fri, 28 Apr 2017 23:33:19 -0700 Subject: [PATCH] [recipes.py] move remote arg parsing to its module R=dnj@chromium.org, phajdan.jr@chromium.org BUG= Review-Url: https://codereview.chromium.org/2839353003 --- recipe_engine/remote.py | 34 ++++++++++++++++++++- recipes.py | 65 +++++++++++----------------------------- unittests/remote_test.py | 2 +- 3 files changed, 52 insertions(+), 49 deletions(-) diff --git a/recipe_engine/remote.py b/recipe_engine/remote.py index 790d5557b..e2a2e6e7b 100644 --- a/recipe_engine/remote.py +++ b/recipe_engine/remote.py @@ -32,7 +32,39 @@ def ensure_workdir(args): shutil.rmtree(args.workdir, ignore_errors=True) -def main(args): +def add_subparser(parser): + remote_p = parser.add_parser( + 'remote', + description='Invoke a recipe command from specified repo and revision') + remote_p.add_argument( + '--repository', required=True, + help='URL of a git repository to fetch') + remote_p.add_argument( + '--revision', + help=( + 'Git commit hash to check out; defaults to latest revision on master' + ' (refs/heads/master)' + )) + remote_p.add_argument( + '--workdir', + type=os.path.abspath, + help='The working directory of repo checkout') + remote_p.add_argument( + '--use-gitiles', action='store_true', + help='Use Gitiles-specific way to fetch repo (potentially cheaper for ' + 'large repos)') + remote_p.add_argument( + 'remote_args', nargs='*', + help='Arguments to pass to fetched repo\'s recipes.py') + + remote_p.set_defaults( + command='remote', + bare_command=True, + func=main, + ) + + +def main(_package_deps, args): with ensure_workdir(args): checkout_dir = os.path.join(args.workdir, 'checkout') revision = args.revision or 'refs/heads/master' diff --git a/recipes.py b/recipes.py index 9a44b5d37..eefff9392 100755 --- a/recipes.py +++ b/recipes.py @@ -220,12 +220,6 @@ def build_annotation_stream_engine(): ret, args.output_result_json, stream_engine, engine_flags) -def remote(args): - from recipe_engine import remote - - return remote.main(args) - - class ProjectOverrideAction(argparse.Action): def __call__(self, parser, namespace, values, option_string=None): p = values.split('=', 2) @@ -355,7 +349,10 @@ def operational_args_type(value): with open(value) as fd: return jsonpb.ParseDict(json.load(fd), arguments_pb2.Arguments()) - parser.set_defaults(operational_args=arguments_pb2.Arguments()) + parser.set_defaults( + operational_args=arguments_pb2.Arguments(), + bare_command=False, # don't call postprocess_func, don't use package_deps + ) parser.add_argument( '--operational-args-path', @@ -366,14 +363,16 @@ def operational_args_type(value): 'be integrated into the runtime parameters.') def post_process_args(parser, args): - if args.command == 'remote': - # TODO(iannucci): this is a hack; remote doesn't behave like ANY other - # commands. A way to solve this will be to allow --package to take - # a remote repo and then simply remove the remote subcommand entirely. - return - - if not args.package: - parser.error('%s requires --package' % args.command) + if args.bare_command: + # TODO(iannucci): this is gross, and only for the remote subcommand; + # remote doesn't behave like ANY other commands. A way to solve this will + # be to allow --package to take a remote repo and then simply remove the + # remote subcommand entirely. + if args.package is not None: + parser.error('%s forbids --package' % args.command) + else: + if not args.package: + parser.error('%s requires --package' % args.command) return post_process_args @@ -385,7 +384,8 @@ def main(): common_postprocess_func = add_common_args(parser) from recipe_engine import fetch, lint_test, bundle, depgraph, autoroll - to_add = [fetch, lint_test, bundle, depgraph, autoroll] + from recipe_engine import remote + to_add = [fetch, lint_test, bundle, depgraph, autoroll, remote] subp = parser.add_subparsers() for module in to_add: @@ -468,33 +468,6 @@ def properties_type(value): 'issue=12345. The property value will be decoded as JSON, but if ' 'this decoding fails the value will be interpreted as a string.') - - remote_p = subp.add_parser( - 'remote', - description='Invoke a recipe command from specified repo and revision') - remote_p.set_defaults(command='remote') - remote_p.add_argument( - '--repository', required=True, - help='URL of a git repository to fetch') - remote_p.add_argument( - '--revision', - help=( - 'Git commit hash to check out; defaults to latest revision on master' - ' (refs/heads/master)' - )) - remote_p.add_argument( - '--workdir', - type=os.path.abspath, - help='The working directory of repo checkout') - remote_p.add_argument( - '--use-gitiles', action='store_true', - help='Use Gitiles-specific way to fetch repo (potentially cheaper for ' - 'large repos)') - remote_p.add_argument( - 'remote_args', nargs='*', - help='Arguments to pass to fetched repo\'s recipes.py') - - refs_p = subp.add_parser( 'refs', description='List places referencing given recipe module(s).') @@ -568,10 +541,8 @@ def properties_type(value): def _real_main(args): from recipe_engine import package - # Commands which do not require config_file, package_deps, and other objects - # initialized later. - if args.command == 'remote': - return remote(args) + if args.bare_command: + return args.func(None, args) config_file = args.package repo_root = package.InfraRepoConfig().from_recipes_cfg(args.package.path) diff --git a/unittests/remote_test.py b/unittests/remote_test.py index 7d1fb30b4..0f46090d1 100755 --- a/unittests/remote_test.py +++ b/unittests/remote_test.py @@ -25,7 +25,7 @@ def test_run(self): 'a_recipe', ], stderr=subprocess.STDOUT) except subprocess.CalledProcessError as ex: - print >> sys.stdout, ex.message + print >> sys.stdout, ex.message, ex.output raise