Thread (204 messages) flat view 204 messages, 5 authors, 2021-07-14

Re: [PATCH v7 15/16] wt-status: expand added sparse directory entries

From: Elijah Newren <hidden>
Date: 2021-07-09 01:04:07

On Mon, Jun 28, 2021 at 7:05 PM Derrick Stolee via GitGitGadget
[off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Derrick Stolee <redacted>

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 <redacted>
---
 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 sparse

Granted, 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.
quoted hunk ↗ jump to hunk
+       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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help