Inter-revision diff: patch 15

Comparing v7 (message) to v9 (message)

--- v7
+++ v9
@@ -1,144 +1,116 @@
 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'.
+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.
 
-This test failed before because the output of 'git status
---porcelain=v2' would not match on the lines for folder1/:
+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.
 
-* The sparse-checkout repo (with a full index) would output each path
-  name that is intended to be added.
+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-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>
 ---
- t/t1092-sparse-checkout-compatibility.sh | 33 +++++++++++++++
- wt-status.c                              | 51 ++++++++++++++++++++++++
- 2 files changed, 84 insertions(+)
+ sparse-index.c              |  7 ++++++
+ t/t7519-status-fsmonitor.sh | 49 +++++++++++++++++++++++++++++++++++++
+ 2 files changed, 56 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
+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..deea88d4431 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
  '
  
-+test_expect_success 'reset mixed and checkout orphan' '
-+	init_repos &&
+@@ -383,4 +384,52 @@ 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_region fsm_hook query trace2.txt &&
++	test_cmp expect actual &&
++	rm trace2.txt &&
++	git sparse-checkout disable
++}
 +
-+	test_all_match git checkout rename-out-to-in &&
++test_expect_success 'status succeeds with sparse index' '
++	git reset --hard &&
 +
-+	# 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 &&
++	test_config core.fsmonitor "$TEST_DIRECTORY/t7519/fsmonitor-all" &&
++	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"
++	EOF
++	git config core.fsmonitor .git/hooks/fsmonitor-test &&
++	check_sparse_index_behavior ! &&
 +
-+test_expect_success 'add everything with deep new file' '
-+	init_repos &&
++	write_script .git/hooks/fsmonitor-test<<-\EOF &&
++		printf "last_update_token\0"
++		printf "dir1/modified\0"
++	EOF
++	check_sparse_index_behavior ! &&
 +
-+	run_on_sparse git sparse-checkout set deep/deeper1/deepest &&
++	cp -r dir1 dir1a &&
++	git add dir1a &&
++	git commit -m "add dir1a" &&
 +
-+	run_on_all touch deep/deeper1/x &&
-+	test_all_match git add . &&
-+	test_all_match git status --porcelain=v2
++	# 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
 +'
 +
  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