Bug 29059

Summary: svn-apply doesn't handle changes to files copied to new directories properly
Product: WebKit Reporter: Cameron McCormack (:heycam) <heycam>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac (Intel)   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 29001, 29058    
Attachments:
Description Flags
Patch v1
eric: review+, commit-queue: commit-queue-
Patch v2 none

Cameron McCormack (:heycam)
Reported 2009-09-08 17:45:50 PDT
The output of svn-create-patch trips up svn-apply when a file has been copied to a newly added directory and changes have been made to the copy. Compare these two diffs: $ cd WebKitTools $ svn cp Makefile Scripts # Copy file into an existing directory. A Scripts/Makefile $ echo >>Makefile # Make a change to the copy. $ svn-create-patch | egrep '^(--|\+\+)' --- Scripts/Makefile (revision 48142) (from Makefile:48142) +++ Scripts/Makefile (working copy) --- Scripts/Makefile (revision 48142) +++ Scripts/Makefile (working copy) and: $ cd WebKitTools $ mkdir NewDirectory $ svn add NewDirectory A NewDirectory $ svn cp Makefile NewDirectory # Copy file into a new directory. A NewDirectory/Makefile $ echo >>Makefile # Make a change to the copy. $ svn-create-patch | egrep '^(--|\+\+)' --- NewDirectory/Makefile (revision 48142) (from Makefile:48142) +++ NewDirectory/Makefile (working copy) --- NewDirectory/Makefile (revision 0) +++ NewDirectory/Makefile (working copy) So when the diff of the copied file in the new directory is created it says "revision 0", making svn-apply think that it is a new file, whereas it has already copied it in the first part of the patch. svn-apply moves the copied file out of the way to Makefile.orig and then fails when attempting to apply the second part of the patch (since NewDirectory/Makefile doesn't exist now). I guess that the "revision 0" output comes from "svn diff" itself; you can see that "svn info" gives different revision info for the two copies: $ svn info Scripts/Makefile | grep ^Revision Revision: 48142 $ svn info NewDirectory/Makefile | grep ^Revision Revision: 0 So one way to fix this would be to change svn-apply to recognise this case and not treat the second part of the patch there as a new file.
Attachments
Patch v1 (1.24 KB, patch)
2009-09-08 17:48 PDT, Cameron McCormack (:heycam) 		eric: review+
commit-queue: commit-queue-
DetailsFormatted DiffDiff
Patch v2 (1.31 KB, patch)
2009-09-09 16:11 PDT, Cameron McCormack (:heycam) 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Cameron McCormack (:heycam)
Comment 1 2009-09-08 17:48:53 PDT
Created attachment 39236 [details] Patch v1 This makes the suggested change.
Eric Seidel (no email)
Comment 2 2009-09-09 08:10:09 PDT
Comment on attachment 39236 [details] Patch v1 Looks sane. Ideally we'd have some unit testing, which is possible (although slightly awkward) using WebKitTools/Scripts/modules/scm_unittest.py.
WebKit Commit Bot
Comment 3 2009-09-09 16:03:07 PDT
Comment on attachment 39236 [details] Patch v1 Rejecting patch 39236 from commit-queue. This patch will require manual commit. Patch https://bugs.webkit.org/attachment.cgi?id=39236 from bug 29059 failed to download and apply.
Eric Seidel (no email)
Comment 4 2009-09-09 16:03:39 PDT
Hunk #1 succeeded at 1 with fuzz 3. can't find file to patch at input line 5 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- |Index: Scripts/svn-apply |=================================================================== |--- Scripts/svn-apply (revision 48142) |+++ Scripts/svn-apply (working copy) -------------------------- No file to patch. Skipping patch. 1 out of 1 hunk ignored
Eric Seidel (no email)
Comment 5 2009-09-09 16:04:32 PDT
Looks like you didn't use svn-create-patch to make this. It's been made from the WebKitTools directory instead of the root. svn-create-patch should automatically create patches with full paths for you.
Cameron McCormack (:heycam)
Comment 6 2009-09-09 16:11:38 PDT
Created attachment 39308 [details] Patch v2 Huh. I'm pretty sure I did use svn-create-patch, but I probably did run it from WebKitTools/ (just to save it churning through the rest of the checkout for minutes only to find no changes). Anyway, here's one with full paths.
Eric Seidel (no email)
Comment 7 2009-09-09 16:15:59 PDT
svn-create-patch will make patches with full paths regardless of where you run it from. Unless you're running an old version somehow?
Eric Seidel (no email)
Comment 8 2009-09-09 16:16:42 PDT
Comment on attachment 39308 [details] Patch v2 OK. Still would love to see more unit tests for these scripts now that we have scm_unittest.py. :)
WebKit Commit Bot
Comment 9 2009-09-09 16:41:51 PDT
Comment on attachment 39308 [details] Patch v2 Clearing flags on attachment: 39308 Committed r48236: <http://trac.webkit.org/changeset/48236>
WebKit Commit Bot
Comment 10 2009-09-09 16:41:55 PDT
All reviewed patches have been landed. Closing bug.
Cameron McCormack (:heycam)
Comment 11 2009-09-09 16:58:53 PDT
(In reply to comment #7) > svn-create-patch will make patches with full paths regardless of where you run > it from. Unless you're running an old version somehow? Seems not (at least not on my machine): $ pwd /Users/cam/svn/WebKit-2/WebKitTools $ echo >>Scripts/svn-create-patch $ ./Scripts/svn-create-patch Index: Scripts/svn-create-patch =================================================================== --- Scripts/svn-create-patch (revision 48236) +++ Scripts/svn-create-patch (working copy) @@ -457,3 +457,4 @@ sub testfilecmp($$) return $fileDataA->{isTestFile} <=> $fileDataB->{isTestFile}; } + $ svn diff -rBASE:HEAD Scripts/svn-create-patch $
Eric Seidel (no email)
Comment 12 2009-09-09 17:00:47 PDT
Such was supposed to be fixed by bug 26999.
Eric Seidel (no email)
Comment 13 2009-09-09 17:01:15 PDT
If you find it's regressed, please file a bug.
Note You need to log in before you can comment on or make changes to this bug.