Re: [PATCH] unpack-trees: also allow get_progress() to work on a different index

From: Elijah Newren <newren@gmail.com>
Date: May 14, 2020, 21:46 UTC
Message-ID: <CABPp-BHM8uS1yxAtjE=Frh_V7JF7-EJ8+47e-cr9TpEMZr4nfw@mail.gmail.com>
In-Reply-To: <xmqq4ksiqnbf.fsf@gitster.c.googlers.com>

On Thu, May 14, 2020 at 1:22 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 26 quoted lines

>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Elijah Newren <newren@gmail.com>
> >
> > commit b0a5a12a60 ("unpack-trees: allow check_updates() to work on a
> > different index", 2020-03-27) allowed check_updates() to work on a
> > different index, but it called get_progress() which was hardcoded to
> > work on o->result much like check_updates() had been.  Update it to also
> > accept an index parameter and have check_updates() pass that parameter
> > along so that both are working on the same index.
> >
> > Noticed-by: Jeff Hostetler <jeffhost@microsoft.com>
> > Signed-off-by: Elijah Newren <newren@gmail.com>
> > ---
> >     unpack-trees: also allow get_progress() to work on a different index
> >
> >     This is a fix to a minor 2.27 regression, from the en/sparse-checkout
> >     series.
>
> OK, so update_sparsity() calls the function with o->src_index,
> get_progress() without this fix would iterate over an unrelated
> o->result to count the cache entries that are marked for update or
> removal and show the progress based on that number without this
> patch?  The other caller in unpack_trees() uses o->result so there
> is no change in the behaviour with or without the patch.

Yes.

> The flag bits on cache entries used to count the entries in the
> index by the get_progress() function are applied only on the
> o->result side when merged_entry() and friends call do_add_entry(),
> no?

Not quite; unpack_trees() also calls apply_sparse_checkout(), passing it o->result, and apply_sparse_checkout() also sets those bits.

> Do we see these CE_UPDATE|CE_WT_REMOVE bits attached to the cache
> entries in the o->src_index array when get_progress() is fed the
> src_index in the first place?

Yes, before calling check_updates(o, o->src_index), update_sparsity() loops over o->src_index and calls apply_sparse_checkout() on each of the non-conflicted cache entries. apply_sparse_checkout() will set either CE_UPDATE or CE_WT_REMOVE whenever items flip from or to having the SKIP_WORKTREE bit set.