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',
+      '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.
 
   Returns:
     Return the arg tuple of two if the upload failed
   """
   (local_file, remote_file, acl) = args
   CANNED_ACLS = ['public-read', 'private', 'bucket-owner-read',
                  'authenticated-read', 'bucket-owner-full-control',
                  'public-read-write']
   acl_cmd = None
   if acl in CANNED_ACLS:
-    cmd = '%s cp -a %s %s %s' % (_GSUTIL_BIN, acl, local_file, remote_file)
+    cmd = [_GSUTIL_BIN, 'cp', '-a', acl, local_file, remote_file]
   else:
     # For private uploads we assume that the overlay board is set up properly
     # and a googlestore_acl.xml is present, if not this script errors
-    cmd = '%s cp -a private %s %s' % (_GSUTIL_BIN, local_file, remote_file)
+    cmd = [_GSUTIL_BIN, 'cp', '-a', 'private', 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)
+    acl_cmd = [_GSUTIL_BIN, 'setacl', 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 53 matching lines @@
       # Build list of files to upload.
       upload_files = GenerateUploadDict(package_path, remote_location, uploads)
       remote_file = '%s/Packages' % remote_location.rstrip('/')
       upload_files[tmp_packages_file.name] = remote_file
 
       failed_uploads = RemoteUpload(self._acl, upload_files)
       if len(failed_uploads) > 1 or (None not in failed_uploads):
         error_msg = ['%s -> %s\n' % args for args in failed_uploads if args]
         raise UploadFailed('Error uploading:\n%s' % error_msg)
     else:
-      pkgs = ' '.join(p['CPV'] + '.tbz2' for p in uploads)
+      pkgs = [p['CPV'] + '.tbz2' for p in uploads]
       ssh_server, remote_path = remote_location.split(':', 1)
-      d = { 'pkg_index': tmp_packages_file.name,
-            '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]
+      remote_path = remote_path.rstrip('/')
+      pkg_index = tmp_packages_file.name
+      remote_location = remote_location.rstrip('/')
+      remote_packages = '%s/Packages' % remote_location
+      cmds = [['ssh', ssh_server, 'mkdir', '-p', remote_path],
+              ['rsync', '-av', '--chmod=a+r', pkg_index, remote_packages]]
       if pkgs:
-        cmds.append('rsync -Rav %(pkgs)s %(remote_location)s/' % d)
+        cmds.append(['rsync', '-Rav'] + pkgs + [remote_location + '/'])
       for cmd in cmds:
-        if not _RetryRun(cmd, shell=True, cwd=package_path):
-          raise UploadFailed('Could not run %s' % cmd)
+        if not _RetryRun(cmd, cwd=package_path):
+          raise UploadFailed('Could not run %r' % cmd)
 
   def _UploadBoardTarball(self, board_path, url_suffix):
     """Upload a tarball of the board at the specified path to Google Storage.
 
     Args:
       board_path: The path to the board dir.
       url_suffix: The remote subdirectory where we should upload the packages.
     """
     remote_location = '%s/%s' % (self._upload_location.rstrip('/'), url_suffix)
     assert remote_location.startswith('gs://')
@@ skipping 204 matching lines @@
                                 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,
                                  options.upload_board_tarball)
 
 if __name__ == '__main__':
   main()