git-clang-format: cleanup: Use run() when possible.

[lodato]

Summary

This makes the code a bit easier to understand. Also add a docstring to run().

Note: This means that we read the entire index into memory when calling git update-index, whereas previously we would send the data line-by-line. Git already loads the entire index into memory anyway (to my knowledge) so this should not cause a regression.

[klimek]

lg

[shafik]

Is this still relevant or can we close it?

google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format

def run_clang_format_and_save_to_tree(changed_lines, revision=None,
   def iteritems(container):
       try:
           return container.iteritems() # Python 2
       except AttributeError:
           return container.items() # Python 3
   def index_info_generator():
     for filename, line_ranges in iteritems(changed_lines):
       if revision:
-        git_metadata_cmd = ['git', 'ls-tree',
-                            '%s:%s' % (revision, os.path.dirname(filename)),
-                            os.path.basename(filename)]
-        git_metadata = subprocess.Popen(git_metadata_cmd, stdin=subprocess.PIPE,
-                                        stdout=subprocess.PIPE)
-        stdout = git_metadata.communicate()[0]
+        stdout = run('git', 'ls-tree',
+                     os.path.basename(filename))
         mode = oct(int(stdout.split()[0], 8))
       else:
         mode = oct(os.stat(filename).st_mode)
       # Adjust python3 octal format so that it matches what git expects
       if mode.startswith('0o'):
           mode = '0' + mode[2:]
       blob_id = clang_format_to_blob(filename, line_ranges,
                                      revision=revision,
                                      binary=binary,
                                      style=style)
       yield '%s %s\t%s' % (mode, blob_id, filename)
   return create_tree(index_info_generator(), '--index-info')
 
 
 def create_tree(input_lines, mode):
   """Create a tree object from the given input.
 
   If mode is '--stdin', it must be a list of filenames.  If mode is
   '--index-info' is must be a list of values suitable for "git update-index
   --index-info", such as "<mode> <SP> <sha1> <TAB> <filename>".  Any other mode
   is invalid."""
   assert mode in ('--stdin', '--index-info')
-  cmd = ['git', 'update-index', '--add', '-z', mode]
   with temporary_index_file():
-    p = subprocess.Popen(cmd, stdin=subprocess.PIPE)
-    for line in input_lines:
-      p.stdin.write(to_bytes('%s\0' % line))
-    p.stdin.close()
-    if p.wait() != 0:
-      die('`%s` failed' % ' '.join(cmd))
+    run('git', 'update-index', '--add', '-z', mode,
+        stdin=to_bytes(''.join('%s\0' % line for line in input_lines)))
     tree_id = run('git', 'write-tree')
     return tree_id
 
 
 def clang_format_to_blob(filename, line_ranges, revision=None,
                          binary='clang-format', style=None):
   """Run clang-format on the given file and save the result to a git blob.
 
if patch_mode:
     index_tree = old_tree
   else:
     with temporary_index_file(new_tree):
       run('git', 'checkout-index', '-a', '-f')
   return changed_files
 
 
 def run(*args, **kwargs):
+  """Runs the given command and returns stdout; exits on command failure."""
   stdin = kwargs.pop('stdin', '')
   verbose = kwargs.pop('verbose', True)
   strip = kwargs.pop('strip', True)
   for name in kwargs:
     raise TypeError("run() got an unexpected keyword argument '%s'" % name)
   p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
                        stdin=subprocess.PIPE)
   stdout, stderr = p.communicate(input=stdin)