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

From: Junio C Hamano <[email-masked]>
Date: Fri, 06 Aug 2021 10:21:05 -0700

Jonathan Nieder <[email-masked]> writes:

> The problem is that although diff generation options are only relevant
> for the displayed diff, a script author can imagine them affecting
> path limiting.  For example, I might run

I am somewhat puzzled.  What does "can imagine" exactly mean and
justify this change?  A script author may imagine "git cat-file" can
be expected to meow, but the command actually does not meow and end
up disappointing the author, but that wouldn't justify a rename of
"cat-file" to something else.

> 	git log -w --format=%H -- README
>
> hoping to list commits that edited README, excluding whitespace-only
> changes.  In fact, a whitespace-only change is not TREESAME so the use
> of -w here has no effect (since we don't apply these diff generation
> flags to the diff_options struct rev_info::pruning used for this
> purpose), but the documentation suggests that it should work
>
> 	Suppose you specified foo as the <paths>. We shall call
> 	commits that modify foo !TREESAME, and the rest TREESAME. (In
> 	a diff filtered for foo, they look different and equal,
> 	respectively.)
>
> and a script author who has not tested whitespace-only changes
> wouldn't notice.

It would need to be corrected by a bugfix of either TREESAME
computation, or a documentation fix, I would think.  I fail to see
the similarity you perceive to the "-m" issue at hand, though.

> Similarly, a script author could include
>
> 	git log -m --first-parent --format=%H -- README
>
> to filter the first-parent history for commits that modified README.
> The -m is a no-op but it reflects the script author's intent.

So the expectation is with "-m" we'd give single parent commits on
the fp chain, and merges from side branches that change README, in
addition to merges from side branches that was forked way before the
README was updated on the trunk (hence had ancient README but the
merge kept the version from the trunk)?

> For
> example, until 1e20a407fe2 (stash list: stop passing "-m" to "git
> log", 2021-05-21), "git stash list" did this.

This is not a example that supports your conclusion, though.  The
reason why 288c67ca (stash: default listing to working-tree diff,
2014-08-06) added "-m" on the command line to make it:

  git log --format="%gd: %gs" -g --first-parent -m "$@" $ref_stash --

is to prepare for the users who may pass "-p" as part of the "$@";
they wil get no patches out of these merge commits that represent
stash entries otherwise, and they'd have to pass "-m -p" instead,
without the change.

> As a result, we can't safely change "-m" to imply "-p" without fear of
> breaking such scripts.  Restore the previous behavior.

So the above is *not* an example of a script that would have been
broken with this change.