Restart codereview issue 6792042

buildbot/prebuilt.py

 #!/usr/bin/python
 # Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import datetime
 import multiprocessing
 import optparse
 import os
 import re
@@ skipping 124 matching lines @@
   """Repo sync and then push git changes in flight.
 
     Args:
       retries: The number of times to retry before giving up, default: 5
 
     Raises:
       GitPushFailed if push was unsuccessful after retries
   """
   for retry in range(1, retries + 1):
     try:
-      cros_build_lib.RunCommand('repo sync .', shell=True)
-      cros_build_lib.RunCommand('git push', shell=True)
+      cros_build_lib.RunCommand(['repo', 'sync', '.'])
+      cros_build_lib.RunCommand(['git', 'push'])
       break
     except cros_build_lib.RunCommandError:
       if retry < retries:
         print 'Error pushing changes trying again (%s/%s)' % (retry, retries)
         time.sleep(5 * retry)
   else:
     raise GitPushFailed('Failed to push change after %s retries' % retries)
 
 
 def RevGitFile(filename, value, retries=5, key='PORTAGE_BINHOST'):
   """Update and push the git file.
 
     Args:
       filename: file to modify that is in a git repo already
       value: string representing the version of the prebuilt that has been
         uploaded.
       retries: The number of times to retry before giving up, default: 5
       key: The variable key to update in the git file.
         (Default: PORTAGE_BINHOST)
   """
   prebuilt_branch = 'prebuilt_branch'
   old_cwd = os.getcwd()
   os.chdir(os.path.dirname(filename))
 
