Bug 45424

Summary:

svn-apply tries to delete directories it shouldn't

Product:

WebKit

Reporter:

Mihai Parparita <mihaip>

Component:

Tools / Tests

Assignee:

Mihai Parparita <mihaip>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

dbates, dglazkov, eric

Priority:

Version:

528+ (Nightly build)

Hardware:

Other

OS:

All

Attachments:

Description	Flags
Patch	none

Mihai Parparita

Reported 2010-09-08 17:18:25 PDT

svn-apply tries to delete directories it shouldn't

Attachments
Patch (1.44 KB, patch) 2010-09-08 17:32 PDT, Mihai Parparita	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Mihai Parparita

Comment 1 2010-09-08 17:29:43 PDT

Eric, can you review this? This is why the EWS failed for bug 45307 (see output at https://webkit-commit-queue.appspot.com/results/3961246). It called isDirectoryEmptyForRemoval on LayoutTests/platform/mac/http. That only has one item in it, the tests subdirectory. We ended up calling scmWillDeleteFile("LayoutTests/platform/mac/http/tests"), which returned true since there were files deleted in it (and for the git case we just look for a leading D).

Mihai Parparita

Comment 2 2010-09-08 17:32:46 PDT

Created attachment 66971 [details] Patch

Eric Seidel (no email)

Comment 3 2010-09-09 12:11:56 PDT

Dan is the man.

Eric Seidel (no email)

Comment 4 2010-09-09 12:12:42 PDT

Comment on attachment 66971 [details] Patch We've been trying to unittest our perl code more. I'm not sure we have a way to test code which is still in svn-apply directly (instead of being in a seprate perl module), but Dan would know. What about svn-unapply?

Daniel Bates

Comment 5 2010-09-09 13:55:06 PDT

(In reply to comment #4) > (From update of attachment 66971 [details]) > We've been trying to unittest our perl code more. I'm not sure we have a way to test code which is still in svn-apply directly (instead of being in a seprate perl module), but Dan would know. What about svn-unapply? Unfortunately, we don't have a way to test code in svn-apply and svn-unapply directly at this time. This function is specific to svn-apply.

Daniel Bates

Comment 6 2010-09-09 13:55:34 PDT

Comment on attachment 66971 [details] Patch r=me.

Mihai Parparita

Comment 7 2010-09-09 13:58:33 PDT

(In reply to comment #6) > (From update of attachment 66971 [details]) > r=me. Thanks. Mind landing this (I don't have committer access yet)?

Daniel Bates

Comment 8 2010-09-09 14:00:27 PDT

(In reply to comment #7) > (In reply to comment #6) > > (From update of attachment 66971 [details] [details]) > > r=me. > > Thanks. Mind landing this (I don't have committer access yet)? Sure.

Daniel Bates

Comment 9 2010-09-09 14:08:00 PDT

Comment on attachment 66971 [details] Patch Clearing flags on attachment: 66971 Committed r67115: <http://trac.webkit.org/changeset/67115>

Daniel Bates

Comment 10 2010-09-09 14:08:09 PDT

All reviewed patches have been landed. Closing bug.

Eric Seidel (no email)

Comment 11 2010-09-09 14:28:40 PDT

You can also always set cq? :) Thanks dan for the info and landage!

Daniel Bates

Comment 12 2010-09-09 14:30:30 PDT

(In reply to comment #11) > You can also always set cq? :) Thanks dan for the info and landage! Yes, I normally would cq+ such a patch, but the queue looked pretty full at the time.

Note You need to log in before you can comment on or make changes to this bug.