Bug 15600 - prepare-ChangeLog and update-webkit create needless ChangeLog conflicts
Summary: prepare-ChangeLog and update-webkit create needless ChangeLog conflicts
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-10-21 14:54 PDT by Eric Seidel (no email)
Modified: 2007-10-26 08:52 PDT (History)
1 user (show)

See Also:


Attachments
my script (1.94 KB, text/plain)
2007-10-21 14:54 PDT, Eric Seidel (no email)
no flags Details
Patch v1 (10.95 KB, patch)
2007-10-21 20:31 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
Patch v2 (13.04 KB, patch)
2007-10-25 00:27 PDT, David Kilzer (:ddkilzer)
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2007-10-21 14:54:03 PDT
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.
Comment 1 Eric Seidel (no email) 2007-10-21 14:54:55 PDT
Created attachment 16772 [details]
my script

Marking this for review to ask for comment from some of the other reviewers...
Comment 2 David Kilzer (:ddkilzer) 2007-10-21 16:19:48 PDT
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

Comment 3 Maciej Stachowiak 2007-10-21 16:22:56 PDT
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.
Comment 4 David Kilzer (:ddkilzer) 2007-10-21 18:15:22 PDT
(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.

Comment 5 Eric Seidel (no email) 2007-10-21 18:23:31 PDT
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. :)
Comment 6 David Kilzer (:ddkilzer) 2007-10-21 20:29:41 PDT
(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.

Comment 7 David Kilzer (:ddkilzer) 2007-10-21 20:31:21 PDT
Created attachment 16779 [details]
Patch v1

Proposed fix.
Comment 8 Darin Adler 2007-10-22 06:19:16 PDT
Comment on attachment 16779 [details]
Patch v1

r=me
Comment 9 David Kilzer (:ddkilzer) 2007-10-22 09:25:40 PDT
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.
Comment 10 David Kilzer (:ddkilzer) 2007-10-22 09:38:39 PDT
(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.

Comment 11 David Kilzer (:ddkilzer) 2007-10-22 10:08:44 PDT
(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.

Comment 12 David Kilzer (:ddkilzer) 2007-10-24 21:06:24 PDT
(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
Comment 13 David Kilzer (:ddkilzer) 2007-10-25 00:27:17 PDT
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.
Comment 14 Darin Adler 2007-10-26 00:21:55 PDT
Comment on attachment 16855 [details]
Patch v2

looks great, r=me
Comment 15 David Kilzer (:ddkilzer) 2007-10-26 08:52:14 PDT
Committed r27112