Bug 34871 – svn-apply errors out when removing directories in git

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

[Eric Seidel (no email)]

svn-apply errors out when removing directories in git

https://webkit-commit-queue.appspot.com/results/259424
Failed to run "['/Users/eseidel/Projects/MacEWS/WebKitTools/Scripts/svn-apply', '--reviewer', 'Adam Barth', '--force']" exit_code: 2

Could not open 'LayoutTests/fast/wcss' to list files: 0 at /Users/eseidel/Projects/MacEWS/WebKitTools/Scripts/svn-apply line 314, <> line 573.

https://bugs.webkit.org/attachment.cgi?id=48606

[Eric Seidel (no email)]

There are two comments about this bug in scm_unittest.py which should be removed or updated when/if this bug is ever fixed.  One in SVNTestRepository.setUp() and one in GitTest.test_apply_git_patch()

[Joseph Pecoraro]

Created attachment 53082 [details]
[PATCH] Remove Directories in Git

Note that git rm's "--ignore-unmatch" is available on my system's (Snow Leopard) default git version 1.6.2 and in the newest git 1.7+. I'm pretty sure it would be available on any system with git installed, but I don't know what the rule is here.

I tested this on a git checkout:

  - remove a directory
  - create a patch > $patch
  - reset hard
  - apply patch to svn-apply
  - svn-apply --force $patch
  - SUCCEEDED

I have not tested this on a svn checkout.

[Eric Seidel (no email)]

Clearly "ignore-unmatch" hides the error.  But why do we have the error in the first place?  Do we try to remove the directory twice?

[Joseph Pecoraro]

(In reply to comment #3)
> Clearly "ignore-unmatch" hides the error.  But why do we have the error in the
> first place?  Do we try to remove the directory twice?

I'm guessing git actually removes the directory when it becomes empty (the files are removed before the directory). I'll investigate this.

[Joseph Pecoraro]

(In reply to comment #4)
> (In reply to comment #3)
> > Clearly "ignore-unmatch" hides the error.  But why do we have the error in the
> > first place?  Do we try to remove the directory twice?
> 
> I'm guessing git actually removes the directory when it becomes empty (the
> files are removed before the directory). I'll investigate this.

Looks like it. As you know, git doesn't handle empty directories very well. When you `git rm` all the files in a directory (making the directory empty) git automatically removes the directory. I whipped up a sample git repo to verify this behavior:

    # Directory bar with two files
    shell> ls -l
    total 0
    drwxr-xr-x  4 pecoraro_mbp  staff   136B Apr 11 11:50 bar/
    shell> ls -l bar/
    total 0
    -rw-r--r--  1 pecoraro_mbp  staff     0B Apr 11 11:50 a.txt
    -rw-r--r--  1 pecoraro_mbp  staff     0B Apr 11 11:50 b.txt
    
    # Remove a file, the directory still exists
    shell> git rm bar/a.txt
    rm 'bar/a.txt'
    shell> ls -l
    total 0
    drwxr-xr-x  4 pecoraro_mbp  staff   136B Apr 11 11:50 bar/
    
    # Remove the last file, the directory gets deleted
    shell> git rm bar/b.txt
    rm 'bar/b.txt'
    shell> ls -l
    # NO OUTPUT
    
    # Trying to remove what was already removed? Priceless
    shell> git rm bar
    fatal: pathspec 'bar' did not match any files

[Eric Seidel (no email)]

OK.  We just need to add a comment in the code to explain why we're using ignore-unmatch then.

[Joseph Pecoraro]

Created attachment 53095 [details]
[PATCH] With Comment

[Joseph Pecoraro]

Created attachment 53096 [details]
[PATCH] With Comment and Updated die() message

I update the die message to be accurate if this change gets approved.

[Eric Seidel (no email)]

Comment on attachment 53096 [details]
[PATCH] With Comment and Updated die() message

LGTM.

[WebKit Commit Bot]

Comment on attachment 53096 [details]
[PATCH] With Comment and Updated die() message

Clearing flags on attachment: 53096

Committed r57455: <http://trac.webkit.org/changeset/57455>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.