"private-kvp" merge reverted other changes since November.

[Geert Janssens]

On Wednesday 07 May 2014 22:46:17 John Ralls wrote:
> On May 7, 2014, at 5:08 PM, Geert Janssens <janssens-geert at telenet.be> 
wrote:
> > Only John can tell why he did remove it during the merge and not
> > beforehand.
> I moved most of the QofInstance API into qofinstnace-p.h in 1f3fbf4b,
> very early in the private-kvp branch, because they involve direct
> access to what should be private implementation details of the class.
> In the case of mark dirty, one of the main motivations for the change
> is to make it impossible to change any persistent object member
> variable without marking the object dirty. This is actually done
> twice in those cases where there's a setter function as well as a
> GObject property if one uses  qof_instance_set() because
> qof_instance_set() does it to ensure that it's done for those
> properties that don't have setters and the setter does it as well to
> cover those cases where it's called directly. I wanted to also wrap
> the whole thing in a begin edit/commit wrapper, but that doesn't work
> for Split which is edited and committed by its parent transaction.
> 
>From superficially reading through the commits it's unclear how commit 
1f3fbf4b could have compiled at all. You moved qof_instance_set_dirty to 
qofinstance-p.h, but src/register/ledger-core/split-register-model-
save.c still calls that function. It may be qofinstance-p.h gets 
included via one or the other header file though. I haven't looked too 
deeply. In any case I take from this your intention was clearly to make 
qof_instance_set_dirty a private function.

But that's not the key point I want to argue here. I'm interested to get 
to the bottom of why your merge went wrong. That could help us in the 
future to avoid similar unexpected situations.

> I made the actual changes in merge commits from merging master into
> private-kvp, but following the recommendations in
> https://www.kernel.org/pub/software/scm/git/docs/git-rerere.html and
> having been yelled at at length on the gtk-devel list a few years ago
> for pushing too many merge commits I used rerere to push all of those
> commits into the merge commit. Mike's concern that something like
> that should be explicitly called out in a separate commit didn't even
> occur to me. I was more concerned about keeping git from turning my
> dozen or so incremental changes into a single giant commit.
> 
I did the merge the other way around. I merged private-kvp into master 
and resolved the merge conflicts by reading the source files in private-
kvp HEAD to learn what your intention was. There were 3 non-obvious 
changes (the other conflicts were trivial):
- some functions got moved into Account.c from import-match-map.c/h
  from what I could see you didn't alter the functions themselves so
  I could still drop the files without making tweaks to Account.c. This
  is something that's not trivial to follow with git diff or friends.
- one merge resulted in declarations to follow code. I think this
  was in a merge conflict but I hadn't spotted it while resolving the
  conflict
- one extern "C" block had to be moved down manually way below a newly
  added set of function declarations.

And I hope I didn't miss any more subtle changes. I was worried about 
the file name changes in qof (qofbook.c->qofbook.cpp for example) but 
apparently git managed that part just fine.

> It may well have been that back-merging with rerere that ate some of
> the changes on the master side of the merge. I'll have to study the
> differences between Geert's merge commit and mine to figure out what
> was different and why.

I wonder indeed if rerere did something odd here or whether we're making 
wrong assumptions on how it works. I'll wait for your analysis when you 
get around to it.
> 
> I'm still not convinced that git can safely merge a long running
> branch in the face of the substantial divergence that we had in this
> case without special and careful handling. I *am* convinced that the
> several days work I did to make the merge work at all wasn't
> sufficient, but I don't yet understand why not, nor do I understand
> why Geert was able to get it to merge correctly when I failed.
> 
Perhaps a question for a git mailing list ?

> Thanks, Geert, for cleaning up my mess for me.
> 
You're welcome. I was too curious to let it pass and it was a good 
opportunity to learn something about git merge in the process.

Geert