On Wednesday 07 May 2014 22:46:17 John Ralls wrote: > On May 7, 2014, at 5:08 PM, Geert Janssens <janssens-ge...@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 _______________________________________________ gnucash-devel mailing list gnucash-devel@gnucash.org https://lists.gnucash.org/mailman/listinfo/gnucash-devel
