llvm-git: More tweaks.

[jyknight]

Summary

On python3, use bytes for reading and applying the patch file, rather
than str. This fixes encoding issues when applying patches with
python3.X (reported by zturner).

Also, simplify the "svn update" call, by using svn's "--parents" flag,
instead of manually computing and supplying the list of parent
directories.

[zturner]

llvm/utils/git-svn/git-llvm

120–121

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.

132–134

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.

[jyknight]

BTW, I've tested this on Linux in both python2 and python3, but not on windows.

llvm/utils/git-svn/git-llvm

120–121

Yes, that's correct. "universal_newlines" in python3 really means "text". If it's enabled, the input/output are str, otherwise they're bytes. And for binary data, we _want_ (and now provide) bytes input, so this line is unneeded and undesirable. (in python3.7, they in fact fixed the confusing name, and added a "text" keyword arg as an alias of the "universal_newlines" one.).

132–134

We don't need them -- the purpose of this change is exactly to allow the use of bytes objects for binary I/O. Note the diff below to use rstrip(b'\r\n') instead of rstrip('\r\n') when !text.

[zturner]

llvm/utils/git-svn/git-llvm

327–328

Should we retain the comment here about why we're passing text=False?

[jyknight]

Forgot to close with the commit, r347766.

llvm/utils/git-svn/git-llvm

def get_dev_null():
     """Lazily create a /dev/null fd for use in shell()"""
     global dev_null_fd
     if dev_null_fd is None:
         dev_null_fd = open(os.devnull, 'w')
     return dev_null_fd
 
 
 def shell(cmd, strip=True, cwd=None, stdin=None, die_on_failure=True,
-          ignore_errors=False, force_binary_stdin=False):
+          ignore_errors=False, text=True):
     log_verbose('Running in %s: %s' % (cwd, ' '.join(cmd)))
 
     err_pipe = subprocess.PIPE
     if ignore_errors:
         # Silence errors if requested.
         err_pipe = get_dev_null()
 
-    if force_binary_stdin and stdin:
-        stdin = stdin.encode('utf-8')
-
     start = time.time()
-    text = not force_binary_stdin
     p = subprocess.Popen(cmd, cwd=cwd, stdout=subprocess.PIPE, stderr=err_pipe,
-                         stdin=subprocess.PIPE, universal_newlines=text)
+                         stdin=subprocess.PIPE,
+                         universal_newlines=text)
     stdout, stderr = p.communicate(input=stdin)
     elapsed = time.time() - start
 
     log_verbose('Command took %0.1fs' % elapsed)
 
-    if not text:
-        stdout = stdout.decode('utf-8')
-        stderr = stderr.decode('utf-8')
-
     if p.returncode == 0 or ignore_errors:
         if stderr and not ignore_errors:
             eprint('`%s` printed to stderr:' % ' '.join(cmd))
             eprint(stderr.rstrip())
         if strip:
+            if text:
                 stdout = stdout.rstrip('\r\n')
+            else:
+                stdout = stdout.rstrip(b'\r\n')
         if VERBOSE:
             for l in stdout.splitlines():
                 log_verbose("STDOUT: %s" % l)
         return stdout
     err_msg = '`%s` returned %s' % (' '.join(cmd), p.returncode)
     eprint(err_msg)
     if stderr:
         eprint(stderr.rstrip())
     if die_on_failure:
         sys.exit(2)
     raise RuntimeError(err_msg)
 
 
 def git(*cmd, **kwargs):
-    return shell(['git'] + list(cmd), kwargs.get('strip', True))
+    return shell(['git'] + list(cmd), **kwargs)
 
 
 def svn(cwd, *cmd, **kwargs):
-    # TODO: Better way to do default arg when we have *cmd?
-    return shell(['svn'] + list(cmd), cwd=cwd, stdin=kwargs.get('stdin', None),
-                 ignore_errors=kwargs.get('ignore_errors', None))
+    return shell(['svn'] + list(cmd), cwd=cwd, **kwargs)
 
 def program_exists(cmd):
     if sys.platform == 'win32' and not cmd.endswith('.exe'):
         cmd += '.exe'
     for path in os.environ["PATH"].split(os.pathsep):
         if os.access(os.path.join(path, cmd), os.X_OK):
             return True
     return False
