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

[Jonathan Nieder <jrnieder@gmail.com>]

Hi Hannes,

Johannes Sixt wrote:
> Am 07.08.21 um 03:55 schrieb Jonathan Nieder:

>> The motivating example (Rust) shows that there is at least one script
>> that _did_ use "-m" in this way.  Rust has mitigation, but the above
>> logic leads me to believe that they are not the only project that will
>> be affected.  And more generally, when a script author has a
>> reasonable reason to believe something will work, they write scripts
>> where it _does_ work, and then an update breaks their script, I think
>> it's reasonable for them not to be happy.
>
> As you know, we have "plumbing" commands with a stable interface and
> "porcelain" commands for which we reserve to change the behavior without
> advance notice. By your reasoning we would not need to distinguish
> between the two categories and were forced to keep all behavior stable.
> This undoing of a behavior change in a "porcelain" command with the
> argument that one script depended on the old behavior and that others
> might do so as well would set an unwanted precedent.

Hm, this is worth elucidating a bit more, since I am definitely in
favor of continuing to change porcelain commands for the better where
we can.  If we decide that "git log --format=<fmt>" is no longer part
of the stable scripting interface we provide, then that would be a
huge change for our callers (and it's probably too late), but I would
certainly be in favor of us going back in time and doing that. :)

More generally, we've been able to make changes to porcelain commands
that don't hurt our ability to act as a platform for scripts, and I
want us to continue to be able to do that.  "Do not break any script"
is certainly not the standard I think we should apply, as illustrated
by my thoughts upthread when I thought '-m' in this Rust example was a
typo.

But by now it's very clear to me that it was not a typo.

In other words:

- this isn't only about one obscure script.  The point of the "this
  was not a typo" logic is to illustrate that in addition to the
  examples that we know about it is very likely that there are
  examples that we don't know about, in teams' script collections
  beyond the reach of search engines.

- In fact, in addition to the motivating example that makes it
  possible to build Rust, we had multiple in-tree scripts that would
  also have broken by this, if they had not been adapted to work
  around that in the same series!  I should have noticed that in
  review, and I'm sorry that I didn't.

> Perhaps we need to point script authors to "plumbing" commands more clearly?

I think the existence of "plumbing" is fairly well known, but users
don't always have an easy time using it.  The "porcelain" is what ends
up getting the most attention in improvements, and so while I
encourage script authors to use 'git rev-list <revs> | git diff-tree
-s --stdin --format=<fmt>' in place of 'git log --format=<fmt>
<revs>', most do not listen, and I can't really blame them given how
much more convenient the latter is and how many more options it
supports.

I don't think that situation will change unless we

 a. Maintain a second, parallel implementation of each porcelain
    command that only uses plumbing.  This would provide an example of
    how to use plumbing and would ensure that the plumbing grows in
    capability at the same time as the corresponding porcelain.  Or

 b. Expose a library interface, so that we can expose the actual
    helpers that support the standard implementation of porcelain
    commands.

I tried a little of (a) years ago by updating contrib/examples/ to
pass tests: https://lore.kernel.org/git/20100817065147.GA18293@burratino/.
It was fun but I don't think it's really sustainable.

In the long term, I think (b) is going to be an important thing to do,
and I think it will be helpful.  Some automated callers would
appreciate the ability to pass structured input instead of having to
pretend to be shell scripts. :)  True shell scripts would also benefit
because the plumbing commands can more directly map to such a library
API.

> (BTW, I have no opinion on whether -m should or should not imply -p.)

Nevertheless, thanks for weighing in.

Jonathan