Bug 15600 – prepare-ChangeLog and update-webkit create needless ChangeLog conflicts

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

update-webkit creates needless ChangeLog conflicts

I have a script which I use locally to deal with such things.  Maybe this logic should be rolled into update-webkit?  The script is really dumb and could be made smarter if necessary.

[Eric Seidel (no email)]

Created attachment 16772 [details]
my script

Marking this for review to ask for comment from some of the other reviewers...

[David Kilzer (:ddkilzer)]

Interesting.  I've never used the update-webkit script, but it may be worth looking into creating a custom "diff3" command that is passed to --diff3-cmd when running "svn update".

This mail message says that returning a non-zero exit status from the command (let's call it update-webkit-diff3) will leave a file in the conflicted state:

http://svn.haxx.se/users/archive-2004-09/1132.shtml

So the script just has to do the following:

1. If the merged filename is not "ChangeLog", return a non-zero exit status.
2. Else merge the ChangeLog entry without conflicts, clean up the files created and return a zero exit status.

This contrib Perl script for Subversion may have some clues as well (although it does 3-way merges after the conflicts occur by using "svn update --diff3-cmd=/bin/false"):

http://svn.collab.net/repos/svn/trunk/contrib/client-side/svn_update.pl

[Maciej Stachowiak]

I'd definitely love to see better conflict avoidance in prepare-ChangeLog. But does just removing the conflict markers always do the right thing? I am not so sure, because I think at times I have seen content duplicated in the two conflict alternatives in such a way that just removing the markers would lead to a broken ChangeLog. What would work better is something like this:

If ChangeLog local changes are only lines at the top, save them, remove them, update ChangeLog, then paste new lines back at the top. I'd love to see a version like that folded into prepare-ChangeLog.

[David Kilzer (:ddkilzer)]

(In reply to comment #2)
> Interesting.  I've never used the update-webkit script, but it may be worth
> looking into creating a custom "diff3" command that is passed to --diff3-cmd
> when running "svn update".

I've written update-webkit-diff3.  It creates a patch using "diff -u" between the local changed file and the previous file, then applies this patch using "patch --fuzz=3" to prevent unwanted conflicts in ChangeLog files (just like svn-apply now does).

(In reply to comment #3)
> If ChangeLog local changes are only lines at the top, save them, remove them,
> update ChangeLog, then paste new lines back at the top. I'd love to see a
> version like that folded into prepare-ChangeLog.

I didn't realize that prepare-ChangeLog could use the script as well.  I'll make this change and submit a patch.

[Eric Seidel (no email)]

Well, if update-webkit is fixed to never give you unnecessary conflicts in your ChangeLog, then it's less needed for prepare-ChangeLog.  In fact, I'm not sure why prepare-ChangeLog would ever want this functionality, as the reason why you have a conflict in the first place is because you already have a changelog entry.  Unless we're discussing updating prepare-ChangeLog to notice and update existing changelog entries. :)

[David Kilzer (:ddkilzer)]

(In reply to comment #5)
> Well, if update-webkit is fixed to never give you unnecessary conflicts in your
> ChangeLog, then it's less needed for prepare-ChangeLog.  In fact, I'm not sure
> why prepare-ChangeLog would ever want this functionality, as the reason why you
> have a conflict in the first place is because you already have a changelog
> entry.  Unless we're discussing updating prepare-ChangeLog to notice and update
> existing changelog entries. :)

Sometimes you want to update the ChangeLog by rerunning prepare-ChangeLog, but don't want to clean up conflicts while you're merging the ChangeLog entries.

[David Kilzer (:ddkilzer)]

Created attachment 16779 [details]
Patch v1

Proposed fix.

[Darin Adler]

Comment on attachment 16779 [details]
Patch v1

r=me

[David Kilzer (:ddkilzer)]

Comment on attachment 16779 [details]
Patch v1

Clearing Darin's review+ flag. The update-webkit-diff3 script needs more work.

I think it needs to run diff3 and print its output to stdout instead of just doing an "exit 1;", otherwise the merged file ends up blank.

[David Kilzer (:ddkilzer)]

(In reply to comment #9)
> I think it needs to run diff3 and print its output to stdout instead of just
> doing an "exit 1;", otherwise the merged file ends up blank.

The svn update command does not "fall back" to its own internal diff3 implementation if the --diff3-cmd script returns 1, so we may either have to invoke diff3 in the update-webkit-diff3 script, or we may need to do the ChangeLog cleanup after-the-fact.

[David Kilzer (:ddkilzer)]

(In reply to comment #10)
> The svn update command does not "fall back" to its own internal diff3
> implementation if the --diff3-cmd script returns 1, so we may either have to
> invoke diff3 in the update-webkit-diff3 script, or we may need to do the
> ChangeLog cleanup after-the-fact.

I think we should do the ChangeLog clean-up after-the-fact since we can watch for ChangeLog files with a "C" status, then run a diff and a patch (similar to what update-webkit-diff3 does in the patch) on the three files left in the directory.

[David Kilzer (:ddkilzer)]

(In reply to comment #11)
> I think we should do the ChangeLog clean-up after-the-fact since we can watch
> for ChangeLog files with a "C" status, then run a diff and a patch (similar to
> what update-webkit-diff3 does in the patch) on the three files left in the
> directory.

I'm going to implement such a script that works for both git and svn.

Note that I committed r27013 and r27014, which was some clean-up originally included in the patch in Attachment #16779 [details].

http://trac.webkit.org/projects/webkit/changeset/27013
http://trac.webkit.org/projects/webkit/changeset/27014

[David Kilzer (:ddkilzer)]

Created attachment 16855 [details]
Patch v2

Introducing the resolve-ChangeLogs script, which does exactly what its name suggests.  It may be run in stand-alone mode (for those that like to run 'svn up' on their own), and is integrated with prepare-ChangeLog and update-webkit in case either of those scripts run into a conflicted ChangeLog.

Note that I've tested resolve-ChangeLogs running forwards and backwards in history, and it works both ways because it matches the current file's revision with the conflict file created with the matching extension *to find the "newer" version of the file).  I also assume git won't lie about the mine/ours/theirs stages regardless of how you're merging.  File names used for merging in git were made to match those used in git-mergetool.

[Darin Adler]

Comment on attachment 16855 [details]
Patch v2

looks great, r=me

[David Kilzer (:ddkilzer)]

Committed r27112