for line in svn(svn_repo, 'status', '--no-ignore').split('\n'):
         filename = line[1:].strip()
         os.remove(os.path.join(svn_repo, filename))
 
 
 def svn_init(svn_root):
     if not os.path.exists(svn_root):
         log('Creating svn staging directory: (%s)' % (svn_root))
         os.makedirs(svn_root)
-        svn(svn_root, 'checkout', '--depth=immediates',
+        svn(svn_root, 'checkout', '--depth=empty',
             'https://llvm.org/svn/llvm-project/', '.')
         log("svn staging area ready in '%s'" % svn_root)
     if not os.path.isdir(svn_root):
         die("Can't initialize svn staging dir (%s)" % svn_root)
 
 
 def fix_eol_style_native(rev, svn_sr_path, files):
     """Fix line endings before applying patches with Unix endings
else:
             if eol_style == 'native':
                 crlf_files.append(f)
     if crlf_files:
         # Reformat all files with native SVN line endings to Unix format. SVN
         # knows files with native line endings are text files. It will commit
         # just the diff, and not a mass line ending change.
         shell(['dos2unix'] + crlf_files, ignore_errors=True, cwd=svn_sr_path)
 
-def get_all_parent_dirs(name):
-    parts = []
-    head, tail = os.path.split(name)
-    while head:
-        parts.append(head)
-        head, tail = os.path.split(head)
-    return parts
-
 def split_subrepo(f):
     # Given a path, splits it into (subproject, rest-of-path). If the path is
     # not in a subproject, returns ('', full-path).
 
     subproject, remainder = split_first_path_component(f)
 
     if subproject in GIT_TO_SVN_DIR:
         return subproject, remainder
def svn_push_one_rev(svn_repo, rev, dry_run):
     if status:
         die("Can't push git rev %s because svn status is not empty:\n%s" %
             (rev, status))
 
     svn_dirs_to_update = set()
     for sr, files in iteritems(subrepo_files):
         svn_sr_path = GIT_TO_SVN_DIR[sr]
         for f in files:
-            svn_dirs_to_update.update(
-                get_all_parent_dirs(os.path.join(svn_sr_path, f)))
-
-    # Sort by length to ensure that the parent directories are passed to svn
-    # before child directories.
-    sorted_dirs_to_update = sorted(svn_dirs_to_update, key=len)
+            svn_dirs_to_update.add(
+                os.path.dirname(os.path.join(svn_sr_path, f)))
 
     # SVN update only in the affected directories.
-    svn(svn_repo, 'update', '--depth=immediates', *sorted_dirs_to_update)
+    svn(svn_repo, 'update', '--depth=immediates', '--parents',
+        *svn_dirs_to_update)
 
     for sr, files in iteritems(subrepo_files):
         svn_sr_path = os.path.join(svn_repo, GIT_TO_SVN_DIR[sr])
         if os.name == 'nt':
             fix_eol_style_native(rev, svn_sr_path, files)
         diff = git('show', '--binary', rev, '--',
                    *(os.path.join(sr, f) for f in files),
-                   strip=False)
+                   strip=False, text=False)
         # git is the only thing that can handle its own patches...
-        log_verbose('Apply patch: %s' % diff)
         if sr == '':
             prefix_strip = '-p1'
         else:
             prefix_strip = '-p2'
         try:
-            # If we allow python to apply the diff in text mode, it will
-            # silently convert \n to \r\n which git doesn't like.
             shell(['git', 'apply', prefix_strip, '-'], cwd=svn_sr_path,
-                  stdin=diff, die_on_failure=False, force_binary_stdin=True)
+                  stdin=diff, die_on_failure=False, text=False)
         except RuntimeError as e:
             eprint("Patch doesn't apply: maybe you should try `git pull -r` "
                    "first?")
             sys.exit(2)
 
     status_lines = svn(svn_repo, 'status', '--no-ignore').split('\n')
 
     for l in (l for l in status_lines if (l.startswith('?') or