Bug 29065

Summary:

svn-unapply doesn't revert directories correctly

Product:

WebKit

Reporter:

Cameron McCormack (:heycam) <heycam>

Component:

Tools / Tests

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Minor

CC:

commit-queue

Priority:

Version:

528+ (Nightly build)

Hardware:

Mac (Intel)

OS:

OS X 10.5

Attachments:

Description	Flags
Patch v1	eric: review-
Patch v2	none

Cameron McCormack (:heycam)

Reported 2009-09-08 18:45:47 PDT

Trying to unapply a patch that added a directory will not revert and then remove that directory: fox:~/svn/WebKit-2/WebKitTools $ cat /tmp/c.patch Index: NewDirectory/NewFile =================================================================== --- NewDirectory/NewFile (revision 0) +++ NewDirectory/NewFile (revision 0) @@ -0,0 +1 @@ + fox:~/svn/WebKit-2/WebKitTools $ ./Scripts/svn-apply /tmp/c.patch A NewDirectory patching file NewDirectory/NewFile A NewDirectory/NewFile fox:~/svn/WebKit-2/WebKitTools $ ./Scripts/svn-unapply /tmp/c.patch patching file NewDirectory/NewFile Reverted 'NewDirectory/NewFile' A NewDirectoryfox:~/svn/WebKit-2/WebKitTools $ Notice also that the svn status line printed by svn-unapply doesn't have a newline. Similarly, trying to unapply a patch that removed a directory will not revert its deletion: fox:~/svn/WebKit-2/WebKitTools $ svn rm record-memory-win D record-memory-win/record-memory-win.vcproj D record-memory-win/main.cpp D record-memory-win fox:~/svn/WebKit-2/WebKitTools $ Scripts/svn-create-patch >/tmp/d.patch fox:~/svn/WebKit-2/WebKitTools $ ./Scripts/svn-unapply /tmp/d.patch patching file record-memory-win/main.cpp Reverted 'record-memory-win/main.cpp' patching file record-memory-win/record-memory-win.vcproj Reverted 'record-memory-win/record-memory-win.vcproj' D record-memory-winfox:~/svn/WebKit-2/WebKitTools $ fox:~/svn/WebKit-2/WebKitTools $ svn status record-memory-win D record-memory-win

Attachments
Patch v1 (2.55 KB, patch) 2009-09-08 18:51 PDT, Cameron McCormack (:heycam)	eric: review-	Details Formatted Diff Diff
Patch v2 (4.16 KB, patch) 2009-09-09 16:34 PDT, Cameron McCormack (:heycam)	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Cameron McCormack (:heycam)

Comment 1 2009-09-08 18:51:47 PDT

Created attachment 39242 [details] Patch v1 The problem is that svnStatus() returns a status line with a newline from one branch, but without a newline from the other branch. That caused the regexes in revertDirectories() to fail to match, because they were looking for a \n at the end.

Eric Seidel (no email)

Comment 2 2009-09-09 08:01:34 PDT

Comment on attachment 39242 [details] Patch v1 it seems this repeated regexp really should be in a function, or at least as a constant. Also, I think perl is slightly more readable when you make use of $_ explicit, so I prefer the old code.

Eric Seidel (no email)

Comment 3 2009-09-09 08:01:48 PDT

Comment on attachment 39242 [details] Patch v1 r- for my personal taste nits.

Cameron McCormack (:heycam)

Comment 4 2009-09-09 16:34:30 PDT

Created attachment 39309 [details] Patch v2 (In reply to comment #2) > it seems this repeated regexp really should be in a function, or at least as a > constant. Done. Note also I fixed a small bug -- the second s/// I added was being applied to $_ when it should have been applied to $svnStatus. > Also, I think perl is slightly more readable when you make use of $_ explicit, > so I prefer the old code. I'd argue that leaving off the "$_ =~" is more idiomatic Perl, but OK, done. Perhaps if I'd included it I would've picked up the above bug first time around. :-)

Eric Seidel (no email)

Comment 5 2009-09-09 17:08:20 PDT

Comment on attachment 39309 [details] Patch v2 OK. I guess these could move to VCSTools.pm at some point... I think it's about time to get you commit bit. I'll look into it.

WebKit Commit Bot

Comment 6 2009-09-09 17:59:57 PDT

Comment on attachment 39309 [details] Patch v2 Rejecting patch 39309 from commit-queue. This patch will require manual commit. ['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--quiet', '--exit-after-n-failures=1'] failed with exit code 1

Eric Seidel (no email)

Comment 7 2009-09-09 18:45:16 PDT

Comment on attachment 39309 [details] Patch v2 The media madness never ends! compositing/color-matching/image-color-matching.html -> timed out bug 28624.

WebKit Commit Bot

Comment 8 2009-09-09 22:17:53 PDT

Comment on attachment 39309 [details] Patch v2 Clearing flags on attachment: 39309 Committed r48242: <http://trac.webkit.org/changeset/48242>

WebKit Commit Bot

Comment 9 2009-09-09 22:17:57 PDT

All reviewed patches have been landed. Closing bug.

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