Re: [PATCH 07/27] unpack-trees: ensure full index

From: Elijah Newren <newren@gmail.com>
Date: Jan 27, 2021, 04:43 UTC
Message-ID: <CABPp-BE-eDAR8E538JD-zy3RP4MEc-pnrQwQOOWxsYD7D4BbtQ@mail.gmail.com>
In-Reply-To: <175c3c62543f89144b03b3bdff750ad29d17ba03.1611596534.git.gitgitgadget@gmail.com>

On Mon, Jan 25, 2021 at 9:42 AM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 42 quoted lines

>
> From: Derrick Stolee <dstolee@microsoft.com>
>
> The next change will translate full indexes into sparse indexes at write
> time. The existing logic provides a way for every sparse index to be
> expanded to a full index at read time. However, there are cases where an
> index is written and then continues to be used in-memory to perform
> further updates.
>
> unpack_trees() is frequently called after such a write. In particular,
> commands like 'git reset' do this double-update of the index.
>
> Ensure that we have a full index when entering unpack_trees(), but only
> when command_requires_full_index is true. This is always true at the
> moment, but we will later relax that after unpack_trees() is updated to
> handle sparse directory entries.
>
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> ---
>  unpack-trees.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/unpack-trees.c b/unpack-trees.c
> index f5f668f532d..4dd99219073 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -1567,6 +1567,7 @@ static int verify_absent(const struct cache_entry *,
>   */
>  int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options *o)
>  {
> +       struct repository *repo = the_repository;
>         int i, ret;
>         static struct cache_entry *dfc;
>         struct pattern_list pl;
> @@ -1578,6 +1579,12 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
>         trace_performance_enter();
>         trace2_region_enter("unpack_trees", "unpack_trees", the_repository);
>
> +       prepare_repo_settings(repo);
> +       if (repo->settings.command_requires_full_index) {
> +               ensure_full_index(o->src_index);
> +               ensure_full_index(o->dst_index);

I was worried about o->result as well, since there is a
    memset(&o->result, 0, sizeof(o->result));
followed by manually initializing the relevant fields of the
index_state.  However, the relevant field here is your new
sparse_index bit, and you want that to be 0, i.e. full.

I also checked ensure_full_index() since it is often the case that o->src_index == o->dst_index, but it'll be safe to be called twice on the same index state -- at least as currently written.

So, this patch seems good.

Show 7 quoted lines

> +       }
> +
>         if (!core_apply_sparse_checkout || !o->update)
>                 o->skip_sparse_checkout = 1;
>         if (!o->skip_sparse_checkout && !o->pl) {
> --
> gitgitgadget