Refactor SubprocessCallAndFilter() to remove positional arguments.

This way the function is much more similar to subprocess.call(). Further changes
will be done, to be able to convert all the function, to be able to parallelize
output without corrupting it.

Used pylint to verify call graph correctness, causing some other unrelated
changes.

TEST=unit tests
BUG=none

Review URL: http://codereview.chromium.org/3117039

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@57369 0039d316-1c4b-4281-b951-d872f2087c98

gcl.py

@@ -143,12 +143,12 @@ def GetCachedFile(filename, max_age=60*60*24*3, use_root=False):
             # stderr into content_array.
             content_array = []
             svn_path = url_path + "/" + filename
-            SVN.RunAndFilterOutput(['cat', svn_path, '--non-interactive'], '.',
-                                   False, False, content_array.append)
+            SVN.RunAndFilterOutput(['cat', svn_path, '--non-interactive'],
+                                   cwd='.', filter_fn=content_array.append)
             # Exit the loop if the file was found. Override content.
             content = '\n'.join(content_array)
             break
-          except gclient_utils.Error, e:
+          except gclient_utils.Error:
             if content_array[0].startswith(
                 'svn: Can\'t get username or password'):
               ErrorExit('Your svn credentials expired. Please run svn update '
@@ -716,7 +716,7 @@ def CMDstatus():
 
 
 @need_change_and_args
-@attrs(usage='[--no_presubmit] [--clobber] [--no_watchlists]')
+@attrs(usage='[--no_presubmit] [--no_watchlists]')
 def CMDupload(change_info, args):
   """Uploads the changelist to the server for review.
 
@@ -736,9 +736,6 @@ def CMDupload(change_info, args):
   if FilterFlag(args, "--send-mail"):
     args.append("--send_mail")
 
-  # Supports --clobber for the try server.
-  clobber = FilterFlag(args, "--clobber")
-
   upload_arg = ["upload.py", "-y"]
   server = GetCodeReviewSetting("CODE_REVIEW_SERVER")
   if not server:

gclient.py

@@ -451,7 +451,8 @@ class Dependency(GClientKeywords, gclient_utils.WorkItem):
     # Use a discrete exit status code of 2 to indicate that a hook action
     # failed.  Users of this script may wish to treat hook action failures
     # differently from VC failures.
-    return gclient_utils.SubprocessCall(command, self.root_dir(), fail_status=2)
+    return gclient_utils.SubprocessCall(command, cwd=self.root_dir(),
+        fail_status=2)
 
   def root_dir(self):
     return self.parent.root_dir()
@@ -605,7 +606,6 @@ solutions = [
                                   'incomplete: %s' % s)
     # .gclient can have hooks.
     self.deps_hooks = config_dict.get('hooks', [])
-    self.direct_reference = True
     self.deps_parsed = True
 
   def SaveConfig(self):
@@ -881,10 +881,10 @@ def CMDrecurse(parser, args):
     scm = gclient_scm.GetScmName(url)
     if scm_set and scm not in scm_set:
       continue
-    dir = os.path.normpath(os.path.join(root, path))
+    cwd = os.path.normpath(os.path.join(root, path))
     env['GCLIENT_SCM'] = scm
     env['GCLIENT_URL'] = url
-    subprocess.Popen(args, cwd=dir, env=env).communicate()
+    subprocess.Popen(args, cwd=cwd, env=env).communicate()
 
 
 @attr('usage', '[url] [safesync url]')

gclient_scm.py

@@ -154,7 +154,7 @@ class GitWrapper(SCMWrapper):
     merge_base = self._Run(['merge-base', 'HEAD', 'origin'])
     command = ['diff', merge_base]
     filterer = DiffFilterer(self.relpath)
-    scm.GIT.RunAndFilterOutput(command, path, False, False, filterer.Filter,
+    scm.GIT.RunAndFilterOutput(command, cwd=path, filter_fn=filterer.Filter,
         stdout=options.stdout)
 
   def update(self, options, args, file_list):
@@ -644,7 +644,7 @@ class GitWrapper(SCMWrapper):
   def _Run(self, args, cwd=None, redirect_stdout=True):
     # TODO(maruel): Merge with Capture or better gclient_utils.CheckCall().
     if cwd is None:
-        cwd = self.checkout_path
+      cwd = self.checkout_path
     stdout = None
     if redirect_stdout:
       stdout = subprocess.PIPE
@@ -671,18 +671,15 @@ class SVNWrapper(SCMWrapper):
 
   def cleanup(self, options, args, file_list):
     """Cleanup working copy."""
-    command = ['cleanup']
-    command.extend(args)
-    scm.SVN.Run(command, os.path.join(self._root_dir, self.relpath))
+    scm.SVN.Run(['cleanup'] + args,
+                cwd=os.path.join(self._root_dir, self.relpath))
 
   def diff(self, options, args, file_list):
     # NOTE: This function does not currently modify file_list.
     path = os.path.join(self._root_dir, self.relpath)
     if not os.path.isdir(path):
       raise gclient_utils.Error('Directory %s is not present.' % path)
-    command = ['diff']
-    command.extend(args)
-    scm.SVN.Run(command, path)
+    scm.SVN.Run(['diff'] + args, cwd=path)
 
   def export(self, options, args, file_list):
     """Export a clean directory tree into the given path."""
@@ -693,9 +690,8 @@ class SVNWrapper(SCMWrapper):
     except OSError:
       pass
     assert os.path.exists(export_path)
-    command = ['export', '--force', '.']
-    command.append(export_path)
-    scm.SVN.Run(command, os.path.join(self._root_dir, self.relpath))
+    scm.SVN.Run(['export', '--force', '.', export_path],
+                cwd=os.path.join(self._root_dir, self.relpath))
 
   def pack(self, options, args, file_list):
     """Generates a patch file which can be applied to the root of the
@@ -707,7 +703,8 @@ class SVNWrapper(SCMWrapper):
     command.extend(args)
 
     filterer = DiffFilterer(self.relpath)
-    scm.SVN.RunAndFilterOutput(command, path, False, False, filterer.Filter,
+    scm.SVN.RunAndFilterOutput(command, cwd=path, print_messages=False,
+        print_stdout=False, filter_fn=filterer.Filter,
         stdout=options.stdout)
 
   def update(self, options, args, file_list):
@@ -765,7 +762,7 @@ class SVNWrapper(SCMWrapper):
     dir_info = scm.SVN.CaptureStatus(os.path.join(checkout_path, '.'))
     if [True for d in dir_info if d[0][2] == 'L' and d[1] == checkout_path]:
       # The current directory is locked, clean it up.
-      scm.SVN.Run(['cleanup'], checkout_path)
+      scm.SVN.Run(['cleanup'], cwd=checkout_path)
 
     # Retrieve the current HEAD version because svn is slow at null updates.
     if options.manually_grab_svn_rev and not revision:
@@ -797,7 +794,7 @@ class SVNWrapper(SCMWrapper):
                    from_info['Repository Root'],
                    to_info['Repository Root'],
                    self.relpath]
-        scm.SVN.Run(command, self._root_dir)
+        scm.SVN.Run(command, cwd=self._root_dir)
         from_info['URL'] = from_info['URL'].replace(
             from_info['Repository Root'],
             to_info['Repository Root'])
@@ -840,7 +837,7 @@ class SVNWrapper(SCMWrapper):
         # Create an empty checkout and then update the one file we want.  Future
         # operations will only apply to the one file we checked out.
         command = ["checkout", "--depth", "empty", self.url, checkout_path]
-        scm.SVN.Run(command, self._root_dir)
+        scm.SVN.Run(command, cwd=self._root_dir)
         if os.path.exists(os.path.join(checkout_path, filename)):
           os.remove(os.path.join(checkout_path, filename))
         command = ["update", filename]
@@ -860,7 +857,7 @@ class SVNWrapper(SCMWrapper):
                  os.path.join(checkout_path, filename)]
       command = self._AddAdditionalUpdateFlags(command, options,
           options.revision)
