RESOLVED FIXED 15600
prepare-ChangeLog and update-webkit create needless ChangeLog conflicts
https://bugs.webkit.org/show_bug.cgi?id=15600
Summary prepare-ChangeLog and update-webkit create needless ChangeLog conflicts
Eric Seidel (no email)
Reported 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.
Attachments
my script (1.94 KB, text/plain)
2007-10-21 14:54 PDT, Eric Seidel (no email)
no flags
Patch v1 (10.95 KB, patch)
2007-10-21 20:31 PDT, David Kilzer (:ddkilzer)
no flags
Patch v2 (13.04 KB, patch)
2007-10-25 00:27 PDT, David Kilzer (:ddkilzer)
darin: review+
Eric Seidel (no email)
Comment 1 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...
David Kilzer (:ddkilzer)
Comment 2 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
Maciej Stachowiak
Comment 3 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.
David Kilzer (:ddkilzer)
Comment 4 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.
Eric Seidel (no email)
Comment 5 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. :)
David Kilzer (:ddkilzer)
Comment 6 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.
David Kilzer (:ddkilzer)
Comment 7 2007-10-21 20:31:21 PDT
Created attachment 16779 [details] Patch v1 Proposed fix.
Darin Adler
Comment 8 2007-10-22 06:19:16 PDT
Comment on attachment 16779 [details] Patch v1 r=me
David Kilzer (:ddkilzer)
Comment 9 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.
David Kilzer (:ddkilzer)
Comment 10 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.
David Kilzer (:ddkilzer)
Comment 11 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.
David Kilzer (:ddkilzer)
Comment 12 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
David Kilzer (:ddkilzer)
Comment 13 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.
Darin Adler
Comment 14 2007-10-26 00:21:55 PDT
Comment on attachment 16855 [details] Patch v2 looks great, r=me
David Kilzer (:ddkilzer)
Comment 15 2007-10-26 08:52:14 PDT
Committed r27112
Note You need to log in before you can comment on or make changes to this bug.