Skip to content

Commit 4b980d2

Browse files
committed
describe,diff: implement --no-optional-locks
Both of these commands may update the index in the background for similar performance reasons to `git-status`. This commit implements the `--no-optional-locks` option for these commands, which allows scripts to bypass this behavior. I'm implementing this as a solution to a stale lockfile issue we've sporadically encountered due to a build script that runs `git describe`. We're mitigating this issue by using `libgit2` in our build script, which does not have the same background update behavior for its `git_describe_workdir` implementation, but it would be nice to have this option supported in the `git` CLI. Tests and documentation changes are included! Signed-off-by: Benjamin Woodruff <benjamin.woodruff@vercel.com>
1 parent 6a64ac7 commit 4b980d2

File tree

8 files changed

+84
-7
lines changed

8 files changed

+84
-7
lines changed

Diff for: Documentation/config/diff.adoc

+3-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
contents in the work tree match the contents in the
77
index. This option defaults to `true`. Note that this
88
affects only `git diff` Porcelain, and not lower level
9-
`diff` commands such as `git diff-files`.
9+
`diff` commands such as `git diff-files`. If
10+
`--no-optional-locks` is set (see linkgit:git[1] for
11+
details), the index file is not updated.
1012

1113
`diff.dirstat`::
1214
ifdef::git-diff[]

Diff for: Documentation/git-describe.adoc

+12
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,18 @@ selected and output. Here fewest commits different is defined as
198198
the number of commits which would be shown by `git log tag..input`
199199
will be the smallest number of commits possible.
200200
201+
BACKGROUND REFRESH
202+
------------------
203+
204+
By default, `git describe --dirty` will automatically refresh the index,
205+
similar to the background refresh functionality of
206+
linkgit:git-status[1]. Writing out the updated index is an optimization
207+
that isn't strictly necessary. When `describe` is run in the background,
208+
the lock held during the write may conflict with other simultaneous
209+
processes, causing them to fail. Scripts running `describe` in the
210+
background should consider using `git --no-optional-locks status` (see
211+
linkgit:git[1] for details).
212+
201213
BUGS
202214
----
203215

Diff for: Documentation/git.adoc

+3-1
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,9 @@ If you just want to run git as if it was started in `<path>` then use
189189

190190
--no-optional-locks::
191191
Do not perform optional operations that require locks. This is
192-
equivalent to setting the `GIT_OPTIONAL_LOCKS` to `0`.
192+
equivalent to setting the `GIT_OPTIONAL_LOCKS` to `0`. This
193+
functionality is implemented for `git status`, `git describe`,
194+
and `git diff`.
193195

194196
--no-advice::
195197
Disable all advice hints from being printed.

Diff for: builtin/describe.c

+7-5
Original file line numberDiff line numberDiff line change
@@ -704,18 +704,20 @@ int cmd_describe(int argc,
704704
} else if (dirty) {
705705
struct lock_file index_lock = LOCK_INIT;
706706
struct rev_info revs;
707-
int fd;
708707

709708
setup_work_tree();
710709
prepare_repo_settings(the_repository);
711710
the_repository->settings.command_requires_full_index = 0;
712711
repo_read_index(the_repository);
713712
refresh_index(the_repository->index, REFRESH_QUIET|REFRESH_UNMERGED,
714713
NULL, NULL, NULL);
715-
fd = repo_hold_locked_index(the_repository,
716-
&index_lock, 0);
717-
if (0 <= fd)
718-
repo_update_index_if_able(the_repository, &index_lock);
714+
if (use_optional_locks()) {
715+
int fd = repo_hold_locked_index(the_repository,
716+
&index_lock, 0);
717+
if (0 <= fd)
718+
repo_update_index_if_able(the_repository,
719+
&index_lock);
720+
}
719721

720722
repo_init_revisions(the_repository, &revs, prefix);
721723

Diff for: builtin/diff.c

+4
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
#include "builtin.h"
1111
#include "config.h"
12+
#include "environment.h"
1213
#include "ewah/ewok.h"
1314
#include "lockfile.h"
1415
#include "color.h"
@@ -239,6 +240,9 @@ static void refresh_index_quietly(void)
239240
struct lock_file lock_file = LOCK_INIT;
240241
int fd;
241242

243+
if (!use_optional_locks())
244+
return;
245+
242246
fd = repo_hold_locked_index(the_repository, &lock_file, 0);
243247
if (fd < 0)
244248
return;

Diff for: t/meson.build

+1
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,7 @@ integration_tests = [
500500
't4067-diff-partial-clone.sh',
501501
't4068-diff-symmetric-merge-base.sh',
502502
't4069-remerge-diff.sh',
503+
't4070-diff-auto-refresh-index.sh',
503504
't4100-apply-stat.sh',
504505
't4101-apply-nonl.sh',
505506
't4102-apply-rename.sh',

Diff for: t/t4070-diff-auto-refresh-index.sh

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
#!/bin/sh
2+
#
3+
# Copyright (c) 2025 Benjamin Woodruff
4+
#
5+
6+
test_description='diff.autoRefreshIndex config option'
7+
8+
. ./test-lib.sh
9+
. "$TEST_DIRECTORY"/lib-diff.sh
10+
11+
test_expect_success 'index is updated when autoRefreshIndex is true' '
12+
>tracked &&
13+
git add tracked &&
14+
15+
# stat() must change (but not file contents) to trigger an index update
16+
test_set_magic_mtime tracked &&
17+
18+
# check the mtime of .git/index does not change without autoRefreshIndex
19+
test_set_magic_mtime .git/index &&
20+
git config diff.autoRefreshIndex false &&
21+
git diff &&
22+
test_is_magic_mtime .git/index &&
23+
24+
# but it does change when autoRefreshIndex is true (the default)
25+
git config diff.autoRefreshIndex true &&
26+
git diff &&
27+
! test_is_magic_mtime .git/index
28+
'
29+
30+
test_expect_success '--no-optional-locks overrides autoRefreshIndex' '
31+
>tracked &&
32+
git add tracked &&
33+
test_set_magic_mtime tracked &&
34+
35+
# `--no-optional-locks` overrides `autoRefreshIndex`
36+
test_set_magic_mtime .git/index &&
37+
git config diff.autoRefreshIndex true &&
38+
git --no-optional-locks diff &&
39+
40+
# sanity check that without `--no-optional-locks` it still updates
41+
test_is_magic_mtime .git/index &&
42+
git diff &&
43+
! test_is_magic_mtime .git/index
44+
'
45+
46+
test_done

Diff for: t/t6120-describe.sh

+8
Original file line numberDiff line numberDiff line change
@@ -749,4 +749,12 @@ test_expect_success 'do not be fooled by invalid describe format ' '
749749
test_must_fail git cat-file -t "refs/tags/super-invalid/./../...../ ~^:/?*[////\\\\\\&}/busted.lock-42-g"$(cat out)
750750
'
751751

752+
test_expect_success '--no-optional-locks prevents index update' '
753+
test_set_magic_mtime .git/index &&
754+
git --no-optional-locks describe --dirty &&
755+
test_is_magic_mtime .git/index &&
756+
git describe --dirty &&
757+
! test_is_magic_mtime .git/index
758+
'
759+
752760
test_done

0 commit comments

Comments
 (0)