Re: [PATCH 9/9] update-index: reduce scope of index expansion in do_reupdate
- From
Elijah Newren <newren@gmail.com>
- Date
- Jan 9, 2022, 04:24 UTC
- Message-ID
- <CABPp-BEmT-2+G_PRvhCFkzOc4oP+_5--ESz9=dHAO7gzDsHUOw@mail.gmail.com>
- In-Reply-To
- <de7fc14356231a60c8cac9aa6f92a7fec1560b6a.1641317820.git.gitgitgadget@gmail.com>
On Tue, Jan 4, 2022 at 9:37 AM Victoria Dye via GitGitGadget <gitgitgadget@gmail.com> wrote:
Show 5 quoted lines
> > From: Victoria Dye <vdye@github.com> > > Expand the full index (and redo reupdate operation) only if a sparse > directory in the index differs from HEAD.
This was a bit hard to parse and follow for me. Perhaps:
Avoid pre-emptively expanding to a full index in update-index's do_reupdate(). However, if a sparse directory in the index differs from HEAD, we will need to then expand the index and restart the operation.
Show 74 quoted lines
> Only the index entries that differ
> between the index and HEAD are updated when performing `git update-index
> --again`, so unmodified sparse directories are safely skipped. The index
> does need to be expanded when sparse directories contain changes, though,
> because `update_one(...)` will not operate on sparse directory index
> entries.
>
> Because the index should only be expanded if a sparse directory is modified,
> add a test ensuring the index is not expanded when differences only exist
> within the sparse cone.
>
> Signed-off-by: Victoria Dye <vdye@github.com>
> ---
> builtin/update-index.c | 14 +++++++++++---
> t/t1092-sparse-checkout-compatibility.sh | 5 ++++-
> 2 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/update-index.c b/builtin/update-index.c
> index 605cc693bbd..52ecc714d99 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -606,7 +606,7 @@ static struct cache_entry *read_one_ent(const char *which,
> error("%s: not in %s branch.", path, which);
> return NULL;
> }
> - if (mode == S_IFDIR) {
> + if (!the_index.sparse_index && mode == S_IFDIR) {
> if (which)
> error("%s: not a blob in %s branch.", path, which);
> return NULL;
> @@ -743,8 +743,6 @@ static int do_reupdate(int ac, const char **av,
> */
> has_head = 0;
> redo:
> - /* TODO: audit for interaction with sparse-index. */
> - ensure_full_index(&the_index);
> for (pos = 0; pos < active_nr; pos++) {
> const struct cache_entry *ce = active_cache[pos];
> struct cache_entry *old = NULL;
> @@ -761,6 +759,16 @@ static int do_reupdate(int ac, const char **av,
> discard_cache_entry(old);
> continue; /* unchanged */
> }
> +
> + /* At this point, we know the contents of the sparse directory are
> + * modified with respect to HEAD, so we expand the index and restart
> + * to process each path individually
> + */
> + if (S_ISSPARSEDIR(ce->ce_mode)) {
> + ensure_full_index(&the_index);
> + goto redo;
> + }
> +
> /* Be careful. The working tree may not have the
> * path anymore, in which case, under 'allow_remove',
> * or worse yet 'allow_replace', active_nr may decrease.
> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> index bc0741c970d..0863c9747c4 100755
> --- a/t/t1092-sparse-checkout-compatibility.sh
> +++ b/t/t1092-sparse-checkout-compatibility.sh
> @@ -1225,7 +1225,10 @@ test_expect_success 'sparse index is not expanded: update-index' '
>
> ensure_not_expanded update-index --add README.md &&
> ensure_not_expanded update-index a &&
> - ensure_not_expanded update-index --remove deep/a
> + ensure_not_expanded update-index --remove deep/a &&
> +
> + ensure_not_expanded reset --soft update-deep &&
> + ensure_not_expanded update-index --add --remove --again
> '
>
> # NEEDSWORK: a sparse-checkout behaves differently from a full checkout
> --
> gitgitgadgetI think the rest makes sense. Thanks for working on these!