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

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.
Herald added a project: All.

Is this still relevant or can we close it?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D41130/new/

https://reviews.llvm.org/D41130

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Manuel Klimek via Phabricator via cfe-commits]

klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

lg


https://reviews.llvm.org/D41130



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[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 "".  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)


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 "".  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)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits