git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: shaoxuan.yuan02@gmail.com, gitster@pobox.com,
	derrickstolee@github.com, Victoria Dye <vdye@github.com>,
	Victoria Dye <vdye@github.com>
Subject: [PATCH] unpack-trees: fix sparse directory recursion check
Date: Thu, 01 Sep 2022 21:02:33 +0000	[thread overview]
Message-ID: <pull.1344.git.1662066153644.gitgitgadget@gmail.com> (raw)

From: Victoria Dye <vdye@github.com>

Ensure 'is_sparse_directory_entry()' receives a valid 'name_entry *' if one
exists in the list of tree(s) being unpacked in 'unpack_callback()'.

Currently, 'is_sparse_directory_entry()' is called with the first
'name_entry' in the 'names' list of entries on 'unpack_callback()'. However,
this entry may be empty even when other elements of 'names' are not (such as
when switching from an orphan branch back to a "normal" branch). As a
result, 'is_sparse_directory_entry()' could incorrectly indicate that a
sparse directory is *not* actually sparse because the name of the index
entry does not match the (empty) 'name_entry' path.

Fix the issue by using the existing 'name_entry *p' value in
'unpack_callback()', which points to the first non-empty entry in 'names'.
Because 'p' is 'const', also update 'is_sparse_directory_entry()'s
'name_entry *' argument to be 'const'.

Finally, add a regression test case.

Reported-by: Derrick Stolee <derrickstolee@github.com>
Signed-off-by: Victoria Dye <vdye@github.com>
---
    unpack-trees: fix sparse directory recursion check
    
    This issue was found when the updates from v2.37.3 introduced a test
    failure in a downstream test suite.
    
    The issue stems from the fact that, before v2.37.3, 'unpack_callback()'
    could previously "assume" that 'names[0]' was non-empty if a cache entry
    was unpacked as a sparse index. When b15207b8cf (unpack-trees: unpack
    new trees as sparse directories, 2022-08-08)) was introduced, it
    invalidated that assumption by allowing sparse directories to be
    unpacked based on the contents of other 'names' entries, rather than
    unnecessarily recursing into them and unpacking files individually. As a
    result, certain scenarios could cause a sparse directory to be unpacked
    then also recursively unpacked via 'traverse_trees_recursive()',
    creating duplicate index entries.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1344%2Fvdye%2Fbugfix%2Fsparse-index-orphan-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1344/vdye/bugfix/sparse-index-orphan-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1344

 t/t1092-sparse-checkout-compatibility.sh | 9 +++++++++
 unpack-trees.c                           | 4 ++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
index 0302e36fd66..b9350c075c2 100755
--- a/t/t1092-sparse-checkout-compatibility.sh
+++ b/t/t1092-sparse-checkout-compatibility.sh
@@ -380,6 +380,15 @@ test_expect_success 'checkout with modified sparse directory' '
 	test_all_match git checkout base
 '
 
+test_expect_success 'checkout orphan then non-orphan' '
+	init_repos &&
+
+	test_all_match git checkout --orphan test-orphan &&
+	test_all_match git status --porcelain=v2 &&
+	test_all_match git checkout base &&
+	test_all_match git status --porcelain=v2
+'
+
 test_expect_success 'add outside sparse cone' '
 	init_repos &&
 
diff --git a/unpack-trees.c b/unpack-trees.c
index 90b92114be8..bae812156c4 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1423,7 +1423,7 @@ static void debug_unpack_callback(int n,
  * from the tree walk at the given traverse_info.
  */
 static int is_sparse_directory_entry(struct cache_entry *ce,
-				     struct name_entry *name,
+				     const struct name_entry *name,
 				     struct traverse_info *info)
 {
 	if (!ce || !name || !S_ISSPARSEDIR(ce->ce_mode))
@@ -1562,7 +1562,7 @@ static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, str
 			}
 		}
 
-		if (!is_sparse_directory_entry(src[0], names, info) &&
+		if (!is_sparse_directory_entry(src[0], p, info) &&
 		    !is_new_sparse_dir &&
 		    traverse_trees_recursive(n, dirmask, mask & ~dirmask,
 						    names, info) < 0) {

base-commit: d42b38dfb5edf1a7fddd9542d722f91038407819
-- 
gitgitgadget

             reply	other threads:[~2022-09-01 21:03 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-01 21:02 Victoria Dye via GitGitGadget [this message]
2022-09-02  9:03 ` [PATCH] unpack-trees: fix sparse directory recursion check Johannes Schindelin
2022-09-02 13:32 ` Derrick Stolee
2022-09-02 16:57   ` Junio C Hamano
2022-09-02 19:03 ` Shaoxuan Yuan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1344.git.1662066153644.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=shaoxuan.yuan02@gmail.com \
    --cc=vdye@github.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).