Call fixChangeLogPatch when generating patches from webkit-patch
Created attachment 257419 [details] Patch
Comment on attachment 257419 [details] Patch r=me
Created attachment 257464 [details] Patch for landing
Comment on attachment 257464 [details] Patch for landing "Reviewed by NOBODY (OOPS!)"
(In reply to comment #4) > Comment on attachment 257464 [details] > Patch for landing > > "Reviewed by NOBODY (OOPS!)" We should really make "webkit-patch land -g commitish" correctly handle the reviewer it got from bugzilla...
Created attachment 257465 [details] Patch for landing
Comment on attachment 257465 [details] Patch for landing Clearing flags on attachment: 257465 Committed r187357: <http://trac.webkit.org/changeset/187357>
All reviewed patches have been landed. Closing bug.
(In reply to comment #8) > All reviewed patches have been landed. Closing bug. This patch is incorrect because it makes the assumption that VCSUtils::fixChangeLogPatch() can be called with the stringified content of an arbitrary patch. But VCSUtils::fixChangeLogPatch() assumes its argument represents a ChangeLog patch as stated in an inline comment in the first line of the function body [1] and implied in the description above VCSUtils::fixChangeLogPatch() [2]. As a result of this bad assumption, webkit-patch may generate an incorrect patch file in the worst case and only adjust the diff of the first ChangeLog file in a patch in the best case. Additional remarks: An incorrect patch is only generated when its contents has the form of two consecutive ChangeLog entries that have one or more identical lines. Looking at incorrect patches generated with this patch. In the best case, this patch will have webkit-patch generate ChangeLog diffs that can only be applied with a fuzz factor > 3. One example of this can be seen in the WebCore ChangeLog diff in attachment #258971 [details], which can only be applied with fuzz factor >= 5 (the fuzz factor of the generated patch is proportional to the number of lines in the diff chunk range for the source ChangeLog file whose ChangeLog entry was adjusted). In the worst case, it can generate an incorrect patch that is not faithful to the actual changes/intentions of the author and may be difficult to identify as incorrect by eye, especially if such a patch applies without warnings/errors and a person or reviewer is not looking out for such incorrect patches. [1] <http://trac.webkit.org/browser/trunk/Tools/Scripts/VCSUtils.pm?rev=185710#L1599> [2] <http://trac.webkit.org/browser/trunk/Tools/Scripts/VCSUtils.pm?rev=185710#L1570>
(In reply to comment #7) > Comment on attachment 257465 [details] > Patch for landing > > Clearing flags on attachment: 257465 > > Committed r187357: <http://trac.webkit.org/changeset/187357> I propose that we roll out this change.
Re-opened since this is blocked by bug 148032