--- v6
+++ v9
@@ -1,113 +1,145 @@
From: Derrick Stolee <dstolee@microsoft.com>
-If we need to expand a sparse-index into a full one, then the FS Monitor
-bitmap is going to be incorrect. Ensure that we start fresh at such an
-event.
+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'.
-While this is currently a performance drawback, the eventual hope of the
-sparse-index feature is that these expansions will be rare and hence we
-will be able to keep the FS Monitor data accurate across multiple Git
-commands.
+This test failed before because the output of 'git status
+--porcelain=v2' would not match on the lines for folder1/:
-These tests are added to demonstrate that the behavior is the same
-across a full index and a sparse index, but also that file modifications
-to a tracked directory outside of the sparse cone will trigger
-ensure_full_index().
+* 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.
+
+Reviewed-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
- sparse-index.c | 7 ++++++
- t/t7519-status-fsmonitor.sh | 48 +++++++++++++++++++++++++++++++++++++
- 2 files changed, 55 insertions(+)
+ t/t1092-sparse-checkout-compatibility.sh | 33 +++++++++++++++
+ wt-status.c | 51 ++++++++++++++++++++++++
+ 2 files changed, 84 insertions(+)
-diff --git a/sparse-index.c b/sparse-index.c
-index ef53bd2198b..53c8f711ccc 100644
---- a/sparse-index.c
-+++ b/sparse-index.c
-@@ -186,6 +186,10 @@ int convert_to_sparse(struct index_state *istate)
- cache_tree_free(&istate->cache_tree);
- cache_tree_update(istate, 0);
-
-+ istate->fsmonitor_has_run_once = 0;
-+ FREE_AND_NULL(istate->fsmonitor_dirty);
-+ FREE_AND_NULL(istate->fsmonitor_last_update);
-+
- istate->sparse_index = 1;
- trace2_region_leave("index", "convert_to_sparse", istate->repo);
- return 0;
-@@ -282,6 +286,9 @@ void ensure_full_index(struct index_state *istate)
- istate->cache = full->cache;
- istate->cache_nr = full->cache_nr;
- istate->cache_alloc = full->cache_alloc;
-+ istate->fsmonitor_has_run_once = 0;
-+ FREE_AND_NULL(istate->fsmonitor_dirty);
-+ FREE_AND_NULL(istate->fsmonitor_last_update);
-
- strbuf_release(&base);
- free(full);
-diff --git a/t/t7519-status-fsmonitor.sh b/t/t7519-status-fsmonitor.sh
-index 637391c6ce4..8f9240b9b1a 100755
---- a/t/t7519-status-fsmonitor.sh
-+++ b/t/t7519-status-fsmonitor.sh
-@@ -73,6 +73,7 @@ test_expect_success 'setup' '
- expect*
- actual*
- marker*
-+ trace2*
- EOF
+diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
+index 751f397cc7f..2394c36d881 100755
+--- a/t/t1092-sparse-checkout-compatibility.sh
++++ b/t/t1092-sparse-checkout-compatibility.sh
+@@ -524,4 +524,37 @@ test_expect_success 'sparse-index is not expanded' '
+ test_region ! index ensure_full_index trace2.txt
'
-@@ -383,4 +384,51 @@ test_expect_success 'status succeeds after staging/unstaging' '
- )
- '
-
-+# Usage:
-+# check_sparse_index_behavior [!]
-+# If "!" is supplied, then we verify that we do not call ensure_full_index
-+# during a call to 'git status'. Otherwise, we verify that we _do_ call it.
-+check_sparse_index_behavior () {
-+ git status --porcelain=v2 >expect &&
-+ git sparse-checkout init --cone --sparse-index &&
-+ git sparse-checkout set dir1 dir2 &&
-+ GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
-+ git status --porcelain=v2 >actual &&
-+ test_region $1 index ensure_full_index trace2.txt &&
-+ test_cmp expect actual &&
-+ rm trace2.txt &&
-+ git sparse-checkout disable
-+}
++test_expect_success 'reset mixed and checkout orphan' '
++ init_repos &&
+
-+test_expect_success 'status succeeds with sparse index' '
-+ git reset --hard &&
++ test_all_match git checkout rename-out-to-in &&
+
-+ test_config core.fsmonitor "$TEST_DIRECTORY/t7519/fsmonitor-all" &&
-+ check_sparse_index_behavior ! &&
++ # 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.
++ test_sparse_match git reset --mixed HEAD~1 &&
++ test_sparse_match test-tool read-cache --table --expand &&
++ test_sparse_match git status --porcelain=v2 &&
+
-+ write_script .git/hooks/fsmonitor-test<<-\EOF &&
-+ printf "last_update_token\0"
-+ EOF
-+ git config core.fsmonitor .git/hooks/fsmonitor-test &&
-+ check_sparse_index_behavior ! &&
++ # 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
++'
+
-+ write_script .git/hooks/fsmonitor-test<<-\EOF &&
-+ printf "last_update_token\0"
-+ printf "dir1/modified\0"
-+ EOF
-+ check_sparse_index_behavior ! &&
++test_expect_success 'add everything with deep new file' '
++ init_repos &&
+
-+ cp -r dir1 dir1a &&
-+ git add dir1a &&
-+ git commit -m "add dir1a" &&
++ run_on_sparse git sparse-checkout set deep/deeper1/deepest &&
+
-+ # This one modifies outside the sparse-checkout definition
-+ # and hence we expect to expand the sparse-index.
-+ write_script .git/hooks/fsmonitor-test<<-\EOF &&
-+ printf "last_update_token\0"
-+ printf "dir1a/modified\0"
-+ EOF
-+ check_sparse_index_behavior
++ 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
+