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.
> +
> 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_done
> diff --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
