RESOLVED FIXED 38507
REGRESSION: svn-apply can't handle patches with leading junk
https://bugs.webkit.org/show_bug.cgi?id=38507
Summary REGRESSION: svn-apply can't handle patches with leading junk
Chris Jerdonek
Reported 2010-05-03 22:41:12 PDT
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.
Attachments
Proposed patch (2.45 KB, patch)
2010-05-03 23:03 PDT, Chris Jerdonek
no flags
Chris Jerdonek
Comment 1 2010-05-03 23:03:50 PDT
Created attachment 54997 [details] Proposed patch
Adam Barth
Comment 2 2010-05-03 23:08:30 PDT
Comment on attachment 54997 [details] Proposed patch I'm not an expert here, but this looks sane.
Chris Jerdonek
Comment 3 2010-05-03 23:14:46 PDT
Comment on attachment 54997 [details] Proposed patch Clearing flags on attachment: 54997 Committed r58735: <http://trac.webkit.org/changeset/58735>
Chris Jerdonek
Comment 4 2010-05-03 23:14:57 PDT
All reviewed patches have been landed. Closing bug.
Eric Seidel (no email)
Comment 5 2010-05-04 07:09:40 PDT
Where are the unit tests for either of the two bugs fixed by this change?
Chris Jerdonek
Comment 6 2010-05-04 08:05:24 PDT
(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.
Chris Jerdonek
Comment 7 2010-05-04 08:09:57 PDT
(In reply to comment #6) > > I'll file a bug report. Filed bug report here: https://bugs.webkit.org/show_bug.cgi?id=38519
Note You need to log in before you can comment on or make changes to this bug.