Inter-revision diff: patch 13

Comparing v9 (message) to v6 (message)

--- v9
+++ v6
@@ -1,115 +1,142 @@
 From: Derrick Stolee <dstolee@microsoft.com>
 
-By testing 'git -c core.fsmonitor= status -uno', we can check for the
-simplest index operations that can be made sparse-aware. The necessary
-implementation details are already integrated with sparse-checkout, so
-modify command_requires_full_index to be zero for cmd_status().
+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'.
 
-In refresh_index(), we loop through the index entries to refresh their
-stat() information. However, sparse directories have no stat()
-information to populate. Ignore these entries.
+This test failed before because the output of 'git status
+--porcelain=v2' would not match on the lines for folder1/:
 
-This allows 'git status' to no longer expand a sparse index to a full
-one. This is further tested by dropping the "-uno" option and adding an
-untracked file into the worktree.
+* The sparse-checkout repo (with a full index) would output each path
+  name that is intended to be added.
 
-The performance test p2000-sparse-checkout-operations.sh demonstrates
-these improvements:
+* The sparse-index repo would only output that "folder1/" is staged for
+  addition.
 
-Test                                  HEAD~1           HEAD
------------------------------------------------------------------------------
-2000.2: git status (full-index-v3)    0.31(0.30+0.05)  0.31(0.29+0.06) +0.0%
-2000.3: git status (full-index-v4)    0.31(0.29+0.07)  0.34(0.30+0.08) +9.7%
-2000.4: git status (sparse-index-v3)  2.35(2.28+0.10)  0.04(0.04+0.05) -98.3%
-2000.5: git status (sparse-index-v4)  2.35(2.24+0.15)  0.05(0.04+0.06) -97.9%
+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.
 
-Note that since HEAD~1 was expanding the sparse index by parsing trees,
-it was artificially slower than the full index case. Thus, the 98%
-improvement is misleading, and instead we should celebrate the 0.34s to
-0.05s improvement of 85%. This is more indicative of the peformance
-gains we are expecting by using a sparse index.
+Somehow, this loop over the cache entries was not guarded by
+ensure_full_index() as intended.
 
-Note: we are dropping the assignment of core.fsmonitor here. This is not
-necessary for the test script as we are not altering the config any
-other way. Correct integration with FS Monitor will be validated in
-later changes.
-
-Reviewed-by: Elijah Newren <newren@gmail.com>
 Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
 ---
- builtin/commit.c                         |  3 +++
- read-cache.c                             | 10 ++++++++--
- t/t1092-sparse-checkout-compatibility.sh | 13 +++++++++----
- 3 files changed, 20 insertions(+), 6 deletions(-)
+ t/t1092-sparse-checkout-compatibility.sh | 33 ++++++++++++++++
+ wt-status.c                              | 49 ++++++++++++++++++++++++
+ 2 files changed, 82 insertions(+)
 
-diff --git a/builtin/commit.c b/builtin/commit.c
-index 190d215d43b..12f51db158a 100644
---- a/builtin/commit.c
-+++ b/builtin/commit.c
-@@ -1510,6 +1510,9 @@ int cmd_status(int argc, const char **argv, const char *prefix)
- 	if (argc == 2 && !strcmp(argv[1], "-h"))
- 		usage_with_options(builtin_status_usage, builtin_status_options);
- 
-+	prepare_repo_settings(the_repository);
-+	the_repository->settings.command_requires_full_index = 0;
-+
- 	status_init_config(&s, git_status_config);
- 	argc = parse_options(argc, argv, prefix,
- 			     builtin_status_options,
-diff --git a/read-cache.c b/read-cache.c
-index 0c3ac3cefc0..6a1337cc905 100644
---- a/read-cache.c
-+++ b/read-cache.c
-@@ -1585,8 +1585,7 @@ int refresh_index(struct index_state *istate, unsigned int flags,
- 	 */
- 	preload_index(istate, pathspec, 0);
- 	trace2_region_enter("index", "refresh", NULL);
--	/* TODO: audit for interaction with sparse-index. */
--	ensure_full_index(istate);
-+
- 	for (i = 0; i < istate->cache_nr; i++) {
- 		struct cache_entry *ce, *new_entry;
- 		int cache_errno = 0;
-@@ -1601,6 +1600,13 @@ int refresh_index(struct index_state *istate, unsigned int flags,
- 		if (ignore_skip_worktree && ce_skip_worktree(ce))
- 			continue;
- 
-+		/*
-+		 * If this entry is a sparse directory, then there isn't
-+		 * any stat() information to update. Ignore the entry.
-+		 */
-+		if (S_ISSPARSEDIR(ce->ce_mode))
-+			continue;
-+
- 		if (pathspec && !ce_path_match(istate, ce, pathspec, seen))
- 			filtered = 1;
- 
 diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
-index 375b0d35565..751f397cc7f 100755
+index 1e9737cb4b7..ef918437908 100755
 --- a/t/t1092-sparse-checkout-compatibility.sh
 +++ b/t/t1092-sparse-checkout-compatibility.sh
-@@ -511,12 +511,17 @@ test_expect_success 'sparse-index is expanded and converted back' '
- 	GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
- 		git -C sparse-index -c core.fsmonitor="" reset --hard &&
- 	test_region index convert_to_sparse trace2.txt &&
--	test_region index ensure_full_index trace2.txt &&
-+	test_region index ensure_full_index trace2.txt
-+'
+@@ -521,4 +521,37 @@ test_expect_success 'sparse-index is not expanded' '
+ 	test_region ! index ensure_full_index trace2.txt
+ '
  
--	rm trace2.txt &&
-+test_expect_success 'sparse-index is not expanded' '
++test_expect_success 'reset mixed and checkout orphan' '
 +	init_repos &&
 +
-+	rm -f trace2.txt &&
-+	echo >>sparse-index/untracked.txt &&
- 	GIT_TRACE2_EVENT="$(pwd)/trace2.txt" GIT_TRACE2_EVENT_NESTING=10 \
--		git -C sparse-index -c core.fsmonitor="" status -uno &&
--	test_region index ensure_full_index trace2.txt
-+		git -C sparse-index status &&
-+	test_region ! index ensure_full_index trace2.txt
- '
++	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.
++	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..a90c7b6aa8a 100644
+--- a/wt-status.c
++++ b/wt-status.c
+@@ -657,6 +657,34 @@ static void wt_status_collect_changes_index(struct wt_status *s)
+ 	clear_pathspec(&rev.prune_data);
+ }
  
- test_done
++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;
++	char *full_name;
++
++	if (S_ISDIR(mode))
++		return READ_TREE_RECURSIVE;
++
++	full_name = xstrfmt("%s%s", base->buf, path);
++	it = string_list_insert(&s->change, full_name);
++	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;
++	return 0;
++}
++
+ static void wt_status_collect_changes_initial(struct wt_status *s)
+ {
+ 	struct index_state *istate = s->repo->index;
+@@ -671,6 +699,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