Bug 38507 – REGRESSION: svn-apply can't handle patches with leading junk

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Chris Jerdonek]

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.

[Chris Jerdonek]

Created attachment 54997 [details]
Proposed patch

[Adam Barth]

Comment on attachment 54997 [details]
Proposed patch

I'm not an expert here, but this looks sane.

[Chris Jerdonek]

Comment on attachment 54997 [details]
Proposed patch

Clearing flags on attachment: 54997

Committed r58735: <http://trac.webkit.org/changeset/58735>

[Chris Jerdonek]

All reviewed patches have been landed.  Closing bug.

[Eric Seidel (no email)]

Where are the unit tests for either of the two bugs fixed by this change?

[Chris Jerdonek]

(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]

(In reply to comment #6)
> 
> I'll file a bug report.

Filed bug report here:

https://bugs.webkit.org/show_bug.cgi?id=38519