Re: [PATCH v3 2/2] blame: enable and test the sparse index

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/3/21 9:47 AM, Junio C Hamano wrote:
"Lessley Dennington via GitGitGadget" <gitgitgadget@xxxxxxxxx>
writes:

We do not include paths outside the sparse checkout cone because blame
currently does not support blaming files outside of the sparse definition.
Attempting to do so fails with the following error:

   fatal: no such path '<path outside sparse definition>' in HEAD

Does this indicate that we need to update how the command line
safety in verify_working_tree_path() works in a sparsely checked out
working tree?  If foo/bar is outside the sparse definition,

     git blame HEAD foo/bar

may get such a message, but shouldn't

     git blame HEAD -- foo/bar

make it work?

This is something we could consider for the future, and I've updated the comment you called out below to better reflect that in the next version. However, I don't know that this change belongs in this series, which aims to enable the sparse index. Perhaps even updating the 'blame with pathspec outside sparse definition' test was a bit too far outside the target scope of this change - I can remove my updates if that is the case.

-# TODO: blame currently does not support blaming files outside of the
-# sparse definition. It complains that the file doesn't exist locally.
-test_expect_failure 'blame with pathspec outside sparse definition' '
+# Blame does not support blaming files outside of the sparse
+# definition, so we verify this scenario.

IOW, why is it a good idea to drop the "TODO" and "currently" and pretend
as if the current behaviour is the desirable one?

Thank you, updated for v4.

+test_expect_success 'blame with pathspec outside sparse definition' '
  	init_repos &&
+	test_sparse_match git sparse-checkout set &&
- test_all_match git blame folder1/a &&
-	test_all_match git blame folder2/a &&
-	test_all_match git blame deep/deeper2/a &&
-	test_all_match git blame deep/deeper2/deepest/a
+	for file in a \
+			deep/a \
+			deep/deeper1/a \
+			deep/deeper1/deepest/a
+	do
+		test_sparse_match test_must_fail git blame $file &&
+		cat >expect <<-EOF &&
+		fatal: Cannot lstat '"'"'$file'"'"': No such file or directory
+		EOF
+		# We compare sparse-checkout-err and sparse-index-err in
+		# `test_sparse_match`. Given we know they are the same, we
+		# only check the content of sparse-index-err here.
+		test_cmp expect sparse-index-err
+	done
  '
test_expect_success 'checkout and reset (mixed)' '
@@ -878,6 +892,18 @@ test_expect_success 'sparse index is not expanded: diff' '
  	ensure_not_expanded diff --staged
  '
+test_expect_success 'sparse index is not expanded: blame' '
+	init_repos &&
+
+	for file in a \
+			deep/a \
+			deep/deeper1/a \
+			deep/deeper1/deepest/a
+	do
+		ensure_not_expanded blame $file
+	done
+'
+
  # NEEDSWORK: a sparse-checkout behaves differently from a full checkout
  # in this scenario, but it shouldn't.
  test_expect_success 'reset mixed and checkout orphan' '

Best,

Lessley



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux