{"thread":{"id":"23363","subject":"gitk pays too much attention to file timestamps","startedAt":"2010-04-06T22:57:00Z","lastAt":"2010-04-07T16:48:29Z","messageCount":15,"participants":["Alexander Gladysh","Markus Heidelberg","Jonathan Nieder","Avery Pennarun","A Large Angry SCM","Junio C Hamano","Jon Seymour"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"138785","messageId":"l2hc6c947f61004061557x8085600fif5e973077d9eb4f3@mail.gmail.com","threadId":"23363","inReplyTo":null,"subject":"gitk pays too much attention to file timestamps","fromName":"Alexander Gladysh","fromEmail":"agladysh@gmail.com","sentAt":"2010-04-06T22:57:00Z","receivedAt":"2010-04-06T22:57:00Z","isPatch":false,"sender":{"key":"agladysh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38239?v=4"},"body":"Hi, list!\n\nOS X 10.6.3\nGit 1.7.0.4\n\nWhen I \"touch\" a file, gitk lists it in \"local uncommitted changes,\nnot checked in to index\" (without a difference, just a name). I\nbelieve that it should not.\n\nGit status, and git commit do ignore such files.\n\nAfter git reset --hard, gitk stops seeing changes as expected.\n\nSee steps to reproduce below.\n\nHTH,\nAlexander.\n\n$ mkdir touch\n$ cd touch\n$ git init\n$ echo \"A\" > alpha\n$ git add alpha\n$ git commit -m \"alpha\"\n$ touch alpha\n$ git status\n# On branch master\nnothing to commit (working directory clean)\n$ gitk --all\n\nObserve \"local uncommitted changes, not checked in to index\"\n"},{"id":"138786","messageId":"201004070115.27285.markus.heidelberg@web.de","threadId":"23363","inReplyTo":"l2hc6c947f61004061557x8085600fif5e973077d9eb4f3@mail.gmail.com","subject":"Re: gitk pays too much attention to file timestamps","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2010-04-06T23:15:27Z","receivedAt":"2010-04-06T23:15:27Z","isPatch":false,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"Alexander Gladysh, 2010-04-07 00:57:\n> Hi, list!\n> \n> OS X 10.6.3\n> Git 1.7.0.4\n> \n> When I \"touch\" a file, gitk lists it in \"local uncommitted changes,\n> not checked in to index\" (without a difference, just a name). I\n> believe that it should not.\n> \n> Git status, and git commit do ignore such files.\n> \n> After git reset --hard, gitk stops seeing changes as expected.\n\nThe problem is that gitk doesn't invoke \"git update-index --refresh\",\nI guess it should on Update (F5) and Reload (Ctrl-F5).\n\nMarkus\n"},{"id":"138787","messageId":"20100406233601.GA27533@progeny.tock","threadId":"23363","inReplyTo":"l2hc6c947f61004061557x8085600fif5e973077d9eb4f3@mail.gmail.com","subject":"Re: gitk pays too much attention to file timestamps","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-06T23:36:01Z","receivedAt":"2010-04-06T23:36:01Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi!\n\nAlexander Gladysh wrote:\n\n> When I \"touch\" a file, gitk lists it in \"local uncommitted changes,\n> not checked in to index\" (without a difference, just a name). I\n> believe that it should not.\n\nFirst, an explanation:\n\nIn general, git keeps some stat(2) information to tell whether an\nentry in the index is \"dirty\".  This way, low-level commands can\ncompare that to the metadata in the file to avoid a costly\ncomparison of actual objects to the content of files on disk.\n\nBefore starting work, the high-level commands like ‘git commit’ will\ngenerally update this stat(2) information in one pass before doing\nanything else.  This turns any \"dirty\" entries without actually\ndifferent content in the index into clean entries, so the user doesn’t\nhave to worry about anything except content for these commands.  You\ncan request this update at any time yourself with the following\ncommand:\n\n  git update-index --refresh -q\n\nUnlike ‘git checkout HEAD -- .’ this does not touch the files in\nyour work tree, and unlike ‘git reset HEAD’, it does not affect the\ncontent registered in the index for your files.\n\nOkay, on to your topic:\n\ngitk is something of a passive observer of the index, which is\nactually something I like about it.  This keeps it relatively fast\nand can be useful when trying to understand other commands.\n\nI am not sure how other people use gitk, though.  Maybe this would\nbe worth changing.  For a reference point, another command in a\nvery similar situation is ‘git diff’: people who want the speedup\nfrom avoiding refreshing the index with that command use\n\n\t[diff]\n\t\tautoRefreshIndex = false\n\nin their configuration file, so the rest of us don’t have to suffer\nfrom the confusing behavior.\n\nAs some kind of evil compromise, it might be worth teaching gitk\nto check the same configuration and run update-index --refresh in\ngetcommits{} if and only if it is unset or set to true.\n\nThoughts?\nJonathan\n"},{"id":"138788","messageId":"n2kc6c947f61004061647ybb6c2f55zc70197362764ef8@mail.gmail.com","threadId":"23363","inReplyTo":"20100406233601.GA27533@progeny.tock","subject":"Re: gitk pays too much attention to file timestamps","fromName":"Alexander Gladysh","fromEmail":"agladysh@gmail.com","sentAt":"2010-04-06T23:47:15Z","receivedAt":"2010-04-06T23:47:15Z","isPatch":false,"sender":{"key":"agladysh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38239?v=4"},"body":"Jonathan,\n\nthank you for the explanation!\n\n<...>\n\n> gitk is something of a passive observer of the index, which is\n> actually something I like about it.  This keeps it relatively fast\n> and can be useful when trying to understand other commands.\n\nI *think* that 1.6.x didn't have this issue. (Sorry, can't check now.)\n\n> I am not sure how other people use gitk, though.  Maybe this would\n> be worth changing.  For a reference point, another command in a\n> very similar situation is ‘git diff’: people who want the speedup\n> from avoiding refreshing the index with that command use\n\n>        [diff]\n>                autoRefreshIndex = false\n\n> in their configuration file, so the rest of us don’t have to suffer\n> from the confusing behavior.\n\n> As some kind of evil compromise, it might be worth teaching gitk\n> to check the same configuration and run update-index --refresh in\n> getcommits{} if and only if it is unset or set to true.\n\n> Thoughts?\n\nThat's fine as far as I'm concerned.\n\nThe current behaviour is really annoying.\n\nIf I want something fast, I do not use GUI tools. Gitk starts up\nrather slowly on my box anyway.\n\nAlexander.\n"},{"id":"138789","messageId":"u2r32541b131004061658r555f21dbgbe011960d9152d3c@mail.gmail.com","threadId":"23363","inReplyTo":"20100406233601.GA27533@progeny.tock","subject":"Re: gitk pays too much attention to file timestamps","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2010-04-06T23:58:41Z","receivedAt":"2010-04-06T23:58:41Z","isPatch":false,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"On Tue, Apr 6, 2010 at 7:36 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> gitk is something of a passive observer of the index, which is\n> actually something I like about it.  This keeps it relatively fast\n> and can be useful when trying to understand other commands.\n>\n> I am not sure how other people use gitk, though.  Maybe this would\n> be worth changing.  For a reference point, another command in a\n> very similar situation is ‘git diff’: people who want the speedup\n> from avoiding refreshing the index with that command use\n\nIsn't it kind of weird, then, that it bothers to check the stat\ninformation at all?  I'm not sure why I'd want to know accurately\nwhether or not my file matches the index, but then have gitk fail to\ntell me about *what* doesn't match.  If it has to check the stat()\ninformation anyway, isn't it already being slow?\n\nAvery\n"},{"id":"138791","messageId":"20100407004353.GA11346@progeny.tock","threadId":"23363","inReplyTo":"n2kc6c947f61004061647ybb6c2f55zc70197362764ef8@mail.gmail.com","subject":"[PATCH/RFC] gitk: refresh index before checking for local changes","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-07T00:43:53Z","receivedAt":"2010-04-07T00:43:53Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Most git porcelain silently refreshes stat-dirty index entries.  Teach\ngitk to, too; this will make the behavior easier to understand when a\nperson makes a change to a file and then changes mind and restores the\nold version in her editor of choice.\n\nThis patch does not change the ‘checkout’ code path, since it is\nassumed that the index is already being cleaned in that case.\n\nTesting is needed to check if this breaks operation with read-only\naccess to a repository.\n\nRequested-by: Alexander Gladysh <agladysh@gmail.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nAlexander Gladysh wrote:\n> Jonathan Nieder wrote:\n\n[some nonsense about configurability]\n\n> That's fine as far as I'm concerned.\n> \n> The current behaviour is really annoying.\n\nMaybe that could be added in the future.  From testing this out,\nrefreshing unconditionally seems fast enough, at least.\n\n gitk |   50 +++++++++++++++++++++++++++++++++++++++++---------\n 1 files changed, 41 insertions(+), 9 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex 1f36a3e..2753446 100755\n--- a/gitk\n+++ b/gitk\n@@ -375,7 +375,7 @@ proc start_rev_list {view} {\n \tget_viewmainhead $view\n     }\n     if {$showlocalchanges && $viewmainheadid($view) ne {}} {\n-\tinterestedin $viewmainheadid($view) dodiffindex\n+\tinterestedin $viewmainheadid($view) dorefreshindex\n     }\n     fconfigure $fd -blocking 0 -translation lf -eofchar {}\n     if {$tclencoding != {}} {\n@@ -4974,9 +4974,9 @@ proc doshowlocalchanges {} {\n \n     if {$viewmainheadid($curview) eq {}} return\n     if {[commitinview $viewmainheadid($curview) $curview]} {\n-\tdodiffindex\n+\tdorefreshindex\n     } else {\n-\tinterestedin $viewmainheadid($curview) dodiffindex\n+\tinterestedin $viewmainheadid($curview) dorefreshindex\n     }\n }\n \n@@ -4992,6 +4992,42 @@ proc dohidelocalchanges {} {\n     incr lserial\n }\n \n+# spawn off a process to refresh the index\n+proc dorefreshindex {} {\n+    global lserial showlocalchanges isworktree vfilelimit curview\n+\n+    if {!$showlocalchanges || !$isworktree} return\n+    incr lserial\n+    set cmd \"|git update-index --refresh -q\"\n+    if {$vfilelimit($curview) ne {}} {\n+\tset cmd [concat $cmd -- $vfilelimit($curview)]\n+    }\n+    set fd [open $cmd r]\n+    fconfigure $fd -blocking 0\n+    set i [reg_instance $fd]\n+    filerun $fd [list readrefreshindex $fd $lserial $i]\n+}\n+\n+# update-index --refresh -q finished?\n+proc readrefreshindex {fd serial inst} {\n+    global lserial\n+\n+    if {$serial != $lserial} {\n+\tstop_instance $inst\n+\treturn 0\n+    }\n+    if {[gets $fd line] == 0} {\n+\t# ignore output\n+\treturn 0\n+    }\n+    if {![eof $fd]} {\n+        return 1\n+    }\n+    stop_instance $inst\n+    dodiffindex\n+    return 0\n+}\n+\n # spawn off a process to do git diff-index --cached HEAD\n proc dodiffindex {} {\n     global lserial showlocalchanges vfilelimit curview\n@@ -7504,13 +7540,9 @@ proc getblobdiffs {ids} {\n     global git_version\n \n     set textconv {}\n-    if {[package vcompare $git_version \"1.6.1\"] >= 0} {\n-\tset textconv \"--textconv\"\n-    }\n+    set textconv \"--textconv\"\n     set submodule {}\n-    if {[package vcompare $git_version \"1.6.6\"] >= 0} {\n-\tset submodule \"--submodule\"\n-    }\n+    set submodule \"--submodule\"\n     set cmd [diffcmd $ids \"-p $textconv $submodule  -C --cc --no-commit-id -U$diffcontext\"]\n     if {$ignorespace} {\n \tappend cmd \" -w\"\n-- \ndebian.1.7.0.3.1.469.g398f8\n"},{"id":"138792","messageId":"20100407010109.GB11346@progeny.tock","threadId":"23363","inReplyTo":"u2r32541b131004061658r555f21dbgbe011960d9152d3c@mail.gmail.com","subject":"Re: gitk pays too much attention to file timestamps","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-07T01:01:09Z","receivedAt":"2010-04-07T01:01:09Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Avery Pennarun wrote:\n\n> I'm not sure why I'd want to know accurately\n> whether or not my file matches the index, but then have gitk fail to\n> tell me about *what* doesn't match.\n\nAh, yes, I am not thinking very well.  Please forget what I said before.\n\nv1.5.3-rc5~10 (git-diff: squelch \"empty\" diffs, 2007-08-03) has the\nanswer for your question.  I am not sure I agree with its conclusion.\n\nv1.5.3~8 (git-diff: resurrect the traditional empty \"diff --git\"\nbehaviour, 2007-08-31) tweaked the porcelain more by dropping the\nwarning message and adding the diff.autorefreshindex variable.\n\nWhat is missing is a --refresh-index option for diff-files and\ndiff-index, to make it easy for porcelain, especially long-running\nporcelain like gitk.\n\nJonathan\n"},{"id":"138794","messageId":"x2kc6c947f61004061807yf6d63879s5d37bd735b39ded8@mail.gmail.com","threadId":"23363","inReplyTo":"20100407004353.GA11346@progeny.tock","subject":"Re: [PATCH/RFC] gitk: refresh index before checking for local changes","fromName":"Alexander Gladysh","fromEmail":"agladysh@gmail.com","sentAt":"2010-04-07T01:07:23Z","receivedAt":"2010-04-07T01:07:23Z","isPatch":true,"sender":{"key":"agladysh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38239?v=4"},"body":"On Wed, Apr 7, 2010 at 04:43, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Most git porcelain silently refreshes stat-dirty index entries.  Teach\n> gitk to, too; this will make the behavior easier to understand when a\n> person makes a change to a file and then changes mind and restores the\n> old version in her editor of choice.\n\nThis patch fixes my problem, thank you!\n\nAlexander.\n"},{"id":"138795","messageId":"20100407011647.GA24187@progeny.tock","threadId":"23363","inReplyTo":"20100407004353.GA11346@progeny.tock","subject":"Re: [PATCH/RFC] gitk: refresh index before checking for local changes","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-07T01:16:48Z","receivedAt":"2010-04-07T01:16:48Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nI’ll start off the reviewing, too:\n\nJonathan Nieder wrote:\n\n> +    if {![eof $fd]} {\n> +        return 1\n\nWhitespace damage.\n\n> @@ -7504,13 +7540,9 @@ proc getblobdiffs {ids} {\n>      global git_version\n>  \n>      set textconv {}\n> -    if {[package vcompare $git_version \"1.6.1\"] >= 0} {\n> -\tset textconv \"--textconv\"\n> -    }\n> +    set textconv \"--textconv\"\n>      set submodule {}\n> -    if {[package vcompare $git_version \"1.6.6\"] >= 0} {\n> -\tset submodule \"--submodule\"\n> -    }\n> +    set submodule \"--submodule\"\n>      set cmd [diffcmd $ids \"-p $textconv $submodule  -C --cc --no-commit-id -U$diffcontext\"]\n>      if {$ignorespace} {\n>  \tappend cmd \" -w\"\n\nWhat does this have to do with the topic at hand?  (Sorry, I lumped\nin a local fix that still needs to be generalized and submitted\nseparately.)\n\nSo this patch is in good enough shape to try out, but please ping me\nfor a new one before it is time to send something like it on to the\nmasses.\n\nSorry for the noise,\nJonathan\n"},{"id":"138796","messageId":"4BBBEC43.5000100@gmail.com","threadId":"23363","inReplyTo":"20100407004353.GA11346@progeny.tock","subject":"Re: [PATCH/RFC] gitk: refresh index before checking for local changes","fromName":"A Large Angry SCM","fromEmail":"gitzilla@gmail.com","sentAt":"2010-04-07T02:21:55Z","receivedAt":"2010-04-07T02:21:55Z","isPatch":true,"sender":{"key":"gitzilla@gmail.com","avatar":"https://gravatar.com/avatar/354625c442439908ff3dd99757dee330e29e9df7847472384faf7a00add247fb?d=mp&s=160"},"body":"Jonathan Nieder wrote:\n> Most git porcelain silently refreshes stat-dirty index entries.  Teach\n> gitk to, too; this will make the behavior easier to understand when a\n> person makes a change to a file and then changes mind and restores the\n> old version in her editor of choice.\n> \n> This patch does not change the ‘checkout’ code path, since it is\n> assumed that the index is already being cleaned in that case.\n> \n> Testing is needed to check if this breaks operation with read-only\n> access to a repository.\n> \n\nNAK - gitk should not modify a repository and/or working dir unless \n_explicitly_ prompted to by the user.\n\nIf you want a new _non-default_ option setting for gitk, that fine also.\n"},{"id":"138797","messageId":"20100407025706.GA6528@progeny.tock","threadId":"23363","inReplyTo":"4BBBEC43.5000100@gmail.com","subject":"Re: [PATCH/RFC] gitk: refresh index before checking for local changes","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-07T02:57:06Z","receivedAt":"2010-04-07T02:57:06Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"A Large Angry SCM wrote:\n\n[context: should gitk run ‘gitk update-index --refresh -q’ transparently?]\n\n> NAK - gitk should not modify a repository and/or working dir unless\n> _explicitly_ prompted to by the user.\n>\n> If you want a new _non-default_ option setting for gitk, that fine also.\n\nI have some sympathy for this point of view.\n\nDoes the same principle apply to ‘git diff’?  If not, why not?\n\nIt should be possible to suppress files with no changes from the\ndisplay without refreshing the index (or not to suppress them and to\nstill refresh the index, nor that matter; the issues are orthogonal),\nbut it would be nice to come up with a clear rationale first.\n\nJonathan\n"},{"id":"138804","messageId":"7vfx37g6f6.fsf@alter.siamese.dyndns.org","threadId":"23363","inReplyTo":"4BBBEC43.5000100@gmail.com","subject":"Re: [PATCH/RFC] gitk: refresh index before checking for local changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-07T05:47:41Z","receivedAt":"2010-04-07T05:47:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"A Large Angry SCM <gitzilla@gmail.com> writes:\n\n> NAK - gitk should not modify a repository and/or working dir unless\n> _explicitly_ prompted to by the user.\n\nI used to think that way, until I realized that gitk has operations like\n\"Tag this commit\" that does write into the repository.\n"},{"id":"138821","messageId":"4BBC6AD1.80202@gmail.com","threadId":"23363","inReplyTo":"7vfx37g6f6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] gitk: refresh index before checking for local changes","fromName":"A Large Angry SCM","fromEmail":"gitzilla@gmail.com","sentAt":"2010-04-07T11:21:53Z","receivedAt":"2010-04-07T11:21:53Z","isPatch":true,"sender":{"key":"gitzilla@gmail.com","avatar":"https://gravatar.com/avatar/354625c442439908ff3dd99757dee330e29e9df7847472384faf7a00add247fb?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> A Large Angry SCM <gitzilla@gmail.com> writes:\n> \n>> NAK - gitk should not modify a repository and/or working dir unless\n>> _explicitly_ prompted to by the user.\n> \n> I used to think that way, until I realized that gitk has operations like\n> \"Tag this commit\" that does write into the repository.\n> \n\nDoes that happen every time you run gitk or only when the gitk user \ninstructs gitk to?\n"},{"id":"138831","messageId":"x2w2cfc40321004070736q7ab306b7i6220ffd58f724cec@mail.gmail.com","threadId":"23363","inReplyTo":"4BBBEC43.5000100@gmail.com","subject":"Re: [PATCH/RFC] gitk: refresh index before checking for local changes","fromName":"Jon Seymour","fromEmail":"jon.seymour@gmail.com","sentAt":"2010-04-07T14:36:13Z","receivedAt":"2010-04-07T14:36:13Z","isPatch":true,"sender":{"key":"jon.seymour@gmail.com","avatar":"https://avatars.githubusercontent.com/u/207131?v=4"},"body":"On Wed, Apr 7, 2010 at 2:21 PM, A Large Angry SCM <gitzilla@gmail.com> wrote:\n>\n> NAK - gitk should not modify a repository and/or working dir unless\n> _explicitly_ prompted to by the user.\n>\n> If you want a new _non-default_ option setting for gitk, that fine also.\n\ngitk and git gui currently behave differently in this regard.\n\ngit gui updates the indexes view of the working tree on start, gitk does not.\n\ngitk's current behaviour is somewhat mysterious to the uninitiated -\n\nuser: \"what? I have local changes?\"\ntool: \"relax dear user what you see is dirty laundry. if you want to\nsee the actual state, start git gui then come back to me and refresh\nme when you are done\"\n\ngitk is effectively lying to the user about the state of their working\ntree. git gui does not.\n\nNeedless to say, I support this change.\n\njon.\n\n\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"138855","messageId":"r2l32541b131004070948o92575f5j4728764482e8a262@mail.gmail.com","threadId":"23363","inReplyTo":"7vfx37g6f6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] gitk: refresh index before checking for local changes","fromName":"Avery Pennarun","fromEmail":"apenwarr@gmail.com","sentAt":"2010-04-07T16:48:29Z","receivedAt":"2010-04-07T16:48:29Z","isPatch":true,"sender":{"key":"apenwarr@gmail.com","avatar":"https://avatars.githubusercontent.com/u/20592?v=4"},"body":"On Wed, Apr 7, 2010 at 1:47 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> A Large Angry SCM <gitzilla@gmail.com> writes:\n>> NAK - gitk should not modify a repository and/or working dir unless\n>> _explicitly_ prompted to by the user.\n>\n> I used to think that way, until I realized that gitk has operations like\n> \"Tag this commit\" that does write into the repository.\n\nAlso 'git status' does the same \"modify a repository\" operation as\nwe're currently discussing, so calling it modification is a bit of an\nexaggeration.\n\nAvery\n"}]}