Inter-revision diff: patch 1

Comparing v2 (message) to v4 (message)

--- v2
+++ v4
@@ -1,78 +1,102 @@
 From: Derrick Stolee <dstolee@microsoft.com>
 
-Before moving to update 'git status' and 'git add' to work with sparse
-indexes, add an explicit test that ensures the sparse-index works the
-same as a normal sparse-checkout when the worktree contains directories
-and files outside of the sparse cone.
+The sparse-index format is designed to be compatible with merge
+conflicts, even those outside the sparse-checkout definition. The reason
+is that when converting a full index to a sparse one, a cache entry with
+nonzero stage will not be collapsed into a sparse directory entry.
 
-Specifically, 'folder1/a' is a file in our test repo, but 'folder1' is
-not in the sparse cone. When 'folder1/a' is modified, the file
-'folder1/a' is shown as modified, but adding it fails. This is new
-behavior as of a20f704 (add: warn when asked to update SKIP_WORKTREE
-entries, 2021-04-08). Before that change, these adds would be silently
-ignored.
+However, this behavior was not tested, and a different behavior within
+convert_to_sparse() fails in this scenario. Specifically,
+cache_tree_update() will fail when unmerged entries exist.
+convert_to_sparse_rec() uses the cache-tree data to recursively walk the
+tree structure, but also to compute the OIDs used in the
+sparse-directory entries.
 
-Untracked files are fine: adding new files both with 'git add .' and
-'git add folder1/' works just as in a full checkout. This may not be
-entirely desirable, but we are not intending to change behavior at the
-moment, only document it. A future change could alter the behavior to
-be more sensible, and this test could be modified to satisfy the new
-expected behavior.
+Add an index scan to convert_to_sparse() that will detect if these merge
+conflict entries exist and skip the conversion before trying to update
+the cache-tree. This is marked as NEEDSWORK because this can be removed
+with a suitable update to cache_tree_update() or a similar method that
+can construct a cache-tree with invalid nodes, but still allow creating
+the nodes necessary for creating sparse directory entries.
+
+It is possible that in the future we will not need to make such an
+update, since if we do not expand a sparse-index into a full one, this
+conversion does not need to happen. Thus, this can be deferred until the
+merge machinery is made to integrate with the sparse-index.
 
 Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
 ---
- t/t1092-sparse-checkout-compatibility.sh | 40 ++++++++++++++++++++++++
- 1 file changed, 40 insertions(+)
+ sparse-index.c                           | 18 ++++++++++++++++++
+ t/t1092-sparse-checkout-compatibility.sh | 22 ++++++++++++++++++++++
+ 2 files changed, 40 insertions(+)
 
+diff --git a/sparse-index.c b/sparse-index.c
+index 6f21397e2ee0..1b49898d0cb7 100644
+--- a/sparse-index.c
++++ b/sparse-index.c
+@@ -125,6 +125,17 @@ int set_sparse_index_config(struct repository *repo, int enable)
+ 	return res;
+ }
+ 
++static int index_has_unmerged_entries(struct index_state *istate)
++{
++	int i;
++	for (i = 0; i < istate->cache_nr; i++) {
++		if (ce_stage(istate->cache[i]))
++			return 1;
++	}
++
++	return 0;
++}
++
+ int convert_to_sparse(struct index_state *istate)
+ {
+ 	int test_env;
+@@ -161,6 +172,13 @@ int convert_to_sparse(struct index_state *istate)
+ 		return -1;
+ 	}
+ 
++	/*
++	 * NEEDSWORK: If we have unmerged entries, then stay full.
++	 * Unmerged entries prevent the cache-tree extension from working.
++	 */
++	if (index_has_unmerged_entries(istate))
++		return 0;
++
+ 	if (cache_tree_update(istate, 0)) {
+ 		warning(_("unable to update cache-tree, staying full"));
+ 		return -1;
 diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
-index 12e6c453024f..0ec487acd283 100755
+index 12e6c453024f..4f2f09b53a32 100755
 --- a/t/t1092-sparse-checkout-compatibility.sh
 +++ b/t/t1092-sparse-checkout-compatibility.sh
-@@ -232,6 +232,46 @@ test_expect_success 'add, commit, checkout' '
- 	test_all_match git checkout -
+@@ -352,6 +352,28 @@ test_expect_success 'merge with outside renames' '
+ 	done
  '
  
-+test_expect_success 'status/add: outside sparse cone' '
++# Sparse-index fails to convert the index in the
++# final 'git cherry-pick' command.
++test_expect_success 'cherry-pick with conflicts' '
 +	init_repos &&
 +
-+	# folder1 is at HEAD, but outside the sparse cone
-+	run_on_sparse mkdir folder1 &&
-+	cp initial-repo/folder1/a sparse-checkout/folder1/a &&
-+	cp initial-repo/folder1/a sparse-index/folder1/a &&
++	write_script edit-conflict <<-\EOF &&
++	echo $1 >conflict
++	EOF
 +
-+	test_sparse_match git status &&
++	test_all_match git checkout -b to-cherry-pick &&
++	run_on_all ../edit-conflict ABC &&
++	test_all_match git add conflict &&
++	test_all_match git commit -m "conflict to pick" &&
 +
-+	write_script edit-contents <<-\EOF &&
-+	echo text >>$1
-+	EOF
-+	run_on_all ../edit-contents folder1/a &&
-+	run_on_all ../edit-contents folder1/new &&
++	test_all_match git checkout -B base HEAD~1 &&
++	run_on_all ../edit-conflict DEF &&
++	test_all_match git add conflict &&
++	test_all_match git commit -m "conflict in base" &&
 +
-+	test_sparse_match git status --porcelain=v2 &&
-+
-+	# This "git add folder1/a" is completely ignored
-+	# by the sparse-checkout repos. It causes the
-+	# full repo to have a different staged environment.
-+	#
-+	# This is not a desirable behavior, but this test
-+	# ensures that the sparse-index is not the cause
-+	# of a behavior change.
-+	test_sparse_match test_must_fail git add folder1/a &&
-+	test_sparse_match test_must_fail git add --refresh folder1/a &&
-+	git -C full-checkout checkout HEAD -- folder1/a &&
-+	test_all_match git status --porcelain=v2 &&
-+
-+	test_all_match git add . &&
-+	test_all_match git status --porcelain=v2 &&
-+	test_all_match git commit -m folder1/new &&
-+
-+	run_on_all ../edit-contents folder1/newer &&
-+	test_all_match git add folder1/ &&
-+	test_all_match git status --porcelain=v2 &&
-+	test_all_match git commit -m folder1/newer
++	test_all_match test_must_fail git cherry-pick to-cherry-pick
 +'
 +
- test_expect_success 'checkout and reset --hard' '
+ test_expect_success 'clean' '
  	init_repos &&
  
 -- 
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help