-      scm.SVN.Run(command, self._root_dir)
+      scm.SVN.Run(command, cwd=self._root_dir)
 
   def revert(self, options, args, file_list):
     """Reverts local modifications. Subversion specific.

gclient_utils.py

@@ -251,52 +251,51 @@ def RemoveDirectory(*path):
     os.rmdir(file_path)
 
 
-def SubprocessCall(command, in_directory, fail_status=None):
-  """Runs command, a list, in directory in_directory.
+def SubprocessCall(args, **kwargs):
+  """Wraps SubprocessCallAndFilter() with different default arguments.
 
-  This function wraps SubprocessCallAndFilter, but does not perform the
-  filtering functions.  See that function for a more complete usage
-  description.
-  """
-  # Call subprocess and capture nothing:
-  SubprocessCallAndFilter(command, in_directory, True, True, fail_status)
+  Calls subprocess and capture nothing."""
+  kwargs['print_messages'] = True
+  kwargs['print_stdout'] = True
+  return SubprocessCallAndFilter(args, **kwargs)
 
 
-def SubprocessCallAndFilter(command,
-                            in_directory,
-                            print_messages,
-                            print_stdout,
-                            fail_status=None,
-                            filter_fn=None,
-                            stdout=None):
-  """Runs command, a list, in directory in_directory.
+def SubprocessCallAndFilter(args, **kwargs):
+  """Runs a command and prints a header line if appropriate.
 
-  If print_messages is true, a message indicating what is being done
-  is printed to stdout. If print_messages is false, the message is printed
-  only if we actually need to print something else as well, so you can
-  get the context of the output. If print_messages is false and print_stdout
-  is false, no output at all is generated.
+  If |print_messages| is True, a message indicating what is being done
+  is printed to stdout. Otherwise the message is printed only if the call
+  generated any ouput. If both |print_messages| and |print_stdout| are False,
+  no output at all is generated.
 
-  Also, if print_stdout is true, the command's stdout is also forwarded
-  to stdout.
+  If |print_stdout| is True, the command's stdout is also forwarded to stdout.
 
-  If a filter_fn function is specified, it is expected to take a single
+  If |filter_fn| function is specified, it is expected to take a single
   string argument, and it will be called with each line of the
   subprocess's output. Each line has had the trailing newline character
   trimmed.
 
   If the command fails, as indicated by a nonzero exit status, gclient will
-  exit with an exit status of fail_status.  If fail_status is None (the
+  exit with an exit status of fail_status. If fail_status is None (the
   default), gclient will raise an Error exception.
+
+  Other subprocess.Popen parameters can be specified.
   """
