Re: git status always modifies index?

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Nov 26, 2017, 21:55 UTC
Message-ID: <alpine.DEB.2.21.1.1711262231250.6482@virtualbox>
In-Reply-To: <20171126192508.GB1501@sigill>

Hi Peff,

On Sun, 26 Nov 2017, Jeff King wrote:

Show 18 quoted lines

> 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.

Show 10 quoted lines

> > 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`.

Show 5 quoted lines

> > 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