[0/2] describe and diff: implement --no-optional-locks

Message

Elijah Newren via GitGitGadget March 6, 2025, 5:58 a.m. UTC

git describe and git diff may update the index in the background for similar
performance reasons to git-status. These patches implement 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!

Benjamin Woodruff (2):
  describe: implement --no-optional-locks
  diff: implement --no-optional-locks

 Documentation/config/diff.adoc     |  4 ++-
 Documentation/git-describe.adoc    | 12 ++++++++
 Documentation/git.adoc             |  4 ++-
 builtin/describe.c                 | 12 ++++----
 builtin/diff.c                     |  4 +++
 t/meson.build                      |  1 +
 t/t4070-diff-auto-refresh-index.sh | 46 ++++++++++++++++++++++++++++++
 t/t6120-describe.sh                |  8 ++++++
 8 files changed, 84 insertions(+), 7 deletions(-)
 create mode 100755 t/t4070-diff-auto-refresh-index.sh


base-commit: e969bc875963a10890d61ba84eab3a460bd9e535
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1872%2Fbgw%2Fbgw%2Fdiff-describe-optional-locks-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1872/bgw/bgw/diff-describe-optional-locks-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1872

Comments

Junio C Hamano March 6, 2025, 4:11 p.m. UTC | #1

"Benjamin Woodruff via GitGitGadget" <[e-mail]>
writes:

> git describe and git diff may update the index in the background for similar
> performance reasons to git-status.

That is a wrong reasoning that is completely opposite, though.

The commands at the Porcelain level, like "status" and "diff",
refresh the index for the CORRECTNESS purposes.

The commands at the plumbing level, which are designed to be used in
your own scripts, like "diff-files" and "diff-index", do not refresh
the index for the performance purposes.  If your own script wants to
produce correct result, your script IS responsible for refreshing
the index after its last modification to working tree files before
it starts to use the plumbing commands to inspect which ones are
modified and which ones are not.  This is so that your script has
more control over when the index is refreshed.  It does not have to
pay cost to run refresh for each Git command it invokes, if it knows
that it does not make any modification between the two invocations;
it can instead refresh just once and then run these two plumbing
commands.

So, it would be absolute no-no to make the Porcelain commands like
"describe" and "diff" not to refresh the index before they work by
default.  As an optional behaviour, it might be acceptable if there
is no other reasonable solutions (like, using plumbing commands if
the callers are not you typing but your scripts calling them).