-  stdout = stdout or sys.stdout
-  logging.debug(command)
+  stdout = kwargs.pop('stdout', sys.stdout) or sys.stdout
+  assert not 'stderr' in kwargs
+  filter_fn = kwargs.pop('filter_fn', None)
+  print_messages = kwargs.pop('print_messages', False)
+  print_stdout = kwargs.pop('print_stdout', False)
+  fail_status = kwargs.pop('fail_status', None)
+
+  logging.debug(args)
   if print_messages:
     stdout.write('\n________ running \'%s\' in \'%s\'\n'
-          % (' '.join(command), in_directory))
+          % (' '.join(args), kwargs['cwd']))
 
-  kid = Popen(command, bufsize=0, cwd=in_directory,
-              stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+  kid = Popen(args, bufsize=0,
+              stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
+              **kwargs)
 
   # Do a flush of sys.stdout before we begin reading from the subprocess's
   # stdout.
@@ -314,7 +313,7 @@ def SubprocessCallAndFilter(command,
       if print_stdout:
         if not print_messages:
           stdout.write('\n________ running \'%s\' in \'%s\'\n'
-              % (' '.join(command), in_directory))
+              % (' '.join(args), kwargs['cwd']))
           print_messages = True
         stdout.write(in_byte)
       if in_byte != '\n':
@@ -337,7 +336,7 @@ def SubprocessCallAndFilter(command,
   rv = kid.wait()
 
   if rv:
-    msg = 'failed to run command: %s' % ' '.join(command)
+    msg = 'failed to run command: %s' % ' '.join(args)
     if fail_status != None:
       sys.stderr.write(msg + '\n')
       sys.exit(fail_status)

scm.py

@@ -117,38 +117,9 @@ class GIT(object):
     return results
 
   @staticmethod
-  def RunAndFilterOutput(args,
-                         in_directory,
-                         print_messages,
-                         print_stdout,
-                         filter_fn,
-                         stdout=None):
-    """Runs a command, optionally outputting to stdout.
-
-    stdout is passed line-by-line to the given filter_fn function. If
-    print_stdout is true, it is also printed to sys.stdout as in Run.
-
-    Args:
-      args: A sequence of command line parameters to be passed.
-      in_directory: The directory where git is to be run.
-      print_messages: Whether to print status messages to stdout about
-        which commands are being run.
-      print_stdout: Whether to forward program's output to stdout.
-      filter_fn: A function taking one argument (a string) which will be
-        passed each line (with the ending newline character removed) of
-        program's output for filtering.
-
-    Raises:
-      gclient_utils.Error: An error occurred while running the command.
-    """
-    command = [GIT.COMMAND]
-    command.extend(args)
-    gclient_utils.SubprocessCallAndFilter(command,
-                                          in_directory,
-                                          print_messages,
-                                          print_stdout,
-                                          filter_fn=filter_fn,
-                                          stdout=stdout)
+  def RunAndFilterOutput(args, **kwargs):
+    """Wrapper to gclient_utils.SubprocessCallAndFilter()."""
+    return gclient_utils.SubprocessCallAndFilter([GIT.COMMAND] + args, **kwargs)
 
   @staticmethod
   def GetEmail(repo_root):
@@ -345,20 +316,9 @@ class SVN(object):
   current_version = None
 
   @staticmethod
-  def Run(args, in_directory):
-    """Runs svn, sending output to stdout.
-
-    Args:
-      args: A sequence of command line parameters to be passed to svn.
-      in_directory: The directory where svn is to be run.
-
-    Raises:
-      Error: An error occurred while running the svn command.
-    """
-    c = [SVN.COMMAND]
-    c.extend(args)
-    # TODO(maruel): This is very gclient-specific.
-    gclient_utils.SubprocessCall(c, in_directory)
+  def Run(args, **kwargs):
+    """Wrappers to gclient_utils.SubprocessCall()."""
+    return gclient_utils.SubprocessCall([SVN.COMMAND] + args, **kwargs)
 
   @staticmethod
   def Capture(args, in_directory=None, print_error=True):
@@ -380,7 +340,7 @@ class SVN(object):
         stderr=stderr).communicate()[0]
 
   @staticmethod
-  def RunAndGetFileList(verbose, args, in_directory, file_list, stdout=None):
+  def RunAndGetFileList(verbose, args, cwd, file_list, stdout=None):
     """Runs svn checkout, update, or status, output to stdout.
 
     The first item in args must be either "checkout", "update", or "status".
@@ -392,13 +352,12 @@ class SVN(object):
     Args:
       verbose: If True, uses verbose output
       args: A sequence of command line parameters to be passed to svn.
-      in_directory: The directory where svn is to be run.
+      cwd: The directory where svn is to be run.
 
     Raises:
       Error: An error occurred while running the svn command.
     """
-    command = [SVN.COMMAND]
-    command.extend(args)
+    stdout = stdout or sys.stdout
 
     # svn update and svn checkout use the same pattern: the first three columns
     # are for file status, property status, and lock status.  This is followed
@@ -434,11 +393,9 @@ class SVN(object):
           failure.append(line)
 
       try:
-        SVN.RunAndFilterOutput(args,
-                               in_directory,
-                               verbose,
-                               True,
-                               CaptureMatchingLines,
+        SVN.RunAndFilterOutput(args, cwd=cwd, print_messages=verbose,
+                               print_stdout=True,
+                               filter_fn=CaptureMatchingLines,
                                stdout=stdout)
       except gclient_utils.Error:
         def IsKnownFailure():
@@ -481,38 +438,9 @@ class SVN(object):
       break
 
   @staticmethod
-  def RunAndFilterOutput(args,
-                         in_directory,
-                         print_messages,
-                         print_stdout,
-                         filter_fn,
-                         stdout=None):
-    """Runs a command, optionally outputting to stdout.
-
-    stdout is passed line-by-line to the given filter_fn function. If
-    print_stdout is true, it is also printed to sys.stdout as in Run.
-
-    Args:
-      args: A sequence of command line parameters to be passed.
-      in_directory: The directory where svn is to be run.
-      print_messages: Whether to print status messages to stdout about
-        which commands are being run.
-      print_stdout: Whether to forward program's output to stdout.
-      filter_fn: A function taking one argument (a string) which will be
-        passed each line (with the ending newline character removed) of
-        program's output for filtering.
-
-    Raises:
-      gclient_utils.Error: An error occurred while running the command.
-    """
-    command = [SVN.COMMAND]
-    command.extend(args)
-    gclient_utils.SubprocessCallAndFilter(command,
-                                          in_directory,
-                                          print_messages,
-                                          print_stdout,
-                                          filter_fn=filter_fn,
-                                          stdout=stdout)
+  def RunAndFilterOutput(args, **kwargs):
+    """Wrapper for gclient_utils.SubprocessCallAndFilter()."""
+    return gclient_utils.SubprocessCallAndFilter([SVN.COMMAND] + args, **kwargs)
 
   @staticmethod
   def CaptureInfo(relpath, in_directory=None, print_error=True):

tests/gclient_scm_test.py

@@ -43,6 +43,7 @@ class BaseTestCase(GCBaseTestCase):
     self.mox.StubOutWithMock(gclient_scm.scm.SVN, 'Run')
     self.mox.StubOutWithMock(gclient_scm.scm.SVN, 'RunAndGetFileList')
     self._scm_wrapper = gclient_scm.CreateSCM
+    gclient_scm.sys.stdout.flush = lambda: None
 
 
 class SVNWrapperTestCase(BaseTestCase):
@@ -68,7 +69,7 @@ class SVNWrapperTestCase(BaseTestCase):
 
   def testDir(self):
     members = [
-        'AddAdditionalFlags', 'FullUrlForRelativeUrl', 'RunCommand',
+        'FullUrlForRelativeUrl', 'RunCommand',
         'cleanup', 'diff', 'export', 'pack', 'relpath', 'revert',
         'revinfo', 'runhooks', 'status', 'update',
         'updatesingle', 'url',
@@ -118,6 +119,8 @@ class SVNWrapperTestCase(BaseTestCase):
     options = self.Options(verbose=True)
     base_path = gclient_scm.os.path.join(self.root_dir, self.relpath)
     gclient_scm.os.path.isdir(base_path).AndReturn(False)
+    gclient_scm.scm.SVN.Capture(['--version']
+        ).AndReturn('svn, version 1.5.1 (r32289)')
     # It'll to a checkout instead.
     gclient_scm.os.path.exists(gclient_scm.os.path.join(base_path, '.git')
                                ).AndReturn(False)
@@ -126,7 +129,8 @@ class SVNWrapperTestCase(BaseTestCase):
     gclient_scm.os.path.exists(base_path).AndReturn(False)
     files_list = self.mox.CreateMockAnything()
     gclient_scm.scm.SVN.RunAndGetFileList(options.verbose,
-                                          ['checkout', self.url, base_path],
+                                          ['checkout', self.url, base_path,
+                                           '--force'],
                                           self.root_dir, files_list)
 
     self.mox.ReplayAll()
@@ -235,7 +239,8 @@ class SVNWrapperTestCase(BaseTestCase):
     gclient_scm.os.path.exists(base_path).AndReturn(False)
     files_list = self.mox.CreateMockAnything()
     gclient_scm.scm.SVN.RunAndGetFileList(options.verbose,
-                                          ['checkout', self.url, base_path],
+                                          ['checkout', self.url, base_path,
+                                           '--force'],
                                           self.root_dir, files_list)
     self.mox.ReplayAll()
     scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir,
@@ -307,7 +312,8 @@ class SVNWrapperTestCase(BaseTestCase):
     # When checking out a single file, we issue an svn checkout and svn update.
     files_list = self.mox.CreateMockAnything()
     gclient_scm.scm.SVN.Run(
-        ['checkout', '--depth', 'empty', self.url, base_path], self.root_dir)
+        ['checkout', '--depth', 'empty', self.url, base_path],
+        cwd=self.root_dir)
     gclient_scm.scm.SVN.RunAndGetFileList(options.verbose, ['update', 'DEPS'],
         gclient_scm.os.path.join(self.root_dir, self.relpath), files_list)
 
@@ -345,7 +351,8 @@ class SVNWrapperTestCase(BaseTestCase):
     files_list = self.mox.CreateMockAnything()
     gclient_scm.scm.SVN.Run(
         ['export', gclient_scm.os.path.join(self.url, 'DEPS'),
-            gclient_scm.os.path.join(base_path, 'DEPS')], self.root_dir)
+            gclient_scm.os.path.join(base_path, 'DEPS')],
+        cwd=self.root_dir)
 
     self.mox.ReplayAll()
     scm = self._scm_wrapper(url=self.url, root_dir=self.root_dir,
@@ -379,7 +386,8 @@ class SVNWrapperTestCase(BaseTestCase):
     # When checking out a single file, we issue an svn checkout and svn update.
     files_list = self.mox.CreateMockAnything()
     gclient_scm.scm.SVN.Run(
-        ['checkout', '--depth', 'empty', self.url, base_path], self.root_dir)
+        ['checkout', '--depth', 'empty', self.url, base_path],
+        cwd=self.root_dir)
     gclient_scm.scm.SVN.RunAndGetFileList(options.verbose, ['update', 'DEPS'],
         gclient_scm.os.path.join(self.root_dir, self.relpath), files_list)
 

