Re: [RFC/PATCH 00/18] Add --index-only option to git merge

From: Elijah Newren <newren@gmail.com>
Date: Apr 9, 2016, 03:09 UTC
Message-ID: <CABPp-BG49Gr3Kf8Q3E6Vc=GF9MG+m10HkhkwbaOBfzs1cFcgVw@mail.gmail.com>
In-Reply-To: <5707ABBF.2050701@drmicha.warpmail.net>

On Fri, Apr 8, 2016 at 6:01 AM, Michael J Gruber <git@drmicha.warpmail.net> wrote:

Show 13 quoted lines

> I haven't looked at your series thoroughly but immediately had to think
> of 'tr/remerge-diff' (on 'pu'), see
> http://permalink.gmane.org/gmane.comp.version-control.git/256591
>
> There, Thomas used index-only merge to reproduce an automatic merge as
> the base for a useful "remerge-diff".
>
> I've been rebasing (and using) that series on 'next' for a while now
> without any problems; some reasons kept it from being merged on next,
> see the thread.
>
> So, it would be interesting whether you solve the same problem
> differently, or face the same problems ;)

Thanks for the link. Looks like a very interesting series, even if we're solving slightly different problems (I don't want conflicts auto-resolved; I want to be able to look at what conflicted and why with commands like 'git ls-files -u' afterward.)

But the problem he's trying to solve is interesting to me too. We have one patch from each of our series that does overlap, the index-only modification to merge-recursive, though implemented slightly differently. I think mine's a little clearer, and I have a hunch that he might be able to use the idea in mine to dramatically simplify some of the other stuff he's doing. In particular, merge-recursive already has code to auto resolve conflicts that he could just re-use instead of reimplementing, which I believe would dramatically simplify his patch #8.

I didn't read all his code super closely, so I might be missing something important, but I got the feeling that he didn't need different behavior than what merge-recursive already implements for virtual merge bases, and that even he wasn't certain whether he had handled all cases (e.g. not only conflict markers and modify/delete, but also rename/delete, rename/add, rename/rename (both 1to2 and 2to1), D/F conflicts, and perhaps others I'm not thinking of at the moment). We already have well reviewed and tested code for all those cases; it's just a subset of the things triggered by o->call_depth for virtual merges bases. Also, like the index-only stuff triggered by o->call_depth, the auto-resolve-conflict stuff is pretty well separated so it should be easy to add a flag to trigger just this portion without getting all the other stuff that o->call_depth normally does.

As far as I could tell, his series stalled out both because of concerns surrounding whether all the automatic-conflict-resolutions were correct, and because it made git log no longer be a read-only operation. Neither of those concerns are applicable to my patchset; I invented totally new problems and concerns instead. :-) I don't have a good solution to the second concern for his patchset, but I think there's probably an easy solution to the first. Once I get my patchset cleaned up, I may take a look at reviving his (if he doesn't beat me to it.)