[PATCH] D54807: llvm-git: More tweaks.

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: llvm/utils/git-svn/git-llvm:120-121
 
-    if force_binary_stdin and stdin:
-        stdin = stdin.encode('utf-8')
-
----------------
Don't we still need this line?  When I wrote this patch, it was because after experimenting I found that `subprocess.Popen` would with `universal_newlines=False` unless `stdin` was a `bytes`.


================
Comment at: llvm/utils/git-svn/git-llvm:132-134
-    if not text:
-        stdout = stdout.decode('utf-8')
-        stderr = stderr.decode('utf-8')
----------------
Don't we need these lines too?  Like before, I found that if `universal_newlines=False` on Python 3, then `stdout` and `stderr` would both be a `bytes` object, and subsequent lines of code where we do something like `stdout.strip('\r\n')` would fail because it would be trying to mix `bytes` and `str`.


https://reviews.llvm.org/D54807