Bug 34871 - svn-apply errors out when removing directories in git
Summary: svn-apply errors out when removing directories in git
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 34879
  Show dependency treegraph
 
Reported: 2010-02-11 19:08 PST by Eric Seidel (no email)
Modified: 2010-04-11 13:19 PDT (History)
3 users (show)

See Also:


Attachments
[PATCH] Remove Directories in Git (1.52 KB, patch)
2010-04-11 01:34 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[PATCH] With Comment (2.04 KB, patch)
2010-04-11 12:55 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
[PATCH] With Comment and Updated die() message (2.05 KB, patch)
2010-04-11 13:00 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2010-02-11 19:08:11 PST
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
Comment 1 Eric Seidel (no email) 2010-04-09 00:03:43 PDT
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()
Comment 2 Joseph Pecoraro 2010-04-11 01:34:16 PDT
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.
Comment 3 Eric Seidel (no email) 2010-04-11 11:24:15 PDT
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?
Comment 4 Joseph Pecoraro 2010-04-11 11:49:04 PDT
(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.
Comment 5 Joseph Pecoraro 2010-04-11 11:55:43 PDT
(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
Comment 6 Eric Seidel (no email) 2010-04-11 12:40:41 PDT
OK.  We just need to add a comment in the code to explain why we're using ignore-unmatch then.
Comment 7 Joseph Pecoraro 2010-04-11 12:55:05 PDT
Created attachment 53095 [details]
[PATCH] With Comment
Comment 8 Joseph Pecoraro 2010-04-11 13:00:10 PDT
Created attachment 53096 [details]
[PATCH] With Comment and Updated die() message

I update the die message to be accurate if this change gets approved.
Comment 9 Eric Seidel (no email) 2010-04-11 13:03:19 PDT
Comment on attachment 53096 [details]
[PATCH] With Comment and Updated die() message

LGTM.
Comment 10 WebKit Commit Bot 2010-04-11 13:19:09 PDT
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>
Comment 11 WebKit Commit Bot 2010-04-11 13:19:15 PDT
All reviewed patches have been landed.  Closing bug.