Re: [PATCH 0/2] Add a way to wait for index lock

[Junio C Hamano <gitster@pobox.com>]

Jun Wu <quark.zju@gmail.com> writes:

> From: Jun Wu <quark@meta.com>
>
> An IDE that runs `git status` periodically in the background has 2
> choices. Both are suboptimal:
> - with `--no-optional-locks`: `status` becomes slower over time.
> - without `--no-optional-locks`: conflicts with other commands like
>   `add`, `commit`, often run by a user from the IDE terminal.

It is unfair to expect that the readers would guess what you left
unsaid here, which probably is "we will introduce the third choice,
which does such and such things, and it improves the situation in
such and such ways because ...", before listing ...

> Patch 1 adds index lock timeout so `add` can wait instead of fail.
> Patch 2 adjusts the read/lock order so `commit` can also wait.

... these.

> To minimize write race, if the index is read before locking, the
> wait will be skipped, as if timeout is 0. Depending on the read/lock
> order, some commands (e.g. add, rm, mv, checkout, restore, read-tree,
> apply, update-index) respect the timeout. Some (e.g. reset, stash,
> merge, rebase, cherry-pick, am) do not. I started with the simple and
> common commands. If there is interest in other commands I can take a
> look too.

Is the above supposed to be the description of the proposed solution?

There are multiple parties (i.e., `git status` that needed to
discard the result with "--no-optional-locks" and other end-user
actions that do need to safely update the index) involved, and it is
unclear whose actions many of verbs in your writing refer to in the
above.  E.g., "if the index is read before locking, the wait will be
skipped" for whom?  "git status" that wants to opportunistically
write out the result of refreshing?  End-user actions like "git add"
that wanted to register new contents to the index?  Both?

> The default timeout is 1s (reasoning in patch 1). I'm happy to change it
> to 0 if people prefer no behavior change.
>
> Related context:
> - (2017-9-22) https://lore.kernel.org/git/20170923033425.6ilykxl5pi7unlj6@sigill.intra.peff.net/
>   proposed index lock timeout.
> - (2017-9-27) Commit 27344d6a6c (git: add --no-optional-locks option, 2017-09-27)
>   introduces `--no-optional-locks` for the same issue.
> - (2019-5-3) https://lore.kernel.org/git/CACsJy8BcBk6Fsfy7o1aBTQAPeH-w+rBwLZrzw4jJ-OM_jhkxMQ@mail.gmail.com/
>   mentioned `update_if_able` and "gap" between read and lock, mitigated
>   in patch 1 with the `index->initialized` check.
>
> Jun Wu (2):
>   repository: add core.indexLockTimeout
>   commit: lock index before reading
>
>  Documentation/config/core.adoc |  6 ++++++
>  builtin/commit.c               |  9 ++-------
>  repository.c                   | 17 +++++++++++++++-
>  t/meson.build                  |  1 +
>  t/t0031-lockfile-pid.sh        |  4 ++++
>  t/t0032-index-lock-timeout.sh  | 37 ++++++++++++++++++++++++++++++++++
>  t/t7400-submodule-basic.sh     |  1 +
>  7 files changed, 67 insertions(+), 8 deletions(-)
>  create mode 100755 t/t0032-index-lock-timeout.sh
>
>
> base-commit: 6de20f6092dcf9bdb1c8efe03db4b70c82b423dd