-  commit = cros_build_lib.RunCommand('git rev-parse HEAD', shell=True,
+  commit = cros_build_lib.RunCommand(['git', 'rev-parse', 'HEAD'],
                                      redirect_stdout=True).output
-  cros_build_lib.RunCommand('git remote update', shell=True)
-  cros_build_lib.RunCommand('repo start %s .' % prebuilt_branch, shell=True)
-  git_ssh_config_cmd = (
-    'git config url.ssh://git@gitrw.chromium.org:9222.pushinsteadof '
-    'http://git.chromium.org/git')
-  cros_build_lib.RunCommand(git_ssh_config_cmd, shell=True)
+  cros_build_lib.RunCommand(['git', 'remote', 'update'])
+  cros_build_lib.RunCommand(['repo', 'start', prebuilt_branch, '.'])
+  git_ssh_config_cmd = ['git', 'config',

[sosa, 2011/04/14 21:22:18]

Style is a big wrong.  If you wish to continue with indent you need to start
'git' on the new line ...

so this [
  'git', 'config', ...
  ... ]

rather than [ ...
  ... ]

[dgarrett, 2011/04/18 21:17:38]

As I understand, you have a choice...

stuff ['git',
       'config',
       ...]

or 

stuff [
   'git',
   'config',
   ...]

[Peter Mayo, 2011/04/19 03:59:39]

Done.

+      'url.ssh://git@gitrw.chromium.org:9222.pushinsteadof',
+      'http://git.chromium.org/git' ]
+  cros_build_lib.RunCommand(git_ssh_config_cmd)
   description = 'Update %s="%s" in %s' % (key, value, filename)
   print description
   try:
     UpdateLocalFile(filename, value, key)
-    cros_build_lib.RunCommand('git config push.default tracking', shell=True)
-    cros_build_lib.RunCommand('git commit -am "%s"' % description, shell=True)
+    cros_build_lib.RunCommand(['git', 'config', 'push.default', 'tracking'])
+    cros_build_lib.RunCommand(['git', 'commit', '-am', description])
     RevGitPushWithRetry(retries)
   finally:
-    cros_build_lib.RunCommand('repo abandon %s .' % prebuilt_branch, shell=True)
-    cros_build_lib.RunCommand('git checkout %s' % commit, shell=True)
+    cros_build_lib.RunCommand(['repo', 'abandon', 'prebuilt_branch', '.'])
+    cros_build_lib.RunCommand(['git', 'checkout', commit])
     os.chdir(old_cwd)
 
 
 def GetVersion():
   """Get the version to put in LATEST and update the git version with."""
   return datetime.datetime.now().strftime('%d.%m.%y.%H%M%S')
 
 
 def LoadPrivateFilters(build_path):
   """Load private filters based on ebuilds found under _PRIVATE_OVERLAY_DIR.
@@ skipping 34 matching lines @@
     False otherwise.
   """
   for name in _FILTER_PACKAGES:
     if name in file_path:
       print 'FILTERING %s' % file_path
       return True
 
   return False
 
 
-def _RetryRun(cmd, print_cmd=True, shell=False, cwd=None):
+def _RetryRun(cmd, print_cmd=True, cwd=None):
   """Run the specified command, retrying if necessary.
 
   Args:
     cmd: The command to run.
     print_cmd: Whether to print out the cmd.
     shell: Whether to treat the command as a shell.
     cwd: Working directory to run command in.
 
   Returns:
     True if the command succeeded. Otherwise, returns False.
   """
 
   # TODO(scottz): port to use _Run or similar when it is available in
   # cros_build_lib.
   for attempt in range(_RETRIES):
     try:
-      output = cros_build_lib.RunCommand(cmd, print_cmd=print_cmd, shell=shell,
+      output = cros_build_lib.RunCommand(cmd, print_cmd=print_cmd,
                                          cwd=cwd)
       return True
     except cros_build_lib.RunCommandError:
-      print 'Failed to run %s' % cmd
+      print 'Failed to run %r' % cmd
   else:
-    print 'Retry failed run %s, giving up' % cmd
+    print 'Retry failed run %r, giving up' % cmd
     return False
 
 
 def _GsUpload(args):
   """Upload to GS bucket.
 
   Args:
     args: a tuple of three arguments that contains local_file, remote_file, and
           the acl used for uploading the file.
 
@@ skipping 13 matching lines @@
     cmd = '%s cp -a private %s %s' % (_GSUTIL_BIN, local_file, remote_file)
     if not os.path.exists(acl):
       print >> sys.stderr, ('You are specifying either a file that does not '
                             'exist or an unknown canned acl: %s. Aborting '
                             'upload') % acl
       # emulate the failing of an upload since we are not uploading the file
       return (local_file, remote_file)
 
     acl_cmd = '%s setacl %s %s' % (_GSUTIL_BIN, acl, remote_file)
 
-  if not _RetryRun(cmd, print_cmd=False, shell=True):
+  if not _RetryRun(cmd, print_cmd=False):
     return (local_file, remote_file)
 
   if acl_cmd:
     # Apply the passed in ACL xml file to the uploaded object.
-    _RetryRun(acl_cmd, print_cmd=False, shell=True)
+    _RetryRun(acl_cmd, print_cmd=False)
 
 
 def RemoteUpload(acl, files, pool=10):
   """Upload to google storage.
 
   Create a pool of process and call _GsUpload with the proper arguments.
 
   Args:
     acl: The canned acl used for uploading. acl can be one of: "public-read",
          "public-read-write", "authenticated-read", "bucket-owner-read",
@@ skipping 99 matching lines @@
     value: New value for key.
   """
   cwd = os.path.dirname(os.path.abspath(path))
   filename = os.path.basename(path)
   if not os.path.isdir(cwd):
     os.makedirs(cwd)
   if not os.path.isfile(path):
     config_file = file(path, 'w')
     config_file.close()
   UpdateLocalFile(path, value, key)
-  cros_build_lib.RunCommand('git add %s' % filename, cwd=cwd, shell=True)
+  cros_build_lib.RunCommand(['git', 'add', filename],  cwd=cwd)
   description = 'Update %s=%s in %s' % (key, value, filename)
-  cros_build_lib.RunCommand('git commit -m "%s"' % description, cwd=cwd,
-      shell=True)
+  cros_build_lib.RunCommand(['git', 'commit', '-m', description], cwd=cwd)
 
 
 def _GrabAllRemotePackageIndexes(binhost_urls):
   """Grab all of the packages files associated with a list of binhost_urls.
 
   Args:
     binhost_urls: The URLs for the directories containing the Packages files we
                   want to grab.
 
   Returns:
@@ skipping 66 matching lines @@
             'pkgs': pkgs,
             'remote_packages': '%s/Packages' % remote_location.rstrip('/'),
             'remote_path': remote_path.rstrip('/'),
             'remote_location': remote_location.rstrip('/'),
             'ssh_server': ssh_server }
       cmds = ['ssh %(ssh_server)s mkdir -p %(remote_path)s' % d,
               'rsync -av --chmod=a+r %(pkg_index)s %(remote_packages)s' % d]
       if pkgs:
         cmds.append('rsync -Rav %(pkgs)s %(remote_location)s/' % d)
       for cmd in cmds:
-        if not _RetryRun(cmd, shell=True, cwd=package_path):
+        if not _RetryRun(cmd, cwd=package_path):
           raise UploadFailed('Could not run %s' % cmd)
 
   def _SyncHostPrebuilts(self, build_path, version, key, git_sync,
                         sync_binhost_conf):
     """Synchronize host prebuilt files.
 
     This function will sync both the standard host packages, plus the host
     packages associated with all targets that have been "setup" with the
     current host's chroot. For instance, if this host has been used to build
     x86-generic, it will sync the host packages associated with
@@ skipping 155 matching lines @@
     uploader._SyncHostPrebuilts(options.build_path, version, options.key,
                                 options.git_sync, options.sync_binhost_conf)
 
   if options.board:
     uploader._SyncBoardPrebuilts(options.board, options.build_path, version,
                                  options.key, options.git_sync,
                                  options.sync_binhost_conf)
 
 if __name__ == '__main__':
   main()