On Wed, Sep 07, 2016 at 03:06:36PM +0200, Johannes Schindelin wrote: > > This is marked as "RFC" because I don't feel entirely confident that I'm > > not missing some clever need for these options. But in both cases my gut > > feeling is that they are simply unintended effects that nobody ever > > noticed, because it would be very rare that they would affect the > > output. And that if they _did_ affect the output, they would probably be > > doing the wrong thing. > > Given that the patch ID is *wrong* for merge commits (it only looks at the > first parent, so each "-s ours" merge will have the same patch ID!), I > would say that we can get away with re-defining the patch ID of merge > commits. > > The only case where it might change things that I can think of would be a > `git rebase --preserve-merges`: it would probably have worked *by chance* > before (or not, in case of "-s ours" merges), and now it would try to pick > the merge commits even if rebased versions were already merged upstream. > > If I read the --preserve-merges code correctly, that would result in the > merge commit's parents to be 'rewritten' to HEAD. And as both parents > would be rewritten to HEAD, they would be condensed into a single new > parent, resulting in a cherry-pick that fails (because it tries to > cherry-pick a merge commit without any -n option). > > Of course, what we could do is to introduce a modifier, e.g. > --cherry-pick=first-parent, that would trigger the old behavior and would > be asked-for in the --preserve-merges mode. > > But quite frankly, personally I would not worry about it *that* much. As > you pointed out, the patch ID for merge commits is incorrect to begin > with, and we may just redeclare all merge commits to be incomparable to > one another when it comes to patch IDs. > > In short: I would be fine with the change of behavior. Thanks for this explanation; it matches what I was thinking, but you went through it in a lot more detail. So it sounds like this is the right thing, but as you pointed out, the implementation is just silly. I'll see if I can come up with a working v2. -Peff