{"thread":{"id":"12275","subject":"Be more verbose when checkout takes a long time","startedAt":"2008-02-23T21:36:08Z","lastAt":"2008-02-24T06:29:26Z","messageCount":7,"participants":["Linus Torvalds","Junio C Hamano","Daniel Barkalow"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"69713","messageId":"alpine.LFD.1.00.0802231323590.21332@woody.linux-foundation.org","threadId":"12275","inReplyTo":null,"subject":"Be more verbose when checkout takes a long time","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-02-23T21:36:08Z","receivedAt":"2008-02-23T21:36:08Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nSo I find it irritating when git thinks for a long time without telling me \nwhat's taking so long. And by \"long time\" I definitely mean less than two \nseconds, which is already way too long for me.\n\nThis hits me when doing a large pull and the checkout takes a long time, \nor when just switching to another branch that is old and again checkout \ntakes a while.\n\nNow, git read-tree already had support for the \"-v\" flag that does nice \nupdates about what's going on, but it was delayed by two seconds, and if \nthe thing had already done more than half by then it would be quiet even \nafter that, so in practice it meant that we migth be quiet for up to four \nseconds. Much too long.\n\nSo this patch changes the timeout to just one second, which makes it much \nmore palatable to me.\n\nThe other thing this patch does is that \"git checkout\" now doesn't disable \nthe \"-v\" flag when doing its thing, and only disables the output when \ngiven the -q flag. Quite frankly, I'm not really sure why it disabled \nerror messages in the first place: it used to do\n\n\tmerge_error=$(git read-tree .. 2>&1) || (\n\t\tcase \"$merge\" in\n\t\t'')\n\t\t\techo >&2 \"$merge_error\"\n\t\t\texit 1 ;;\n\t\t...\n\nwhich obviously meant that the \"-v\" flag was useless, because it was \nsuppressed by the fact that any outpu just went to \"merge_error\" and then \nprinted just once if we didn't do a merge.\n\nNow, I'm sure this had a good reason (for the \"git checkout -m\" case), but \nit did make the common case of git-checkout really annoying. So I just \nremoved that whole \"suppress error messages from git-read-tree\" thing. \nPeople who use -m all the time probably disagree with this patch. I dunno.\n\nAnyway, with this I no longer get that annoying pregnant pause when doing \nbig branch switches.\n\nComments?\n\n\t\tLinus\n\n---\n git-checkout.sh |    3 +--\n unpack-trees.c  |    2 +-\n 2 files changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/git-checkout.sh b/git-checkout.sh\nindex bd74d70..4b07fc4 100755\n--- a/git-checkout.sh\n+++ b/git-checkout.sh\n@@ -210,10 +210,9 @@ then\n     git read-tree $v --reset -u $new\n else\n     git update-index --refresh >/dev/null\n-    merge_error=$(git read-tree -m -u --exclude-per-directory=.gitignore $old $new 2>&1) || (\n+    git read-tree $v -m -u --exclude-per-directory=.gitignore $old $new || (\n \tcase \"$merge\" in\n \t'')\n-\t\techo >&2 \"$merge_error\"\n \t\texit 1 ;;\n \tesac\n \ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex ec558f9..0f62609 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -301,7 +301,7 @@ static void check_updates(struct cache_entry **src, int nr,\n \t\t}\n \n \t\tprogress = start_progress_delay(\"Checking out files\",\n-\t\t\t\t\t\ttotal, 50, 2);\n+\t\t\t\t\t\ttotal, 50, 1);\n \t\tcnt = 0;\n \t}\n \n"},{"id":"69720","messageId":"7v8x1b1fiu.fsf@gitster.siamese.dyndns.org","threadId":"12275","inReplyTo":"alpine.LFD.1.00.0802231323590.21332@woody.linux-foundation.org","subject":"Re: Be more verbose when checkout takes a long time","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-23T22:20:25Z","receivedAt":"2008-02-23T22:20:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> So I find it irritating when git thinks for a long time without telling me \n> what's taking so long. And by \"long time\" I definitely mean less than two \n> seconds, which is already way too long for me.\n\nDo you mean more than two or less than two?\n\n> Now, git read-tree already had support for the \"-v\" flag that does nice \n> updates about what's going on, but it was delayed by two seconds, and if \n> the thing had already done more than half by then it would be quiet even \n> after that, so in practice it meant that we migth be quiet for up to four \n> seconds. Much too long.\n\nGeez you are impatient ;-).\n\nThe other user of start_progress_delay uses 95% as cutoff.  and\nprobably 50% was too low, but that may just be bikeshedding.\n\n> ... Quite frankly, I'm not really sure why it disabled \n> error messages in the first place: ...\n> ...\n> Now, I'm sure this had a good reason (for the \"git checkout -m\" case), but \n> it did make the common case of git-checkout really annoying.\n\nI agree.  Perhaps we can add some message when \"-m\" codepath\nfalls back to the three-way merge to make \"merge-error\" less\nscary.  Perhaps like:\n\n git-checkout.sh |   11 +++++++----\n 1 files changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/git-checkout.sh b/git-checkout.sh\nindex bd74d70..5e36136 100755\n--- a/git-checkout.sh\n+++ b/git-checkout.sh\n@@ -210,11 +210,14 @@ then\n     git read-tree $v --reset -u $new\n else\n     git update-index --refresh >/dev/null\n-    merge_error=$(git read-tree -m -u --exclude-per-directory=.gitignore $old $new 2>&1) || (\n-\tcase \"$merge\" in\n-\t'')\n-\t\techo >&2 \"$merge_error\"\n+    git read-tree -$v m -u --exclude-per-directory=.gitignore $old $new || (\n+\tcase \"$merge,$v\" in\n+\t,*)\n \t\texit 1 ;;\n+\t1,)\n+\t\t;; # quiet\n+\t*)\n+\t\techo >&2 \"Falling back to 3-way merge...\" ;;\n \tesac\n \n \t# Match the index to the working tree, and do a three-way.\n"},{"id":"69723","messageId":"alpine.LFD.1.00.0802231430100.21332@woody.linux-foundation.org","threadId":"12275","inReplyTo":"7v8x1b1fiu.fsf@gitster.siamese.dyndns.org","subject":"Re: Be more verbose when checkout takes a long time","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-02-23T22:32:35Z","receivedAt":"2008-02-23T22:32:35Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 23 Feb 2008, Junio C Hamano wrote:\n\n> Linus Torvalds <torvalds@linux-foundation.org> writes:\n> \n> > So I find it irritating when git thinks for a long time without telling me \n> > what's taking so long. And by \"long time\" I definitely mean less than two \n> > seconds, which is already way too long for me.\n> \n> Do you mean more than two or less than two?\n\nI mean that \"long time\" starts at a point that is less than two seconds.\n\nAnything over a second is a long time for me.\n\n> Geez you are impatient ;-).\n\nI like to call it \"discerning in my time usage\".\n\n> The other user of start_progress_delay uses 95% as cutoff.  and\n> probably 50% was too low, but that may just be bikeshedding.\n\nI did think that 50% was a bit low, and considered upping it to 75, but \nwith the one-second thing it wasn't as much of a deal any more. \n\n> I agree.  Perhaps we can add some message when \"-m\" codepath\n> falls back to the three-way merge to make \"merge-error\" less\n> scary.  Perhaps like:\n\nSounds sane to me.\n\n\t\tLinus\n"},{"id":"69725","messageId":"7vzltrz4cl.fsf@gitster.siamese.dyndns.org","threadId":"12275","inReplyTo":"alpine.LFD.1.00.0802231430100.21332@woody.linux-foundation.org","subject":"Re: Be more verbose when checkout takes a long time","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-23T22:37:46Z","receivedAt":"2008-02-23T22:37:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Sat, 23 Feb 2008, Junio C Hamano wrote:\n>\n>> Linus Torvalds <torvalds@linux-foundation.org> writes:\n>> \n>> > So I find it irritating when git thinks for a long time without telling me \n>> > what's taking so long. And by \"long time\" I definitely mean less than two \n>> > seconds, which is already way too long for me.\n>> \n>> Do you mean more than two or less than two?\n>\n> I mean that \"long time\" starts at a point that is less than two seconds.\n>\n> Anything over a second is a long time for me.\n>\n>> Geez you are impatient ;-).\n>\n> I like to call it \"discerning in my time usage\".\n>\n>> The other user of start_progress_delay uses 95% as cutoff.  and\n>> probably 50% was too low, but that may just be bikeshedding.\n>\n> I did think that 50% was a bit low, and considered upping it to 75, but \n> with the one-second thing it wasn't as much of a deal any more. \n>\n>> I agree.  Perhaps we can add some message when \"-m\" codepath\n>> falls back to the three-way merge to make \"merge-error\" less\n>> scary.  Perhaps like:\n>\n> Sounds sane to me.\n\nOk, then.\n\nUnfortunately it will be short-lived on 'master' as I have been\nplanning to merge Daniel's rewrite soon ;-)\n"},{"id":"69740","messageId":"7vve4fxmsx.fsf@gitster.siamese.dyndns.org","threadId":"12275","inReplyTo":"7vzltrz4cl.fsf@gitster.siamese.dyndns.org","subject":"Re: Be more verbose when checkout takes a long time","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-23T23:42:06Z","receivedAt":"2008-02-23T23:42:06Z","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> Linus Torvalds <torvalds@linux-foundation.org> writes:\n> ...\n>> Sounds sane to me.\n>\n> Ok, then.\n>\n> Unfortunately it will be short-lived on 'master' as I have been\n> planning to merge Daniel's rewrite soon ;-)\n\nSo here it is ;-)\n\nThanks to the unpack_trees_options.gently flag, this does not\nhave to say \"Falling back to...\" because the first \"attempt to\nswitch normally\" would silently fail.\n\n---\n\n builtin-checkout.c |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-checkout.c b/builtin-checkout.c\nindex 5f176c6..283831e 100644\n--- a/builtin-checkout.c\n+++ b/builtin-checkout.c\n@@ -232,6 +232,7 @@ static int merge_working_tree(struct checkout_opts *opts,\n \t\t\ttopts.update = 1;\n \t\t\ttopts.merge = 1;\n \t\t\ttopts.gently = opts->merge;\n+\t\t\ttopts.verbose_update = !opts->quiet;\n \t\t\ttopts.fn = twoway_merge;\n \t\t\ttopts.dir = xcalloc(1, sizeof(*topts.dir));\n \t\t\ttopts.dir->show_ignored = 1;\n"},{"id":"69741","messageId":"7voda7xmnk.fsf_-_@gitster.siamese.dyndns.org","threadId":"12275","inReplyTo":"7vzltrz4cl.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] checkout: error out when index is unmerged even with -m","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-02-23T23:45:19Z","receivedAt":"2008-02-23T23:45:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Even when -m is given to allow fallilng back to 3-way merge\nwhile switching branches, we should refuse if the original index\nis unmerged.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * I think this bug was inherited from the scripted version.\n   Fixing it is much easier here.\n\n builtin-checkout.c |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-checkout.c b/builtin-checkout.c\nindex 283831e..e028270 100644\n--- a/builtin-checkout.c\n+++ b/builtin-checkout.c\n@@ -226,8 +226,8 @@ static int merge_working_tree(struct checkout_opts *opts,\n \t\trefresh_cache(REFRESH_QUIET);\n \n \t\tif (unmerged_cache()) {\n-\t\t\tret = opts->merge ? -1 :\n-\t\t\t\terror(\"you need to resolve your current index first\");\n+\t\t\terror(\"you need to resolve your current index first\");\n+\t\t\treturn 1;\n \t\t} else {\n \t\t\ttopts.update = 1;\n \t\t\ttopts.merge = 1;\n"},{"id":"69761","messageId":"alpine.LNX.1.00.0802240121170.19024@iabervon.org","threadId":"12275","inReplyTo":"7voda7xmnk.fsf_-_@gitster.siamese.dyndns.org","subject":"Re: [PATCH] checkout: error out when index is unmerged even with -m","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-02-24T06:29:26Z","receivedAt":"2008-02-24T06:29:26Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Sat, 23 Feb 2008, Junio C Hamano wrote:\n\n> Even when -m is given to allow fallilng back to 3-way merge\n> while switching branches, we should refuse if the original index\n> is unmerged.\n\nAcked-by: Daniel Barkalow <barkalow@iabervon.org>\n\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n> \n>  * I think this bug was inherited from the scripted version.\n>    Fixing it is much easier here.\n\nI'm pretty sure it was. I wasn't clear on all the motivations for checkout \n-m behavior, so I tried to make it exactly the same.\n\n>  builtin-checkout.c |    4 ++--\n>  1 files changed, 2 insertions(+), 2 deletions(-)\n> \n> diff --git a/builtin-checkout.c b/builtin-checkout.c\n> index 283831e..e028270 100644\n> --- a/builtin-checkout.c\n> +++ b/builtin-checkout.c\n> @@ -226,8 +226,8 @@ static int merge_working_tree(struct checkout_opts *opts,\n>  \t\trefresh_cache(REFRESH_QUIET);\n>  \n>  \t\tif (unmerged_cache()) {\n> -\t\t\tret = opts->merge ? -1 :\n> -\t\t\t\terror(\"you need to resolve your current index first\");\n> +\t\t\terror(\"you need to resolve your current index first\");\n> +\t\t\treturn 1;\n>  \t\t} else {\n>  \t\t\ttopts.update = 1;\n>  \t\t\ttopts.merge = 1;\n\nDitch the \"else\" now that it's not needed? And, actually, I think you can \nditch most of the use of \"ret\" when there's only one case for getting to \nneeding a real merge.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"}]}