{"thread":{"id":"30456","subject":"[PATCH/RFC] clone: inform the user we are checking out","startedAt":"2012-05-07T09:09:47Z","lastAt":"2012-05-07T19:03:46Z","messageCount":7,"participants":["Erik Faye-Lund","Johannes Sixt","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"190971","messageId":"1336381787-6484-1-git-send-email-kusmabite@gmail.com","threadId":"30456","inReplyTo":null,"subject":"[PATCH/RFC] clone: inform the user we are checking out","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-05-07T09:09:47Z","receivedAt":"2012-05-07T09:09:47Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"When cloning a large repository over a local file-system, git\ncan use hard-links to the old repository files, making the\nrepository-cloning very fast. However, git also perform an\nimplicit checkout of the files, which can be a lengthy\noperation.\n\nInform the user about this, and move the logic to notify the\nuser that we are done until we actually are.\n\nSigned-off-by: Erik Faye-Lund <kusmabite@gmail.com>\n---\n\nI just tried to clone a very large (proprietary) repository over\na local filesystyem, and the output struck me as confusing:\n\n---8<---\n$ git clone some-repo.git some-other-repo\nCloning into 'some-other-repo'...    <happens instantly>\ndone.                                <hangs for minutes>\n$\n---8<---\n\nNow, seems to be because repo gets hard-linked, so the cloning\npart did not actually take much time. However, we perform a\ncheck-out at the end, and this does take time for my large repo.\nSo the behavior probably makes sense from a git-internals point\nof view.\n\nBut from an end-user point of view, it's confusing. I asked git\nto clone, and it told me it finished, only to hang around for\nseveral minutes while, judging by the output, doing nothing.\n\nSo, perhaps it could make sense to do something along these lines?\n\nThis gives me this output instead:\n\n---8<---\n$ git clone some-repo.git some-other-repo\nCloning into 'some-repo'...    <happens instantly>\nChecking out HEAD...           <hangs for minutes>\ndone.\n$\n---8<---\n\n...which seems much more informative to me.\n\n builtin/clone.c | 10 +++++++---\n 1 file changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex bbd5c96..3f863a1 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -368,9 +368,6 @@ static void clone_local(const char *src_repo, const char *dest_repo)\n \t\tstrbuf_release(&src);\n \t\tstrbuf_release(&dest);\n \t}\n-\n-\tif (0 <= option_verbosity)\n-\t\tprintf(_(\"done.\\n\"));\n }\n \n static const char *junk_work_tree;\n@@ -544,6 +541,9 @@ static int checkout(void)\n \tif (option_no_checkout)\n \t\treturn 0;\n \n+\tif (0 <= option_verbosity)\n+\t\tprintf(_(\"Checking out HEAD...\\n\"));\n+\n \thead = resolve_refdup(\"HEAD\", sha1, 1, NULL);\n \tif (!head) {\n \t\twarning(_(\"remote HEAD refers to nonexistent ref, \"\n@@ -870,5 +870,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \tstrbuf_release(&key);\n \tstrbuf_release(&value);\n \tjunk_pid = 0;\n+\n+\tif (is_local && 0 <= option_verbosity)\n+\t\tprintf(_(\"done.\\n\"));\n+\n \treturn err;\n }\n-- \n1.7.10.1.457.g8275905\n"},{"id":"190974","messageId":"4FA7952C.90702@viscovery.net","threadId":"30456","inReplyTo":"1336381787-6484-1-git-send-email-kusmabite@gmail.com","subject":"Re: [PATCH/RFC] clone: inform the user we are checking out","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2012-05-07T09:26:04Z","receivedAt":"2012-05-07T09:26:04Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 5/7/2012 11:09, schrieb Erik Faye-Lund:\n> When cloning a large repository over a local file-system\n...\n> $ git clone some-repo.git some-other-repo\n> Cloning into 'some-other-repo'...    <happens instantly>\n> done.                                <hangs for minutes>\n> $\n...\n> I asked git\n> to clone, and it told me it finished, only to hang around for\n> several minutes while, judging by the output, doing nothing.\n\nWe have a nice \"Checking out files\" progress indicator. I wonder why you\ndo not see it.\n\nAt any rate, it's better to write \"done\" only after we're really done.\n\n-- Hannes\n"},{"id":"190975","messageId":"CABPQNSZ+FtMJ1msSmqvROP6YqRGKvz1uz8w1z5mF3YDXZyiPQw@mail.gmail.com","threadId":"30456","inReplyTo":"4FA7952C.90702@viscovery.net","subject":"Re: [PATCH/RFC] clone: inform the user we are checking out","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-05-07T09:35:08Z","receivedAt":"2012-05-07T09:35:08Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, May 7, 2012 at 11:26 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:\n> Am 5/7/2012 11:09, schrieb Erik Faye-Lund:\n>> When cloning a large repository over a local file-system\n> ...\n>> $ git clone some-repo.git some-other-repo\n>> Cloning into 'some-other-repo'...    <happens instantly>\n>> done.                                <hangs for minutes>\n>> $\n> ...\n>> I asked git\n>> to clone, and it told me it finished, only to hang around for\n>> several minutes while, judging by the output, doing nothing.\n>\n> We have a nice \"Checking out files\" progress indicator. I wonder why you\n> do not see it.\n>\n\nThat's a very good point, I forgot about that one. I'll investigate\nthat part. Just to be clear, I'm seeing this problem on version\n1.7.10.1.457.g8275905 (junio's 'master' as of right now, AFAICT) on\nCentOS. I've built with:\nNO_OPENSSL = YesPlease\nNO_CURL = YesPlease\n\n\"git config -l\" does not reveal that any progress related\nconfig-options are set.\n\n> At any rate, it's better to write \"done\" only after we're really done.\n\nWell, perhaps, but at the same time perhaps not: We only write it for\nlocal clones, because the remote-clones are quite chatty to begin\nwith, and the remote-stuff seems to inform about it's actions by\nprinting \"<performing task x>...done!\". Moving the final\n\"done.\"-printing to the very end might be a step in making these\noutputs even less consistent. But it might not be important.\n"},{"id":"190978","messageId":"CABPQNSbbH3ULjgYxW6RdH0emrgXoKf1mkiEAm2fR19LsC5q8_w@mail.gmail.com","threadId":"30456","inReplyTo":"CABPQNSZ+FtMJ1msSmqvROP6YqRGKvz1uz8w1z5mF3YDXZyiPQw@mail.gmail.com","subject":"Re: [PATCH/RFC] clone: inform the user we are checking out","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-05-07T09:46:53Z","receivedAt":"2012-05-07T09:46:53Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, May 7, 2012 at 11:35 AM, Erik Faye-Lund <kusmabite@gmail.com> wrote:\n> On Mon, May 7, 2012 at 11:26 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:\n>> Am 5/7/2012 11:09, schrieb Erik Faye-Lund:\n>>> When cloning a large repository over a local file-system\n>> ...\n>>> $ git clone some-repo.git some-other-repo\n>>> Cloning into 'some-other-repo'...    <happens instantly>\n>>> done.                                <hangs for minutes>\n>>> $\n>> ...\n>>> I asked git\n>>> to clone, and it told me it finished, only to hang around for\n>>> several minutes while, judging by the output, doing nothing.\n>>\n>> We have a nice \"Checking out files\" progress indicator. I wonder why you\n>> do not see it.\n>>\n>\n> That's a very good point, I forgot about that one. I'll investigate\n> that part. Just to be clear, I'm seeing this problem on version\n> 1.7.10.1.457.g8275905 (junio's 'master' as of right now, AFAICT) on\n> CentOS. I've built with:\n> NO_OPENSSL = YesPlease\n> NO_CURL = YesPlease\n>\n> \"git config -l\" does not reveal that any progress related\n> config-options are set.\n>\n\nIt seems progress is only enabled when cloning with the verbose-flag.\nThis is because check_updates in unpack-trees.c only enabled progress\nif both o->update and o->verbose_update is set, and checkout in\nbuiltin/clone.c fills in opts.verbose_update with (option_verbosity >\n0).\n\nSo that's that mystery. I still think the output without the\nverbose-flag is confusing as-is.\n"},{"id":"190981","messageId":"CABPQNSYe6dDf20D2pQpF=K97rZnWxqe5weEdO+FdKck9rwwzWg@mail.gmail.com","threadId":"30456","inReplyTo":"CABPQNSbbH3ULjgYxW6RdH0emrgXoKf1mkiEAm2fR19LsC5q8_w@mail.gmail.com","subject":"Re: [PATCH/RFC] clone: inform the user we are checking out","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-05-07T10:02:46Z","receivedAt":"2012-05-07T10:02:46Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Mon, May 7, 2012 at 11:46 AM, Erik Faye-Lund <kusmabite@gmail.com> wrote:\n> On Mon, May 7, 2012 at 11:35 AM, Erik Faye-Lund <kusmabite@gmail.com> wrote:\n>> On Mon, May 7, 2012 at 11:26 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:\n>>> Am 5/7/2012 11:09, schrieb Erik Faye-Lund:\n>>>> I asked git\n>>>> to clone, and it told me it finished, only to hang around for\n>>>> several minutes while, judging by the output, doing nothing.\n>>>\n>>> We have a nice \"Checking out files\" progress indicator. I wonder why you\n>>> do not see it.\n>\n> It seems progress is only enabled when cloning with the verbose-flag.\n> This is because check_updates in unpack-trees.c only enabled progress\n> if both o->update and o->verbose_update is set, and checkout in\n> builtin/clone.c fills in opts.verbose_update with (option_verbosity >\n> 0).\n>\n> So that's that mystery. I still think the output without the\n> verbose-flag is confusing as-is.\n\nOK, some blaming shows that this changed in 5bd631b3 (\"clone: support\nmultiple levels of verbosity\"), back in February 2010. Before this\npatch, one would have to specify the \"quiet\"-flag to clone to suppress\nprogress-output, after the patch the default is progress being off.\n\nThis seems like the right thing to do if we want to resurrect the\nprogress-output's default-on behavior:\n---8<---\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 3f863a1..f48e603 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -569,7 +569,7 @@ static int checkout(void)\n \topts.update = 1;\n \topts.merge = 1;\n \topts.fn = oneway_merge;\n-\topts.verbose_update = (option_verbosity > 0);\n+\topts.verbose_update = (option_verbosity >= 0);\n \topts.src_index = &the_index;\n \topts.dst_index = &the_index;\n\n---8<---\n"},{"id":"191000","messageId":"20120507135446.GB4860@sigill.intra.peff.net","threadId":"30456","inReplyTo":"CABPQNSYe6dDf20D2pQpF=K97rZnWxqe5weEdO+FdKck9rwwzWg@mail.gmail.com","subject":"Re: [PATCH/RFC] clone: inform the user we are checking out","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-07T13:54:46Z","receivedAt":"2012-05-07T13:54:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 07, 2012 at 12:02:46PM +0200, Erik Faye-Lund wrote:\n\n> OK, some blaming shows that this changed in 5bd631b3 (\"clone: support\n> multiple levels of verbosity\"), back in February 2010. Before this\n> patch, one would have to specify the \"quiet\"-flag to clone to suppress\n> progress-output, after the patch the default is progress being off.\n\nYeah, I think this is a bug. We already show progress indicators from\nthe transports without \"-v\", so there is no reason to require it for the\ncheckout progress meter. And reading fbd631b3, it looks like the change\nis simply a mistake in the patch.\n\n> This seems like the right thing to do if we want to resurrect the\n> progress-output's default-on behavior:\n> ---8<---\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index 3f863a1..f48e603 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -569,7 +569,7 @@ static int checkout(void)\n>  \topts.update = 1;\n>  \topts.merge = 1;\n>  \topts.fn = oneway_merge;\n> -\topts.verbose_update = (option_verbosity > 0);\n> +\topts.verbose_update = (option_verbosity >= 0);\n>  \topts.src_index = &the_index;\n>  \topts.dst_index = &the_index;\n> \n> ---8<---\n\nYes, that looks correct to me. Thanks for looking into this.\n\n-Peff\n"},{"id":"191025","messageId":"7v397b6cfh.fsf@alter.siamese.dyndns.org","threadId":"30456","inReplyTo":"CABPQNSYe6dDf20D2pQpF=K97rZnWxqe5weEdO+FdKck9rwwzWg@mail.gmail.com","subject":"Re: [PATCH/RFC] clone: inform the user we are checking out","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-07T19:03:46Z","receivedAt":"2012-05-07T19:03:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund <kusmabite@gmail.com> writes:\n\n> OK, some blaming shows that this changed in 5bd631b3 (\"clone: support\n> multiple levels of verbosity\"), back in February 2010. Before this\n> patch, one would have to specify the \"quiet\"-flag to clone to suppress\n> progress-output, after the patch the default is progress being off.\n>\n> This seems like the right thing to do if we want to resurrect the\n> progress-output's default-on behavior:\n> ---8<---\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index 3f863a1..f48e603 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -569,7 +569,7 @@ static int checkout(void)\n>  \topts.update = 1;\n>  \topts.merge = 1;\n>  \topts.fn = oneway_merge;\n> -\topts.verbose_update = (option_verbosity > 0);\n> +\topts.verbose_update = (option_verbosity >= 0);\n>  \topts.src_index = &the_index;\n>  \topts.dst_index = &the_index;\n>\n> ---8<---\n\nSounds sensible, as the original said \"verbosely update unless we were\ntold to be quiet\", so the normal case should be verbose_update which will\ngive progress only if it takes too long.\n\nCare to roll a signed-off patch?\n"}]}