On Sun, Oct 31, 2021 at 11:40:05PM -0700, Junio C Hamano wrote:
> Johannes Altmanninger <aclopte@xxxxxxxxx> writes:
>
> > Commit cff5dc09ed (apply: add --intent-to-add, 2018-05-26) introduced
> > "apply -N" plus a test to make sure it behaves exactly as "add -N"
> > when given equivalent changes. However, the test only checks working
> > tree changes. Now "apply -N" forgot to read the index, so it left
> > all tracked files as deleted, except for the ones it touched.
> >
> > Fix this by reading the index file, like we do for "apply --cached".
> > and test that we leave no content changes in the index.
> >
> > Reported-by: Ryan Hodges <rhodges@xxxxxxxxx>
> > Signed-off-by: Johannes Altmanninger <aclopte@xxxxxxxxx>
> > ---
> >
> > Sorry I used the wrong Reported-by: address in v1
> >
> > apply.c | 2 +-
> > t/t2203-add-intent.sh | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/apply.c b/apply.c
> > index 43a0aebf4e..4f740e373b 100644
> > --- a/apply.c
> > +++ b/apply.c
> > @@ -4771,7 +4771,7 @@ static int apply_patch(struct apply_state *state,
> > LOCK_DIE_ON_ERROR);
> > }
> >
> > - if (state->check_index && read_apply_cache(state) < 0) {
> > + if ((state->check_index || state->ita_only) && read_apply_cache(state) < 0) {
> > error(_("unable to read index file"));
> > res = -128;
> > goto end;
>
> Thanks for an attempt, but I am not sure if it is wise to keep
> ita_only independent from check_index like this patch does.
I must confess, I didn't even consider alternative solutions.
>
> There are many safety/correctness related checks that check_index
> enables, and that is why not just the "--index" option, but "--3way"
> and "--cached" enable it internally. As "instead of adding the
> contents to the index, mark the new path with i-t-a bit" is also
> futzing with the index, it should enable the same safety checks by
> enabling check_index _much_ earlier. And if you did so, the above
> hunk will become a totally unnecessary change, because by the time
> the control gets there, because you accepted ita_only earlier and
> enabled check_index, just like you did for "--3way" and "--cached".
>
> One thing that check_index does is that it drops unsafe_paths bit,
> which means we would be protected from patch application that tries
> to step out of our narrow cone of the directory hierarchy, which is
> a safety measure. There are probably others I am forgetting.
To be clear, check_index *disables* the unsafe_paths check, but it enables
a stronger check: verify_index_match(), which makes sure that the touched
paths exist in the index.
