git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] unpack-trees: don't update files with CE_WT_REMOVE set

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 21, 2015, 20:52 UTC
Message-ID
<xmqqlhe9ters.fsf@gitster.dls.corp.google.com>
In-Reply-To
<xmqqpp3pyxh0.fsf@gitster.dls.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 10 quoted lines
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> Thank you both for catching this. Just a small suggestion. Perhaps we
>> should do this instead. apply_sparse_checkout() is the function where
>> all "action" manipulation (add, delete, update files..) for sparse
>> checkout occurs and it should not ask to delete and update both at the
>> same time.
>
> Sounds good.  The first hunk may merely be a noise, but the second
> one is the true bugfix for the issue observed, I think.
I've replaced as/sparse-checkout-removal with the following.
Thanks, all.
-- >8 --
From: David Turner <dturner@twopensource.com>
Date: Fri, 17 Jul 2015 17:19:27 -0400
Subject: [PATCH] unpack-trees: don't update files with CE_WT_REMOVE set

Don't update files in the worktree from cache entries which are flagged with CE_WT_REMOVE.

When a user does a sparse checkout, git removes files that are marked with CE_WT_REMOVE (because they are out-of-scope for the sparse checkout). If those files are also marked CE_UPDATE (for instance, because they differ in the branch that is being checked out and the outgoing branch), git would previously recreate them. This patch prevents them from being recreated.

These erroneously-created files would also interfere with merges, causing pre-merge revisions of out-of-scope files to appear in the worktree.

apply_sparse_checkout() is the function where all "action" manipulation (add, delete, update files..) for sparse checkout occurs; it should not ask to delete and update both at the same time.

Signed-off-by: Anatole Shaw <git-devel@omni.poc.net>
Signed-off-by: David Turner <dturner@twopensource.com>
Helped-by: Duy Nguyen <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t1090-sparse-checkout-scope.sh | 52 ++++++++++++++++++++++++++++++++++++++++
 unpack-trees.c                   |  4 ++++
 2 files changed, 56 insertions(+)
 create mode 100755 t/t1090-sparse-checkout-scope.sh
diff --git a/t/t1090-sparse-checkout-scope.sh b/t/t1090-sparse-checkout-scope.sh
new file mode 100755
index 0000000..1f61eb3
--- /dev/null
+++ b/t/t1090-sparse-checkout-scope.sh
@@ -0,0 +1,52 @@
+#!/bin/sh
+
+test_description='sparse checkout scope tests'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo "initial" >a &&
+	echo "initial" >b &&
+	echo "initial" >c &&
+	git add a b c &&
+	git commit -m "initial commit"
+'
+
+test_expect_success 'create feature branch' '
+	git checkout -b feature &&
+	echo "modified" >b &&
+	echo "modified" >c &&
+	git add b c &&
+	git commit -m "modification"
+'
+
+test_expect_success 'perform sparse checkout of master' '
+	git config --local --bool core.sparsecheckout true &&
+	echo "!/*" >.git/info/sparse-checkout &&
+	echo "/a" >>.git/info/sparse-checkout &&
+	echo "/c" >>.git/info/sparse-checkout &&
+	git checkout master &&
+	test_path_is_file a &&
+	test_path_is_missing b &&
+	test_path_is_file c
+'
+
+test_expect_success 'merge feature branch into sparse checkout of master' '
+	git merge feature &&
+	test_path_is_file a &&
+	test_path_is_missing b &&
+	test_path_is_file c &&
+	test "$(cat c)" = "modified"
+'
+
+test_expect_success 'return to full checkout of master' '
+	git checkout feature &&
+	echo "/*" >.git/info/sparse-checkout &&
+	git checkout master &&
+	test_path_is_file a &&
+	test_path_is_file b &&
+	test_path_is_file c &&
+	test "$(cat b)" = "modified"
+'
+
+test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index 02f69ae..f177c0e 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -227,6 +227,9 @@ static int check_updates(struct unpack_trees_options *o)
 		struct cache_entry *ce = index->cache[i];
 
 		if (ce->ce_flags & CE_UPDATE) {
+			if (ce->ce_flags & CE_WT_REMOVE)
+				die("BUG: both update and delete flags are set on %s",
+				    ce->name);
 			display_progress(progress, ++cnt);
 			ce->ce_flags &= ~CE_UPDATE;
 			if (o->update && !o->dry_run) {
@@ -290,6 +293,7 @@ static int apply_sparse_checkout(struct cache_entry *ce, struct unpack_trees_opt
 		if (!(ce->ce_flags & CE_UPDATE) && verify_uptodate_sparse(ce, o))
 			return -1;
 		ce->ce_flags |= CE_WT_REMOVE;
+		ce->ce_flags &= ~CE_UPDATE;
 	}
 	if (was_skip_worktree && !ce_skip_worktree(ce)) {
 		if (verify_absent_sparse(ce, ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o))
-- 
2.5.0-rc2-392-g7dbe568
Previous: Junio C HamanoNext: David Turner
Message 7 of 8 in “unpack-trees: don't update files flagged for deletion”
  1. unpack-trees: don't update files flagged for deletionDavid Turner, Jul 17, 2015
  2. Junio C HamanoJul 17, 2015
  3. unpack-trees: don't update files with CE_WT_REMOVE setDavid Turner, Jul 17, 2015
  4. Junio C HamanoJul 17, 2015
  5. Duy NguyenJul 18, 2015
  6. Junio C HamanoJul 18, 2015
  7. Junio C HamanoJul 21, 2015
  8. David TurnerJul 20, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.