Skip to content

describe and diff: implement --no-optional-locks #1872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion Documentation/config/diff.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
contents in the work tree match the contents in the
index. This option defaults to `true`. Note that this
affects only `git diff` Porcelain, and not lower level
`diff` commands such as `git diff-files`.
`diff` commands such as `git diff-files`. If
`--no-optional-locks` is set (see linkgit:git[1] for
details), the index file is not updated.

`diff.dirstat`::
ifdef::git-diff[]
Expand Down
12 changes: 12 additions & 0 deletions Documentation/git-describe.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,18 @@ selected and output. Here fewest commits different is defined as
the number of commits which would be shown by `git log tag..input`
will be the smallest number of commits possible.

BACKGROUND REFRESH
------------------

By default, `git describe --dirty` will automatically refresh the index,
similar to the background refresh functionality of
linkgit:git-status[1]. Writing out the updated index is an optimization
that isn't strictly necessary. When `describe` is run in the background,
the lock held during the write may conflict with other simultaneous
processes, causing them to fail. Scripts running `describe` in the
background should consider using `git --no-optional-locks status` (see
linkgit:git[1] for details).

BUGS
----

Expand Down
4 changes: 3 additions & 1 deletion Documentation/git.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,9 @@ If you just want to run git as if it was started in `<path>` then use

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

--no-advice::
Disable all advice hints from being printed.
Expand Down
12 changes: 7 additions & 5 deletions builtin/describe.c
Original file line number Diff line number Diff line change
Expand Up @@ -704,18 +704,20 @@ int cmd_describe(int argc,
} else if (dirty) {
struct lock_file index_lock = LOCK_INIT;
struct rev_info revs;
int fd;

setup_work_tree();
prepare_repo_settings(the_repository);
the_repository->settings.command_requires_full_index = 0;
repo_read_index(the_repository);
refresh_index(the_repository->index, REFRESH_QUIET|REFRESH_UNMERGED,
NULL, NULL, NULL);
fd = repo_hold_locked_index(the_repository,
&index_lock, 0);
if (0 <= fd)
repo_update_index_if_able(the_repository, &index_lock);
if (use_optional_locks()) {
int fd = repo_hold_locked_index(the_repository,
&index_lock, 0);
if (0 <= fd)
repo_update_index_if_able(the_repository,
&index_lock);
}

repo_init_revisions(the_repository, &revs, prefix);

Expand Down
4 changes: 4 additions & 0 deletions builtin/diff.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "builtin.h"
#include "config.h"
#include "environment.h"
#include "ewah/ewok.h"
#include "lockfile.h"
#include "color.h"
Expand Down Expand Up @@ -239,6 +240,9 @@ static void refresh_index_quietly(void)
struct lock_file lock_file = LOCK_INIT;
int fd;

if (!use_optional_locks())
return;

fd = repo_hold_locked_index(the_repository, &lock_file, 0);
if (fd < 0)
return;
Expand Down
1 change: 1 addition & 0 deletions t/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@ integration_tests = [
't4067-diff-partial-clone.sh',
't4068-diff-symmetric-merge-base.sh',
't4069-remerge-diff.sh',
't4070-diff-auto-refresh-index.sh',
't4100-apply-stat.sh',
't4101-apply-nonl.sh',
't4102-apply-rename.sh',
Expand Down
46 changes: 46 additions & 0 deletions t/t4070-diff-auto-refresh-index.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#!/bin/sh
#
# Copyright (c) 2025 Benjamin Woodruff
#

test_description='diff.autoRefreshIndex config option'

. ./test-lib.sh
. "$TEST_DIRECTORY"/lib-diff.sh

test_expect_success 'index is updated when autoRefreshIndex is true' '
>tracked &&
git add tracked &&

# stat() must change (but not file contents) to trigger an index update
test_set_magic_mtime tracked &&

# check the mtime of .git/index does not change without autoRefreshIndex
test_set_magic_mtime .git/index &&
git config diff.autoRefreshIndex false &&
git diff &&
test_is_magic_mtime .git/index &&

# but it does change when autoRefreshIndex is true (the default)
git config diff.autoRefreshIndex true &&
git diff &&
! test_is_magic_mtime .git/index
'

test_expect_success '--no-optional-locks overrides autoRefreshIndex' '
>tracked &&
git add tracked &&
test_set_magic_mtime tracked &&

# `--no-optional-locks` overrides `autoRefreshIndex`
test_set_magic_mtime .git/index &&
git config diff.autoRefreshIndex true &&
git --no-optional-locks diff &&

# sanity check that without `--no-optional-locks` it still updates
test_is_magic_mtime .git/index &&
git diff &&
! test_is_magic_mtime .git/index
'

test_done
8 changes: 8 additions & 0 deletions t/t6120-describe.sh
Original file line number Diff line number Diff line change
Expand Up @@ -749,4 +749,12 @@ test_expect_success 'do not be fooled by invalid describe format ' '
test_must_fail git cat-file -t "refs/tags/super-invalid/./../...../ ~^:/?*[////\\\\\\&}/busted.lock-42-g"$(cat out)
'

test_expect_success '--no-optional-locks prevents index update' '
test_set_magic_mtime .git/index &&
git --no-optional-locks describe --dirty &&
test_is_magic_mtime .git/index &&
git describe --dirty &&
! test_is_magic_mtime .git/index
'

test_done
Loading