[recipes.py] Move fetch, lint and bundle parsers to separate modules.

recipes.py

 #!/usr/bin/env python
 # Copyright 2015 The LUCI Authors. All rights reserved.
 # Use of this source code is governed under the Apache License, Version 2.0
 # that can be found in the LICENSE file.
 
 """Tool to interact with recipe repositories.
 
 This tool operates on the nearest ancestor directory containing an
 infra/config/recipes.cfg.
 """
@@ skipping 41 matching lines @@
   universe_view = loader.UniverseView(universe, package_deps.root_package)
 
   # Prevent flakiness caused by stale pyc files.
   package.cleanup_pyc(package_deps.root_package.recipes_dir)
 
   return test.main(
       universe_view, raw_args=args.args,
       engine_flags=args.operational_args.engine_flags)
 
 
-def lint(config_file, package_deps, args):
-  from recipe_engine import lint_test
-  from recipe_engine import loader
-
-  universe = loader.RecipeUniverse(package_deps, config_file)
-  universe_view = loader.UniverseView(universe, package_deps.root_package)
-
-  lint_test.main(universe_view, args.whitelist or [])
-
-
-def bundle(config_file, package_deps, args):
-  from recipe_engine import bundle
-  from recipe_engine import loader
-
-  universe = loader.RecipeUniverse(package_deps, config_file)
-
-  bundle.main(package_deps.root_package, universe, args.destination)
-
-
 def handle_recipe_return(recipe_result, result_filename, stream_engine,
                          engine_flags):
   if engine_flags and engine_flags.use_result_proto:
     return new_handle_recipe_return(
         recipe_result, result_filename, stream_engine)
 
   if 'recipe_result' in recipe_result.result:
     result_string = json.dumps(
         recipe_result.result['recipe_result'], indent=2)
     if result_filename:
@@ skipping 71 matching lines @@
 
   def get_properties_from_operational_args(op_args):
     if not op_args.properties.property:
       return None
     return _op_properties_to_dict(op_args.properties.property)
 
   op_args = args.operational_args
   op_properties = get_properties_from_operational_args(op_args)
   if args.properties and op_properties:
     raise ValueError(
-      "Got operational args properties as well as CLI properties.")
+      'Got operational args properties as well as CLI properties.')
 
   properties = op_properties
   if not properties:
     properties = args.properties
 
   properties['recipe'] = args.recipe
 
   properties = recipe_util.strip_unicode(properties)
 
   os.environ['PYTHONUNBUFFERED'] = '1'
@@ skipping 69 matching lines @@
     print >> sys.stderr, '--verbose-json passed without --output-json'
     return 1
 
   return autoroll.main(args, repo_root, config_file)
 
 
 class ProjectOverrideAction(argparse.Action):
   def __call__(self, parser, namespace, values, option_string=None):
     p = values.split('=', 2)
     if len(p) != 2:
-      raise ValueError("Override must have the form: repo=path")
+      raise ValueError('Override must have the form: repo=path')
     project_id, path = p
 
     v = getattr(namespace, self.dest, None)
     if v is None:
       v = {}
       setattr(namespace, self.dest, v)
 
     if v.get(project_id):
