29059 2009-09-08 17:45:50 -0700 svn-apply doesn't handle changes to files copied to new directories properly 2009-09-09 17:01:15 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) Mac (Intel) OS X 10.5 RESOLVED FIXED P2 Normal --- 29001 29058 1 heycam webkit-unassigned commit-queue eric oldest_to_newest 145606 0 heycam 2009-09-08 17:45:50 -0700 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. 145608 1 39236 heycam 2009-09-08 17:48:53 -0700 Created attachment 39236 Patch v1 This makes the suggested change. 145760 2 39236 eric 2009-09-09 08:10:09 -0700 Comment on attachment 39236 Patch v1 Looks sane. Ideally we'd have some unit testing, which is possible (although slightly awkward) using WebKitTools/Scripts/modules/scm_unittest.py. 145989 3 39236 commit-queue 2009-09-09 16:03:07 -0700 Comment on attachment 39236 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. 145990 4 eric 2009-09-09 16:03:39 -0700 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 145991 5 eric 2009-09-09 16:04:32 -0700 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. 145993 6 39308 heycam 2009-09-09 16:11:38 -0700 Created attachment 39308 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. 145995 7 eric 2009-09-09 16:15:59 -0700 svn-create-patch will make patches with full paths regardless of where you run it from. Unless you're running an old version somehow? 145996 8 39308 eric 2009-09-09 16:16:42 -0700 Comment on attachment 39308 Patch v2 OK. Still would love to see more unit tests for these scripts now that we have scm_unittest.py. :) 146004 9 39308 commit-queue 2009-09-09 16:41:51 -0700 Comment on attachment 39308 Patch v2 Clearing flags on attachment: 39308 Committed r48236: <http://trac.webkit.org/changeset/48236> 146005 10 commit-queue 2009-09-09 16:41:55 -0700 All reviewed patches have been landed. Closing bug. 146015 11 heycam 2009-09-09 16:58:53 -0700 (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 $ 146017 12 eric 2009-09-09 17:00:47 -0700 Such was supposed to be fixed by bug 26999. 146018 13 eric 2009-09-09 17:01:15 -0700 If you find it's regressed, please file a bug. 39236 2009-09-08 17:48:53 -0700 2009-09-09 16:11:38 -0700 Patch v1 29059-v1.patch text/plain 1271 heycam SW5kZXg6IENoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBDaGFuZ2VMb2cJKHJldmlzaW9uIDQ4MTk1 KQorKysgQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTMgQEAKKzIwMDktMDkt MDggIENhbWVyb24gTWNDb3JtYWNrICA8Y2FtQG1jYy5pZC5hdT4KKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBzdm4tYXBwbHkgZG9lc24ndCBoYW5kbGUg Y2hhbmdlcyB0byBmaWxlcyBjb3BpZWQgdG8gbmV3IGRpcmVjdG9yaWVzIHByb3Blcmx5CisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yOTA1OQorCisgICAg ICAgICogU2NyaXB0cy9zdm4tYXBwbHk6IERvbid0IHRyZWF0ICItLS0gcmV2aXNpb24gMCIgcGF0 Y2hlcyBhcyBiZWluZworICAgICAgICBhZGRpdGlvbnMgaWYgd2Uga25vdyB0aGF0IHdlJ3ZlIGp1 c3QgY29waWVkIGEgZmlsZSB0byB0aGlzIG5hbWUuCisKIDIwMDktMDktMDggIE1hcmsgUm93ZSAg PG1yb3dlQGFwcGxlLmNvbT4KIAogICAgICAgICBGaXggYW4gaW5jb3JyZWN0IHZhcmlhYmxlIG5h bWUgaW4gVXBkYXRlQ2hyb21pdW1Tb3VyY2UuCkluZGV4OiBTY3JpcHRzL3N2bi1hcHBseQo9PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09Ci0tLSBTY3JpcHRzL3N2bi1hcHBseQkocmV2aXNpb24gNDgxNDIpCisrKyBTY3JpcHRz L3N2bi1hcHBseQkod29ya2luZyBjb3B5KQpAQCAtMzgwLDcgKzM4MCw3IEBAIHN1YiBwYXRjaCgk KQogICAgIG15ICRhZGRpdGlvbiA9IDA7CiAgICAgbXkgJGlzQmluYXJ5ID0gMDsKIAotICAgICRh ZGRpdGlvbiA9IDEgaWYgKCRwYXRjaCA9fiAvXG4tLS0gLitcKHJldmlzaW9uIDBcKVxyP1xuLyB8 fCAkcGF0Y2ggPX4gL1xuQEAgLTAsMCAuKiBAQC8pOworICAgICRhZGRpdGlvbiA9IDEgaWYgKCRw YXRjaCA9fiAvXG4tLS0gLitcKHJldmlzaW9uIDBcKVxyP1xuLyB8fCAkcGF0Y2ggPX4gL1xuQEAg LTAsMCAuKiBAQC8pICYmICFleGlzdHMoJGNvcGllZEZpbGVzeyRmdWxsUGF0aH0pOwogICAgICRk ZWxldGlvbiA9IDEgaWYgJHBhdGNoID1+IC9cbkBAIC4qIFwrMCwwIEBALzsKICAgICAkaXNCaW5h cnkgPSAxIGlmICRwYXRjaCA9fiAvXG5DYW5ub3QgZGlzcGxheTogZmlsZSBtYXJrZWQgYXMgYSBi aW5hcnkgdHlwZVwuLzsKIAo= 39308 2009-09-09 16:11:38 -0700 2009-09-09 16:41:51 -0700 Patch v2 29059-v2.patch text/plain 1343 heycam SW5kZXg6IFdlYktpdFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJLaXRUb29scy9D aGFuZ2VMb2cJKHJldmlzaW9uIDQ4MTk2KQorKysgV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCSh3b3Jr aW5nIGNvcHkpCkBAIC0xLDMgKzEsMTMgQEAKKzIwMDktMDktMDkgIENhbWVyb24gTWNDb3JtYWNr ICA8Y2FtQG1jYy5pZC5hdT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICBzdm4tYXBwbHkgZG9lc24ndCBoYW5kbGUgY2hhbmdlcyB0byBmaWxlcyBjb3Bp ZWQgdG8gbmV3IGRpcmVjdG9yaWVzIHByb3Blcmx5CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yOTA1OQorCisgICAgICAgICogU2NyaXB0cy9zdm4tYXBw bHk6IERvbid0IHRyZWF0ICItLS0gcmV2aXNpb24gMCIgcGF0Y2hlcyBhcyBiZWluZworICAgICAg ICBhZGRpdGlvbnMgaWYgd2Uga25vdyB0aGF0IHdlJ3ZlIGp1c3QgY29waWVkIGEgZmlsZSB0byB0 aGlzIG5hbWUuCisKIDIwMDktMDktMDggIE1hcmsgUm93ZSAgPG1yb3dlQGFwcGxlLmNvbT4KIAog ICAgICAgICBGaXggYW4gaW5jb3JyZWN0IHZhcmlhYmxlIG5hbWUgaW4gVXBkYXRlQ2hyb21pdW1T b3VyY2UuCkluZGV4OiBXZWJLaXRUb29scy9TY3JpcHRzL3N2bi1hcHBseQo9PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0t LSBXZWJLaXRUb29scy9TY3JpcHRzL3N2bi1hcHBseQkocmV2aXNpb24gNDgxNDIpCisrKyBXZWJL aXRUb29scy9TY3JpcHRzL3N2bi1hcHBseQkod29ya2luZyBjb3B5KQpAQCAtMzgwLDcgKzM4MCw3 IEBAIHN1YiBwYXRjaCgkKQogICAgIG15ICRhZGRpdGlvbiA9IDA7CiAgICAgbXkgJGlzQmluYXJ5 ID0gMDsKIAotICAgICRhZGRpdGlvbiA9IDEgaWYgKCRwYXRjaCA9fiAvXG4tLS0gLitcKHJldmlz aW9uIDBcKVxyP1xuLyB8fCAkcGF0Y2ggPX4gL1xuQEAgLTAsMCAuKiBAQC8pOworICAgICRhZGRp dGlvbiA9IDEgaWYgKCRwYXRjaCA9fiAvXG4tLS0gLitcKHJldmlzaW9uIDBcKVxyP1xuLyB8fCAk cGF0Y2ggPX4gL1xuQEAgLTAsMCAuKiBAQC8pICYmICFleGlzdHMoJGNvcGllZEZpbGVzeyRmdWxs UGF0aH0pOwogICAgICRkZWxldGlvbiA9IDEgaWYgJHBhdGNoID1+IC9cbkBAIC4qIFwrMCwwIEBA LzsKICAgICAkaXNCaW5hcnkgPSAxIGlmICRwYXRjaCA9fiAvXG5DYW5ub3QgZGlzcGxheTogZmls ZSBtYXJrZWQgYXMgYSBiaW5hcnkgdHlwZVwuLzsKIAo=