Bug 9571 – Teach svn-apply and svn-unapply to handle directory adds and removes better

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[David Kilzer (:ddkilzer)]

Currently svn-apply does a chdir() to every directory it adds.  This is unnecessary.  See:

http://bugzilla.opendarwin.org/show_bug.cgi?id=9485#c6

The svn-apply script doesn't know when to remove directories, e.g., when they are empty after all files and directories under them have been marked for removal.

Finally, the svn-unapply script doesn't know how to remove previously-added directories or add (revert) previously removed directories.

[David Kilzer (:ddkilzer)]

Created attachment 9013 [details]
Patch v1

Changes to svn-apply:

- Sorted subroutines alphabetically; handleBinaryChange() moved but did not change; addDirectoriesIfNeeded() was moved and had major changes.
- Changed addDirectoriesIfNeeded() subroutine to operation without changing directories.  Keep track of directories checked for adding in %checkedDirectories global.  Removed $startDir global variable.
- Extracted svnStatus() subroutine from existing code.
- Added removeDirectoriesIfNeeded() subroutine to check for empty directories.  It is called after all patches have been applied.
- Added isDirectoryEmptyForRemoval() subroutine used by removeDirectoriesIfNeeded() to determine if a directory is empty or not.  This sub uses the %removeDirectoryIgnoreList global to filter out directories to ignore when checking if a given directory is empty.  Note that "return" can't be used in the grep{} code block, otherwise that will cause the entire function to return.

Changes to svn-unapply:

- Sorted subroutines alphabetically; unapplyPatch() was moved but did not change.
- Extracted svnStatus() subroutine from existing code.
- Removed unused $startDir global variable.
- Added revertDirectories() subroutine.  Uses %directoriesToCheck global variable when running.  This variable is set as patches are unapplied.

[Darin Adler]

Comment on attachment 9013 [details]
Patch v1

+        if (exists $removeDirectoryIgnoreList{$_}) {
+            0;
+        }
+        elsif (! -d File::Spec->catdir($dir, $_)) {
+            1;
+        }
+        else {

Our formatting would be:

    } elsif

not

    }
    elsif

I'm not sure that svn revert is the right thing to do in svn-unapply, but I can't think of any other way to bring the file back. Ideally the file would come back with the contents from the patch rather than the current contents in Subversion?

r=me

[David Kilzer (:ddkilzer)]

Committed revision 15028.

Fixed formatting in svn-apply in revision 15029.

[David Kilzer (:ddkilzer)]

(In reply to comment #2)
> I'm not sure that svn revert is the right thing to do in svn-unapply, but I
> can't think of any other way to bring the file back. Ideally the file would
> come back with the contents from the patch rather than the current contents in
> Subversion?

This will get fixed when the "svn revert" todo is done.  The svn-unapply script should do a smart comparison between the original (from "svn revert") and the reversed patch.