| Message ID | de7fc14356231a60c8cac9aa6f92a7fec1560b6a.1641317820.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Sparse index: integrate with 'clean', 'checkout-index', 'update-index' | expand |
On Tue, Jan 4, 2022 at 9:37 AM Victoria Dye via GitGitGadget <gitgitgadget@gmail.com> wrote: > > 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. > 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 > -- > gitgitgadget I think the rest makes sense. Thanks for working on these!
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