Skip to content

Commit 27344d6

Browse files
peffgitster
authored andcommitted
git: add --no-optional-locks option
Some tools like IDEs or fancy editors may periodically run commands like "git status" in the background to keep track of the state of the repository. Some of these commands may refresh the index and write out the result in an opportunistic way: if they can get the index lock, then they update the on-disk index with any updates they find. And if not, then their in-core refresh is lost and just has to be recomputed by the next caller. But taking the index lock may conflict with other operations in the repository. Especially ones that the user is doing themselves, which _aren't_ opportunistic. In other words, "git status" knows how to back off when somebody else is holding the lock, but other commands don't know that status would be happy to drop the lock if somebody else wanted it. There are a couple possible solutions: 1. Have some kind of "pseudo-lock" that allows other commands to tell status that they want the lock. This is likely to be complicated and error-prone to implement (and maybe even impossible with just dotlocks to work from, as it requires some inter-process communication). 2. Avoid background runs of commands like "git status" that want to do opportunistic updates, preferring instead plumbing like diff-files, etc. This is awkward for a couple of reasons. One is that "status --porcelain" reports a lot more about the repository state than is available from individual plumbing commands. And two is that we actually _do_ want to see the refreshed index. We just don't want to take a lock or write out the result. Whereas commands like diff-files expect us to refresh the index separately and write it to disk so that they can depend on the result. But that write is exactly what we're trying to avoid. 3. Ask "status" not to lock or write the index. This is easy to implement. The big downside is that any work done in refreshing the index for such a call is lost when the process exits. So a background process may end up re-hashing a changed file multiple times until the user runs a command that does an index refresh themselves. This patch implements the option 3. The idea (and the test) is largely stolen from a Git for Windows patch by Johannes Schindelin, 67e5ce7 (status: offer *not* to lock the index and update it, 2016-08-12). The twist here is that instead of making this an option to "git status", it becomes a "git" option and matching environment variable. The reason there is two-fold: 1. An environment variable is carried through to sub-processes. And whether an invocation is a background process or not should apply to the whole process tree. So you could do "git --no-optional-locks foo", and if "foo" is a script or alias that calls "status", you'll still get the effect. 2. There may be other programs that want the same treatment. I've punted here on finding more callers to convert, since "status" is the obvious one to call as a repeated background job. But "git diff"'s opportunistic refresh of the index may be a good candidate. The test is taken from 67e5ce7, and it's worth repeating Johannes's explanation: Note that the regression test added in this commit does not *really* verify that no index.lock file was written; that test is not possible in a portable way. Instead, we verify that .git/index is rewritten *only* when `git status` is run without `--no-optional-locks`. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 4010f1d commit 27344d6

File tree

6 files changed

+41
-1
lines changed

6 files changed

+41
-1
lines changed

Documentation/git.txt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,10 @@ foo.bar= ...`) sets `foo.bar` to the empty string which ` git config
159159
Add "icase" magic to all pathspec. This is equivalent to setting
160160
the `GIT_ICASE_PATHSPECS` environment variable to `1`.
161161

162+
--no-optional-locks::
163+
Do not perform optional operations that require locks. This is
164+
equivalent to setting the `GIT_OPTIONAL_LOCKS` to `0`.
165+
162166
GIT COMMANDS
163167
------------
164168

@@ -697,6 +701,14 @@ of clones and fetches.
697701
which feed potentially-untrusted URLS to git commands. See
698702
linkgit:git-config[1] for more details.
699703

704+
`GIT_OPTIONAL_LOCKS`::
705+
If set to `0`, Git will complete any requested operation without
706+
performing any optional sub-operations that require taking a lock.
707+
For example, this will prevent `git status` from refreshing the
708+
index as a side effect. This is useful for processes running in
709+
the background which do not want to cause lock contention with
710+
other operations on the repository. Defaults to `1`.
711+
700712
Discussion[[Discussion]]
701713
------------------------
702714

builtin/commit.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1385,7 +1385,10 @@ int cmd_status(int argc, const char **argv, const char *prefix)
13851385
read_cache_preload(&s.pathspec);
13861386
refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, &s.pathspec, NULL, NULL);
13871387

1388-
fd = hold_locked_index(&index_lock, 0);
1388+
if (use_optional_locks())
1389+
fd = hold_locked_index(&index_lock, 0);
1390+
else
1391+
fd = -1;
13891392

13901393
s.is_initial = get_sha1(s.reference, oid.hash) ? 1 : 0;
13911394
if (!s.is_initial)

cache.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,7 @@ static inline enum object_type object_type(unsigned int mode)
443443
#define GIT_NOGLOB_PATHSPECS_ENVIRONMENT "GIT_NOGLOB_PATHSPECS"
444444
#define GIT_ICASE_PATHSPECS_ENVIRONMENT "GIT_ICASE_PATHSPECS"
445445
#define GIT_QUARANTINE_ENVIRONMENT "GIT_QUARANTINE_PATH"
446+
#define GIT_OPTIONAL_LOCKS_ENVIRONMENT "GIT_OPTIONAL_LOCKS"
446447

447448
/*
448449
* This environment variable is expected to contain a boolean indicating
@@ -782,6 +783,11 @@ extern int protect_ntfs;
782783
*/
783784
extern int ref_paranoia;
784785

786+
/*
787+
* Returns the boolean value of $GIT_OPTIONAL_LOCKS (or the default value).
788+
*/
789+
int use_optional_locks(void);
790+
785791
/*
786792
* The character that begins a commented line in user-editable file
787793
* that is subject to stripspace.

environment.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,3 +336,8 @@ void reset_shared_repository(void)
336336
{
337337
need_shared_repository_from_config = 1;
338338
}
339+
340+
int use_optional_locks(void)
341+
{
342+
return git_env_bool(GIT_OPTIONAL_LOCKS_ENVIRONMENT, 1);
343+
}

git.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,10 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
182182
setenv(GIT_ICASE_PATHSPECS_ENVIRONMENT, "1", 1);
183183
if (envchanged)
184184
*envchanged = 1;
185+
} else if (!strcmp(cmd, "--no-optional-locks")) {
186+
setenv(GIT_OPTIONAL_LOCKS_ENVIRONMENT, "0", 1);
187+
if (envchanged)
188+
*envchanged = 1;
185189
} else if (!strcmp(cmd, "--shallow-file")) {
186190
(*argv)++;
187191
(*argc)--;

t/t7508-status.sh

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1670,4 +1670,14 @@ test_expect_success '"Initial commit" should not be noted in commit template' '
16701670
test_i18ngrep ! "Initial commit" output
16711671
'
16721672

1673+
test_expect_success '--no-optional-locks prevents index update' '
1674+
test-chmtime =1234567890 .git/index &&
1675+
git --no-optional-locks status &&
1676+
test-chmtime -v +0 .git/index >out &&
1677+
grep ^1234567890 out &&
1678+
git status &&
1679+
test-chmtime -v +0 .git/index >out &&
1680+
! grep ^1234567890 out
1681+
'
1682+
16731683
test_done

0 commit comments

Comments
 (0)