Subject: Re: [PATCH v1 5/9] diff-merges: move specific diff-index "-m" handling to diff-index

From: Junio C Hamano <[email-masked]>
Date: Tue, 18 May 2021 05:29:12 +0900

Sergey Organov <[email-masked]> writes:

> Move specific handling of "-m" for diff-index to diff-index.c, so
> diff-merges is left to handle only diff for merges options.
>
> Being a better design by itself, this is especially essential in
> preparation for letting -m imply -p, as "diff-index -m" obviously
> should not imply -p, as it's entirely unrelated.
>
> To handle this, in addition to moving specific diff-index "-m" code
> out of diff-merges, we introduce new
>
>   diff_merges_suppress_options_parsing()
>
> and call it before generic options processing in cmd_diff_index().

This change has a small but obvious fallout.

    $ git diff-index -c --cached HEAD^

now starts failing loudly.  Earlier, it silently fell back to
"combined" diff of one parent, which is "-p".

I think the end result is good (and luckily, "DIFF FORMAT FOR
MERGES" section explicitly limits "-c" and "--cc" to diff-tree,
diff-files and diff (and by implication excludes diff-index) so I am
sure there are small but non-zero number of people somewhere in the
world who has "diff-index -c" in their scripts that suddenly starts
failing with the version of Git with this change, but we can just
say their use was broken ;-)

Having said all that, I have to wonder if it still is needed to keep
the "diff-index -m" working, or we would be better off breaking it
to avoid a change like this that makes us bend over backwards to
work around the command line parsing infrastructure.

The only reason why "diff-index -m" exists is because it was part of
the idea Linus had for the merge implementation that we ended up
deciding not taking, where merges and possibly other bulk operations
that would affect the working tree is done in a separate, temporary
directory that is sparsely populated, the user is asked to edit away
conflicts in the temporary directory and expected to monitor his or
her own progress using "diff-index -m".  Our plan was to populate
such a temporary directory with only paths that are involved in the
operation in progress, without instantiating paths that are not
touched, so "treat missing files as if they haven't been modified"
was a handy ingredient for such a mode of operation.

But we ended up going with a different design, in which the main
working tree area is used to perform merges and to resolve
conflicts, which made this "pretend missing files as unmodified"
unnecessary feature.  In the end, we made a good move, as the
current design allows users to verify their changes in the context
of a full checkout (e.g. "make" would not have been a good way to
validate the conflict resolution if it is done in a separate
temporary directory that is sparsely populated with only the paths
involved in the merge---you need all files for building, including
the ones that are not modified, and "make" does not know to treat
missing files as if they are unmodified).