completion: cache the path to the repository
After the previous changes in this series there are only a handful of $(__gitdir) command substitutions left in the completion script, but there is still a bit of room for improvements: 1. The command substitution involves the forking of a subshell, which has considerable overhead on some platforms. 2. There are a few cases, where this command substitution is executed more than once during a single completion, which means multiple subshells and possibly multiple 'git rev-parse' executions. __gitdir() is invoked twice while completing refs for e.g. 'git log', 'git rebase', 'gitk', or while completing remote refs for 'git fetch' or 'git push'. Both of these points can be addressed by using the __git_find_repo_path() helper function introduced in the previous commit: 1. __git_find_repo_path() stores the path to the repository in a variable instead of printing it, so the command substitution around the function can be avoided. Or rather: the command substitution should be avoided to make the new value of the variable set inside the function visible to the callers. (Yes, there is now a command substitution inside __git_find_repo_path() around each 'git rev-parse', but that's executed only if necessary, and only once per completion, see point 2. below.) 2. $__git_repo_path, the variable holding the path to the repository, is declared local in the toplevel completion functions __git_main() and __gitk_main(). Thus, once set, the path is visible in all completion functions, including all subsequent calls to __git_find_repo_path(), meaning that they wouldn't have to re-discover the path to the repository. So call __git_find_repo_path() and use $__git_repo_path instead of the $(__gitdir) command substitution to access paths in the .git directory. Turn tests checking __gitdir()'s repository discovery into tests of __git_find_repo_path() such that only the tested function changes but the expected results don't, ensuring that repo discovery keeps working as it did before. As __gitdir() is not used anymore in the completion script, mark it as deprecated and direct users' attention to __git_find_repo_path() and $__git_repo_path. Yet keep four __gitdir() tests to ensure that it handles success and failure of __git_find_repo_path() and that it still handles its optional remote argument, because users' custom completion scriptlets might depend on it. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
SZEDER Gábor committed
Feb 3, 2017 at 03:48 UTC
fad9484f0aebc818112fb52270ebcddc768a7573
2 files changed
+132
-75
contrib/completion/git-completion.bash
+29
-17
@@ -39,6 +39,11 @@ esac
39
# variable.
40
__git_find_repo_path ()
41
{
42
+ if [ -n "$__git_repo_path" ]; then
43
+ # we already know where it is
44
+ return
45
+ fi
46
+
47
if [ -n "${__git_C_args-}" ]; then
48
__git_repo_path="$(git "${__git_C_args[@]}" \
49
${__git_dir:+--git-dir="$__git_dir"} \
@@ -56,6 +61,7 @@ __git_find_repo_path ()
61
fi
62
}
63
64
+# Deprecated: use __git_find_repo_path() and $__git_repo_path instead
65
# __gitdir accepts 0 or 1 arguments (i.e., location)
66
# returns location of .git repo
67
__gitdir ()
@@ -350,10 +356,13 @@ __git_tags ()
356
# 'git checkout's tracking DWIMery (optional; ignored, if set but empty).
357
__git_refs ()
358
{
353
- local i hash dir="$(__gitdir)" track="${2-}"
359
+ local i hash dir track="${2-}"
360
local list_refs_from=path remote="${1-}"
361
local format refs pfx
362
363
+ __git_find_repo_path
364
+ dir="$__git_repo_path"
365
+
366
if [ -z "$remote" ]; then
367
if [ -z "$dir" ]; then
368
return
@@ -458,8 +467,8 @@ __git_refs_remotes ()
467
468
__git_remotes ()
469
{
461
- local d="$(__gitdir)"
462
- test -d "$d/remotes" && ls -1 "$d/remotes"
470
+ __git_find_repo_path
471
+ test -d "$__git_repo_path/remotes" && ls -1 "$__git_repo_path/remotes"
472
__git remote
473
}
474
@@ -957,8 +966,8 @@ __git_whitespacelist="nowarn warn error error-all fix"
966
967
_git_am ()
968
{
960
- local dir="$(__gitdir)"
961
- if [ -d "$dir"/rebase-apply ]; then
969
+ __git_find_repo_path
970
+ if [ -d "$__git_repo_path"/rebase-apply ]; then
971
__gitcomp "--skip --continue --resolved --abort"
972
return
973
fi
@@ -1041,7 +1050,8 @@ _git_bisect ()
1050
local subcommands="start bad good skip reset visualize replay log run"
1051
local subcommand="$(__git_find_on_cmdline "$subcommands")"
1052
if [ -z "$subcommand" ]; then
1044
- if [ -f "$(__gitdir)"/BISECT_START ]; then
1053
+ __git_find_repo_path
1054
+ if [ -f "$__git_repo_path"/BISECT_START ]; then
1055
__gitcomp "$subcommands"
1056
else
1057
__gitcomp "replay start"
@@ -1146,8 +1156,8 @@ _git_cherry ()
1156
1157
_git_cherry_pick ()
1158
{
1149
- local dir="$(__gitdir)"
1150
- if [ -f "$dir"/CHERRY_PICK_HEAD ]; then
1159
+ __git_find_repo_path
1160
+ if [ -f "$__git_repo_path"/CHERRY_PICK_HEAD ]; then
1161
__gitcomp "--continue --quit --abort"
1162
return
1163
fi
@@ -1538,10 +1548,10 @@ __git_log_date_formats="relative iso8601 rfc2822 short local default raw"
1548
_git_log ()
1549
{
1550
__git_has_doubledash && return
1551
+ __git_find_repo_path
1552
1542
- local g="$(__gitdir)"
1553
local merge=""
1544
- if [ -f "$g/MERGE_HEAD" ]; then
1554
+ if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
1555
merge="--merge"
1556
fi
1557
case "$cur" in
@@ -1788,11 +1798,12 @@ _git_push ()
1798
1799
_git_rebase ()
1800
{
1791
- local dir="$(__gitdir)"
1792
- if [ -f "$dir"/rebase-merge/interactive ]; then
1801
+ __git_find_repo_path
1802
+ if [ -f "$__git_repo_path"/rebase-merge/interactive ]; then
1803
__gitcomp "--continue --skip --abort --quit --edit-todo"
1804
return
1795
- elif [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
1805
+ elif [ -d "$__git_repo_path"/rebase-apply ] || \
1806
+ [ -d "$__git_repo_path"/rebase-merge ]; then
1807
__gitcomp "--continue --skip --abort --quit"
1808
return
1809
fi
@@ -2467,8 +2478,8 @@ _git_reset ()
2478
2479
_git_revert ()
2480
{
2470
- local dir="$(__gitdir)"
2471
- if [ -f "$dir"/REVERT_HEAD ]; then
2481
+ __git_find_repo_path
2482
+ if [ -f "$__git_repo_path"/REVERT_HEAD ]; then
2483
__gitcomp "--continue --quit --abort"
2484
return
2485
fi
@@ -2865,9 +2876,10 @@ __gitk_main ()
2876
__git_has_doubledash && return
2877
2878
local __git_repo_path
2868
- local g="$(__gitdir)"
2879
+ __git_find_repo_path
2880
+
2881
local merge=""
2870
- if [ -f "$g/MERGE_HEAD" ]; then
2882
+ if [ -f "$__git_repo_path/MERGE_HEAD" ]; then
2883
merge="--merge"
2884
fi
2885
case "$cur" in
t/t9902-completion.sh
+103
-58
@@ -131,213 +131,217 @@ else
131
ROOT="$(pwd)"
132
fi
133
134
-test_expect_success 'setup for __gitdir tests' '
134
+test_expect_success 'setup for __git_find_repo_path/__gitdir tests' '
135
mkdir -p subdir/subsubdir &&
136
+ mkdir -p non-repo &&
137
git init otherrepo
138
'
139
139
-test_expect_success '__gitdir - from command line (through $__git_dir)' '
140
+test_expect_success '__git_find_repo_path - from command line (through $__git_dir)' '
141
echo "$ROOT/otherrepo/.git" >expected &&
142
(
143
__git_dir="$ROOT/otherrepo/.git" &&
143
- __gitdir >"$actual"
144
- ) &&
145
- test_cmp expected "$actual"
146
-'
147
-
148
-test_expect_success '__gitdir - repo as argument' '
149
- echo "otherrepo/.git" >expected &&
150
- (
151
- __gitdir "otherrepo" >"$actual"
144
+ __git_find_repo_path &&
145
+ echo "$__git_repo_path" >"$actual"
146
) &&
147
test_cmp expected "$actual"
148
'
149
156
-test_expect_success '__gitdir - remote as argument' '
157
- echo "remote" >expected &&
158
- (
159
- __gitdir "remote" >"$actual"
160
- ) &&
161
- test_cmp expected "$actual"
162
-'
163
-
164
-test_expect_success '__gitdir - .git directory in cwd' '
150
+test_expect_success '__git_find_repo_path - .git directory in cwd' '
151
echo ".git" >expected &&
152
(
167
- __gitdir >"$actual"
153
+ __git_find_repo_path &&
154
+ echo "$__git_repo_path" >"$actual"
155
) &&
156
test_cmp expected "$actual"
157
'
158
172
-test_expect_success '__gitdir - .git directory in parent' '
159
+test_expect_success '__git_find_repo_path - .git directory in parent' '
160
echo "$ROOT/.git" >expected &&
161
(
162
cd subdir/subsubdir &&
176
- __gitdir >"$actual"
163
+ __git_find_repo_path &&
164
+ echo "$__git_repo_path" >"$actual"
165
) &&
166
test_cmp expected "$actual"
167
'
168
181
-test_expect_success '__gitdir - cwd is a .git directory' '
169
+test_expect_success '__git_find_repo_path - cwd is a .git directory' '
170
echo "." >expected &&
171
(
172
cd .git &&
185
- __gitdir >"$actual"
173
+ __git_find_repo_path &&
174
+ echo "$__git_repo_path" >"$actual"
175
) &&
176
test_cmp expected "$actual"
177
'
178
190
-test_expect_success '__gitdir - parent is a .git directory' '
179
+test_expect_success '__git_find_repo_path - parent is a .git directory' '
180
echo "$ROOT/.git" >expected &&
181
(
182
cd .git/refs/heads &&
194
- __gitdir >"$actual"
183
+ __git_find_repo_path &&
184
+ echo "$__git_repo_path" >"$actual"
185
) &&
186
test_cmp expected "$actual"
187
'
188
199
-test_expect_success '__gitdir - $GIT_DIR set while .git directory in cwd' '
189
+test_expect_success '__git_find_repo_path - $GIT_DIR set while .git directory in cwd' '
190
echo "$ROOT/otherrepo/.git" >expected &&
191
(
192
GIT_DIR="$ROOT/otherrepo/.git" &&
193
export GIT_DIR &&
204
- __gitdir >"$actual"
194
+ __git_find_repo_path &&
195
+ echo "$__git_repo_path" >"$actual"
196
) &&
197
test_cmp expected "$actual"
198
'
199
209
-test_expect_success '__gitdir - $GIT_DIR set while .git directory in parent' '
200
+test_expect_success '__git_find_repo_path - $GIT_DIR set while .git directory in parent' '
201
echo "$ROOT/otherrepo/.git" >expected &&
202
(
203
GIT_DIR="$ROOT/otherrepo/.git" &&
204
export GIT_DIR &&
205
cd subdir &&
215
- __gitdir >"$actual"
206
+ __git_find_repo_path &&
207
+ echo "$__git_repo_path" >"$actual"
208
) &&
209
test_cmp expected "$actual"
210
'
211
220
-test_expect_success '__gitdir - from command line while "git -C"' '
212
+test_expect_success '__git_find_repo_path - from command line while "git -C"' '
213
echo "$ROOT/.git" >expected &&
214
(
215
__git_dir="$ROOT/.git" &&
216
__git_C_args=(-C otherrepo) &&
225
- __gitdir >"$actual"
217
+ __git_find_repo_path &&
218
+ echo "$__git_repo_path" >"$actual"
219
) &&
220
test_cmp expected "$actual"
221
'
222
230
-test_expect_success '__gitdir - relative dir from command line and "git -C"' '
223
+test_expect_success '__git_find_repo_path - relative dir from command line and "git -C"' '
224
echo "$ROOT/otherrepo/.git" >expected &&
225
(
226
cd subdir &&
227
__git_dir="otherrepo/.git" &&
228
__git_C_args=(-C ..) &&
236
- __gitdir >"$actual"
229
+ __git_find_repo_path &&
230
+ echo "$__git_repo_path" >"$actual"
231
) &&
232
test_cmp expected "$actual"
233
'
234
241
-test_expect_success '__gitdir - $GIT_DIR set while "git -C"' '
235
+test_expect_success '__git_find_repo_path - $GIT_DIR set while "git -C"' '
236
echo "$ROOT/.git" >expected &&
237
(
238
GIT_DIR="$ROOT/.git" &&
239
export GIT_DIR &&
240
__git_C_args=(-C otherrepo) &&
247
- __gitdir >"$actual"
241
+ __git_find_repo_path &&
242
+ echo "$__git_repo_path" >"$actual"
243
) &&
244
test_cmp expected "$actual"
245
'
246
252
-test_expect_success '__gitdir - relative dir in $GIT_DIR and "git -C"' '
247
+test_expect_success '__git_find_repo_path - relative dir in $GIT_DIR and "git -C"' '
248
echo "$ROOT/otherrepo/.git" >expected &&
249
(
250
cd subdir &&
251
GIT_DIR="otherrepo/.git" &&
252
export GIT_DIR &&
253
__git_C_args=(-C ..) &&
259
- __gitdir >"$actual"
254
+ __git_find_repo_path &&
255
+ echo "$__git_repo_path" >"$actual"
256
) &&
257
test_cmp expected "$actual"
258
'
259
264
-test_expect_success '__gitdir - "git -C" while .git directory in cwd' '
260
+test_expect_success '__git_find_repo_path - "git -C" while .git directory in cwd' '
261
echo "$ROOT/otherrepo/.git" >expected &&
262
(
263
__git_C_args=(-C otherrepo) &&
268
- __gitdir >"$actual"
264
+ __git_find_repo_path &&
265
+ echo "$__git_repo_path" >"$actual"
266
) &&
267
test_cmp expected "$actual"
268
'
269
273
-test_expect_success '__gitdir - "git -C" while cwd is a .git directory' '
270
+test_expect_success '__git_find_repo_path - "git -C" while cwd is a .git directory' '
271
echo "$ROOT/otherrepo/.git" >expected &&
272
(
273
cd .git &&
274
__git_C_args=(-C .. -C otherrepo) &&
278
- __gitdir >"$actual"
275
+ __git_find_repo_path &&
276
+ echo "$__git_repo_path" >"$actual"
277
) &&
278
test_cmp expected "$actual"
279
'
280
283
-test_expect_success '__gitdir - "git -C" while .git directory in parent' '
281
+test_expect_success '__git_find_repo_path - "git -C" while .git directory in parent' '
282
echo "$ROOT/otherrepo/.git" >expected &&
283
(
284
cd subdir &&
285
__git_C_args=(-C .. -C otherrepo) &&
288
- __gitdir >"$actual"
286
+ __git_find_repo_path &&
287
+ echo "$__git_repo_path" >"$actual"
288
) &&
289
test_cmp expected "$actual"
290
'
291
293
-test_expect_success '__gitdir - non-existing path in "git -C"' '
292
+test_expect_success '__git_find_repo_path - non-existing path in "git -C"' '
293
(
294
__git_C_args=(-C non-existing) &&
296
- test_must_fail __gitdir >"$actual"
295
+ test_must_fail __git_find_repo_path &&
296
+ printf "$__git_repo_path" >"$actual"
297
) &&
298
test_must_be_empty "$actual"
299
'
300
301
-test_expect_success '__gitdir - non-existing path in $__git_dir' '
301
+test_expect_success '__git_find_repo_path - non-existing path in $__git_dir' '
302
(
303
__git_dir="non-existing" &&
304
- test_must_fail __gitdir >"$actual"
304
+ test_must_fail __git_find_repo_path &&
305
+ printf "$__git_repo_path" >"$actual"
306
) &&
307
test_must_be_empty "$actual"
308
'
309
309
-test_expect_success '__gitdir - non-existing $GIT_DIR' '
310
+test_expect_success '__git_find_repo_path - non-existing $GIT_DIR' '
311
(
312
GIT_DIR="$ROOT/non-existing" &&
313
export GIT_DIR &&
313
- test_must_fail __gitdir >"$actual"
314
+ test_must_fail __git_find_repo_path &&
315
+ printf "$__git_repo_path" >"$actual"
316
) &&
317
test_must_be_empty "$actual"
318
'
319
318
-test_expect_success '__gitdir - gitfile in cwd' '
320
+test_expect_success '__git_find_repo_path - gitfile in cwd' '
321
echo "$ROOT/otherrepo/.git" >expected &&
322
echo "gitdir: $ROOT/otherrepo/.git" >subdir/.git &&
323
test_when_finished "rm -f subdir/.git" &&
324
(
325
cd subdir &&
324
- __gitdir >"$actual"
326
+ __git_find_repo_path &&
327
+ echo "$__git_repo_path" >"$actual"
328
) &&
329
test_cmp expected "$actual"
330
'
331
329
-test_expect_success '__gitdir - gitfile in parent' '
332
+test_expect_success '__git_find_repo_path - gitfile in parent' '
333
echo "$ROOT/otherrepo/.git" >expected &&
334
echo "gitdir: $ROOT/otherrepo/.git" >subdir/.git &&
335
test_when_finished "rm -f subdir/.git" &&
336
(
337
cd subdir/subsubdir &&
335
- __gitdir >"$actual"
338
+ __git_find_repo_path &&
339
+ echo "$__git_repo_path" >"$actual"
340
) &&
341
test_cmp expected "$actual"
342
'
343
340
-test_expect_success SYMLINKS '__gitdir - resulting path avoids symlinks' '
344
+test_expect_success SYMLINKS '__git_find_repo_path - resulting path avoids symlinks' '
345
echo "$ROOT/otherrepo/.git" >expected &&
346
mkdir otherrepo/dir &&
347
test_when_finished "rm -rf otherrepo/dir" &&
@@ -345,16 +349,57 @@ test_expect_success SYMLINKS '__gitdir - resulting path avoids symlinks' '
349
test_when_finished "rm -f link" &&
350
(
351
cd link &&
352
+ __git_find_repo_path &&
353
+ echo "$__git_repo_path" >"$actual"
354
+ ) &&
355
+ test_cmp expected "$actual"
356
+'
357
+
358
+test_expect_success '__git_find_repo_path - not a git repository' '
359
+ (
360
+ cd non-repo &&
361
+ GIT_CEILING_DIRECTORIES="$ROOT" &&
362
+ export GIT_CEILING_DIRECTORIES &&
363
+ test_must_fail __git_find_repo_path &&
364
+ printf "$__git_repo_path" >"$actual"
365
+ ) &&
366
+ test_must_be_empty "$actual"
367
+'
368
+
369
+test_expect_success '__gitdir - finds repo' '
370
+ echo "$ROOT/.git" >expected &&
371
+ (
372
+ cd subdir/subsubdir &&
373
__gitdir >"$actual"
374
) &&
375
test_cmp expected "$actual"
376
'
377
353
-test_expect_success '__gitdir - not a git repository' '
354
- nongit test_must_fail __gitdir >"$actual" &&
378
+
379
+test_expect_success '__gitdir - returns error when cant find repo' '
380
+ (
381
+ __git_dir="non-existing" &&
382
+ test_must_fail __gitdir >"$actual"
383
+ ) &&
384
test_must_be_empty "$actual"
385
'
386
387
+test_expect_success '__gitdir - repo as argument' '
388
+ echo "otherrepo/.git" >expected &&
389
+ (
390
+ __gitdir "otherrepo" >"$actual"
391
+ ) &&
392
+ test_cmp expected "$actual"
393
+'
394
+
395
+test_expect_success '__gitdir - remote as argument' '
396
+ echo "remote" >expected &&
397
+ (
398
+ __gitdir "remote" >"$actual"
399
+ ) &&
400
+ test_cmp expected "$actual"
401
+'
402
+
403
test_expect_success '__gitcomp - trailing space - options' '
404
test_gitcomp "--re" "--dry-run --reuse-message= --reedit-message=
405
--reset-author" <<-EOF