Pass args in file from task_runner to run_isolated

client/run_isolated.py

 #!/usr/bin/env python
 # Copyright 2012 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.
 
 """Runs a command with optional isolated input/output.
 
 Despite name "run_isolated", can run a generic non-isolated command specified as
 args.
 
 If input isolated hash is provided, fetches it, creates a tree of hard links,
 appends args to the command in the fetched isolated and runs it.
 To improve performance, keeps a local cache.
 The local cache can safely be deleted.
 
 Any ${EXECUTABLE_SUFFIX} on the command line will be replaced with ".exe" string
 on Windows and "" on other platforms.
 
 Any ${ISOLATED_OUTDIR} on the command line will be replaced by the location of a
 temporary directory upon execution of the command specified in the .isolated
 file. All content written to this directory will be uploaded upon termination
 and the .isolated file describing this directory will be printed to stdout.
 
 Any ${SWARMING_BOT_FILE} on the command line will be replaced by the value of
 the --bot-file parameter. This file is used by a swarming bot to communicate
 state of the host to tasks. It is written to by the swarming bot's
 on_before_task() hook in the swarming server's custom bot_config.py.
 """
 
-__version__ = '0.8.5'
+__version__ = '0.8.6'
 
+import argparse
 import base64
 import collections
+import json
 import logging
 import optparse
 import os
 import sys
 import tempfile
 import time
 
 from third_party.depot_tools import fix_encoding
 
 from utils import file_path
@@ skipping 680 matching lines @@
            'non-zero only on internal failure')
   parser.add_option(
       '--hard-timeout', type='float', help='Enforce hard timeout in execution')
   parser.add_option(
       '--grace-period', type='float',
       help='Grace period between SIGTERM and SIGKILL')
   parser.add_option(
       '--bot-file',
       help='Path to a file describing the state of the host. The content is '
            'defined by on_before_task() in bot_config.')
+  parser.add_option(
+      '-a', '--argsfile',
+      # This is actually handled in parse_args; it's included here purely so it
+      # can make it into the help text.
+      help='Specify a file containing a JSON array of arguments to this '
+           'script. If --argsfile is provided, no other argument may be '
+           'provided on the command line.')
   data_group = optparse.OptionGroup(parser, 'Data source')
   data_group.add_option(
       '-s', '--isolated',
       help='Hash of the .isolated to grab from the isolate server.')
   isolateserver.add_isolate_server_options(data_group)
   parser.add_option_group(data_group)
 
   isolateserver.add_cache_options(parser)
 
   cipd.add_cipd_options(parser)
 
   debug_group = optparse.OptionGroup(parser, 'Debugging')
   debug_group.add_option(
       '--leak-temp-dir',
       action='store_true',
       help='Deliberately leak isolate\'s temp dir for later examination. '
            'Default: %default')
   debug_group.add_option(
       '--root-dir', help='Use a directory instead of a random one')
   parser.add_option_group(debug_group)
 
   auth.add_auth_options(parser)
 
   parser.set_defaults(cache='cache', cipd_cache='cipd_cache')
   return parser
 
 
-def main(args):
+def parse_args(args):
+  # Create a fake mini-parser just to get out the "-a" command. Note that
+  # it's not documented here; instead, it's documented in create_option_parser
+  # even though that parser will never actually get to parse it. This is
+  # because -f is exclusive with all other options and arguments.

[M-A Ruel, 2016/10/24 21:19:28]

-f?

[aludwin, 2016/10/24 21:27:00]

Now --argsfile; fixed.

+  file_argparse = argparse.ArgumentParser(add_help=False)
+  file_argparse.add_argument("-a", "--argsfile")

[M-A Ruel, 2016/10/24 21:19:28]

single quotes

[aludwin, 2016/10/24 21:27:00]

Done.

+  (file_args, nonfile_args) = file_argparse.parse_known_args(args)
+  if not file_args.argsfile == None:

[M-A Ruel, 2016/10/24 21:19:28]

don't use this pattern, do:
if file_args.argsfile:

[aludwin, 2016/10/24 21:27:00]

Done.

+    if len(nonfile_args) > 0:

[M-A Ruel, 2016/10/24 21:19:28]

if nonfile_args:

[aludwin, 2016/10/24 21:27:00]

Done, didn't realize [] was falsy.

+      file_argparse.error('Can\'t specify --argsfile with'
+                          'any other arguments (%s)' % nonfile_args)
+    try:
+      with open(file_args.argsfile, 'r') as f:
+        args = json.loads(f.read())

[M-A Ruel, 2016/10/24 21:19:28]

args = json.load(f)

+    except (IOError, OSError, ValueError) as e:
+      # We don't need to error out here - "args" is now empty,
+      # so the call below to parser.parse_args(args) will fail
+      # and print the full help text.
+      print >> sys.stderr, "Couldn't read arguments: %s" % e

[M-A Ruel, 2016/10/24 21:19:28]

use single quote and escape \'

[aludwin, 2016/10/24 21:27:00]

Done.

+
+  # Even if we failed to read the args, just call the normal parser now since it
+  # will print the correct help message.
   parser = create_option_parser()
   options, args = parser.parse_args(args)
+  return (parser, options, args)
+
+
+def main(args):
+  (parser, options, args) = parse_args(args)
 
   isolated_cache = isolateserver.process_cache_options(options)
   if options.clean:
     if options.isolated:
       parser.error('Can\'t use --isolated with --clean.')
     if options.isolate_server:
       parser.error('Can\'t use --isolate-server with --clean.')
     if options.json:
       parser.error('Can\'t use --json with --clean.')
     isolated_cache.cleanup()
@@ skipping 48 matching lines @@
   except cipd.Error as ex:
     print >> sys.stderr, ex.message
     return 1
 
 
 if __name__ == '__main__':
   subprocess42.inhibit_os_error_reporting()
   # Ensure that we are always running with the correct encoding.
   fix_encoding.fix_encoding()
   file_path.enable_symlink()
+
   sys.exit(main(sys.argv[1:]))