[gmx-developers] Git master broken

[Mark Abraham]

----- Original Message -----
From: Christoph Junghans <junghans at mpip-mainz.mpg.de>
Date: Thursday, August 26, 2010 1:57
Subject: Re: [gmx-developers] Git master broken
To: Discussion list for GROMACS development <gmx-developers at gromacs.org>

> Hi,
> 
> the merge was not 100% trivial, there were two simple conflicts
> due to a define renaming in include/vec.h and a argument change in
> split of include/string2.h, which only happened in the master branch.
> That is why I committed it with the standard commit message.

"git log -p 5e3473a" has a merge commit message, but has new content as well. This fools some parts of git-log into not showing that new content. The graphical depiction in gitk on master branch makes this clear too... that commit forms a single thread of development until DvdS tried to merge it.

Anyone who's done development work on either master or release-4-5-patches might have this kind of problem at some point. Only if your development footprint interacts with current progress on will you have an issue, and there's no getting around having to resolve merge conflicts regardless of the branch you're on. Things are less hairy if people know which branch is supposed to get updated (currently, release-4-5-patches). Having started work from master (say) and committed your present work to your private copy of the master branch, coping strategies for merging your material into release-4-5-patches include:

* Use "git rebase release-4-5-patches" to switch the base of that private branch from master to release-4-5-patches. You will still have to resolve merge conflicts, but the result will be immediately ready to push into origin/release-4-5-patches. [IMO the most elegant strategy, because there's only a single merge, and pulling future updates will be easy]

* "git pull master" (resolve conflicts), then "git pull release-4-5-patches" (resolve conflicts), then merge into release-4-5-patches trivially. [IMO this is more work and creates a more cluttered development history, but depending on the nature of the conflicts, the two merging stages might be conceptually simpler than the one-stage merge above.]

* Merge release-4-5-patches into master, then "git pull master" (resolve conflicts), then merge into release-4-5-patches. [IMO ugly, but I can't think of a technical flaw]

> I just did the merge, because the master branch missed a header 
> of some kernel, which was fixed in 4.5 branch. After the merge I 
> did a build check and it worked, so this was not a build break.
> 
> 
> Concerning the current merge policy:
> I think the problem by now is that the main work happens on the 
> 4.5 branch and this has to be merged into the master branch 
> immediately. Otherwise the first one, who wants to merge 
> something in the master branch, in the above case me, has to 
> decide about all the 4.5 changes of all the other developers. At 
> the current point of development this is not a big deal, because 
> master and 4.5 are not very different.

Work on release-4-5-patches doesn't have to be merged into master immediately, absent some artificial policy. In fact, if nobody's committing to master then there's no reason to merge to master at all, and there's no reason to have branched. However, branching is easy and lightweight and should be encouraged.

It must be noted that the merge conflict problem occurs independently of whether branching occurred. Here, Christoph wanted to do:

diff --git a/include/resall.h b/include/resall.h
index f4e547e..4349551 100644
--- a/include/resall.h
+++ b/include/resall.h
@@ -45,18 +45,18 @@
 extern "C" {
 #endif

-extern t_restp *search_rtp(const char *key,int nrtp,t_restp rtp[]);
+t_restp *search_rtp(const char *key,int nrtp,t_restp rtp[]);
 /* Search for an entry in the rtp database */

and Berk wanted to do

diff --git a/include/resall.h b/include/resall.h
index 4349551..e23f808 100644
--- a/include/resall.h
+++ b/include/resall.h
@@ -45,8 +45,16 @@
 extern "C" {
 #endif

-t_restp *search_rtp(const char *key,int nrtp,t_restp rtp[]);
-/* Search for an entry in the rtp database */
+char *search_rtp(const char *key,int nrtp,t_restp rtp[]);
 
Now that's trivial to merge with a word-based diff (change 'extern t_restp *' to 'char *' by the two stages of "remove 'extern'" and "change 't_restp' to 'char'"), but the default diff is line-based. Even if there had never been a release-4-5-patches branch, whoever got their commit in first was home free, and the other would have to have resolved this merge.

> However, what still puzzles me is the origin of the conflicts in 
> the case of David's merge. For human eyes most of them look 
> trivial, but git doesn't get it.

I hope I helped there.

Mark

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://maillist.sys.kth.se/pipermail/gromacs.org_gmx-developers/attachments/20100826/df9b3858/attachment.html>