Skip to content

Commit

Permalink
grep: honor sparse-checkout on working tree searches
Browse files Browse the repository at this point in the history
On a sparse checked out repository, `git grep` (without --cached) ends
up searching the cache when an entry matches the search pathspec and has
the SKIP_WORKTREE bit set. This is confusing both because the sparse
paths are not expected to be in a working tree search (as they are not
checked out), and because the output mixes working tree and cache
results without distinguishing them. (Note that grep also resorts to the
cache on working tree searches that include --assume-unchanged paths.
But the whole point in that case is to assume that the contents of the
index entry and the file are the same. This does not apply to the case
of sparse paths, where the file isn't even expected to be present.)

Fix that by teaching grep to honor the sparse-checkout rules for working
tree searches. If the user wants to grep paths outside the current
sparse-checkout definition, they may either update the sparsity rules to
materialize the files, or use --cached to search all blobs registered in
the index.

Note: it might also be interesting to add a configuration option that
allow users to search paths that are present despite having the
SKIP_WORKTREE bit set, and/or to restrict searches in the index and past
revisions too. These ideas are left as future improvements to avoid
conflicting with other sparse-checkout topics currently in flight.

Suggested-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
Reviewed-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
matheustavares authored and gitster committed Feb 10, 2021
1 parent 773e25a commit 42d906b
Show file tree
Hide file tree
Showing 3 changed files with 179 additions and 11 deletions.
7 changes: 5 additions & 2 deletions builtin/grep.c
Expand Up @@ -508,6 +508,10 @@ static int grep_cache(struct grep_opt *opt,

for (nr = 0; nr < repo->index->cache_nr; nr++) {
const struct cache_entry *ce = repo->index->cache[nr];

if (!cached && ce_skip_worktree(ce))
continue;

strbuf_setlen(&name, name_base_len);
strbuf_addstr(&name, ce->name);

Expand All @@ -520,8 +524,7 @@ static int grep_cache(struct grep_opt *opt,
* cache entry are identical, even if worktree file has
* been modified, so use cache version instead
*/
if (cached || (ce->ce_flags & CE_VALID) ||
ce_skip_worktree(ce)) {
if (cached || (ce->ce_flags & CE_VALID)) {
if (ce_stage(ce) || ce_intent_to_add(ce))
continue;
hit |= grep_oid(opt, &ce->oid, name.buf,
Expand Down
9 changes: 0 additions & 9 deletions t/t7011-skip-worktree-reading.sh
Expand Up @@ -109,15 +109,6 @@ test_expect_success 'ls-files --modified' '
test -z "$(git ls-files -m)"
'

test_expect_success 'grep with skip-worktree file' '
git update-index --no-skip-worktree 1 &&
echo test > 1 &&
git update-index 1 &&
git update-index --skip-worktree 1 &&
rm 1 &&
test "$(git grep --no-ext-grep test)" = "1:test"
'

echo ":000000 100644 $ZERO_OID $EMPTY_BLOB A 1" > expected
test_expect_success 'diff-index does not examine skip-worktree absent entries' '
setup_absent &&
Expand Down
174 changes: 174 additions & 0 deletions t/t7817-grep-sparse-checkout.sh
@@ -0,0 +1,174 @@
#!/bin/sh

test_description='grep in sparse checkout
This test creates a repo with the following structure:
.
|-- a
|-- b
|-- dir
| `-- c
|-- sub
| |-- A
| | `-- a
| `-- B
| `-- b
`-- sub2
`-- a
Where the outer repository has non-cone mode sparsity patterns, sub is a
submodule with cone mode sparsity patterns and sub2 is a submodule that is
excluded by the superproject sparsity patterns. The resulting sparse checkout
should leave the following structure in the working tree:
.
|-- a
|-- sub
| `-- B
| `-- b
`-- sub2
`-- a
But note that sub2 should have the SKIP_WORKTREE bit set.
'

. ./test-lib.sh

test_expect_success 'setup' '
echo "text" >a &&
echo "text" >b &&
mkdir dir &&
echo "text" >dir/c &&
git init sub &&
(
cd sub &&
mkdir A B &&
echo "text" >A/a &&
echo "text" >B/b &&
git add A B &&
git commit -m sub &&
git sparse-checkout init --cone &&
git sparse-checkout set B
) &&
git init sub2 &&
(
cd sub2 &&
echo "text" >a &&
git add a &&
git commit -m sub2
) &&
git submodule add ./sub &&
git submodule add ./sub2 &&
git add a b dir &&
git commit -m super &&
git sparse-checkout init --no-cone &&
git sparse-checkout set "/*" "!b" "!/*/" "sub" &&
git tag -am tag-to-commit tag-to-commit HEAD &&
tree=$(git rev-parse HEAD^{tree}) &&
git tag -am tag-to-tree tag-to-tree $tree &&
test_path_is_missing b &&
test_path_is_missing dir &&
test_path_is_missing sub/A &&
test_path_is_file a &&
test_path_is_file sub/B/b &&
test_path_is_file sub2/a &&
git branch -m main
'

# The test below covers a special case: the sparsity patterns exclude '/b' and
# sparse checkout is enabled, but the path exists in the working tree (e.g.
# manually created after `git sparse-checkout init`). git grep should skip it.
test_expect_success 'working tree grep honors sparse checkout' '
cat >expect <<-EOF &&
a:text
EOF
test_when_finished "rm -f b" &&
echo "new-text" >b &&
git grep "text" >actual &&
test_cmp expect actual
'

test_expect_success 'grep searches unmerged file despite not matching sparsity patterns' '
cat >expect <<-EOF &&
b:modified-b-in-branchX
b:modified-b-in-branchY
EOF
test_when_finished "test_might_fail git merge --abort && \
git checkout main && git sparse-checkout init" &&
git sparse-checkout disable &&
git checkout -b branchY main &&
test_commit modified-b-in-branchY b &&
git checkout -b branchX main &&
test_commit modified-b-in-branchX b &&
git sparse-checkout init &&
test_path_is_missing b &&
test_must_fail git merge branchY &&
git grep "modified-b" >actual &&
test_cmp expect actual
'

test_expect_success 'grep --cached searches entries with the SKIP_WORKTREE bit' '
cat >expect <<-EOF &&
a:text
b:text
dir/c:text
EOF
git grep --cached "text" >actual &&
test_cmp expect actual
'

# Note that sub2/ is present in the worktree but it is excluded by the sparsity
# patterns, so grep should not recurse into it.
test_expect_success 'grep --recurse-submodules honors sparse checkout in submodule' '
cat >expect <<-EOF &&
a:text
sub/B/b:text
EOF
git grep --recurse-submodules "text" >actual &&
test_cmp expect actual
'

test_expect_success 'grep --recurse-submodules --cached searches entries with the SKIP_WORKTREE bit' '
cat >expect <<-EOF &&
a:text
b:text
dir/c:text
sub/A/a:text
sub/B/b:text
sub2/a:text
EOF
git grep --recurse-submodules --cached "text" >actual &&
test_cmp expect actual
'

test_expect_success 'working tree grep does not search the index with CE_VALID and SKIP_WORKTREE' '
cat >expect <<-EOF &&
a:text
EOF
test_when_finished "git update-index --no-assume-unchanged b" &&
git update-index --assume-unchanged b &&
git grep text >actual &&
test_cmp expect actual
'

test_expect_success 'grep --cached searches index entries with both CE_VALID and SKIP_WORKTREE' '
cat >expect <<-EOF &&
a:text
b:text
dir/c:text
EOF
test_when_finished "git update-index --no-assume-unchanged b" &&
git update-index --assume-unchanged b &&
git grep --cached text >actual &&
test_cmp expect actual
'

test_done

0 comments on commit 42d906b

Please sign in to comment.