Bug 46061 – svn-apply updates date of wrong change log entry for a change log diff that contains two consecutive entries with the same author and date

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

[Daniel Bates]

Consider the following change log patch:

--- ChangeLog
+++ ChangeLog
@@ -70,6 +70,14 @@

 2010-09-18  Alice  <[e-mail]>

+        Reviewed by NOBODY (OOPS!).
+
+        Changed some more code on 2010-09-18.
+
+        * File:
+
[phone]  Alice  <[e-mail]>
+
         Reviewed by Ray.

         Changed some code on 2010-09-18.

Suppose we apply this patch using svn-apply on 09/19/2010. Looking at the resulting ChangeLog, we have:

2010-09-18  Alice  <[e-mail]>

        Reviewed by NOBODY (OOPS!).

        Changed some more code on 2010-09-18.

        * File:

2010-09-19  Alice  <[e-mail]>

        Reviewed by Ray.

        Changed some code on 2010-09-18.

Notice, the date of the earlier entry was changed.

[Daniel Bates]

Created attachment 68037 [details]
Unit test

[Daniel Bates]

(In reply to comment #0)

> Suppose we apply this patch using svn-apply on 09/19/2010. Looking at the resulting ChangeLog, we have:
> 

I mean't to say: "Looking at the resulting ChangeLog and assuming the name of the reviewer(s) was not specified to svn-apply, we have:"

[Daniel Bates]

Created attachment 77480 [details]
Patch with test cases

Moved unit test into new file fixChangeLogPatchThenSetChangeLogDateAndReviewer.pl because it tests setChangeLogDateAndReviewer(fixChangeLogPatch()).

Note, this patch modifies the functionality of fixChangeLogPatch() to move entries that are inserted earlier in a ChangeLog to the top of the file. Therefore, future patches that deliberately insert a change log entry earlier (why?) will not be landed by the commit-queue/webkit-patch as the author intended. Instead, the author will need to land such a patch by hand.

Maybe we should consider outputting a warning message when svn-apply detects this?

[Daniel Bates]

Created attachment 77589 [details]
Patch with test cases

Removed extraneous comment in added unit test file fixChangeLogPatchThenSetChangeLogDateAndReviewer.pl.

Strengthened for-loop for updating the date state index by noticing that if there exists a line L_i that matches the regular expression $dateStartRegEx after shifting the overlapping lines towards the front then i \in [$chunkStartIndex, $dateStartIndex - 1].

[Daniel Bates]

Comment on attachment 77589 [details]
Patch with test cases

Clearing flags on attachment: 77589

Committed r74780: <http://trac.webkit.org/changeset/74780>

[Daniel Bates]

All reviewed patches have been landed.  Closing bug.