Re: [PATCH] Revert 'diff-merges: let "-m" imply "-p"'

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Aug 7, 2021, 18:08 UTC
Message-ID: <YQ7MGoNswyZJHjkZ@google.com>
In-Reply-To: <xmqqv94hi3zw.fsf@gitster.g>

Hi,

Junio C Hamano wrote:

Show 7 quoted lines

> But I didn't see how you think your Rust thing is not a typo, and I
> still don't.  Unless you think Rust folks expected "-m" to do what
> "-m" was not designed to do, that is, and I do not think that
> "people thought it did something entirely differently, when it was a
> no-op, so we shouldn't suddenly make it not a no-op" is a good
> rationale that affects how we choose the evolution path for our
> tools.

Please don't treat this as an attempt to be argumentative: as you've said, there's plenty of other reason for us to know in retrospect that making "-m" imply "-p" is problematic for scripts. Since you asked, I think it's still worth describing my logic about the Rust example.

I believe the Rust folks expected "-m" to do something that it is designed to do. They _also_ overlooked a different subtlety about the interaction between diff generation and path limiting. It's good that Rust's bootstrap.py is fixed now to be more straightforward (by now it's even using the plumbing command); but it is very easy for another script author to have had the same confusion, which I might add was a harmless confusion until this change. If we changed the behavior to match their expectation _better_ then it would be a perfectly fine compatibility break that would be expected to improve the behavior of more scripts than it hurts. This change was not in that category.

1. When I add "-m --first-parent" to my "git log -p" invocation, it
   changes what diff it generates.  Until 9ab89a24390 (log: enable
   "-m" automatically with "--first-parent", 2020-07-29), the diff
   shown for a merge with --first-parent was simply "no diff".  A
   script written before mid-2020 that wants to operate on the
   --first-parent diff is highly likely to pass -m.

2. The -m only affects diff generation and does not affect path
   limiting.  So when no diff is being generated it is in fact a
   no-op.  This point is fairly subtle, though, and because it is not
   documented, script authors _in practice_ would only discover it by
   experimentation.

3. A script using -m with the intent of affecting path limiting
   doesn't get any feedback via experimentation that they've made a
   mistake because path limiting with --first-parent already does what
   the script author was hoping for.

What's relevant is not whether the script author was in the wrong or in the right: it's whether we expect there to be a significant number of scripts negatively affected by the change. Because of (1) to (3) above, I do.

Jonathan