Re: [PATCH 1/3] Resurrect "diff-lib.c: adjust position of i-t-a entries in diff"

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 6, 2016, 19:15 UTC
Message-ID: <xmqqwphljnlj.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CACsJy8D7c8Z_ugasn_scf391+C6GxJp1CYwHY4ndvVtLiJzxnQ@mail.gmail.com>

Duy Nguyen <pclouds@gmail.com> writes:

Show 9 quoted lines

> On Tue, Oct 4, 2016 at 11:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Duy Nguyen <pclouds@gmail.com> writes:
>>
>>> We don't use it internally _yet_. I need to go through all the
>>> external diff code and see --shift-ita should be there. The end goal
>>> is still changing the default behavior and getting rid of --shift-ita,
>>
>> I do not agree with that endgame, and quite honestly I do not want
>> to waste time reviewing such a series.

I definitely shouldn't have said that, especially "waste". Many issues around i-t-a and diff make my head hurt when I think about them [*1*], but not wanting to spend time that gets my head hurt and not wanting to waste time are totally different things. My apologies.

I missed something curious in your statement above, i.e. "external diff". I thought we have pretty much got rid of all the invocation of "git diff" via the run_command() interface and we do not need the command line option (we only need the options->shift_ita so that callers like "git status" can seletively ask for it when making internal calls), and that is why I didn't want to see it.

Show 5 quoted lines

> I do not believe current "diff" behavior wrt. i-t-a entries is right
> either. There's no point in pursuing this series then. Feel free to
> revert 3f6d56d (commit: ignore intent-to-add entries instead of
> refusing - 2012-02-07) and bring back the old i-t-a behavior. All the
> problems would go away.

It is way too late to revert 3f6d56d. Even though I think it was overall a mistake to treat i-t-a as "not exist" while committing, people are now used to the behaviour with that change, and we need to make our progress within the constraint of the real world.

[Footnote]

Here is one of the things around i-t-a and diff. If you make "git diff" (between the index and the working tree) report "new" file, it would imply that "git apply" run without "--index" should create an ita entry in the index for symmetry, wouldn't it? That by itself can be seen as an improvement (we no longer would have to say that "git apply patchfile && git commit -a" that is run in a clean state will forget new files the patchfile creates), but it also means we now need a repository in order to run "git apply" (without "--index"), which is a problem, as "git apply" is often used as a better "patch".

"git apply --cached" may also become "interesting". A patch that would apply cleanly to HEAD should apply cleanly if you did this:

    $ git read-tree HEAD
    $ git apply --cached <patch

no matter what the working tree state is. Should a patch that creates a "new" file add contents to the index, or just an i-t-a entry? I could argue it both ways, but either is quite satisfactory and makes my head hurt.