Re: [PATCH] diff-files: treat "i-t-a" files as "not-in-index"

Thanks for replying!

On Thu, Jun 11, 2020 at 01:27:22PM -0700, Junio C Hamano wrote:

Srinidhi Kaushik [off-list ref] writes:

quoted

The `diff-files' command and related commands which call `cmd_diff_files()',
consider the "intent-to-add" files as a part of the index when comparing the
work-tree against it. This was previously addressed in [1] and [2] by turning
the option `--ita-invisible-in-index' (introduced in [3]) on by default.

For `diff-files' (and `add -p' as a consequence) to show the i-t-a files as
as new, `ita_invisible_in_index' will be enabled by default here as well.

[1] 0231ae71d3 (diff: turn --ita-invisible-in-index on by default, 2018-05-26)
[2] 425a28e0a4 (diff-lib: allow ita entries treated as "not yet exist in
                index", 2016-10-24)
[3] b42b451919 (diff: add --ita-[in]visible-in-index, 2016-10-24)

Is there any place where we still run the internal diff machinery to
compare the index and the working tree without setting the
ita_invisible_in_index bit on with this patch applied, and if so,
why?  Does the justification why that other place needs to leave
the bit off apply to this codepath as well?

Yes, I believe that there exist some use cases for `ita_invisible_in_index'
to be unset. For instance, `index_differs_from' which is used in a quite a
few places -- like "commit", "revert", and "rebase" -- which require a
"no change" to be returned.

This commit: [1] addressed the issue where the cache-tree was producing
the same tree as the current commit when it involved "intent-to-add"
entries, instead of aborting.

[1] 018ec3c820 (commit: fix empty commit creation when there's no changes
                but ita entries, 2016-10-24) 

What I am trying to get at is if this is helping only one usecase
for "diff-files" while breaking other usecases.

Currently, `run_add_p' (for "add"; which this patch addresses
the fix), and `push_to_deploy' (in "receive-pack"; where this
is the intended behavior), call "diff-files" as a subprocess,
in which case the `ita_invisible_in_index' bit is explicitly
set. For all other cases, calls are made directly
to `run_diff_files' and will be unaffected by this change.
 

On the other hand, if there is no longer anybody who wants
ita_invisible_in_index off, perhaps we can get rid of the bit and
lose many conditionals.