grep: honor sparse-checkout on working tree searches
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>
Matheus Tavares committed
Feb 9, 2021 at 18:33 UTC
42d906bec48c95d66cb669ee41ce04330f74a98b
3 files changed
+179
-11
builtin/grep.c
+5
-2
@@ -508,6 +508,10 @@ static int grep_cache(struct grep_opt *opt,
508
509
for (nr = 0; nr < repo->index->cache_nr; nr++) {
510
const struct cache_entry *ce = repo->index->cache[nr];
511
+
512
+ if (!cached && ce_skip_worktree(ce))
513
+ continue;
514
+
515
strbuf_setlen(&name, name_base_len);
516
strbuf_addstr(&name, ce->name);
517
@@ -520,8 +524,7 @@ static int grep_cache(struct grep_opt *opt,
524
* cache entry are identical, even if worktree file has
525
* been modified, so use cache version instead
526
*/
523
- if (cached || (ce->ce_flags & CE_VALID) ||
524
- ce_skip_worktree(ce)) {
527
+ if (cached || (ce->ce_flags & CE_VALID)) {
528
if (ce_stage(ce) || ce_intent_to_add(ce))
529
continue;
530
hit |= grep_oid(opt, &ce->oid, name.buf,
t/t7011-skip-worktree-reading.sh
-9
@@ -109,15 +109,6 @@ test_expect_success 'ls-files --modified' '
109
test -z "$(git ls-files -m)"
110
'
111
112
-test_expect_success 'grep with skip-worktree file' '
113
- git update-index --no-skip-worktree 1 &&
114
- echo test > 1 &&
115
- git update-index 1 &&
116
- git update-index --skip-worktree 1 &&
117
- rm 1 &&
118
- test "$(git grep --no-ext-grep test)" = "1:test"
119
-'
120
-
112
echo ":000000 100644 $ZERO_OID $EMPTY_BLOB A 1" > expected
113
test_expect_success 'diff-index does not examine skip-worktree absent entries' '
114
setup_absent &&
t/t7817-grep-sparse-checkout.sh
new
+174
@@ -0,0 +1,174 @@
1
+#!/bin/sh
2
+
3
+test_description='grep in sparse checkout
4
+
5
+This test creates a repo with the following structure:
6
+
7
+.
8
+|-- a
9
+|-- b
10
+|-- dir
11
+| `-- c
12
+|-- sub
13
+| |-- A
14
+| | `-- a
15
+| `-- B
16
+| `-- b
17
+`-- sub2
18
+ `-- a
19
+
20
+Where the outer repository has non-cone mode sparsity patterns, sub is a
21
+submodule with cone mode sparsity patterns and sub2 is a submodule that is
22
+excluded by the superproject sparsity patterns. The resulting sparse checkout
23
+should leave the following structure in the working tree:
24
+
25
+.
26
+|-- a
27
+|-- sub
28
+| `-- B
29
+| `-- b
30
+`-- sub2
31
+ `-- a
32
+
33
+But note that sub2 should have the SKIP_WORKTREE bit set.
34
+'
35
+
36
+. ./test-lib.sh
37
+
38
+test_expect_success 'setup' '
39
+ echo "text" >a &&
40
+ echo "text" >b &&
41
+ mkdir dir &&
42
+ echo "text" >dir/c &&
43
+
44
+ git init sub &&
45
+ (
46
+ cd sub &&
47
+ mkdir A B &&
48
+ echo "text" >A/a &&
49
+ echo "text" >B/b &&
50
+ git add A B &&
51
+ git commit -m sub &&
52
+ git sparse-checkout init --cone &&
53
+ git sparse-checkout set B
54
+ ) &&
55
+
56
+ git init sub2 &&
57
+ (
58
+ cd sub2 &&
59
+ echo "text" >a &&
60
+ git add a &&
61
+ git commit -m sub2
62
+ ) &&
63
+
64
+ git submodule add ./sub &&
65
+ git submodule add ./sub2 &&
66
+ git add a b dir &&
67
+ git commit -m super &&
68
+ git sparse-checkout init --no-cone &&
69
+ git sparse-checkout set "/*" "!b" "!/*/" "sub" &&
70
+
71
+ git tag -am tag-to-commit tag-to-commit HEAD &&
72
+ tree=$(git rev-parse HEAD^{tree}) &&
73
+ git tag -am tag-to-tree tag-to-tree $tree &&
74
+
75
+ test_path_is_missing b &&
76
+ test_path_is_missing dir &&
77
+ test_path_is_missing sub/A &&
78
+ test_path_is_file a &&
79
+ test_path_is_file sub/B/b &&
80
+ test_path_is_file sub2/a &&
81
+ git branch -m main
82
+'
83
+
84
+# The test below covers a special case: the sparsity patterns exclude '/b' and
85
+# sparse checkout is enabled, but the path exists in the working tree (e.g.
86
+# manually created after `git sparse-checkout init`). git grep should skip it.
87
+test_expect_success 'working tree grep honors sparse checkout' '
88
+ cat >expect <<-EOF &&
89
+ a:text
90
+ EOF
91
+ test_when_finished "rm -f b" &&
92
+ echo "new-text" >b &&
93
+ git grep "text" >actual &&
94
+ test_cmp expect actual
95
+'
96
+
97
+test_expect_success 'grep searches unmerged file despite not matching sparsity patterns' '
98
+ cat >expect <<-EOF &&
99
+ b:modified-b-in-branchX
100
+ b:modified-b-in-branchY
101
+ EOF
102
+ test_when_finished "test_might_fail git merge --abort && \
103
+ git checkout main && git sparse-checkout init" &&
104
+
105
+ git sparse-checkout disable &&
106
+ git checkout -b branchY main &&
107
+ test_commit modified-b-in-branchY b &&
108
+ git checkout -b branchX main &&
109
+ test_commit modified-b-in-branchX b &&
110
+
111
+ git sparse-checkout init &&
112
+ test_path_is_missing b &&
113
+ test_must_fail git merge branchY &&
114
+ git grep "modified-b" >actual &&
115
+ test_cmp expect actual
116
+'
117
+
118
+test_expect_success 'grep --cached searches entries with the SKIP_WORKTREE bit' '
119
+ cat >expect <<-EOF &&
120
+ a:text
121
+ b:text
122
+ dir/c:text
123
+ EOF
124
+ git grep --cached "text" >actual &&
125
+ test_cmp expect actual
126
+'
127
+
128
+# Note that sub2/ is present in the worktree but it is excluded by the sparsity
129
+# patterns, so grep should not recurse into it.
130
+test_expect_success 'grep --recurse-submodules honors sparse checkout in submodule' '
131
+ cat >expect <<-EOF &&
132
+ a:text
133
+ sub/B/b:text
134
+ EOF
135
+ git grep --recurse-submodules "text" >actual &&
136
+ test_cmp expect actual
137
+'
138
+
139
+test_expect_success 'grep --recurse-submodules --cached searches entries with the SKIP_WORKTREE bit' '
140
+ cat >expect <<-EOF &&
141
+ a:text
142
+ b:text
143
+ dir/c:text
144
+ sub/A/a:text
145
+ sub/B/b:text
146
+ sub2/a:text
147
+ EOF
148
+ git grep --recurse-submodules --cached "text" >actual &&
149
+ test_cmp expect actual
150
+'
151
+
152
+test_expect_success 'working tree grep does not search the index with CE_VALID and SKIP_WORKTREE' '
153
+ cat >expect <<-EOF &&
154
+ a:text
155
+ EOF
156
+ test_when_finished "git update-index --no-assume-unchanged b" &&
157
+ git update-index --assume-unchanged b &&
158
+ git grep text >actual &&
159
+ test_cmp expect actual
160
+'
161
+
162
+test_expect_success 'grep --cached searches index entries with both CE_VALID and SKIP_WORKTREE' '
163
+ cat >expect <<-EOF &&
164
+ a:text
165
+ b:text
166
+ dir/c:text
167
+ EOF
168
+ test_when_finished "git update-index --no-assume-unchanged b" &&
169
+ git update-index --assume-unchanged b &&
170
+ git grep --cached text >actual &&
171
+ test_cmp expect actual
172
+'
173
+
174
+test_done