[PATCH] D41130: git-clang-format: cleanup: Use run() when possible.

[Mark Lodato via Phabricator via cfe-commits]

lodato created this revision.
lodato added reviewers: djasper, klimek.

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* so this should not cause a regression.

- To my knowledge.


https://reviews.llvm.org/D41130

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


Index: google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
===================================================================
--- google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
+++ google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
@@ -356,12 +356,9 @@
   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',
+                     '%s:%s' % (revision, os.path.dirname(filename)),
+                     os.path.basename(filename))
         mode = oct(int(stdout.split()[0], 8))
       else:
         mode = oct(os.stat(filename).st_mode)
@@ -384,14 +381,9 @@
   --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
 
@@ -522,6 +514,7 @@
 
 
 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)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41130.126613.patch
Type: text/x-patch
Size: 1992 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171212/1002c5fc/attachment-0001.bin>