Re: git status always modifies index?

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Peff,

On Sun, 26 Nov 2017, Jeff King wrote:

> On Sat, Nov 25, 2017 at 10:55:25PM +0100, Johannes Schindelin wrote:
> 
> > > Right, I went a little off track of your original point.
> > > 
> > > What I was trying to get at is that naming it "status --no-lock-index"
> > > would not be the same thing (even though with the current implementation
> > > it would behave the same). IOW, can we improve the documentation of
> > > "status" to point to make it easier to discover this use case.
> > 
> > I had the hunch that renaming the option (and moving it away from `git
> > status`, even if it is currently only affecting `git status` and even if
> > it will most likely be desirable to have the option to really only prevent
> > `git status` from writing .lock files) was an unfortunate decision (and
> > made my life as Git for Windows quite a bit harder than really necessary,
> > it cost me over one workday of a bug hunt, mainly due to a false flag
> > indicating `git rebase` to be the culprit). And I hinted at it, too.
> 
> I remain unconvinced that we have actually uncovered a serious problem.

You did not. A colleague of mine did. And it was a problem in Git for
Windows only, caused by the changes necessitated by yours (which even used
my tests, which made it easy for my conflict resolution to do the wrong
thing by removing my --no-lock-index test in favor of your
--no-optional-locks test, breaking --no-lock-index).

It cost me almost two work days, and a lot of hair. And all I meant by "I
hinted at it, too" was that I felt that something like that was coming
when I saw your variation of my patches making it into git/git's master.

This kind of stuff really throws my upstreaming back quite a bit, not only
due to lost time, but also due to the frustration with the caused
regressions.

Now, the report indicates that not only Git for Windows had a problem, but
that the new feature is unnecessarily unintuitive. I would even claim that
the --no-lock-index option (even if it does not say "--read-only") would
have made for a better user experience because it is at least in the
expected place: the `git status` man page.

> Somebody asked if Git could do a thing, and people pointed him to the
> right option.

If people have to ask on the mailing list even after reading the man
pages, that's a strong indicator that we could do better.

> That option is new in the latest release.

In Git, yes. In Git for Windows, no. And it worked beautifully in Git for
Windows before v2.15.0.

> > I really never understood why --no-optional-locks had to be introduced
> > when it did exactly the same as --no-lock-index, and when the latter has a
> > right to exist in the first place, even in the purely hypothetical case
> > that we teach --no-optional-locks to handle more cases than just `git
> > status`' writing of the index (and in essence, it looks like premature
> > optimization): it is a very concrete use case that a user may want `git
> > status` to refrain from even trying to write any file, as this thread
> > shows very eloquently.
> 
> Besides potentially handling more than just "git status",

... which is a premature optimization...

> it differs in one other way: it can be triggered via and is carried
> through the environment.

... which Git for Windows' --no-lock-index *also* had to do (think
submodules). We simply figured that out only after introducing the option,
therefore it was carried as an add-on commit, and the plan was to squash
it in before upstreaming (obviously!).

So I contest your claim. `--no-lock-index` must be propagated to callees
in the same way as the (still hypothetical) `--no-optional-locks` that
would cover more than just `git status`.

> > Maybe it is time to reintroduce --no-lock-index, and make
> > --no-optional-locks' functionality a true superset of --no-lock-index'.
> 
> I'm not against having a separate option for "never write to the
> repository".

Whoa, slow down. We already introduced the `--no-optional-locks` option
for a completely hypothetical use case covering more than just `git
status`, a use case that may very well never see the light of day. (At
least it was my undederstanding that the conjecture of something like that
maybe being needed by somebody some time in the future was the entire
reason tobutcher the --no-lock-index approach into a very different
looking --no-optional-locks that is much harder to find in the
documentation.)

Let's not introduce yet another option for a completely hypothetical use
case that may be even more theoretical.

> I think it's potentially different than "don't lock", as I
> mentioned earlier.

I don't see the need at all at the moemnt.

> Frankly I don't see much value in "--no-lock-index" if we already have
> "--no-optional-locks".

Funny. I did not (and still do not) see the need for renaming `git status
--no-lock-index` to `git --no-optional-locks status` (i.e. cluttering the
global option space for something that really only `git status` needs).

> But I figured you would carry "status --no-lock-index" forever in Git
> for Windows anyway (after all, if you remove it now, you're breaking
> compatibility for existing users).

I will not carry it forever. Most definitely not. It was marked as
experimental for a reason: I suspected that major changes would be
required to get it accepted into git.git (even if I disagree from a purely
practicial point of view that those changes are required, but that's what
you have to accept when working in Open Source, that you sometimes have to
change something solely to please the person who can reject your patches).

Sure, I am breaking compatibility for existing users, but that is more the
fault of --no-optional-locks being introduced than anything else.

I am pretty much done talking about this subject at this point. I only
started talking about it because I wanted you to understand that I will
insist on my hunches more forcefully in the future, and I hope you will
see why I do that. But then, you may not even see the problems caused by
the renaming (and forced broader scope for currently no good reason) of
--no-lock-index, so maybe you disagree that acting on my hunch would have
prevented those problems.

Ciao,
Dscho