Re: [PATCH v7 15/16] wt-status: expand added sparse directory entries
- From
Elijah Newren <newren@gmail.com>
- Date
- Jul 9, 2021, 01:03 UTC
- Message-ID
- <CABPp-BE=qw9pO89hggUFbu=eovfL=7Os5BY8DZNd5Z=qU==wFg@mail.gmail.com>
- In-Reply-To
- <717a3f49f97753149d5c435a83b3f1773dd4b1bb.1624932294.git.gitgitgadget@gmail.com>
On Mon, Jun 28, 2021 at 7:05 PM Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> wrote:
Show 51 quoted lines
> > From: Derrick Stolee <dstolee@microsoft.com> > > It is difficult, but possible, to get into a state where we intend to > add a directory that is outside of the sparse-checkout definition. Add a > test to t1092-sparse-checkout-compatibility.sh that demonstrates this > using a combination of 'git reset --mixed' and 'git checkout --orphan'. > > This test failed before because the output of 'git status > --porcelain=v2' would not match on the lines for folder1/: > > * The sparse-checkout repo (with a full index) would output each path > name that is intended to be added. > > * The sparse-index repo would only output that "folder1/" is staged for > addition. > > The status should report the full list of files to be added, and so this > sparse-directory entry should be expanded to a full list when reaching > it inside the wt_status_collect_changes_initial() method. Use > read_tree_at() to assist. > > Somehow, this loop over the cache entries was not guarded by > ensure_full_index() as intended. > > Signed-off-by: Derrick Stolee <dstolee@microsoft.com> > --- > t/t1092-sparse-checkout-compatibility.sh | 33 +++++++++++++++ > wt-status.c | 51 ++++++++++++++++++++++++ > 2 files changed, 84 insertions(+) > > diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh > index fed0440bafe..df217a2d10b 100755 > --- a/t/t1092-sparse-checkout-compatibility.sh > +++ b/t/t1092-sparse-checkout-compatibility.sh > @@ -545,4 +545,37 @@ test_expect_success 'sparse-index is not expanded' ' > test_region ! index ensure_full_index trace2.txt > ' > > +test_expect_success 'reset mixed and checkout orphan' ' > + init_repos && > + > + test_all_match git checkout rename-out-to-in && > + > + # Sparse checkouts do not agree with full checkouts about > + # how to report a directory/file conflict during a reset. > + # This command would fail with test_all_match because the > + # full checkout reports "T folder1/0/1" while a sparse > + # checkout reports "D folder1/0/1". This matches because > + # the sparse checkouts skip "adding" the other side of > + # the conflict.
The same issue I highlighted last time is still present. If you
insert an "exit 1" right here, then run
./t1092-sparse-checkout-compatibility.sh --ver --imm -x
until it stops, then
cd t/trash directory.t1092-sparse-checkout-compatibility/sparse-checkout
git ls-files -t | grep folder # Note the files that are sparse
git reset --mixed HEAD~1
git ls-files -t | grep folder # Note the files that are sparse --
there are some that aren't that should be
git sparse-checkout reapply
git ls-files -t | grep folder # Note the files that are sparseGranted, this is a bug with sparse-checkout without sparse-index, so not something new to your series. But since you are using comparisons between regular sparse-checkouts and sparse-index to verify correctness, this seems problematic to me.
Show 93 quoted lines
> + test_sparse_match git reset --mixed HEAD~1 &&
> + test_sparse_match test-tool read-cache --table --expand &&
> + test_sparse_match git status --porcelain=v2 &&
> +
> + # At this point, sparse-checkouts behave differently
> + # from the full-checkout.
> + test_sparse_match git checkout --orphan new-branch &&
> + test_sparse_match test-tool read-cache --table --expand &&
> + test_sparse_match git status --porcelain=v2
> +'
> +
> +test_expect_success 'add everything with deep new file' '
> + init_repos &&
> +
> + run_on_sparse git sparse-checkout set deep/deeper1/deepest &&
> +
> + run_on_all touch deep/deeper1/x &&
> + test_all_match git add . &&
> + test_all_match git status --porcelain=v2
> +'
> +
> test_done
> diff --git a/wt-status.c b/wt-status.c
> index 96db3e74962..0317baef87e 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -657,6 +657,36 @@ static void wt_status_collect_changes_index(struct wt_status *s)
> clear_pathspec(&rev.prune_data);
> }
>
> +static int add_file_to_list(const struct object_id *oid,
> + struct strbuf *base, const char *path,
> + unsigned int mode, void *context)
> +{
> + struct string_list_item *it;
> + struct wt_status_change_data *d;
> + struct wt_status *s = context;
> + struct strbuf full_name = STRBUF_INIT;
> +
> + if (S_ISDIR(mode))
> + return READ_TREE_RECURSIVE;
> +
> + strbuf_add(&full_name, base->buf, base->len);
> + strbuf_addstr(&full_name, path);
> + it = string_list_insert(&s->change, full_name.buf);
> + d = it->util;
> + if (!d) {
> + CALLOC_ARRAY(d, 1);
> + it->util = d;
> + }
> +
> + d->index_status = DIFF_STATUS_ADDED;
> + /* Leave {mode,oid}_head zero for adds. */
> + d->mode_index = mode;
> + oidcpy(&d->oid_index, oid);
> + s->committable = 1;
> + strbuf_release(&full_name);
> + return 0;
> +}
> +
> static void wt_status_collect_changes_initial(struct wt_status *s)
> {
> struct index_state *istate = s->repo->index;
> @@ -671,6 +701,27 @@ static void wt_status_collect_changes_initial(struct wt_status *s)
> continue;
> if (ce_intent_to_add(ce))
> continue;
> + if (S_ISSPARSEDIR(ce->ce_mode)) {
> + /*
> + * This is a sparse directory entry, so we want to collect all
> + * of the added files within the tree. This requires recursively
> + * expanding the trees to find the elements that are new in this
> + * tree and marking them with DIFF_STATUS_ADDED.
> + */
> + struct strbuf base = STRBUF_INIT;
> + struct pathspec ps = { 0 };
> + struct tree *tree = lookup_tree(istate->repo, &ce->oid);
> +
> + ps.recursive = 1;
> + ps.has_wildcard = 1;
> + ps.max_depth = -1;
> +
> + strbuf_add(&base, ce->name, ce->ce_namelen);
> + read_tree_at(istate->repo, tree, &base, &ps,
> + add_file_to_list, s);
> + continue;
> + }
> +
> it = string_list_insert(&s->change, ce->name);
> d = it->util;
> if (!d) {
> --
> gitgitgadget