All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: gitster@pobox.com, vdye@github.com, shaoxuan.yuan02@gmail.com,
	Philip Oakley <philipoakley@iee.email>,
	Josh Steadmon <steadmon@google.com>,
	Derrick Stolee <derrickstolee@github.com>,
	Derrick Stolee <derrickstolee@github.com>
Subject: [PATCH v2 4/5] object-name: diagnose trees in index properly
Date: Tue, 26 Apr 2022 20:43:19 +0000	[thread overview]
Message-ID: <b730457fccc8b4d98beca70c82400446de4b69c4.1651005800.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1207.v2.git.1651005800.gitgitgadget@gmail.com>

From: Derrick Stolee <derrickstolee@github.com>

When running 'git show :<path>' where '<path>' is a directory, then
there is a subtle difference between a full checkout and a sparse
checkout. The error message from diagnose_invalid_index_path() reports
whether the path is on disk or not. The full checkout will have the
directory on disk, but the path will not be in the index. The sparse
checkout could have the directory not exist, specifically when that
directory is outside of the sparse-checkout cone.

In the case of a sparse index, we have yet another state: the path can
be a sparse directory in the index. In this case, the error message from
diagnose_invalid_index_path() would erroneously say "path '<path>' is in
the index, but not at stage 0", which is false.

Add special casing around sparse directory entries so we get to the
correct error message. This requires two checks in order to get parity
with the normal sparse-checkout case.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 object-name.c                            |  6 ++++--
 t/t1092-sparse-checkout-compatibility.sh | 18 ++++++++++++------
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/object-name.c b/object-name.c
index 2dc5d2549b8..4d2746574cd 100644
--- a/object-name.c
+++ b/object-name.c
@@ -1832,7 +1832,8 @@ static void diagnose_invalid_index_path(struct repository *r,
 		pos = -pos - 1;
 	if (pos < istate->cache_nr) {
 		ce = istate->cache[pos];
-		if (ce_namelen(ce) == namelen &&
+		if (!S_ISSPARSEDIR(ce->ce_mode) &&
+		    ce_namelen(ce) == namelen &&
 		    !memcmp(ce->name, filename, namelen))
 			die(_("path '%s' is in the index, but not at stage %d\n"
 			    "hint: Did you mean ':%d:%s'?"),
@@ -1848,7 +1849,8 @@ static void diagnose_invalid_index_path(struct repository *r,
 		pos = -pos - 1;
 	if (pos < istate->cache_nr) {
 		ce = istate->cache[pos];
-		if (ce_namelen(ce) == fullname.len &&
+		if (!S_ISSPARSEDIR(ce->ce_mode) &&
+		    ce_namelen(ce) == fullname.len &&
 		    !memcmp(ce->name, fullname.buf, fullname.len))
 			die(_("path '%s' is in the index, but not '%s'\n"
 			    "hint: Did you mean ':%d:%s' aka ':%d:./%s'?"),
diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
index 08c9cfd359e..fa1d5603605 100755
--- a/t/t1092-sparse-checkout-compatibility.sh
+++ b/t/t1092-sparse-checkout-compatibility.sh
@@ -1158,15 +1158,21 @@ test_expect_success 'show (cached blobs/trees)' '
 	test_all_match git show :deep/a &&
 	test_sparse_match git show :folder1/a &&
 
-	# Asking "git show" for directories in the index
-	# had different behavior depending on the existence
-	# of a sparse index.
+	# The error message differs depending on whether
+	# the directory exists in the worktree.
 	test_all_match test_must_fail git show :deep/ &&
 	test_must_fail git -C full-checkout show :folder1/ &&
-	test_must_fail git -C sparse-checkout show :folder1/ &&
+	test_sparse_match test_must_fail git show :folder1/ &&
 
-	test_must_fail git -C sparse-index show :folder1/ 2>err &&
-	grep "is in the index, but not at stage 0" err
+	# Change the sparse cone for an extra case:
+	run_on_sparse git sparse-checkout set deep/deeper1 &&
+
+	# deep/deeper2 is a sparse directory in the sparse index.
+	test_sparse_match test_must_fail git show :deep/deeper2/ &&
+
+	# deep/deeper2/deepest is not in the sparse index, but
+	# will trigger an index expansion.
+	test_sparse_match test_must_fail git show :deep/deeper2/deepest/
 '
 
 test_expect_success 'submodule handling' '
-- 
gitgitgadget


  parent reply	other threads:[~2022-04-26 20:43 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-07 16:37 [PATCH 0/4] Sparse index integration with 'git show' Derrick Stolee via GitGitGadget
2022-04-07 16:37 ` [PATCH 1/4] t1092: add compatibility tests for " Derrick Stolee via GitGitGadget
2022-04-14 18:37   ` Josh Steadmon
2022-04-18 12:23     ` Derrick Stolee
2022-04-07 16:37 ` [PATCH 2/4] show: integrate with the sparse index Derrick Stolee via GitGitGadget
2022-04-14 18:50   ` Josh Steadmon
2022-04-18 12:28     ` Derrick Stolee
2022-04-07 16:37 ` [PATCH 3/4] object-name: reject trees found in the index Derrick Stolee via GitGitGadget
2022-04-14 18:57   ` Josh Steadmon
2022-04-18 12:31     ` Derrick Stolee
2022-04-07 16:37 ` [PATCH 4/4] object-name: diagnose trees in index properly Derrick Stolee via GitGitGadget
2022-04-07 20:46   ` Philip Oakley
2022-04-12  6:32   ` Junio C Hamano
2022-04-12 13:52     ` Derrick Stolee
2022-04-12 15:45       ` Junio C Hamano
2022-04-14 18:37 ` [PATCH 0/4] Sparse index integration with 'git show' Josh Steadmon
2022-04-14 21:14   ` Junio C Hamano
2022-04-18 12:42     ` Derrick Stolee
2022-04-26 20:43 ` [PATCH v2 0/5] " Derrick Stolee via GitGitGadget
2022-04-26 20:43   ` [PATCH v2 1/5] t1092: add compatibility tests for " Derrick Stolee via GitGitGadget
2022-04-26 20:43   ` [PATCH v2 2/5] show: integrate with the sparse index Derrick Stolee via GitGitGadget
2022-04-26 20:43   ` [PATCH v2 3/5] object-name: reject trees found in the index Derrick Stolee via GitGitGadget
2022-04-26 20:43   ` Derrick Stolee via GitGitGadget [this message]
2022-04-26 20:43   ` [PATCH v2 5/5] rev-parse: integrate with sparse index Derrick Stolee via GitGitGadget
2022-04-26 20:55   ` [PATCH v2 0/5] Sparse index integration with 'git show' Junio C Hamano
2022-04-27 13:47     ` Derrick Stolee

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

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

  git send-email \
    --in-reply-to=b730457fccc8b4d98beca70c82400446de4b69c4.1651005800.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=philipoakley@iee.email \
    --cc=shaoxuan.yuan02@gmail.com \
    --cc=steadmon@google.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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.