{"thread":{"id":"47299","subject":"git status always modifies index?","startedAt":"2017-11-22T15:19:51Z","lastAt":"2017-12-03T00:38:02Z","messageCount":33,"participants":["Nathan Neulinger","Santiago Torres","Jonathan Nieder","Jeff King","Johannes Schindelin","Junio C Hamano","Kaartic Sivaraam"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"333280","messageId":"a039d139-dba5-683e-afbf-4044cd32ab1d@neulinger.org","threadId":"47299","inReplyTo":null,"subject":"git status always modifies index?","fromName":"Nathan Neulinger","fromEmail":"nneul@neulinger.org","sentAt":"2017-11-22T15:19:43Z","receivedAt":"2017-11-22T15:19:51Z","isPatch":false,"sender":{"key":"nneul@neulinger.org","avatar":"https://gravatar.com/avatar/6bf7d938addba77b29fe46572be573fd52d1e4dbea47218c4397d760c87aabd1?d=mp&s=160"},"body":"Current code appears to always attempt an index refresh, which results in file permission changes if you run a 'git \nstatus' as a privileged account.\n\nWould be nice if there were an option available to ask git status to NOT update the index.\n\nEven better would be if it was smart about the situation and would not refresh the index if it can see that file \nownership would change as a result of updating the index. To me this is following principle of least surprise. Running a \n\"query\" operation would not normally be expected to result in write/modify activity.\n\n-- Nathan\n------------------------------------------------------------\nNathan Neulinger                       nneul@neulinger.org\nNeulinger Consulting                   (573) 612-1412\n"},{"id":"333281","messageId":"20171122153028.olssotkcf3dd6ron@LykOS.localdomain","threadId":"47299","inReplyTo":"a039d139-dba5-683e-afbf-4044cd32ab1d@neulinger.org","subject":"Re: git status always modifies index?","fromName":"Santiago Torres","fromEmail":"santiago@nyu.edu","sentAt":"2017-11-22T15:30:30Z","receivedAt":"2017-11-22T15:29:12Z","isPatch":false,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"Hi Nathan.\n\nDo you mean git-status writing an index file? What would you suggest for\ngit-status to compute which files have changed without modifying an\nindex-file? Or are you suggesting git-status to fail if the index file\ndoesn't belong to the user-id who invoked the command...\n\nThanks,\n-Santiago\n"},{"id":"333282","messageId":"5050d779-2981-6f06-49f7-0ecb4efb25b8@neulinger.org","threadId":"47299","inReplyTo":"20171122153028.olssotkcf3dd6ron@LykOS.localdomain","subject":"Re: git status always modifies index?","fromName":"Nathan Neulinger","fromEmail":"nneul@neulinger.org","sentAt":"2017-11-22T15:37:09Z","receivedAt":"2017-11-22T15:37:16Z","isPatch":false,"sender":{"key":"nneul@neulinger.org","avatar":"https://gravatar.com/avatar/6bf7d938addba77b29fe46572be573fd52d1e4dbea47218c4397d760c87aabd1?d=mp&s=160"},"body":"What I'm meaning is - why does it need to write the index back out to disk?\n\n From looking at the code in builtin/commit.c it looks like it takes a lock on the index, collects the status, and then \nunconditionally rewrites the index file.\n\nI'm proposing that the update_index_if_able call not actually be issued if it would result in a ownership change on the \nunderlying file - such as a simple case of root user or other privileged account issuing 'git status' in a directory.\n\n\nI understand completely that it would be expected to be altered if the privileged user did a commit/add or any other \noperation that was inherently a 'write' operation, but doesn't seem like status should be one of those cases.\n\n-- Nathan\n\nOn 11/22/17 9:30 AM, Santiago Torres wrote:\n> Hi Nathan.\n> \n> Do you mean git-status writing an index file? What would you suggest for\n> git-status to compute which files have changed without modifying an\n> index-file? Or are you suggesting git-status to fail if the index file\n> doesn't belong to the user-id who invoked the command...\n> \n> Thanks,\n> -Santiago\n> \n\n-- \n------------------------------------------------------------\nNathan Neulinger                       nneul@neulinger.org\nNeulinger Consulting                   (573) 612-1412\n"},{"id":"333286","messageId":"20171122161014.djkdygmclk227xmq@LykOS.localdomain","threadId":"47299","inReplyTo":"5050d779-2981-6f06-49f7-0ecb4efb25b8@neulinger.org","subject":"Re: git status always modifies index?","fromName":"Santiago Torres","fromEmail":"santiago@nyu.edu","sentAt":"2017-11-22T16:10:16Z","receivedAt":"2017-11-22T16:08:56Z","isPatch":false,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"On Wed, Nov 22, 2017 at 09:37:09AM -0600, Nathan Neulinger wrote:\n> What I'm meaning is - why does it need to write the index back out to disk?\n> \n> From looking at the code in builtin/commit.c it looks like it takes a lock\n> on the index, collects the status, and then unconditionally rewrites the\n> index file.\n>\nHmm, I just took a look at those lines and I see what you mean. From\nwhat I understand, this is to cache the result of the index computation\nfor ensuing git calls.\n\n> I'm proposing that the update_index_if_able call not actually be issued if\n> it would result in a ownership change on the underlying file - such as a\n> simple case of root user or other privileged account issuing 'git status' in\n> a directory.\n\nI don't think this would be a desire-able default behavior. I'd wager\nthat running git status using different accounts is not a great idea ---\nspecially interacting with a user repository as root.\n\n> I understand completely that it would be expected to be altered if the\n> privileged user did a commit/add or any other operation that was inherently\n> a 'write' operation, but doesn't seem like status should be one of those\n> cases.\n\nI think it's because of the reasons above. That being said, I don't know\nwhat the rest of the community would think of something akin to a\n--no-update-index type flag.\n\nCheers!\n-Santiago.\n"},{"id":"333287","messageId":"dfbf4af3-e87c-bdcb-7544-685572925a50@neulinger.org","threadId":"47299","inReplyTo":"20171122161014.djkdygmclk227xmq@LykOS.localdomain","subject":"Re: git status always modifies index?","fromName":"Nathan Neulinger","fromEmail":"nneul@neulinger.org","sentAt":"2017-11-22T16:20:43Z","receivedAt":"2017-11-22T16:20:50Z","isPatch":false,"sender":{"key":"nneul@neulinger.org","avatar":"https://gravatar.com/avatar/6bf7d938addba77b29fe46572be573fd52d1e4dbea47218c4397d760c87aabd1?d=mp&s=160"},"body":"I just got an answer to my stackoverflow question on this, apparently it's already implemented:\n\nhttps://stackoverflow.com/questions/47436939/how-to-run-git-status-without-modifying-git-index-such-as-in-a-prompt-command\n\nThere is a \"--no-optional-locks\" command in 2.15 that looks like it does exactly what I need.\n\n-- Nathan\n\nOn 11/22/17 10:10 AM, Santiago Torres wrote:\n> On Wed, Nov 22, 2017 at 09:37:09AM -0600, Nathan Neulinger wrote:\n>> What I'm meaning is - why does it need to write the index back out to disk?\n>>\n>>  From looking at the code in builtin/commit.c it looks like it takes a lock\n>> on the index, collects the status, and then unconditionally rewrites the\n>> index file.\n>>\n> Hmm, I just took a look at those lines and I see what you mean. From\n> what I understand, this is to cache the result of the index computation\n> for ensuing git calls.\n> \n>> I'm proposing that the update_index_if_able call not actually be issued if\n>> it would result in a ownership change on the underlying file - such as a\n>> simple case of root user or other privileged account issuing 'git status' in\n>> a directory.\n> \n> I don't think this would be a desire-able default behavior. I'd wager\n> that running git status using different accounts is not a great idea ---\n> specially interacting with a user repository as root.\n> \n>> I understand completely that it would be expected to be altered if the\n>> privileged user did a commit/add or any other operation that was inherently\n>> a 'write' operation, but doesn't seem like status should be one of those\n>> cases.\n> \n> I think it's because of the reasons above. That being said, I don't know\n> what the rest of the community would think of something akin to a\n> --no-update-index type flag.\n> \n> Cheers!\n> -Santiago.\n> \n\n-- \n------------------------------------------------------------\nNathan Neulinger                       nneul@neulinger.org\nNeulinger Consulting                   (573) 612-1412\n"},{"id":"333289","messageId":"20171122162405.rr2uyggfv3xj4bqb@LykOS.localdomain","threadId":"47299","inReplyTo":"dfbf4af3-e87c-bdcb-7544-685572925a50@neulinger.org","subject":"Re: git status always modifies index?","fromName":"Santiago Torres","fromEmail":"santiago@nyu.edu","sentAt":"2017-11-22T16:24:06Z","receivedAt":"2017-11-22T16:22:46Z","isPatch":false,"sender":{"key":"santiago@nyu.edu","avatar":"https://avatars.githubusercontent.com/u/3579933?v=4"},"body":"Ah, my bad. I missed this patch...\n\nGood luck!\n-Santiago.\n"},{"id":"333321","messageId":"20171122202720.GD11671@aiede.mtv.corp.google.com","threadId":"47299","inReplyTo":"dfbf4af3-e87c-bdcb-7544-685572925a50@neulinger.org","subject":"Re: git status always modifies index?","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-11-22T20:27:20Z","receivedAt":"2017-11-22T20:27:29Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nNathan Neulinger wrote[1]:\n\n> I just got an answer to my stackoverflow question on this,\n> apparently it's already implemented:\n>\n> https://stackoverflow.com/questions/47436939/how-to-run-git-status-without-modifying-git-index-such-as-in-a-prompt-command\n>\n> There is a \"--no-optional-locks\" command in 2.15 that looks like it\n> does exactly what I need.\n\nI was about to point to\nhttps://public-inbox.org/git/20170921043214.pyhdsrpy4omy54rm@sigill.intra.peff.net/\nabout exactly this thing. :)\n\nThat said, I wonder if this use case is an illustration that a name\nlike --no-lock-index (as was used in Git for Windows when this feature\nfirst appeared) or --no-refresh-on-disk-index (sorry, I am terrible at\ncoming up with option names) would make the feature easier to\ndiscover.\n\nThanks,\nJonathan\n\n[1] https://public-inbox.org/git/dfbf4af3-e87c-bdcb-7544-685572925a50@neulinger.org/\n"},{"id":"333323","messageId":"20171122211729.GA2854@sigill","threadId":"47299","inReplyTo":"20171122202720.GD11671@aiede.mtv.corp.google.com","subject":"Re: git status always modifies index?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-22T21:17:29Z","receivedAt":"2017-11-22T21:17:37Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 22, 2017 at 12:27:20PM -0800, Jonathan Nieder wrote:\n\n> Nathan Neulinger wrote[1]:\n> \n> > I just got an answer to my stackoverflow question on this,\n> > apparently it's already implemented:\n> >\n> > https://stackoverflow.com/questions/47436939/how-to-run-git-status-without-modifying-git-index-such-as-in-a-prompt-command\n> >\n> > There is a \"--no-optional-locks\" command in 2.15 that looks like it\n> > does exactly what I need.\n> \n> I was about to point to\n> https://public-inbox.org/git/20170921043214.pyhdsrpy4omy54rm@sigill.intra.peff.net/\n> about exactly this thing. :)\n> \n> That said, I wonder if this use case is an illustration that a name\n> like --no-lock-index (as was used in Git for Windows when this feature\n> first appeared) or --no-refresh-on-disk-index (sorry, I am terrible at\n> coming up with option names) would make the feature easier to\n> discover.\n\nYeah, it's interesting that Nathan does not care about the simultaneous\nlocking here, but rather about the effect of writing to the repo for\nwhat would otherwise be a read-only operation.\n\nUnder the original intent of --no-optional-locks I think if we could\nsomehow magically update the on-disk index without lock contention, it\nwould be OK to do so. But that would make it no longer work for this\nparticular case.\n\nAnd I would also not be surprised if there are other cases where we\nwrite in a lockless way that would best be avoided in a multi-user\nsetup. I'm thinking specifically of the way that some merge-y operations\nmay write out intermediate objects, even though they're only needed\ninside the process. It _should_ be a read-only operation to ask \"can\nthese two things be merged cleanly\", and you should be able to ask that\nwithout accidentally creating root-owned files in .git/objects.\n\nSo I actually think what Nathan wants is not exactly the same as\n--no-optional-locks in the first place. But in practice, for a limited\nset of operations and with the way that locks work in Git, it\naccomplishes the same thing. Maybe that points to having a broader\noption. Or maybe having two separate options that largely have the same\neffect. Or maybe just living with the minor philosophical rough edges,\nsince it seems OK in practice.\n\n-Peff\n"},{"id":"333329","messageId":"20171122215635.GE11671@aiede.mtv.corp.google.com","threadId":"47299","inReplyTo":"20171122211729.GA2854@sigill","subject":"Re: git status always modifies index?","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-11-22T21:56:35Z","receivedAt":"2017-11-22T21:56:47Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jeff King wrote:\n> On Wed, Nov 22, 2017 at 12:27:20PM -0800, Jonathan Nieder wrote:\n\n>> That said, I wonder if this use case is an illustration that a name\n>> like --no-lock-index (as was used in Git for Windows when this feature\n>> first appeared) or --no-refresh-on-disk-index (sorry, I am terrible at\n>> coming up with option names) would make the feature easier to\n>> discover.\n[...]\n>         Or maybe just living with the minor philosophical rough edges,\n> since it seems OK in practice.\n\nTo be clear, my concern is not philosophical but practical: I'm saying\nif it's a \"git status\" option (or at least shows up in the \"git\nstatus\" manpage) and it is memorably about $GIT_DIR/index (at least\nmentions that in its description) then it is more likely to help\npeople.\n\nThanks,\nJonathan\n"},{"id":"333331","messageId":"20171122220627.GE2854@sigill","threadId":"47299","inReplyTo":"20171122215635.GE11671@aiede.mtv.corp.google.com","subject":"Re: git status always modifies index?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-22T22:06:27Z","receivedAt":"2017-11-22T22:06:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 22, 2017 at 01:56:35PM -0800, Jonathan Nieder wrote:\n\n> Jeff King wrote:\n> > On Wed, Nov 22, 2017 at 12:27:20PM -0800, Jonathan Nieder wrote:\n> \n> >> That said, I wonder if this use case is an illustration that a name\n> >> like --no-lock-index (as was used in Git for Windows when this feature\n> >> first appeared) or --no-refresh-on-disk-index (sorry, I am terrible at\n> >> coming up with option names) would make the feature easier to\n> >> discover.\n> [...]\n> >         Or maybe just living with the minor philosophical rough edges,\n> > since it seems OK in practice.\n> \n> To be clear, my concern is not philosophical but practical: I'm saying\n> if it's a \"git status\" option (or at least shows up in the \"git\n> status\" manpage) and it is memorably about $GIT_DIR/index (at least\n> mentions that in its description) then it is more likely to help\n> people.\n\nRight, I went a little off track of your original point.\n\nWhat I was trying to get at is that naming it \"status --no-lock-index\"\nwould not be the same thing (even though with the current implementation\nit would behave the same). IOW, can we improve the documentation of\n\"status\" to point to make it easier to discover this use case.\n\n-Peff\n"},{"id":"333490","messageId":"alpine.DEB.2.21.1.1711252240300.6482@virtualbox","threadId":"47299","inReplyTo":"20171122220627.GE2854@sigill","subject":"Re: git status always modifies index?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-25T21:55:25Z","receivedAt":"2017-11-25T21:56:34Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Wed, 22 Nov 2017, Jeff King wrote:\n\n> On Wed, Nov 22, 2017 at 01:56:35PM -0800, Jonathan Nieder wrote:\n> \n> > Jeff King wrote:\n> > > On Wed, Nov 22, 2017 at 12:27:20PM -0800, Jonathan Nieder wrote:\n> > \n> > >> That said, I wonder if this use case is an illustration that a name\n> > >> like --no-lock-index (as was used in Git for Windows when this feature\n> > >> first appeared) or --no-refresh-on-disk-index (sorry, I am terrible at\n> > >> coming up with option names) would make the feature easier to\n> > >> discover.\n> > [...]\n> > >         Or maybe just living with the minor philosophical rough edges,\n> > > since it seems OK in practice.\n> > \n> > To be clear, my concern is not philosophical but practical: I'm saying\n> > if it's a \"git status\" option (or at least shows up in the \"git\n> > status\" manpage) and it is memorably about $GIT_DIR/index (at least\n> > mentions that in its description) then it is more likely to help\n> > people.\n> \n> Right, I went a little off track of your original point.\n> \n> What I was trying to get at is that naming it \"status --no-lock-index\"\n> would not be the same thing (even though with the current implementation\n> it would behave the same). IOW, can we improve the documentation of\n> \"status\" to point to make it easier to discover this use case.\n\nI had the hunch that renaming the option (and moving it away from `git\nstatus`, even if it is currently only affecting `git status` and even if\nit will most likely be desirable to have the option to really only prevent\n`git status` from writing .lock files) was an unfortunate decision (and\nmade my life as Git for Windows quite a bit harder than really necessary,\nit cost me over one workday of a bug hunt, mainly due to a false flag\nindicating `git rebase` to be the culprit). And I hinted at it, too.\n\nMaybe I should trust my instincts and act on them more. It is not like it\nwas the first time that I had doubts that turned out to have merit, see\ne.g. the regression introduced into the formerly rock-solid\nset_hidden_flag() patches due to changes I made reluctantly during\nupstreaming, or the regression introduced during v1->v2 in my regex-buf\npatches that caused problems with mulibc and AIX.\n\nI really never understood why --no-optional-locks had to be introduced\nwhen it did exactly the same as --no-lock-index, and when the latter has a\nright to exist in the first place, even in the purely hypothetical case\nthat we teach --no-optional-locks to handle more cases than just `git\nstatus`' writing of the index (and in essence, it looks like premature\noptimization): it is a very concrete use case that a user may want `git\nstatus` to refrain from even trying to write any file, as this thread\nshows very eloquently.\n\nMaybe it is time to reintroduce --no-lock-index, and make\n--no-optional-locks' functionality a true superset of --no-lock-index'.\n\nAt least that is what my gut feeling tells me should be done.\n\nCiao,\nDscho\n"},{"id":"333514","messageId":"xmqqwp2diuki.fsf@gitster.mtv.corp.google.com","threadId":"47299","inReplyTo":"20171122220627.GE2854@sigill","subject":"Re: git status always modifies index?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-26T03:32:13Z","receivedAt":"2017-11-26T03:32:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> What I was trying to get at is that naming it \"status --no-lock-index\"\n> would not be the same thing (even though with the current implementation\n> it would behave the same). IOW, can we improve the documentation of\n> \"status\" to point to make it easier to discover this use case.\n\nYeah, the name is unfortunate. \n\nWhat the end user really wants to see, I suspect, is a \"--read-only\"\noption that applies to any filesystem entity and to any command, in\nthe context of this thread, and also in the original discussion that\nled to the introduction of that option.  \n\nWhile I think the variable losing \"index\" from its name was a vast\nimprovement relative to \"--no-lock-index\", simply because it\nexpresses what we do a bit closer to \"do not just do things without\nmodifying anything my repository\", it did not go far enough.\n\n"},{"id":"333524","messageId":"xmqq7eudidqb.fsf@gitster.mtv.corp.google.com","threadId":"47299","inReplyTo":"xmqqwp2diuki.fsf@gitster.mtv.corp.google.com","subject":"Re: git status always modifies index?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-26T09:35:56Z","receivedAt":"2017-11-26T09:36:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Jeff King <peff@peff.net> writes:\n>\n>> What I was trying to get at is that naming it \"status --no-lock-index\"\n>> would not be the same thing (even though with the current implementation\n>> it would behave the same). IOW, can we improve the documentation of\n>> \"status\" to point to make it easier to discover this use case.\n>\n> Yeah, the name is unfortunate. \n>\n> What the end user really wants to see, I suspect, is a \"--read-only\"\n> option that applies to any filesystem entity and to any command, in\n> the context of this thread, and also in the original discussion that\n> led to the introduction of that option.  \n>\n> While I think the variable losing \"index\" from its name was a vast\n> improvement relative to \"--no-lock-index\", simply because it\n> expresses what we do a bit closer to \"do not just do things without\n> modifying anything my repository\", it did not go far enough.\n\nYuck, the last sentence was garbled.  What I meant as the ideal\n\"read-only\" was \"do things without modifying anything in my\nrepository\".\n\nAnd to avoid any misunderstanding, what I mean by \"it did not go far\nenough\" is *NOT* this:\n\n    We added a narrow feature and gave it a narrow name.  Instead we\n    should have added a \"--read-only\" feature, which this change may\n    be a small part of, and waited releasing the whole thing until\n    it is reasonably complete.\n\nBy going far enough, I was wondering if we should have done\nsomething that we historically did not do.  An \"aspirational\"\nfeature that is incrementally released with a known bug and that\nwill give users what they want in the larger picture when completed.\n\nIOW, we could have made this \"git --read-only <cmd>\", that is\nexplained initially as \"tell Git not to modify repository when it\ndoes not have to (e.g. avoid opportunistic update)\" and perhaps\nlater as \"tell Git not to modify anything in the repository--if it\nabsolutely has to (e.g. \"git --read-only commit\" is impossible to\ncomplete without modifying anything in the repository), error out\ninstead\".  And with a known-bug section to clearly state that this\nfeature is not something we vetted every codepath to ensure the\nread-only operation, but is still a work in progress.\n\nAfter all, \"status\" does not have to stay to be the only command\nwith opportunistic modification (in the current implementation, it\ndoes \"update-index --refresh\" to update the index).  And the index\ndoes not have to stay to be the only thing that is opportunistically\nmodified (e.g. \"git diff --cached\" could not just opportunistically\nupdate the index, but also it could be taught to write out tree\nobjects for intermediate directories when it does its cache-tree\nrefresh, which would help the diff-index algorithm quite a bit in\nthe performance department).  \n\nHaving a large picture option like \"--read-only\" instead of ending\nup with dozens of \"we implemented a knob to tweak only this little\npiece, and here is an option to trigger it\" would help users in the\nlong run, but we traditionally did not do so because we tend to\navoid shipping \"incomplete\" features, but being perfectionist with\nsuch a large undertaking can stall topics with feature bloat.  In a\ncase like this, however, I suspect that an aspirational feature that\nstarts small, promises little and can be extended over time may be a\ngood way to move forward.\n\n\n\n"},{"id":"333533","messageId":"20171126192508.GB1501@sigill","threadId":"47299","inReplyTo":"alpine.DEB.2.21.1.1711252240300.6482@virtualbox","subject":"Re: git status always modifies index?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-26T19:25:08Z","receivedAt":"2017-11-26T19:25:15Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Nov 25, 2017 at 10:55:25PM +0100, Johannes Schindelin wrote:\n\n> > Right, I went a little off track of your original point.\n> > \n> > What I was trying to get at is that naming it \"status --no-lock-index\"\n> > would not be the same thing (even though with the current implementation\n> > it would behave the same). IOW, can we improve the documentation of\n> > \"status\" to point to make it easier to discover this use case.\n> \n> I had the hunch that renaming the option (and moving it away from `git\n> status`, even if it is currently only affecting `git status` and even if\n> it will most likely be desirable to have the option to really only prevent\n> `git status` from writing .lock files) was an unfortunate decision (and\n> made my life as Git for Windows quite a bit harder than really necessary,\n> it cost me over one workday of a bug hunt, mainly due to a false flag\n> indicating `git rebase` to be the culprit). And I hinted at it, too.\n\nI remain unconvinced that we have actually uncovered a serious problem.\nSomebody asked if Git could do a thing, and people pointed him to the\nright option. That option is new in the latest release. So it is\nentirely plausible to me that the new option is just fine and:\n\n  1. We could adjust the documentation to cross-reference from\n     git-status.\n\n  2. Now that the new option exists, informal documentation will start\n     to mention it. Including this thread in the mailing list archive,\n     and the stack overflow thread that was linked.\n\n> I really never understood why --no-optional-locks had to be introduced\n> when it did exactly the same as --no-lock-index, and when the latter has a\n> right to exist in the first place, even in the purely hypothetical case\n> that we teach --no-optional-locks to handle more cases than just `git\n> status`' writing of the index (and in essence, it looks like premature\n> optimization): it is a very concrete use case that a user may want `git\n> status` to refrain from even trying to write any file, as this thread\n> shows very eloquently.\n\nBesides potentially handling more than just \"git status\", it differs in\none other way: it can be triggered via and is carried through the\nenvironment.\n\n> Maybe it is time to reintroduce --no-lock-index, and make\n> --no-optional-locks' functionality a true superset of --no-lock-index'.\n\nI'm not against having a separate option for \"never write to the\nrepository\". I think it's potentially different than \"don't lock\", as I\nmentioned earlier. Frankly I don't see much value in \"--no-lock-index\"\nif we already have \"--no-optional-locks\". But I figured you would carry\n\"status --no-lock-index\" forever in Git for Windows anyway (after all,\nif you remove it now, you're breaking compatibility for existing users).\n\n-Peff\n"},{"id":"333534","messageId":"20171126192749.GC1501@sigill","threadId":"47299","inReplyTo":"xmqqwp2diuki.fsf@gitster.mtv.corp.google.com","subject":"Re: git status always modifies index?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-26T19:27:49Z","receivedAt":"2017-11-26T19:27:55Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 26, 2017 at 12:32:13PM +0900, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > What I was trying to get at is that naming it \"status --no-lock-index\"\n> > would not be the same thing (even though with the current implementation\n> > it would behave the same). IOW, can we improve the documentation of\n> > \"status\" to point to make it easier to discover this use case.\n> \n> Yeah, the name is unfortunate. \n> \n> What the end user really wants to see, I suspect, is a \"--read-only\"\n> option that applies to any filesystem entity and to any command, in\n> the context of this thread, and also in the original discussion that\n> led to the introduction of that option.\n\nI'm not sure I agree. Lockless writes are actually fine for the original\nuse case of --no-optional-locks (which is a process for the same user\nthat just happens to run in the background). I can buy the distinction\nbetween that and \"--read-only\" as premature optimization, though, since\nit's not common for most operations to do non-locking writes (pretty\nmuch object writes are the only thing, and most \"semantically read-only\"\noperations like status or diff do not write any objects).\n\nSo there's very little lost by people in the first boat saying\n\"--read-only\".\n\n-Peff\n"},{"id":"333551","messageId":"alpine.DEB.2.21.1.1711262231250.6482@virtualbox","threadId":"47299","inReplyTo":"20171126192508.GB1501@sigill","subject":"Re: git status always modifies index?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-26T21:55:01Z","receivedAt":"2017-11-26T21:55:23Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Sun, 26 Nov 2017, Jeff King wrote:\n\n> On Sat, Nov 25, 2017 at 10:55:25PM +0100, Johannes Schindelin wrote:\n> \n> > > Right, I went a little off track of your original point.\n> > > \n> > > What I was trying to get at is that naming it \"status --no-lock-index\"\n> > > would not be the same thing (even though with the current implementation\n> > > it would behave the same). IOW, can we improve the documentation of\n> > > \"status\" to point to make it easier to discover this use case.\n> > \n> > I had the hunch that renaming the option (and moving it away from `git\n> > status`, even if it is currently only affecting `git status` and even if\n> > it will most likely be desirable to have the option to really only prevent\n> > `git status` from writing .lock files) was an unfortunate decision (and\n> > made my life as Git for Windows quite a bit harder than really necessary,\n> > it cost me over one workday of a bug hunt, mainly due to a false flag\n> > indicating `git rebase` to be the culprit). And I hinted at it, too.\n> \n> I remain unconvinced that we have actually uncovered a serious problem.\n\nYou did not. A colleague of mine did. And it was a problem in Git for\nWindows only, caused by the changes necessitated by yours (which even used\nmy tests, which made it easy for my conflict resolution to do the wrong\nthing by removing my --no-lock-index test in favor of your\n--no-optional-locks test, breaking --no-lock-index).\n\nIt cost me almost two work days, and a lot of hair. And all I meant by \"I\nhinted at it, too\" was that I felt that something like that was coming\nwhen I saw your variation of my patches making it into git/git's master.\n\nThis kind of stuff really throws my upstreaming back quite a bit, not only\ndue to lost time, but also due to the frustration with the caused\nregressions.\n\nNow, the report indicates that not only Git for Windows had a problem, but\nthat the new feature is unnecessarily unintuitive. I would even claim that\nthe --no-lock-index option (even if it does not say \"--read-only\") would\nhave made for a better user experience because it is at least in the\nexpected place: the `git status` man page.\n\n> Somebody asked if Git could do a thing, and people pointed him to the\n> right option.\n\nIf people have to ask on the mailing list even after reading the man\npages, that's a strong indicator that we could do better.\n\n> That option is new in the latest release.\n\nIn Git, yes. In Git for Windows, no. And it worked beautifully in Git for\nWindows before v2.15.0.\n\n> > I really never understood why --no-optional-locks had to be introduced\n> > when it did exactly the same as --no-lock-index, and when the latter has a\n> > right to exist in the first place, even in the purely hypothetical case\n> > that we teach --no-optional-locks to handle more cases than just `git\n> > status`' writing of the index (and in essence, it looks like premature\n> > optimization): it is a very concrete use case that a user may want `git\n> > status` to refrain from even trying to write any file, as this thread\n> > shows very eloquently.\n> \n> Besides potentially handling more than just \"git status\",\n\n... which is a premature optimization...\n\n> it differs in one other way: it can be triggered via and is carried\n> through the environment.\n\n... which Git for Windows' --no-lock-index *also* had to do (think\nsubmodules). We simply figured that out only after introducing the option,\ntherefore it was carried as an add-on commit, and the plan was to squash\nit in before upstreaming (obviously!).\n\nSo I contest your claim. `--no-lock-index` must be propagated to callees\nin the same way as the (still hypothetical) `--no-optional-locks` that\nwould cover more than just `git status`.\n\n> > Maybe it is time to reintroduce --no-lock-index, and make\n> > --no-optional-locks' functionality a true superset of --no-lock-index'.\n> \n> I'm not against having a separate option for \"never write to the\n> repository\".\n\nWhoa, slow down. We already introduced the `--no-optional-locks` option\nfor a completely hypothetical use case covering more than just `git\nstatus`, a use case that may very well never see the light of day. (At\nleast it was my undederstanding that the conjecture of something like that\nmaybe being needed by somebody some time in the future was the entire\nreason tobutcher the --no-lock-index approach into a very different\nlooking --no-optional-locks that is much harder to find in the\ndocumentation.)\n\nLet's not introduce yet another option for a completely hypothetical use\ncase that may be even more theoretical.\n\n> I think it's potentially different than \"don't lock\", as I\n> mentioned earlier.\n\nI don't see the need at all at the moemnt.\n\n> Frankly I don't see much value in \"--no-lock-index\" if we already have\n> \"--no-optional-locks\".\n\nFunny. I did not (and still do not) see the need for renaming `git status\n--no-lock-index` to `git --no-optional-locks status` (i.e. cluttering the\nglobal option space for something that really only `git status` needs).\n\n> But I figured you would carry \"status --no-lock-index\" forever in Git\n> for Windows anyway (after all, if you remove it now, you're breaking\n> compatibility for existing users).\n\nI will not carry it forever. Most definitely not. It was marked as\nexperimental for a reason: I suspected that major changes would be\nrequired to get it accepted into git.git (even if I disagree from a purely\npracticial point of view that those changes are required, but that's what\nyou have to accept when working in Open Source, that you sometimes have to\nchange something solely to please the person who can reject your patches).\n\nSure, I am breaking compatibility for existing users, but that is more the\nfault of --no-optional-locks being introduced than anything else.\n\nI am pretty much done talking about this subject at this point. I only\nstarted talking about it because I wanted you to understand that I will\ninsist on my hunches more forcefully in the future, and I hope you will\nsee why I do that. But then, you may not even see the problems caused by\nthe renaming (and forced broader scope for currently no good reason) of\n--no-lock-index, so maybe you disagree that acting on my hunch would have\nprevented those problems.\n\nCiao,\nDscho\n"},{"id":"333560","messageId":"xmqq7euch7jb.fsf@gitster.mtv.corp.google.com","threadId":"47299","inReplyTo":"20171126192749.GC1501@sigill","subject":"Re: git status always modifies index?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-27T00:47:20Z","receivedAt":"2017-11-27T00:47:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I'm not sure I agree. Lockless writes are actually fine for the original\n> use case of --no-optional-locks (which is a process for the same user\n> that just happens to run in the background).\n\nThe phrase \"lockless write\" scares me---it sounds as if you\noverwrite the index file no matter what other people (including\nanother instance of yourself) are doing to it.  \n\n    Side note: What 'use-optional-locks' actually does is not to\n    give any file descriptor to write into when we invoke the\n    wt-status helpers (which would want to make an opportunistic\n    update to the index under the lock), so \"--no-optional-locks\" is\n    quite different from \"lockless write\".  Whew.  It is part of\n    what \"semantically read-only things do not write\" would have\n    been.\n\nHow would a true \"lockless write\" that is OK for background\nopportunistic refresh work?  Read, compute and then open the final\nindex file under its final name for writing and write it out,\nwithout involving any rename?  As long as it finishes writing the\nresult in full and closes, its competing with a real \"lockful write\"\nwould probably be safe when it loses (the lockful one will rename\nits result over to the refreshed one).  It cannot \"win\" the race by\nwriting into the temporary lock file the other party is using ;-)\nBut it may lose the race in a messy way---the lockful one tries to\nrename its result over to the real index, which the lockless one has\nstill open and writing.  Unix variants are probably OK with it and\nthe lockless one would lose gracefully, but on other platforms the\nlockful one would fail to rename, I suspect?  Or the lockless one\ncan crash while it is writing even if there is no race.\n\nOr do you mean something different by \"lockless write\"?\n"},{"id":"333577","messageId":"20171127044314.GA6236@sigill","threadId":"47299","inReplyTo":"xmqq7eudidqb.fsf@gitster.mtv.corp.google.com","subject":"Re: git status always modifies index?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-27T04:43:14Z","receivedAt":"2017-11-27T04:43:25Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 26, 2017 at 06:35:56PM +0900, Junio C Hamano wrote:\n\n> Having a large picture option like \"--read-only\" instead of ending\n> up with dozens of \"we implemented a knob to tweak only this little\n> piece, and here is an option to trigger it\" would help users in the\n> long run, but we traditionally did not do so because we tend to\n> avoid shipping \"incomplete\" features, but being perfectionist with\n> such a large undertaking can stall topics with feature bloat.  In a\n> case like this, however, I suspect that an aspirational feature that\n> starts small, promises little and can be extended over time may be a\n> good way to move forward.\n\nI actually consider \"--no-optional-locks\" to be such an aspirational\nfeature. I didn't go digging for other cases (though I'm fairly certain\nthat \"diff\" has one), but hoped to leave it for further bug reports (\"I\nused the option, ran command X, and saw lock contention\").\n\nI would be fine with having a further aspirational \"read only\" mode. As\nI said before, that's not quite the same thing as no-optional-locks, but\nI think they're close enough that I'd be fine having only one of them.\nBut now that we've shipped a version with the locking one, we're stuck\nwith at least for the duration of a deprecation cycle.\n\n-Peff\n"},{"id":"333585","messageId":"xmqqd144e2uu.fsf@gitster.mtv.corp.google.com","threadId":"47299","inReplyTo":"20171127044314.GA6236@sigill","subject":"Re: git status always modifies index?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-27T04:56:41Z","receivedAt":"2017-11-27T04:56:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I actually consider \"--no-optional-locks\" to be such an aspirational\n> feature. I didn't go digging for other cases (though I'm fairly certain\n> that \"diff\" has one), but hoped to leave it for further bug reports (\"I\n> used the option, ran command X, and saw lock contention\").\n\nOK, then we are essentially on the same page.  I just was hoping\nthat we can restrain ourselves from adding these \"non essential\"\nknobs at too fine granularity, ending up forcing end users to use\nall of them.\n"},{"id":"333586","messageId":"20171127050031.GA6858@sigill","threadId":"47299","inReplyTo":"xmqqd144e2uu.fsf@gitster.mtv.corp.google.com","subject":"Re: git status always modifies index?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-27T05:00:31Z","receivedAt":"2017-11-27T05:00:38Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 27, 2017 at 01:56:41PM +0900, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > I actually consider \"--no-optional-locks\" to be such an aspirational\n> > feature. I didn't go digging for other cases (though I'm fairly certain\n> > that \"diff\" has one), but hoped to leave it for further bug reports (\"I\n> > used the option, ran command X, and saw lock contention\").\n> \n> OK, then we are essentially on the same page.  I just was hoping\n> that we can restrain ourselves from adding these \"non essential\"\n> knobs at too fine granularity, ending up forcing end users to use\n> all of them.\n\nYes, I agree we should try not to have too many knobs. That's actually\none of the reasons I avoided a status-only option in the first place.\n\nIn retrospect, I agree that the current option probably doesn't get the\ngranularity quite right. The idea of \"totally read-only\" just didn't\ncross my mind at all when working on the earlier feature.\n\n-Peff\n"},{"id":"333593","messageId":"20171127052443.GB5946@sigill","threadId":"47299","inReplyTo":"alpine.DEB.2.21.1.1711262231250.6482@virtualbox","subject":"Re: git status always modifies index?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-27T05:24:44Z","receivedAt":"2017-11-27T05:24:51Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 26, 2017 at 10:55:01PM +0100, Johannes Schindelin wrote:\n\n> > I remain unconvinced that we have actually uncovered a serious problem.\n> \n> You did not. A colleague of mine did. And it was a problem in Git for\n> Windows only, caused by the changes necessitated by yours (which even used\n> my tests, which made it easy for my conflict resolution to do the wrong\n> thing by removing my --no-lock-index test in favor of your\n> --no-optional-locks test, breaking --no-lock-index).\n> \n> It cost me almost two work days, and a lot of hair. And all I meant by \"I\n> hinted at it, too\" was that I felt that something like that was coming\n> when I saw your variation of my patches making it into git/git's master.\n\nI was confused by your mention of a problem, since this was the first I\nheard about it. Looking at the GfW repo, I assume you mean the bits\ntouched by 45538830baf.\n\nIf so, then yes, I'm sad that the combination of the features caused\nextra work for you. But I also don't think that is a compelling reason\nto say that \"--no-optional-locks\" is the wrong approach.\n\nIt _does_ argue for trying to take features intact between the two\ncodebases. But I am not sure I buy that argument. The original feature\ngot no review on the list, and in fact most of us weren't even aware of\nit until encountering the problem independently. IMHO it argues for GfW\ntrying to land patches upstream first, and then having them trickle in\nas you merge upstream releases.\n\nI suspect you are going to say \"but I am busy and don't have time for\nthat\". And I know it takes time. I maintain the fork that GitHub runs on\nits servers, and I have a backlog of features to upstream. Some of them\nend up quite different when I do that, and it's a huge pain. But\nultimately I've forked the upstream project, and that's the price I pay.\nUpstream doesn't care about my fork's problems.\n\nI dunno. Maybe you do not see Git for Windows as such a fork. But\nspeaking as somebody who works on git.git, that is my perception of it\n(that GfW is downstream). So I'm sympathetic, but I don't like the idea\nof taking non-Windows-specific patches wholesale and skipping the list\nreview and design process.\n\n> > Somebody asked if Git could do a thing, and people pointed him to the\n> > right option.\n> \n> If people have to ask on the mailing list even after reading the man\n> pages, that's a strong indicator that we could do better.\n\nSure. That's why I suggested improving the documentation in my last\nemail. But in all the discussion, I haven't seen any patch to that\neffect.\n\n> In Git, yes. In Git for Windows, no. And it worked beautifully in Git for\n> Windows before v2.15.0.\n\nIt didn't in git.git, because it wasn't there. ;)\n\n> > But I figured you would carry \"status --no-lock-index\" forever in Git\n> > for Windows anyway (after all, if you remove it now, you're breaking\n> > compatibility for existing users).\n> \n> I will not carry it forever. Most definitely not. It was marked as\n> experimental for a reason: I suspected that major changes would be\n> required to get it accepted into git.git (even if I disagree from a purely\n> practicial point of view that those changes are required, but that's what\n> you have to accept when working in Open Source, that you sometimes have to\n> change something solely to please the person who can reject your patches).\n\nYes, I saw just now that you continue to recognize it and give a\ndeprecation warning, which seems like quite a reasonable thing to do.\n\n> Sure, I am breaking compatibility for existing users, but that is more the\n> fault of --no-optional-locks being introduced than anything else.\n\nHopefully the text at the start of this mail explains why I disagree on\nthe \"fault\" here.\n\n> I am pretty much done talking about this subject at this point. I only\n> started talking about it because I wanted you to understand that I will\n> insist on my hunches more forcefully in the future, and I hope you will\n> see why I do that. But then, you may not even see the problems caused by\n> the renaming (and forced broader scope for currently no good reason) of\n> --no-lock-index, so maybe you disagree that acting on my hunch would have\n> prevented those problems.\n\nAgain, maybe the bit above explains my viewpoint a bit more. I'm\ncertainly sympathetic to the pain of upstreaming.\n\nI do disagree with the \"no good reason\" for this particular patch.\n\nCertainly you should feel free to present your hunches. I'd expect you\nto as part of the review (I'm pretty sure I even solicited your opinion\nwhen I sent the original patch). But I also think it's important for\npatches sent upstream to get thorough review (both for code and design).\nThe patches having been in another fork (and thus presumably being\nstable) is one point in their favor, but I don't think it should trumps\nall other discussion.\n\n-Peff\n"},{"id":"333595","messageId":"xmqqmv38cl6a.fsf@gitster.mtv.corp.google.com","threadId":"47299","inReplyTo":"20171127052443.GB5946@sigill","subject":"Re: git status always modifies index?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-27T06:03:57Z","receivedAt":"2017-11-27T06:04:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Again, maybe the bit above explains my viewpoint a bit more. I'm\n> certainly sympathetic to the pain of upstreaming.\n>\n> I do disagree with the \"no good reason\" for this particular patch.\n>\n> Certainly you should feel free to present your hunches. I'd expect you\n> to as part of the review (I'm pretty sure I even solicited your opinion\n> when I sent the original patch). But I also think it's important for\n> patches sent upstream to get thorough review (both for code and design).\n> The patches having been in another fork (and thus presumably being\n> stable) is one point in their favor, but I don't think it should trumps\n> all other discussion.\n\nI haven't been following this subthread closely, but I agree.  I\nthink your turning a narrow option that was only about status into\nsomething that can be extended as a more general option resulted in\na better design overall.\n\nI am guessing that a little voice in his head said \"this may be\napplicable wider than Windows and it will be better to be humble and\nreceptive to others' suggestions by going to the list and get this\npatch reviewed\" was overridden by other needs, like expediency, when\nhe did the initial \"covers only status and its opportunistic writing\nof the index\" as a Windows only thing, and Dscho is now regretting\nnot following that initial hunch, as that resulted in unnecessary\npain for both himself and his users.  I am sympathetic, but we are\nall normal human and I do not think and mistakes like this can be\navoided.  Often we are blinded by the immediate issue in front of us\nand we lose sight of a bigger picture, and it is OK as long as we\nlearn from our (or better yet, others') mistakes.\n\nThanks.\n"},{"id":"333596","messageId":"20171127060412.GA1247@sigill","threadId":"47299","inReplyTo":"20171127052443.GB5946@sigill","subject":"[PATCH] git-status.txt: mention --no-optional-locks","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-27T06:04:12Z","receivedAt":"2017-11-27T06:04:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 27, 2017 at 12:24:43AM -0500, Jeff King wrote:\n\n> > If people have to ask on the mailing list even after reading the man\n> > pages, that's a strong indicator that we could do better.\n> \n> Sure. That's why I suggested improving the documentation in my last\n> email. But in all the discussion, I haven't seen any patch to that\n> effect.\n\nMaybe like this.\n\n-- >8 --\nSubject: [PATCH] git-status.txt: mention --no-optional-locks\n\nIf you come to the documentation thinking \"I do not want Git\nto take any locks for my background processes\", then you may\neasily run across \"--no-optional-locks\" in git.txt.\n\nBut it's quite reasonable to hit a specific instance of the\nproblem: you have \"git status\" running in the background,\nand you notice that it causes lock contention with other\nprocesses. So you look in git-status.txt to see if there is\na way to disable it, but there's no mention of the flag.\n\nLet's add a short note mentioning that status does indeed\ntouch the index (and why), with a pointer to the global\noption. That can point users in the right direction and help\nthem make a more informed decision about what they're\ndisabling.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/git-status.txt | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\ndiff --git a/Documentation/git-status.txt b/Documentation/git-status.txt\nindex fc282e0a92..81cab9aefb 100644\n--- a/Documentation/git-status.txt\n+++ b/Documentation/git-status.txt\n@@ -387,6 +387,19 @@ ignored submodules you can either use the --ignore-submodules=dirty command\n line option or the 'git submodule summary' command, which shows a similar\n output but does not honor these settings.\n \n+BACKGROUND REFRESH\n+------------------\n+\n+By default, `git status` will automatically refresh the index, updating\n+the cached stat information from the working tree and writing out the\n+result. Writing out the updated index is an optimization that isn't\n+strictly necessary (`status` computes the values for itself, but writing\n+them out is just to save subsequent programs from repeating our\n+computation). When `status` is run in the background, the lock held\n+during the write may conflict with other simultaneous processes, causing\n+them to fail. Scripts running `status` in the background should consider\n+using `git --no-optional-locks status` (see linkgit:git[1] for details).\n+\n SEE ALSO\n --------\n linkgit:gitignore[5]\n-- \n2.15.0.687.g5a800c9f78\n\n"},{"id":"333597","messageId":"xmqqindwcl00.fsf@gitster.mtv.corp.google.com","threadId":"47299","inReplyTo":"20171127060412.GA1247@sigill","subject":"Re: [PATCH] git-status.txt: mention --no-optional-locks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-11-27T06:07:43Z","receivedAt":"2017-11-27T06:07:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, Nov 27, 2017 at 12:24:43AM -0500, Jeff King wrote:\n>\n>> > If people have to ask on the mailing list even after reading the man\n>> > pages, that's a strong indicator that we could do better.\n>> \n>> Sure. That's why I suggested improving the documentation in my last\n>> email. But in all the discussion, I haven't seen any patch to that\n>> effect.\n>\n> Maybe like this.\n\nI gave it only a single read, and it was a quite easy read.\n\nWill queue but not immediately merge to 'next' before I hear from\nothers.\n\nThanks.\n\n> -- >8 --\n> Subject: [PATCH] git-status.txt: mention --no-optional-locks\n>\n> If you come to the documentation thinking \"I do not want Git\n> to take any locks for my background processes\", then you may\n> easily run across \"--no-optional-locks\" in git.txt.\n>\n> But it's quite reasonable to hit a specific instance of the\n> problem: you have \"git status\" running in the background,\n> and you notice that it causes lock contention with other\n> processes. So you look in git-status.txt to see if there is\n> a way to disable it, but there's no mention of the flag.\n>\n> Let's add a short note mentioning that status does indeed\n> touch the index (and why), with a pointer to the global\n> option. That can point users in the right direction and help\n> them make a more informed decision about what they're\n> disabling.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  Documentation/git-status.txt | 13 +++++++++++++\n>  1 file changed, 13 insertions(+)\n>\n> diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt\n> index fc282e0a92..81cab9aefb 100644\n> --- a/Documentation/git-status.txt\n> +++ b/Documentation/git-status.txt\n> @@ -387,6 +387,19 @@ ignored submodules you can either use the --ignore-submodules=dirty command\n>  line option or the 'git submodule summary' command, which shows a similar\n>  output but does not honor these settings.\n>  \n> +BACKGROUND REFRESH\n> +------------------\n> +\n> +By default, `git status` will automatically refresh the index, updating\n> +the cached stat information from the working tree and writing out the\n> +result. Writing out the updated index is an optimization that isn't\n> +strictly necessary (`status` computes the values for itself, but writing\n> +them out is just to save subsequent programs from repeating our\n> +computation). When `status` is run in the background, the lock held\n> +during the write may conflict with other simultaneous processes, causing\n> +them to fail. Scripts running `status` in the background should consider\n> +using `git --no-optional-locks status` (see linkgit:git[1] for details).\n> +\n>  SEE ALSO\n>  --------\n>  linkgit:gitignore[5]\n"},{"id":"333598","messageId":"20171127061257.GB1247@sigill","threadId":"47299","inReplyTo":"xmqq7euch7jb.fsf@gitster.mtv.corp.google.com","subject":"Re: git status always modifies index?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-27T06:12:57Z","receivedAt":"2017-11-27T06:13:04Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 27, 2017 at 09:47:20AM +0900, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > I'm not sure I agree. Lockless writes are actually fine for the original\n> > use case of --no-optional-locks (which is a process for the same user\n> > that just happens to run in the background).\n> \n> The phrase \"lockless write\" scares me---it sounds as if you\n> overwrite the index file no matter what other people (including\n> another instance of yourself) are doing to it.  \n\nIck, no, that would be quite bad. ;)\n\nI only meant that if we \"somehow\" had a way in the future to update the\nstat cache without affecting the other parts of the index, and without\ncausing lock contention that causes other readers to barf, it could be\ntriggered even under this option.\n\nThat would be quite different from the current index and stat-cache\ndesign, and I have no plans in that area.\n\nWrites to the object database _are_ lockless now (it is OK if two\nwriters collide, because they are by definition writing the same data).\nAnd I wouldn't expect them to be affected by --no-optional-locks.  I\nthink elsewhere in the thread you mentioned writing out trees for\ncache-tree, which seems like a plausible example. Usually there's not\nmuch point if you're not going to write out the index with the new\ncache-tree entries, too. But I could see a program wanting to convert\nthe index into a tree in order to speed up a series of tree-to-index\ndiffs within a single program.\n\nThis is all pretty hypothetical, though.\n\n-Peff\n"},{"id":"333604","messageId":"b63ecdab-2283-9479-0de6-29a604c09670@gmail.com","threadId":"47299","inReplyTo":"xmqqindwcl00.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] git-status.txt: mention --no-optional-locks","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2017-11-27T10:22:31Z","receivedAt":"2017-11-27T10:22:45Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Monday 27 November 2017 11:37 AM, Junio C Hamano wrote:\n> Jeff King <peff@peff.net> writes:\n>> +using `git --no-optional-locks status` (see linkgit:git[1] for details).\n\nIt strikes me just now that `--no-side-effects` might have been a better \nname for the option (of course, iff this avoid all kinds of side \neffects. I'm not sure about the side affects other than index refreshing \nof \"git status\"). And in case we didn't care about the predictability of \noption names even a little, `--do-what-i-say` might have been a catchy \nalternative ;-)\n\n\n---\nKaartic\n"},{"id":"333631","messageId":"alpine.DEB.2.21.1.1711272142120.6482@virtualbox","threadId":"47299","inReplyTo":"20171127052443.GB5946@sigill","subject":"Re: git status always modifies index?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-27T20:44:29Z","receivedAt":"2017-11-27T20:44:51Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Mon, 27 Nov 2017, Jeff King wrote:\n\n> [...] IMHO it argues for GfW trying to land patches upstream first, and\n> then having them trickle in as you merge upstream releases.\n\nYou know that I tried that, and you know why I do not do that anymore: it\nsimply takes too long, and the review on the list focuses on things I\ncannot focus on as much, I need to make sure that the patches *work*\nfirst, whereas the patch review on the Git mailing list tends to ensure\nthat they have the proper form first.\n\nI upstream patches when I have time.\n\nCiao,\nDscho\n"},{"id":"333632","messageId":"20171127204909.GA27469@aiede.mtv.corp.google.com","threadId":"47299","inReplyTo":"alpine.DEB.2.21.1.1711272142120.6482@virtualbox","subject":"Re: git status always modifies index?","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-11-27T20:49:09Z","receivedAt":"2017-11-27T20:49:41Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJohannes Schindelin wrote:\n> On Mon, 27 Nov 2017, Jeff King wrote:\n\n>> [...] IMHO it argues for GfW trying to land patches upstream first, and\n>> then having them trickle in as you merge upstream releases.\n>\n> You know that I tried that, and you know why I do not do that anymore: it\n> simply takes too long, and the review on the list focuses on things I\n> cannot focus on as much, I need to make sure that the patches *work*\n> first, whereas the patch review on the Git mailing list tends to ensure\n> that they have the proper form first.\n>\n> I upstream patches when I have time.\n\nYou have been developing in the open, so no complaints from me, just a\nsecond point of reference:\n\nFor Google's internal use we sometimes have needed a patch faster than\nupstream can review it.  Our approach in those cases has been to send\na patch to the mailing list and then apply it internally immediately.\nIf upstream is stalled for months on review, so be it --- we already\nhave the patch.  But this tends to help ensure that we are moving in\nthe same direction.\n\nThat said, I don't think that was the main issue with\n--no-optional-locks.  I'll comment more on that in another subthread.\n\nThanks,\nJonathan\n"},{"id":"333633","messageId":"alpine.DEB.2.21.1.1711272146540.6482@virtualbox","threadId":"47299","inReplyTo":"xmqqmv38cl6a.fsf@gitster.mtv.corp.google.com","subject":"Re: git status always modifies index?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-27T20:50:30Z","receivedAt":"2017-11-27T20:50:56Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 27 Nov 2017, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > Again, maybe the bit above explains my viewpoint a bit more. I'm\n> > certainly sympathetic to the pain of upstreaming.\n> >\n> > I do disagree with the \"no good reason\" for this particular patch.\n> >\n> > Certainly you should feel free to present your hunches. I'd expect you\n> > to as part of the review (I'm pretty sure I even solicited your opinion\n> > when I sent the original patch). But I also think it's important for\n> > patches sent upstream to get thorough review (both for code and design).\n> > The patches having been in another fork (and thus presumably being\n> > stable) is one point in their favor, but I don't think it should trumps\n> > all other discussion.\n> \n> I haven't been following this subthread closely, but I agree.  I\n> think your turning a narrow option that was only about status into\n> something that can be extended as a more general option resulted in\n> a better design overall.\n\nThe --no-optional-locks feature is\n\n- hard to find,\n\n- in the current scenarios less desirable than a very concrete \"do not\n  write index.lock files in `git status`\",\n\n- too simple to introduce to merit introducing it *before* a need for it\n  arises that is larger than `git status --no-lock-index`, and you would\n  still have to keep the latter because it is a very concrete and real use\n  case that is unlikely to want to avoid other .lock files too.\n\nSo while you two are happily on agreeing with one another, the reality is\nthat this supposedly better design is nothing else than premature\noptimization.\n\nCiao,\nDscho\n"},{"id":"333634","messageId":"alpine.DEB.2.21.1.1711272152510.6482@virtualbox","threadId":"47299","inReplyTo":"b63ecdab-2283-9479-0de6-29a604c09670@gmail.com","subject":"Re: [PATCH] git-status.txt: mention --no-optional-locks","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-11-27T20:54:35Z","receivedAt":"2017-11-27T20:55:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Kaartic,\n\nOn Mon, 27 Nov 2017, Kaartic Sivaraam wrote:\n\n> On Monday 27 November 2017 11:37 AM, Junio C Hamano wrote:\n> > Jeff King <peff@peff.net> writes:\n> > > +using `git --no-optional-locks status` (see linkgit:git[1] for details).\n> \n> It strikes me just now that `--no-side-effects` might have been a better\n> name for the option (of course, iff this avoid all kinds of side\n> effects. I'm not sure about the side affects other than index refreshing\n> of \"git status\"). And in case we didn't care about the predictability of\n> option names even a little, `--do-what-i-say` might have been a catchy\n> alternative ;-)\n\nYour reasoning points to an important insight: while writing index.lock\nfiles is a side effect of `git status`, and while there may be other side\neffects in other operations, it is highly doubtful that any caller would\njust want to switch them off wholesale. Instead, it is much more likely\nthat callers will want to pick the side effect they want to switch off.\n\nCiao,\nDscho\n"},{"id":"333636","messageId":"20171127205731.GB27469@aiede.mtv.corp.google.com","threadId":"47299","inReplyTo":"20171127044314.GA6236@sigill","subject":"Re: git status always modifies index?","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-11-27T20:57:31Z","receivedAt":"2017-11-27T20:57:38Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJeff King wrote:\n> On Sun, Nov 26, 2017 at 06:35:56PM +0900, Junio C Hamano wrote:\n\n>> Having a large picture option like \"--read-only\" instead of ending\n>> up with dozens of \"we implemented a knob to tweak only this little\n>> piece, and here is an option to trigger it\" would help users in the\n>> long run, but we traditionally did not do so because we tend to\n>> avoid shipping \"incomplete\" features, but being perfectionist with\n>> such a large undertaking can stall topics with feature bloat.  In a\n>> case like this, however, I suspect that an aspirational feature that\n>> starts small, promises little and can be extended over time may be a\n>> good way to move forward.\n>\n> I actually consider \"--no-optional-locks\" to be such an aspirational\n> feature. I didn't go digging for other cases (though I'm fairly certain\n> that \"diff\" has one), but hoped to leave it for further bug reports (\"I\n> used the option, ran command X, and saw lock contention\").\n\nI am worried that the project is not learning from what happened here.\n\nMy main issue with the --no-optional-locks name is that it does not\nconnect to the underlying user need.  Your main argument for it is\nthat it exactly describes the underlying user need.  One of us has to\nbe wrong.\n\nSo let me describe my naive reading:\n\nAs a user, I want to inspect the state of the repository without\ndisrupting it in any way.  That means not breaking concurrent\nprocesses and not upsetting permissions.  --read-only seems to\ndescribe this use case to me perfectly.\n\nIf I understood correctly, your objection is that --read-only is not\nspecific enough.  What I really want, you might say, is not to break\nconcurrent processes.  Any other aspects of being read-only are not\nrelevant.  E.g. if I can refresh the on-disk index using O_APPEND\nwithout disrupting concurrent processes then I should be satisfied\nwith that.\n\nFair enough, though that feels like overengineering.  But I *still*\ndon't see what that has to do with the name \"no-optional-locks\".  When\nis a lock *optional*?  And how am I supposed to discover this option?\n\nThis also came up during review, and I am worried that this review\nfeedback is being ignored.  In other words, I have no reason to\nbelieve it won't happen again.\n\n> I would be fine with having a further aspirational \"read only\" mode.\n\nExcellent, we seem to agree on this much.  If I can find time for it\ntoday then I'll write a patch.\n\nThanks,\nJonathan\n"},{"id":"333645","messageId":"20171127225020.GA29384@sigill.intra.peff.net","threadId":"47299","inReplyTo":"20171127205731.GB27469@aiede.mtv.corp.google.com","subject":"Re: git status always modifies index?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-27T22:50:21Z","receivedAt":"2017-11-27T22:50:27Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 27, 2017 at 12:57:31PM -0800, Jonathan Nieder wrote:\n\n> > I actually consider \"--no-optional-locks\" to be such an aspirational\n> > feature. I didn't go digging for other cases (though I'm fairly certain\n> > that \"diff\" has one), but hoped to leave it for further bug reports (\"I\n> > used the option, ran command X, and saw lock contention\").\n> \n> I am worried that the project is not learning from what happened here.\n> \n> My main issue with the --no-optional-locks name is that it does not\n> connect to the underlying user need.  Your main argument for it is\n> that it exactly describes the underlying user need.  One of us has to\n> be wrong.\n\nOr there's a false dichotomy. ;) We could be talking about two different\nusers.\n\n> So let me describe my naive reading:\n> \n> As a user, I want to inspect the state of the repository without\n> disrupting it in any way.  That means not breaking concurrent\n> processes and not upsetting permissions.  --read-only seems to\n> describe this use case to me perfectly.\n\nThat does not match the request that I got from real script writers who\nwere having a problem. They wanted to avoid lock contention with\nbackground tasks.  They don't care if the repository is modified as long\nas it is done in a safe and non-conflicting way.\n\nI agree (as I think I've said already in this thread) that --read-only\nwould be a superset of that. And that it would probably be OK to have\njust gone there in the first place, sacrificing a small amount of\nspecificity in the name of having fewer knobs for the user to turn.\n\n> If I understood correctly, your objection is that --read-only is not\n> specific enough.  What I really want, you might say, is not to break\n> concurrent processes.  Any other aspects of being read-only are not\n> relevant.  E.g. if I can refresh the on-disk index using O_APPEND\n> without disrupting concurrent processes then I should be satisfied\n> with that.\n\nDo I have an objection? It's not clear to me that anybody is actually\nproposing anything concrete for me to object to.\n\nAre we adding \"--read-only\"? Are we going back to \"status\n--no-lock-index\"? In either case, are we deprecating\n\"--no-optional-locks\"?\n\nIt sounds like you are arguing for the first, and it sounds like Dscho\nis arguing for the second. Frankly, I don't really care that much. I've\nsaid all that I can on why I chose the direction I did, and I remain\nunconvinced that we have evidence that the current option is somehow\nimpossible to find. If somebody wants to take us down one of the other\nroads, that's fine by me.\n\n> Fair enough, though that feels like overengineering.  But I *still*\n> don't see what that has to do with the name \"no-optional-locks\".  When\n> is a lock *optional*?  And how am I supposed to discover this option?\n\nI kind of feel like any answer I give to these questions is just going\nto be waved aside. But here are my earnest answers:\n\n  1. You are bit by lock contention, where running operation X ends up\n     with some error like \"unable to create index.lock: file exists\".\n     \"X\" is probably something like \"commit\".\n\n  2. You search the documentation for options related to locks. You're\n     not likely to find it in the manpage for X, since the root of the\n     problem actually has nothing to do with X in the first place. It's\n     a background task running \"status\" that is the problem.\n\n  3. You might find it in git(1) while searching for information on\n     locks, since \"lock\" is in the name of the option (and is in fact\n     the only hit in that page). The index is also mentioned there\n     (though searching for \"index\" yields a lot more hits).\n\n  4. You might find it in git-status(1) if you suspect that \"status\" is\n     at work. Searching for \"index\" or \"lock\" turns up the addition I\n     just proposed yesterday.\n\nThere are obviously a lot of places where that sequence might fail to\nfind a hit. But the same is true of just about any option, including\nputting \"--read-only\" into git(1).\n\n> This also came up during review, and I am worried that this review\n> feedback is being ignored.  In other words, I have no reason to\n> believe it won't happen again.\n\nI'm having a hard time figuring out what you mean here. Do you mean that\nI ignored feedback on this topic during the initial review?\n\nLooking at the original thread, I just don't see it. There was some\nquestion about the name. I tried to lay out my thinking here:\n\n  https://public-inbox.org/git/20170921050835.mrbgx2zryy3jusdk@sigill.intra.peff.net/\n\nand ended with:\n\n  I am open to a better name, but I could not come up with one.\n\nThere was no meaningful response on the topic. When I reposted v2, I\ntried to bring attention to that with:\n\n    - there was some discussion over the name. I didn't see other\n      suggestions, and I didn't come up with anything better.\n\nSo...am I missing something? Am I misunderstanding your point?\n\n> > I would be fine with having a further aspirational \"read only\" mode.\n> \n> Excellent, we seem to agree on this much.  If I can find time for it\n> today then I'll write a patch.\n\nGreat.\n\n-Peff\n"},{"id":"333974","messageId":"xmqq7eu4mysc.fsf@gitster.mtv.corp.google.com","threadId":"47299","inReplyTo":"20171127205731.GB27469@aiede.mtv.corp.google.com","subject":"Re: git status always modifies index?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-03T00:37:55Z","receivedAt":"2017-12-03T00:38:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> I am worried that the project is not learning from what happened here.\n> ...\n> Fair enough, though that feels like overengineering.  But I *still*\n> don't see what that has to do with the name \"no-optional-locks\".  When\n> is a lock *optional*?  And how am I supposed to discover this option?\n>\n> This also came up during review, and I am worried that this review\n> feedback is being ignored.  In other words, I have no reason to\n> believe it won't happen again.\n\nI too would like to see this part explained a bit better.\n"}]}