Re: [PATCH v6 2/2] diff-files: integrate with sparse index
From: Victoria Dye <hidden>
Date: 2023-03-21 22:34:09
Shuqi Liang wrote:
Originally, diff-files a pathspec that is out-of-cone in a sparse-index environment, Git dies with "pathspec '<x>' did not match any files", mainly because it does not expand the index so nothing is matched. Expand the index when the <pathspec> needs an expanded index, i.e. the <pathspec> contains wildcard that may need a full-index or the <pathspec> is simply outside of sparse-checkout definition.
...
+ if (pathspec_needs_expanded_index(the_repository->index, &rev.diffopt.pathspec)) + ensure_full_index(the_repository->index);
Looks good! I'm glad you were able to use the tests to confirm that this pathspec-based expansion was needed.
quoted hunk ↗ jump to hunk
+ result = run_diff_files(&rev, options); result = diff_result_code(&rev.diffopt, result); cleanup:diff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh index 3242cfe91a..82751f2ca3 100755 --- a/t/perf/p2000-sparse-operations.sh +++ b/t/perf/p2000-sparse-operations.sh@@ -125,5 +125,7 @@ test_perf_on_all git checkout-index -f --all test_perf_on_all git update-index --add --remove $SPARSE_CONE/a test_perf_on_all "git rm -f $SPARSE_CONE/a && git checkout HEAD -- $SPARSE_CONE/a" test_perf_on_all git grep --cached --sparse bogus -- "f2/f1/f1/*" +test_perf_on_all git diff-files +test_perf_on_all git diff-files $SPARSE_CONE/a test_donediff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index c1329e2f16..6cbbc51a16 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh@@ -2097,4 +2097,35 @@ test_expect_failure 'diff-files with pathspec outside sparse definition' ' test_all_match git diff-files folder1/a ' +test_expect_success 'diff-files pathspec expands index when necessary' ' + init_repos && + + write_script edit-contents <<-\EOF && + echo text >>"$1" + EOF + + run_on_all ../edit-contents deep/a && + + # pathspec that should expand index + ! ensure_not_expanded diff-files "*/a" && + test_must_be_empty sparse-index-err && + + ! ensure_not_expanded diff-files "**a" && + test_must_be_empty sparse-index-err +'
Thanks for adding these, it's a good idea to show when the sparse index *is*
expanded in addition to when it is not. However, checking that the
'sparse-index-err' is empty won't handle silent failures, so it's probably
better to create an 'ensure_expanded' to mirror 'ensure_not_expanded'. The
two functions could share pretty much all of their code except for the last
line ('test_region ...').
+ +test_expect_success 'sparse index is not expanded: diff-files' ' + init_repos && + + write_script edit-contents <<-\EOF && + echo text >>"$1" + EOF + + run_on_all ../edit-contents deep/a && + + ensure_not_expanded diff-files && + ensure_not_expanded diff-files deep/a && + ensure_not_expanded diff-files deep/* +' + test_done