tests/gclient_utils_test.py

@@ -37,29 +37,29 @@ class GclientUtilsUnittest(GclientUtilBase):
 
 class CheckCallTestCase(GclientUtilBase):
   def testCheckCallSuccess(self):
-    command = ['boo', 'foo', 'bar']
+    args = ['boo', 'foo', 'bar']
     process = self.mox.CreateMockAnything()
     process.returncode = 0
     env = gclient_utils.os.environ.copy()
     env['LANGUAGE'] = 'en'
     gclient_utils.subprocess.Popen(
-        command, cwd=None,
+        args, cwd=None,
         stderr=None,
         env=env,
         stdout=gclient_utils.subprocess.PIPE,
         shell=gclient_utils.sys.platform.startswith('win')).AndReturn(process)
     process.communicate().AndReturn(['bleh', 'foo'])
     self.mox.ReplayAll()
-    gclient_utils.CheckCall(command)
+    gclient_utils.CheckCall(args)
 
   def testCheckCallFailure(self):
-    command = ['boo', 'foo', 'bar']
+    args = ['boo', 'foo', 'bar']
     process = self.mox.CreateMockAnything()
     process.returncode = 42
     env = gclient_utils.os.environ.copy()
     env['LANGUAGE'] = 'en'
     gclient_utils.subprocess.Popen(
-        command, cwd=None,
+        args, cwd=None,
         stderr=None,
         env=env,
         stdout=gclient_utils.subprocess.PIPE,
@@ -67,10 +67,10 @@ class CheckCallTestCase(GclientUtilBase):
     process.communicate().AndReturn(['bleh', 'foo'])
     self.mox.ReplayAll()
     try:
-      gclient_utils.CheckCall(command)
+      gclient_utils.CheckCall(args)
       self.fail()
     except gclient_utils.CheckCallError, e:
-      self.assertEqual(e.command, command)
+      self.assertEqual(e.command, args)
       self.assertEqual(e.cwd, None)
       self.assertEqual(e.retcode, 42)
       self.assertEqual(e.stdout, 'bleh')
@@ -84,8 +84,8 @@ class SubprocessCallAndFilterTestCase(GclientUtilBase):
     def wait(self):
         pass
 
-  def _inner(self, command, test_string):
-    in_directory = 'bleh'
+  def _inner(self, args, test_string):
+    cwd = 'bleh'
     env = gclient_utils.os.environ.copy()
     env['LANGUAGE'] = 'en'
     gclient_utils.sys.stdout.write(
@@ -93,8 +93,8 @@ class SubprocessCallAndFilterTestCase(GclientUtilBase):
     for i in test_string:
       gclient_utils.sys.stdout.write(i)
     gclient_utils.subprocess.Popen(
-        command,
-        cwd=in_directory,
+        args,
+        cwd=cwd,
         shell=(gclient_utils.sys.platform == 'win32'),
         env=env,
         stdout=gclient_utils.subprocess.PIPE,
@@ -111,20 +111,21 @@ class SubprocessCallAndFilterTestCase(GclientUtilBase):
       if match:
         capture_list.append(match.group(1))
     gclient_utils.SubprocessCallAndFilter(
-        command, in_directory, True, True, None, FilterLines)
+        args, cwd=cwd, print_messages=True, print_stdout=True,
+        filter_fn=FilterLines)
     self.assertEquals(line_list, ['ahah', 'accb', 'allo', 'addb'])
     self.assertEquals(capture_list, ['cc', 'dd'])
 
   def testSubprocessCallAndFilter(self):
-    command = ['boo', 'foo', 'bar']
+    args = ['boo', 'foo', 'bar']
     test_string = 'ahah\naccb\nallo\naddb\n'
-    self._inner(command, test_string)
+    self._inner(args, test_string)
 
   def testNoLF(self):
     # Exactly as testSubprocessCallAndFilter without trailing \n
-    command = ['boo', 'foo', 'bar']
+    args = ['boo', 'foo', 'bar']
     test_string = 'ahah\naccb\nallo\naddb'
-    self._inner(command, test_string)
+    self._inner(args, test_string)
 
 
 class SplitUrlRevisionTestCase(GclientUtilBase):

tests/scm_unittest.py

@@ -316,9 +316,9 @@ class SVNTestCase(BaseSCMTestCase):
   def testRun(self):
     param2 = 'bleh'
     scm.gclient_utils.SubprocessCall(['svn', 'foo', 'bar'],
-                                     param2).AndReturn(None)
+                                     cwd=param2).AndReturn(None)
     self.mox.ReplayAll()
-    scm.SVN.Run(['foo', 'bar'], param2)
+    scm.SVN.Run(['foo', 'bar'], cwd=param2)
 
   def testCaptureStatusEmpty(self):
     text = r"""<?xml version="1.0"?>

trychange.py

@@ -97,6 +97,7 @@ class SCM(object):
     self.options.files = None
     self.codereview_settings = None
     self.codereview_settings_file = 'codereview.settings'
+    self.gclient_root = None
 
   def GetFileNames(self):
     """Return the list of files in the diff."""
@@ -136,7 +137,6 @@ class SCM(object):
 
   def _GclientStyleSettings(self):
     """Find the root, assuming a gclient-style checkout."""
-    self.gclient_root = None
     if not self.options.no_gclient and not self.options.root:
       root = self.checkout_root
       self.gclient_root = gclient_utils.FindGclientRoot(root)
@@ -301,7 +301,7 @@ def _SendChangeHTTP(options):
         'server port to connect to.')
 
   values = _ParseSendChangeOptions(options)
-  description = ''.join("%s=%s\n" % (k,v) for (k,v) in values.iteritems())
+  description = ''.join("%s=%s\n" % (k, v) for (k, v) in values.iteritems())
   values['patch'] = options.diff
 
   url = 'http://%s:%s/send_try_patch' % (options.host, options.port)
@@ -345,7 +345,7 @@ def _SendChangeSVN(options):
                             ' try server svn repository to connect to.')
 
   values = _ParseSendChangeOptions(options)
-  description = ''.join("%s=%s\n" % (k,v) for (k,v) in values.iteritems())
+  description = ''.join("%s=%s\n" % (k, v) for (k, v) in values.iteritems())
   logging.info('Sending by SVN')
   logging.info(description)
   logging.info(options.svn_repo)