Fix git-llvm to not delete non-empty directories.

[jyknight]

Summary

Previously, if a directory contained only other sub-directories, one
of which was being removed, git llvm would delete the parent and all
its subdirs, even though only one should've been deleted.

This error occurred in r366590, where the commit attempted to remove
lldb/packages/Python/lldbsuite/test/tools/lldb-mi, but git-llvm
erroneously removed the entire contents of
lldb/packages/Python/lldbsuite/test/tools.

This happened because "git apply" automatically removes empty
directories locally, and the absence of a local directory was
previously taken as an indication to call 'svn rm' on that
directory. However, an empty local directory does not necessarily
indicate that the directory is truly empty.

Fix that by removing directories only when they're empty on the git
side.

[mehdi_amini]

llvm/utils/git-svn/git-llvm

370 ↗

(On Diff #212219)

I assume we can't just check here the status in git instead because the mapping from SVN -> monorepo is not straightforward to get?

[jyknight]

llvm/utils/git-svn/git-llvm

370 ↗

(On Diff #212219)

We don't have a reverse mapping at the moment.

But the larger issue is that svn status only reports the toplevel missing directory -- not the subdirs/files under it. So, we'd have to mkdir the toplevel dir, then parse svn status again, to test if we need to recreate or remove the newly-exposed subdirs, and repeat until nothing changes. That seemed a lot more complex.

I also started out thinking I'd remove the svn status parsing in favor of using the git diff status outputs for file addition too. But then realized you cannot 'svn add' files if their containing directory hasn't been added yet, and it's not entirely trivial to determine when to call 'svn add' on the dir. So, in the end I left directory/file addition alone, and only switched over deletion.

[jyknight]

.

llvm/trunk/utils/git-svn/git-llvm

def get_all_parent_dirs(name):
     head, tail = os.path.split(name)
     while head:
         parts.append(head)
         head, tail = os.path.split(head)
     return parts
 
 
 def svn_push_one_rev(svn_repo, rev, git_to_svn_mapping, dry_run):
-    files = git('diff-tree', '--no-commit-id', '--name-only', '-r',
-                rev).split('\n')
-    if not files:
+    def split_status(x):
+        x = x.split('\t')
+        return x[1], x[0]
+    files_status = [split_status(x) for x in
+                    git('diff-tree', '--no-commit-id', '--name-status',
+                        '--no-renames', '-r', rev).split('\n')]
+    if not files_status:
         raise RuntimeError('Empty diff for rev %s?' % rev)
 
     # Split files by subrepo
     subrepo_files = collections.defaultdict(list)
-    for f in files:
+    for f, st in files_status:
         subrepo, remainder = split_subrepo(f, git_to_svn_mapping)
-        subrepo_files[subrepo].append(remainder)
+        subrepo_files[subrepo].append((remainder, st))
 
     status = svn(svn_repo, 'status', '--no-ignore')
     if status:
         die("Can't push git rev %s because status in svn staging dir (%s) is "
             "not empty:\n%s" % (rev, svn_repo, status))
 
     svn_dirs_to_update = set()
-    for sr, files in iteritems(subrepo_files):
+    for sr, files_status in iteritems(subrepo_files):
         svn_sr_path = git_to_svn_mapping[sr]
-        for f in files:
+        for f, _ in files_status:
             svn_dirs_to_update.add(
                 os.path.dirname(os.path.join(svn_sr_path, f)))
 
     # We also need to svn update any parent directories which are not yet
     # present
     parent_dirs = set()
     for dir in svn_dirs_to_update:
         parent_dirs.update(get_all_parent_dirs(dir))
     parent_dirs = set(dir for dir in parent_dirs
                       if not os.path.exists(os.path.join(svn_repo, dir)))
     svn_dirs_to_update.update(parent_dirs)
 
     # 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 update only in the affected directories.
     svn(svn_repo, 'update', '--depth=files', *sorted_dirs_to_update)
 
-    for sr, files in iteritems(subrepo_files):
+    for sr, files_status in iteritems(subrepo_files):
         svn_sr_path = os.path.join(svn_repo, git_to_svn_mapping[sr])
         if os.name == 'nt':
-            fix_eol_style_native(rev, svn_sr_path, files)
+            fix_eol_style_native(rev, svn_sr_path,
+                                 [f for f, _ in files_status])
+
         # We use text=False (and pass '--binary') so that we can get an exact
         # diff that can be passed as-is to 'git apply' without any line ending,
         # encoding, or other mangling.
         diff = git('show', '--binary', rev, '--',
-                   *(os.path.join(sr, f) for f in files),
+                   *(os.path.join(sr, f) for f, _ in files_status),
                    strip=False, text=False)
         # git is the only thing that can handle its own patches...
         if sr == '':
             prefix_strip = '-p1'
         else:
             prefix_strip = '-p2'
         try:
             shell(['git', 'apply', prefix_strip, '-'], cwd=svn_sr_path,
                   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)
 
+        # Handle removed files and directories. We need to be careful not to
+        # remove directories just because they _look_ empty in the svn tree, as
+        # we might be missing sibling directories in the working copy. So, only
+        # remove parent directories if they're empty on both the git and svn
+        # sides.
+        maybe_dirs_to_remove = set()
+        for f, st in files_status:
+            if st == 'D':
+                maybe_dirs_to_remove.update(get_all_parent_dirs(f))
+                svn(svn_sr_path, 'remove', f)
+            elif not (st == 'A' or st == 'M' or st == 'T'):
+                # Add is handled below, and nothing needs to be done for Modify.
+                # (FIXME: Type-change between symlink and file might need some
+                # special handling, but let's ignore that for now.)
+                die("Unexpected git status for %r: %r" % (f, st))
+
+        maybe_dirs_to_remove = sorted(maybe_dirs_to_remove, key=len)
+        for f in maybe_dirs_to_remove:
+            if(not os.path.exists(os.path.join(svn_sr_path, f)) and
+               git('ls-tree', '-d', rev, os.path.join(sr, f)) == ''):
+                svn(svn_sr_path, 'remove', f)
+
     status_lines = svn(svn_repo, 'status', '--no-ignore').split('\n')
 
-    for l in (l for l in status_lines if (l.startswith('?') or
-                                          l.startswith('I'))):
-        svn(svn_repo, 'add', '--no-ignore', l[1:].strip())
-    for l in (l for l in status_lines if l.startswith('!')):
-        svn(svn_repo, 'remove', l[1:].strip())
+    for l in status_lines:
+        f = l[1:].strip()
+        if l.startswith('?') or l.startswith('I'):
+            svn(svn_repo, 'add', '--no-ignore', f)
 
     # Now we're ready to commit.
     commit_msg = git('show', '--pretty=%B', '--quiet', rev)
     if not dry_run:
         commit_args = ['commit', '-m', commit_msg]
         if '--force-interactive' in svn(svn_repo, 'commit', '--help'):
             commit_args.append('--force-interactive')
         log(svn(svn_repo, *commit_args))