Bug 83702 – resolve-ChangeLogs incorrectly duplicates entries landed via CQ

• Status: NEW
• Component: Tools / Tests

[Ami Fischman]

Whenever CQ lands a patch of mine and I [git pull --rebase origin] to pull the CQ-landed patch into my local branch (and presumably return said local branch to pristine state matching origin) the code diff is rebased away to nothing, but I always also get a duplicate ChangeLog entry at the top of the file (in addition to the correctly-placed one in mid-file from origin).  I've got the O/A/B version of the merge.changelog.driver config from http://trac.webkit.org/wiki/UsingGitWithWebKit#WebKitScriptsupportforGit set up:

[merge "changelog"]
	driver = perl /home/fischman/src/chromium/src/third_party/WebKit/Tools/Scripts/resolve-ChangeLogs --merge-driver %O %A %B

[Ami Fischman]

Tor/Eric: any ideas about this?
(if no, do you know if there's anything like a test for resolve-ChangeLogs in case I take a crack at this myself?  Or is it just "try it out and see how it goes"?)

[Eric Seidel (no email)]

Adam Roben is really your man here, sadly he's left the project.

[Eric Seidel (no email)]

Maybe Adam isn't quite as gone as I assumed him to be. :)

[Adam Roben (:aroben)]

I don't think `git pull --rebase` is really what you want here. Your patch has been landed for you, so you want to throw away your local commit entirely. If you know that you don't have any other changes on your branch that you need to keep around, you could just do `git fetch && git reset --hard origin/master` or similar.

I guess another option would be to write a script that first does `git pull --rebase` then looks for these ChangeLog-only commits and removes them.

[Ami Fischman]

(In reply to comment #4)
> I don't think `git pull --rebase` is really what you want here. Your patch has been landed for you, so you want to throw away your local commit entirely. If you know that you don't have any other changes on your branch that you need to keep around, you could just do `git fetch && git reset --hard origin/master` or similar.

Right.  The point of using rebase is to *verify* that the upstream change includes everything from the branch (and getting the warm fuzzies from [git branch -d] as opposed to getting the cold willies from [git branch -D]).

> I guess another option would be to write a script that first does `git pull --rebase` then looks for these ChangeLog-only commits and removes them.

Well, that script would ideally look for the same ChangeLog change earlier in the file (modulo "Reviewed by") to confirm that the entry landed as intended, too.  At that point ISTM it would be better to have resolve-ChangeLogs do that work.

Adam: do you know if there's anything like a test for resolve-ChangeLogs?

[Adam Roben (:aroben)]

(In reply to comment #5)
> Adam: do you know if there's anything like a test for resolve-ChangeLogs?

There is not. All our Perl tests live in Tools/Scripts/webkitperl/*_unittest. It would be awesome to add some!

[Ami Fischman]

I dug into this a bit tonight and was disappointed to find that r-CL is a wrapper around diff+patch, as opposed to something that grokked CL entries directly.  Played with some heuristics around classifying a base->ours diff as ignorable in the context of a base->theirs diff, but couldn't come up with a reasonable story.

Sadly I think actually implementing this will require teaching VCSUtils.pm how to parse ChangeLogs and reason about adds/removes/edits and how to combine them.

On the bright side, r-CL as a git merge-driver is just a wrapper around VCSUtils.pm:mergeChangeLogs(), which *is* unittested: http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitperl/VCSUtils_unittest/mergeChangeLogs.pl