git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Nathan Neulinger <nneul@neulinger.org>,
	Santiago Torres <santiago@nyu.edu>,
	git@vger.kernel.org,
	Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: git status always modifies index?
Date: Mon, 27 Nov 2017 17:50:21 -0500	[thread overview]
Message-ID: <20171127225020.GA29384@sigill.intra.peff.net> (raw)
In-Reply-To: <20171127205731.GB27469@aiede.mtv.corp.google.com>

On Mon, Nov 27, 2017 at 12:57:31PM -0800, Jonathan Nieder wrote:

> > I actually consider "--no-optional-locks" to be such an aspirational
> > feature. I didn't go digging for other cases (though I'm fairly certain
> > that "diff" has one), but hoped to leave it for further bug reports ("I
> > used the option, ran command X, and saw lock contention").
> 
> I am worried that the project is not learning from what happened here.
> 
> My main issue with the --no-optional-locks name is that it does not
> connect to the underlying user need.  Your main argument for it is
> that it exactly describes the underlying user need.  One of us has to
> be wrong.

Or there's a false dichotomy. ;) We could be talking about two different
users.

> So let me describe my naive reading:
> 
> As a user, I want to inspect the state of the repository without
> disrupting it in any way.  That means not breaking concurrent
> processes and not upsetting permissions.  --read-only seems to
> describe this use case to me perfectly.

That does not match the request that I got from real script writers who
were having a problem. They wanted to avoid lock contention with
background tasks.  They don't care if the repository is modified as long
as it is done in a safe and non-conflicting way.

I agree (as I think I've said already in this thread) that --read-only
would be a superset of that. And that it would probably be OK to have
just gone there in the first place, sacrificing a small amount of
specificity in the name of having fewer knobs for the user to turn.

> If I understood correctly, your objection is that --read-only is not
> specific enough.  What I really want, you might say, is not to break
> concurrent processes.  Any other aspects of being read-only are not
> relevant.  E.g. if I can refresh the on-disk index using O_APPEND
> without disrupting concurrent processes then I should be satisfied
> with that.

Do I have an objection? It's not clear to me that anybody is actually
proposing anything concrete for me to object to.

Are we adding "--read-only"? Are we going back to "status
--no-lock-index"? In either case, are we deprecating
"--no-optional-locks"?

It sounds like you are arguing for the first, and it sounds like Dscho
is arguing for the second. Frankly, I don't really care that much. I've
said all that I can on why I chose the direction I did, and I remain
unconvinced that we have evidence that the current option is somehow
impossible to find. If somebody wants to take us down one of the other
roads, that's fine by me.

> Fair enough, though that feels like overengineering.  But I *still*
> don't see what that has to do with the name "no-optional-locks".  When
> is a lock *optional*?  And how am I supposed to discover this option?

I kind of feel like any answer I give to these questions is just going
to be waved aside. But here are my earnest answers:

  1. You are bit by lock contention, where running operation X ends up
     with some error like "unable to create index.lock: file exists".
     "X" is probably something like "commit".

  2. You search the documentation for options related to locks. You're
     not likely to find it in the manpage for X, since the root of the
     problem actually has nothing to do with X in the first place. It's
     a background task running "status" that is the problem.

  3. You might find it in git(1) while searching for information on
     locks, since "lock" is in the name of the option (and is in fact
     the only hit in that page). The index is also mentioned there
     (though searching for "index" yields a lot more hits).

  4. You might find it in git-status(1) if you suspect that "status" is
     at work. Searching for "index" or "lock" turns up the addition I
     just proposed yesterday.

There are obviously a lot of places where that sequence might fail to
find a hit. But the same is true of just about any option, including
putting "--read-only" into git(1).

> This also came up during review, and I am worried that this review
> feedback is being ignored.  In other words, I have no reason to
> believe it won't happen again.

I'm having a hard time figuring out what you mean here. Do you mean that
I ignored feedback on this topic during the initial review?

Looking at the original thread, I just don't see it. There was some
question about the name. I tried to lay out my thinking here:

  https://public-inbox.org/git/20170921050835.mrbgx2zryy3jusdk@sigill.intra.peff.net/

and ended with:

  I am open to a better name, but I could not come up with one.

There was no meaningful response on the topic. When I reposted v2, I
tried to bring attention to that with:

    - there was some discussion over the name. I didn't see other
      suggestions, and I didn't come up with anything better.

So...am I missing something? Am I misunderstanding your point?

> > I would be fine with having a further aspirational "read only" mode.
> 
> Excellent, we seem to agree on this much.  If I can find time for it
> today then I'll write a patch.

Great.

-Peff

  reply	other threads:[~2017-11-27 22:50 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-22 15:19 git status always modifies index? Nathan Neulinger
2017-11-22 15:30 ` Santiago Torres
2017-11-22 15:37   ` Nathan Neulinger
2017-11-22 16:10     ` Santiago Torres
2017-11-22 16:20       ` Nathan Neulinger
2017-11-22 16:24         ` Santiago Torres
2017-11-22 20:27         ` Jonathan Nieder
2017-11-22 21:17           ` Jeff King
2017-11-22 21:56             ` Jonathan Nieder
2017-11-22 22:06               ` Jeff King
2017-11-25 21:55                 ` Johannes Schindelin
2017-11-26 19:25                   ` Jeff King
2017-11-26 21:55                     ` Johannes Schindelin
2017-11-27  5:24                       ` Jeff King
2017-11-27  6:03                         ` Junio C Hamano
2017-11-27 20:50                           ` Johannes Schindelin
2017-11-27  6:04                         ` [PATCH] git-status.txt: mention --no-optional-locks Jeff King
2017-11-27  6:07                           ` Junio C Hamano
2017-11-27 10:22                             ` Kaartic Sivaraam
2017-11-27 20:54                               ` Johannes Schindelin
2017-11-27 20:44                         ` git status always modifies index? Johannes Schindelin
2017-11-27 20:49                           ` Jonathan Nieder
2017-11-26  3:32                 ` Junio C Hamano
2017-11-26  9:35                   ` Junio C Hamano
2017-11-27  4:43                     ` Jeff King
2017-11-27  4:56                       ` Junio C Hamano
2017-11-27  5:00                         ` Jeff King
2017-11-27 20:57                       ` Jonathan Nieder
2017-11-27 22:50                         ` Jeff King [this message]
2017-12-03  0:37                         ` Junio C Hamano
2017-11-26 19:27                   ` Jeff King
2017-11-27  0:47                     ` Junio C Hamano
2017-11-27  6:12                       ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171127225020.GA29384@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    --cc=jrnieder@gmail.com \
    --cc=nneul@neulinger.org \
    --cc=santiago@nyu.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).