Arcanist fails to apply patch deleting non-empty directory


#1

Observed Behavior:
Due to a change in subversion 1.10 ( ‘svn status’ without -v: Stop showing uninteresting deletions (r1664533), see https://svn.apache.org/repos/asf/subversion/trunk/CHANGES and https://svn.apache.org/viewvc?view=revision&revision=1664533) the output of svn status --xml no longer contains individual deleted files, but only their ancestor.

The ArcanistSubversionAPI::getSVNStatus then is not able to properly get a correct list of affected files when running arc diff --create which leads to issues with arc patch which attempts to delete files after the parent directory gets removed (which removes nested files as well).

Expected Behavior:
Arcanist used with SVN > 1.10 should be able to properly create and apply a patch deleting directory which contains files.

Phabricator Version:
phabricator a6e17fb702d1de3b6a38ea951bd711f72232f0c2 (Tue, Mar 12)
arcanist 9830c9316d38988b2dc283ac1a124b73bc8e6c5f (Thu, Mar 7)
phutil 34b68955748bb1b2fcbfa86ad69c0e4856d21906 (Tue, Mar 12)
php 7.2.15-0ubuntu0.18.04.1
diff 3.6 at /usr/bin/diff
git 2.17.1 at /usr/bin/git
hg Not Available
pygmentize Not Available
svn 1.11.1 at /usr/local/bin/svn

Reproduction Steps:

  1. setup a new svn repository or use an existing one

  2. create and commit a new directory containing at least one file (eg: mkdir foo && echo "bar" > foo/bar.txt && svn add foo && svn ci)

  3. remove the directory svn rm foo

  4. Create a new differential ( arc diff --create )

  5. arc patch the differential created in previous step

  6. The arc patch should fail with something similar to below:

    $ arc patch D11
    D foo
    D foo/bar.txt
    The next patch would delete the file foo/bar.txt,
    which does not exist! Assume -R? [n]
    Apply anyway? [n]
    Skipping patch.
    1 out of 1 hunk ignored

    WARNING Some hunks could not be applied cleanly by the unix ‘patch’ utility. Your working copy may be different from the revision’s base, or you may be in the wrong subdirectory. You can export the raw patch file using ‘arc export --unified’, and then try to apply it by fiddling with options to ‘patch’ (particularly, -p), or manually. The output above, from ‘patch’, may be helpful in figuring out what went wrong.

Proposed fix:
Below, I’m attaching a patch which seems to fix the issue in my testing environment:

diff --git a/src/repository/api/ArcanistSubversionAPI.php        
b/src/repository/api/ArcanistSubversionAPI.php
index d8066e96..03d2bb63 100644
--- a/src/repository/api/ArcanistSubversionAPI.php
+++ b/src/repository/api/ArcanistSubversionAPI.php
@@ -70,10 +70,10 @@ final class ArcanistSubversionAPI extends ArcanistRepositoryAPI {
     if ($this->svnStatus === null) {
   if ($this->statusPaths) {
         list($status) = $this->execxLocal(
-          '--xml status %Ls',
+          '--xml --verbose status %Ls',
           $this->statusPaths);
       } else {
-        list($status) = $this->execxLocal('--xml status');
+        list($status) = $this->execxLocal('--xml --verbose status');
       }
       $xml = new SimpleXMLElement($status);

@@ -121,6 +121,11 @@ final class ArcanistSubversionAPI extends ArcanistRepositoryAPI {
             $mask |= self::FLAG_CONFLICT;
           }

+         // Nothing happens here, just got there due to the verbose status, bail.
+         if ( $item == 'normal' && $mask == 0 ) {
+           continue;
+         }
+
           $files[$path] = $mask;
         }
       }