38507 2010-05-03 22:41:12 -0700 REGRESSION: svn-apply can't handle patches with leading junk 2010-05-04 08:09:57 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 cjerdonek cjerdonek abarth cjerdonek dbates eric oldest_to_newest 220478 0 cjerdonek 2010-05-03 22:41:12 -0700 svn-apply no longer handles patches with leading junk, e.g. e-mail diffs: https://bugs.webkit.org/show_bug.cgi?id=38439#c5 https://bugs.webkit.org/show_bug.cgi?id=38455#c6 The explanation is as follows: Previously, if a patch contained leading junk, the code for svn-apply would parse the leading junk into its own "patch" in the array of patches to apply. When svn-apply's patch() subroutine got to this bad patch, svn-apply would skip over it provided the --force option was used (which is what the commit-queue uses). After the following revision-- http://trac.webkit.org/changeset/58495 svn-apply combined any leading junk in with the patch following it. This meant that svn-apply's patch() subroutine would now skip over the patch corresponding to that file whenever the --force option was used. In cases where the ChangeLog entry was the first valid patch after any leading junk (e.g. in the case of many e-mail diffs), this meant that svn-apply would skip over applying the ChangeLog part of the patch -- leading to error messages of the form "Found no modified ChangeLogs". One possible solution to this issue is pretty easy and will also solve the following issue: https://bugs.webkit.org/show_bug.cgi?id=33119 I will submit a fix shortly. 220480 1 54997 cjerdonek 2010-05-03 23:03:50 -0700 Created attachment 54997 Proposed patch 220481 2 54997 abarth 2010-05-03 23:08:30 -0700 Comment on attachment 54997 Proposed patch I'm not an expert here, but this looks sane. 220482 3 54997 cjerdonek 2010-05-03 23:14:46 -0700 Comment on attachment 54997 Proposed patch Clearing flags on attachment: 54997 Committed r58735: <http://trac.webkit.org/changeset/58735> 220483 4 cjerdonek 2010-05-03 23:14:57 -0700 All reviewed patches have been landed. Closing bug. 220578 5 eric 2010-05-04 07:09:40 -0700 Where are the unit tests for either of the two bugs fixed by this change? 220590 6 cjerdonek 2010-05-04 08:05:24 -0700 (In reply to comment #5) > Where are the unit tests for either of the two bugs fixed by this change? Bug 33119 is a special case of this bug so it can be handled by the same test cases. The problem was in an area of the Perl code that is not currently covered by any unit tests (the patch() subroutines of both the svn-apply file and the svn-unapply file). It would take a bit more scaffolding or refactoring to add test coverage to that change. The most that could probably be done at this point would be to add an "end-to-end" test to scm_unittest.py that would run as part of "test-webkitpy --all". Perhaps we could get away with simply adding leading junk to a couple of the existing unit test cases: one for Git and one for SVN. I'll file a bug report. 220593 7 cjerdonek 2010-05-04 08:09:57 -0700 (In reply to comment #6) > > I'll file a bug report. Filed bug report here: https://bugs.webkit.org/show_bug.cgi?id=38519 54997 2010-05-03 23:03:50 -0700 2010-05-03 23:14:46 -0700 Proposed patch _patch-38507-1.diff text/plain 2508 cjerdonek ZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL0NoYW5nZUxvZyBiL1dlYktpdFRvb2xzL0NoYW5nZUxv ZwppbmRleCA3NjJlN2E2Li42ZWJjNWE1IDEwMDY0NAotLS0gYS9XZWJLaXRUb29scy9DaGFuZ2VM b2cKKysrIGIvV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsMjcgQEAKIDIwMTAtMDUt MDMgIENocmlzIEplcmRvbmVrICA8Y2plcmRvbmVrQHdlYmtpdC5vcmc+CiAKKyAgICAgICAgUmV2 aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgRml4ZWQgYSByZWNlbnQgUkVHUkVT U0lPTiB0aGF0IGNhdXNlZCBzdm4tYXBwbHkgYW5kIC11bmFwcGx5IHRvCisgICAgICAgIHNraXAg b3ZlciBjaGFuZ2VzIHRvIHRoZSBmaXJzdCBmaWxlIGluIGEgZGlmZiBpZiBsZWFkaW5nIGp1bmsg d2FzCisgICAgICAgIHByZXNlbnQgKGxpa2UgaW4gYW4gZS1tYWlsIGRpZmYpIGFuZCBpZiB0aGUg LS1mb3JjZSBvcHRpb24gd2FzIHVzZWQuCisKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTM4NTA3CisKKyAgICAgICAgKiBTY3JpcHRzL3N2bi1hcHBseToK KyAgICAgICAgICAtIFJlbW92ZWQgdGhlIGJpdCBvZiBjb2RlIGF0IHRoZSBiZWdpbm5pbmcgb2Yg dGhlIHBhdGNoKCkKKyAgICAgICAgICAgIHN1YnJvdXRpbmUgdGhhdCBjaGVja3MgZm9yIHRoZSAi SW5kZXg6IiBsaW5lIGF0IHRoZSBiZWdpbm5pbmcKKyAgICAgICAgICAgIG9mIGEgZmlsZSBkaWZm IChzaW5jZSB0aGUgcGFyc2VQYXRjaCgpIHN1YnJvdXRpbmUgYWxyZWFkeQorICAgICAgICAgICAg Y2hlY2tzIHRoaXMpLgorCisgICAgICAgICogU2NyaXB0cy9zdm4tdW5hcHBseToKKyAgICAgICAg ICAtIFJlbW92ZWQgdGhlIGJpdCBvZiBjb2RlIGF0IHRoZSBiZWdpbm5pbmcgb2YgdGhlIHBhdGNo KCkKKyAgICAgICAgICAgIHN1YnJvdXRpbmUgdGhhdCBjaGVja3MgZm9yIHRoZSAiSW5kZXg6IiBs aW5lIGF0IHRoZSBiZWdpbm5pbmcKKyAgICAgICAgICAgIG9mIGEgZmlsZSBkaWZmIChzaW5jZSB0 aGUgcGFyc2VQYXRjaCgpIHN1YnJvdXRpbmUgYWxyZWFkeQorICAgICAgICAgICAgY2hlY2tzIHRo aXMpLgorCisyMDEwLTA1LTAzICBDaHJpcyBKZXJkb25layAgPGNqZXJkb25la0B3ZWJraXQub3Jn PgorCiAgICAgICAgIFVucmV2aWV3ZWQuCiAKICAgICAgICAgQWRqdXN0ZWQgdGhlIENoYW5nZUxv ZyBlbnRyeSBiZWxvdyBmb3IgcjU4NzMyIChidWcgMzU4MDQpIHRvIHJlZmxlY3QKZGlmZiAtLWdp dCBhL1dlYktpdFRvb2xzL1NjcmlwdHMvc3ZuLWFwcGx5IGIvV2ViS2l0VG9vbHMvU2NyaXB0cy9z dm4tYXBwbHkKaW5kZXggYTg2YzEyMy4uY2Q3NWM5OSAxMDA3NTUKLS0tIGEvV2ViS2l0VG9vbHMv U2NyaXB0cy9zdm4tYXBwbHkKKysrIGIvV2ViS2l0VG9vbHMvU2NyaXB0cy9zdm4tYXBwbHkKQEAg LTMwNCwxNCArMzA0LDcgQEAgc3ViIHBhdGNoKCQpCiAgICAgbXkgKCRkaWZmSGFzaFJlZikgPSBA XzsKIAogICAgIG15ICRwYXRjaCA9ICRkaWZmSGFzaFJlZi0+e3N2bkNvbnZlcnRlZFRleHR9Owot Ci0gICAgdW5sZXNzICgkcGF0Y2ggPX4gbXxeSW5kZXg6IChbXlxyXG5dKyl8KSB7Ci0gICAgICAg IG15ICRzZXBhcmF0b3IgPSAnLScgeCA2NzsKLSAgICAgICAgd2FybiAiRmFpbGVkIHRvIGZpbmQg J0luZGV4OicgaW46XG4kc2VwYXJhdG9yXG4kcGF0Y2hcbiRzZXBhcmF0b3JcbiI7Ci0gICAgICAg IGRpZSB1bmxlc3MgJGZvcmNlOwotICAgICAgICByZXR1cm47Ci0gICAgfQotICAgIG15ICRmdWxs UGF0aCA9ICQxOworICAgIG15ICRmdWxsUGF0aCA9ICRkaWZmSGFzaFJlZi0+e2luZGV4UGF0aH07 CiAKICAgICBteSAkZGVsZXRpb24gPSAwOwogICAgIG15ICRhZGRpdGlvbiA9IDA7CmRpZmYgLS1n aXQgYS9XZWJLaXRUb29scy9TY3JpcHRzL3N2bi11bmFwcGx5IGIvV2ViS2l0VG9vbHMvU2NyaXB0 cy9zdm4tdW5hcHBseQppbmRleCA0NzViZjY4Li40NjVhZDU3IDEwMDc1NQotLS0gYS9XZWJLaXRU b29scy9TY3JpcHRzL3N2bi11bmFwcGx5CisrKyBiL1dlYktpdFRvb2xzL1NjcmlwdHMvc3ZuLXVu YXBwbHkKQEAgLTE0MCwxMyArMTQwLDggQEAgc3ViIHBhdGNoKCQpCiAgICAgbXkgKCRkaWZmSGFz aFJlZikgPSBAXzsKIAogICAgIG15ICRwYXRjaCA9ICRkaWZmSGFzaFJlZi0+e3N2bkNvbnZlcnRl ZFRleHR9OworICAgIG15ICRmdWxsUGF0aCA9ICRkaWZmSGFzaFJlZi0+e2luZGV4UGF0aH07CiAK LSAgICB1bmxlc3MgKCRwYXRjaCA9fiBtfF5JbmRleDogKFteXHJcbl0rKXwpIHsKLSAgICAgICAg bXkgJHNlcGFyYXRvciA9ICctJyB4IDY3OwotICAgICAgICB3YXJuICJGYWlsZWQgdG8gZmluZCAn SW5kZXg6JyBpbjpcbiRzZXBhcmF0b3JcbiRwYXRjaFxuJHNlcGFyYXRvclxuIjsKLSAgICAgICAg cmV0dXJuOwotICAgIH0KLSAgICBteSAkZnVsbFBhdGggPSAkMTsKICAgICAkZGlyZWN0b3JpZXNU b0NoZWNre2Rpcm5hbWUoJGZ1bGxQYXRoKX0gPSAxOwogCiAgICAgbXkgJGRlbGV0aW9uID0gMDsK