WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
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.
Top of Page
Format For Printing
XML
Clone This Bug