Re: [PATCH v2] apply: make --intent-to-add not stomp index

> Johannes Altmanninger <aclopte@gmail.com> 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@cisco.com>
> > Signed-off-by: Johannes Altmanninger <aclopte@gmail.com>
> > ---
> >
> > 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.

> 
> 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.