-      raise ValueError("An override is already defined for [%s] (%s)" % (
+      raise ValueError('An override is already defined for [%s] (%s)' % (
                        project_id, v[project_id]))
     path = os.path.abspath(os.path.expanduser(path))
     if not os.path.isdir(path):
-      raise ValueError("Override path [%s] is not a directory" % (path,))
+      raise ValueError('Override path [%s] is not a directory' % (path,))
     v[project_id] = path
 
 
 def depgraph(config_file, package_deps, args):
   from recipe_engine import depgraph
   from recipe_engine import loader
 
   universe = loader.RecipeUniverse(package_deps, config_file)
 
   depgraph.main(universe, package_deps.root_package,
@@ skipping 34 matching lines @@
     'list': lambda prop: [_op_property_value(v) for v in prop.list.property],
 }
 
 def _op_property_value(prop):
   """Returns the Python-converted value of an arguments_pb2.Property.
 
   Args:
     prop (arguments_pb2.Property): property to convert.
   Returns: The converted value.
   Raises:
-    ValueError: If "prop" is incomplete or invalid.
+    ValueError: If 'prop' is incomplete or invalid.
   """
   typ = prop.WhichOneof('value')
   conv = _OP_PROPERTY_CONV.get(typ)
   if not conv:
     raise ValueError('Unknown property field [%s]' % (typ,))
   return conv(prop)
 
 
 def _op_properties_to_dict(pmap):
   """Creates a properties dictionary from an arguments_pb2.PropertyMap entry.
 
   Args:
     pmap (arguments_pb2.PropertyMap): Map to convert to dictionary form.
-  Returns (dict): A dictionary derived from the properties in "pmap".
+  Returns (dict): A dictionary derived from the properties in 'pmap'.
   """
   return dict((k, _op_property_value(pmap[k])) for k in pmap)
 
 
 def add_common_args(parser):
   from recipe_engine import package_io
 
   def package_type(value):
     if not os.path.isfile(value):
       raise argparse.ArgumentTypeError(
@@ skipping 15 matching lines @@
       '--verbose', '-v', action='count',
       help='Increase logging verboisty')
   # TODO(phajdan.jr): Figure out if we need --no-fetch; remove if not.
   parser.add_argument(
       '--no-fetch', action='store_true',
       help='Disable automatic fetching')
   parser.add_argument('-O', '--project-override', metavar='ID=PATH',
       action=ProjectOverrideAction,
       help='Override a project repository path with a local one.')
   parser.add_argument(
-      # Use "None" as default so that we can recognize when none of the
+      # Use 'None' as default so that we can recognize when none of the
       # bootstrap options were passed.
       '--use-bootstrap', action='store_true', default=None,
       help='Use bootstrap/bootstrap.py to create a isolated python virtualenv'
            ' with required python dependencies.')
   parser.add_argument(
       '--disable-bootstrap', action='store_false', dest='use_bootstrap',
       help='Disables bootstrap (see --use-bootstrap)')
 
   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.add_argument(
       '--operational-args-path',
       dest='operational_args',
       type=operational_args_type,
       help='The path to an operational Arguments file. If provided, this file '
            'must contain a JSONPB-encoded Arguments protobuf message, and will '
            '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
 
-def post_process_common_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 not args.package:
-    parser.error('%s requires --package' % args.command)
+  return post_process_args
 
 
 def main():
   parser = argparse.ArgumentParser(
       description='Interact with the recipe system.')
 
-  add_common_args(parser)
+  common_postprocess_func = add_common_args(parser)
+
+  from recipe_engine import fetch, lint_test, bundle
+  to_add = [fetch, lint_test, bundle]
 
   subp = parser.add_subparsers()
-
-  fetch_p = subp.add_parser(
-      'fetch',
-      description='Fetch and update dependencies.')
-  fetch_p.set_defaults(command='fetch')
+  for module in to_add:
+    module.add_subparser(subp)
 
   test_p = subp.add_parser(
     'test',
     description='Generate or check expectations by simulation')
   test_p.set_defaults(command='test')
   test_p.add_argument('args', nargs=argparse.REMAINDER)
 
-  lint_p = subp.add_parser(
-      'lint',
-      description='Check recipes for stylistic and hygenic issues')
-  lint_p.set_defaults(command='lint')
-
-  lint_p.add_argument(
-      '--whitelist', '-w', action='append',
-      help='A regexp matching module names to add to the default whitelist. '
-           'Use multiple times to add multiple patterns,')
-
-  bundle_p = subp.add_parser(
-      'bundle',
-      description=(
-        'Create a hermetically runnable recipe bundle. This captures the result'
-        ' of all network operations the recipe_engine might normally do to'
-        ' bootstrap itself.'))
-  bundle_p.set_defaults(command='bundle')
-  bundle_p.add_argument(
-      '--destination', default='./bundle',
-      type=os.path.abspath,
-      help='The directory of where to put the bundle (default: %(default)r).')
 
   def properties_file_type(filename):
     with (sys.stdin if filename == '-' else open(filename)) as f:
       obj = json.load(f)
       if not isinstance(obj, dict):
         raise argparse.ArgumentTypeError(
-          "must contain a JSON object, i.e. `{}`.")
+          'must contain a JSON object, i.e. `{}`.')
       return obj
 
   def parse_prop(prop):
     key, val = prop.split('=', 1)
     try:
       val = json.loads(val)
     except (ValueError, SyntaxError):
       pass  # If a value couldn't be evaluated, keep the string version
     return {key: val}
 
   def properties_type(value):
     obj = json.loads(value)
     if not isinstance(obj, dict):
-      raise argparse.ArgumentTypeError("must contain a JSON object, i.e. `{}`.")
+      raise argparse.ArgumentTypeError('must contain a JSON object, i.e. `{}`.')
     return obj
 
   run_p = subp.add_parser(
       'run',
       description='Run a recipe locally')
   run_p.set_defaults(command='run', properties={})
 
   run_p.add_argument(
       '--workdir',
       type=os.path.abspath,
@@ skipping 30 matching lines @@
       'recipe',
       help='The recipe to execute')
   run_p.add_argument(
       'props',
       nargs=argparse.REMAINDER,
       type=parse_prop,
       help='A list of property pairs; e.g. mastername=chromium.linux '
            '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=(
@@ skipping 61 matching lines @@
       'doc',
       description='List all known modules reachable from the current package, '
           'with their documentation')
   doc_p.add_argument('recipe', nargs='?',
                      help='Restrict documentation to this recipe')
   doc_p.add_argument('--kind', default='jsonpb', choices=doc_kinds,
                      help='Output this kind of documentation')
   doc_p.set_defaults(command='doc')
 
   args = parser.parse_args()
-  post_process_common_args(parser, args)
+  common_postprocess_func(parser, args)
+  if hasattr(args, 'postprocess_func'):

[dnj, 2017/04/27 16:33:53]

ew ... maybe ABC / classes are actually the way to go here? :( Leaving it up to
you.

+    args.postprocess_func(parser, args)
 
   # TODO(iannucci): We should always do logging.basicConfig() (probably with
   # logging.WARNING), even if no verbose is passed. However we need to be
   # careful as this could cause issues with spurious/unexpected output. I think
   # it's risky enough to do in a different CL.
 
   if args.verbose > 0:
     logging.basicConfig()
     logging.getLogger().setLevel(logging.INFO)
   if args.verbose > 1:
@@ skipping 93 matching lines @@
     # to automatically find a consistent state, rather than bailing out.
     # Especially that only some subcommands refer to package_deps.
     package_deps = package.PackageDeps.create(
         repo_root, config_file, allow_fetch=not args.no_fetch,
         deps_path=args.deps_path, overrides=args.project_override)
   except subprocess.CalledProcessError:
     # A git checkout failed somewhere. Return 2, which is the sign that this is
     # an infra failure, rather than a test failure.
     return 2
 
-  if args.command == 'fetch':
-    # We already did everything in the create() call above.
-    assert not args.no_fetch, 'Fetch? No-fetch? Make up your mind!'
-    return 0
-  elif args.command == 'test':
+  if hasattr(args, 'func'):
+    return args.func(package_deps, args)
+
+  if args.command == 'test':
     return test(config_file, package_deps, args)
-  elif args.command == 'bundle':
-    return bundle(config_file, package_deps, args)
-  elif args.command == 'lint':
-    return lint(config_file, package_deps, args)
   elif args.command == 'run':
     return run(config_file, package_deps, args)
   elif args.command == 'autoroll':
     return autoroll(repo_root, config_file, args)
   elif args.command == 'depgraph':
     return depgraph(config_file, package_deps, args)
   elif args.command == 'refs':
     return refs(config_file, package_deps, args)
   elif args.command == 'doc':
     return doc(config_file, package_deps, args)
@@ skipping 29 matching lines @@
 
   if not isinstance(ret, int):
     if ret is None:
       ret = 0
     else:
       print >> sys.stderr, ret
       ret = 1
   sys.stdout.flush()
   sys.stderr.flush()
   os._exit(ret)