{"thread":{"id":"19639","subject":"[Patch] Prevent cloning over http from spewing","startedAt":"2009-06-02T17:42:29Z","lastAt":"2009-07-20T15:24:04Z","messageCount":23,"participants":["sparse@infidigm.net","Erik Faye-Lund","Jakub Narebski","Junio C Hamano","Jeff King","Shawn O. Pearce","Tay Ray Chuan"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"115263","messageId":"20090602174229.GA14455@infidigm.net","threadId":"19639","inReplyTo":null,"subject":"[Patch] Prevent cloning over http from spewing","fromName":"","fromEmail":"sparse@infidigm.net","sentAt":"2009-06-02T17:42:29Z","receivedAt":"2009-06-02T17:42:29Z","isPatch":true,"sender":{"key":"sparse@infidigm.net","avatar":null},"body":"When cloning over http git spews a bunch of hashs that don't really convey much.\nThe attached patch disables them unless --verbose is specified.\n\n\ncommit 67be7a94c94100d24bcccf1248a26afc1dec8d05\nAuthor: Jeff Muizelaar <jmuizelaar@mozilla.com>\nDate:   Tue Jun 2 13:32:41 2009 -0400\n\n    Prevent cloning over http from spewing\n    \n    This propogates the verbose flag to the transport and makes\n    the walker verbose only if the transport is verbose.\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex 5c46496..f25d60b 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -502,8 +502,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \t\tif (option_quiet)\n \t\t\ttransport->verbose = -1;\n-\t\telse if (option_verbose)\n+\t\telse if (option_verbose) {\n \t\t\ttransport->progress = 1;\n+\t\t\ttransport->verbose = 1;\n+\t\t}\n \n \t\tif (option_upload_pack)\n \t\t\ttransport_set_option(transport, TRANS_OPT_UPLOADPACK,\ndiff --git a/transport.c b/transport.c\nindex 17891d5..3bb87f3 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -366,7 +366,7 @@ static int fetch_objs_via_walker(struct transport *transport,\n \twalker->get_all = 1;\n \twalker->get_tree = 1;\n \twalker->get_history = 1;\n-\twalker->get_verbosely = transport->verbose >= 0;\n+\twalker->get_verbosely = transport->verbose > 0;\n \twalker->get_recover = 0;\n \n \tfor (i = 0; i < nr_objs; i++)\n"},{"id":"115329","messageId":"40aa078e0906030321u11ec3bbag47e66ca23ec94e09@mail.gmail.com","threadId":"19639","inReplyTo":"20090602174229.GA14455@infidigm.net","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-06-03T10:21:07Z","receivedAt":"2009-06-03T10:21:07Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Jun 2, 2009 at 7:42 PM,  <sparse@infidigm.net> wrote:\n> When cloning over http git spews a bunch of hashs that don't really convey much.\n> The attached patch disables them unless --verbose is specified.\n>\n\nWe prefer patches inline on this mailing-list. See\nDocumentation/SubmittingPatches, \"(3) Sending your patches.\" for more\ndetails.\n\n-- \nErik \"kusma\" Faye-Lund\nkusmabite@gmail.com\n(+47) 986 59 656\n"},{"id":"115330","messageId":"m3vdnda9f7.fsf@localhost.localdomain","threadId":"19639","inReplyTo":"20090602174229.GA14455@infidigm.net","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-06-03T10:39:11Z","receivedAt":"2009-06-03T10:39:11Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"sparse@infidigm.net writes:\n\n> When cloning over http git spews a bunch of hashs that don't really convey much.\n> The attached patch disables them unless --verbose is specified.\n> \n\nPlease follow guidelines stated in Documentation/SubmittingPatches:\n * it is preferred to have patch inline, rather than as attachement\n * put comments between \"---\" and the diffstat\n\nAlso please do the following:\n * use git-format-patch output rather than that of git-show to\n   generate patch to send.\n\nAlso \"Prevent cloning over http from spewing\" is not, in my opinion,\ngood description; \"Make cloning over http non-verbose by default\"\nwould be better.\n\nFinally a question: if we turn off verbose output, do we have any kind\nof progress info fot git-clone over http?\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"115359","messageId":"7vmy8p8947.fsf@alter.siamese.dyndns.org","threadId":"19639","inReplyTo":"m3vdnda9f7.fsf@localhost.localdomain","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-03T18:28:40Z","receivedAt":"2009-06-03T18:28:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> Finally a question: if we turn off verbose output, do we have any kind\n> of progress info fot git-clone over http?\n\nThis was discussed before.\n\nFor example, Shawn wrote in September 2007 in $gmane/59433\n\n    ... Few people really grok the pack-objects/index-pack output, nor do they\n    care.  They just want to know two things:\n\n      - Is git working or frozen in limbo?\n      - Is git frozen because my network sucks, or git sucks?\n      - When will this be done?  Please $DIETY, I want to go home!\n        Make the fetch finish!\n\n    The C based git-fetch made the http protocol almost totally silent.\n    (No more got/walk messages.)  But that's actually also bad as it\n    now doesn't even let you know its transferring anything at all.\n\nYou _could_ do something like this patch.  Instead of showing \"walk %s\"\nand \"got %s\" lines, with occasional \"Getting pack %s\\n which contains %s\",\nit shows and recycles a single line that shows the number of packs, walk\nactions and got actions.\n\nThis is a toy patch; it hiccups for too long while getting each pack, and\nit does not cleanly restore the display after it finishes, but I'll leave\nit to interested readers as an exercise to properly do this using the\nprogress API.\n\nCurrently many http related patches are in flight, so it may not be the\nbest time to do so, though.\n\n http-walker.c |   11 +++--------\n walker.c      |   21 +++++++++++++++++----\n walker.h      |   10 +++++++++-\n 3 files changed, 29 insertions(+), 13 deletions(-)\n\ndiff --git a/http-walker.c b/http-walker.c\nindex 7321ccc..3303bc1 100644\n--- a/http-walker.c\n+++ b/http-walker.c\n@@ -256,7 +256,7 @@ static void finish_object_request(struct object_request *obj_req)\n \t\tmove_temp_to_file(obj_req->tmpfile, obj_req->filename);\n \n \tif (obj_req->rename == 0)\n-\t\twalker_say(obj_req->walker, \"got %s\\n\", sha1_to_hex(obj_req->sha1));\n+\t\twalker_progress(obj_req->walker, WALKER_GOT);\n }\n \n static void process_object_response(void *callback_data)\n@@ -733,12 +733,7 @@ static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned cha\n \tif (!target)\n \t\treturn -1;\n \n-\tif (walker->get_verbosely) {\n-\t\tfprintf(stderr, \"Getting pack %s\\n\",\n-\t\t\tsha1_to_hex(target->sha1));\n-\t\tfprintf(stderr, \" which contains %s\\n\",\n-\t\t\tsha1_to_hex(sha1));\n-\t}\n+\twalker_progress(walker, WALKER_PACK);\n \n \turl = xmalloc(strlen(repo->base) + 65);\n \tsprintf(url, \"%s/objects/pack/pack-%s.pack\",\n@@ -907,7 +902,7 @@ struct walker *get_http_walker(const char *url, struct remote *remote)\n {\n \tchar *s;\n \tstruct walker_data *data = xmalloc(sizeof(struct walker_data));\n-\tstruct walker *walker = xmalloc(sizeof(struct walker));\n+\tstruct walker *walker = xcalloc(1, sizeof(struct walker));\n \n \thttp_init(remote);\n \ndiff --git a/walker.c b/walker.c\nindex e57630e..aaf3e2c 100644\n--- a/walker.c\n+++ b/walker.c\n@@ -9,10 +9,23 @@\n \n static unsigned char current_commit_sha1[20];\n \n-void walker_say(struct walker *walker, const char *fmt, const char *hex)\n+void walker_progress(struct walker *walker, enum walker_action action)\n {\n-\tif (walker->get_verbosely)\n-\t\tfprintf(stderr, fmt, hex);\n+\tswitch (action) {\n+\tdefault:\n+\t\tbreak;\n+\tcase WALKER_GOT:\n+\t\twalker->got++;\n+\t\tbreak;\n+\tcase WALKER_WALK:\n+\t\twalker->walked++;\n+\t\tbreak;\n+\tcase WALKER_PACK:\n+\t\twalker->got_pack++;\n+\t\tbreak;\n+\t}\n+\tfprintf(stderr, \"pack %-5u walk %-10lu got %-10lu\\r\",\n+\t\twalker->got_pack, walker->walked, walker->got);\n }\n \n static void report_missing(const struct object *obj)\n@@ -83,7 +96,7 @@ static int process_commit(struct walker *walker, struct commit *commit)\n \n \thashcpy(current_commit_sha1, commit->object.sha1);\n \n-\twalker_say(walker, \"walk %s\\n\", sha1_to_hex(commit->object.sha1));\n+\twalker_progress(walker, WALKER_WALK);\n \n \tif (walker->get_tree) {\n \t\tif (process(walker, &commit->tree->object))\ndiff --git a/walker.h b/walker.h\nindex 8a149e1..5e7c1f6 100644\n--- a/walker.h\n+++ b/walker.h\n@@ -9,6 +9,9 @@ struct walker {\n \tvoid (*prefetch)(struct walker *, unsigned char *sha1);\n \tint (*fetch)(struct walker *, unsigned char *sha1);\n \tvoid (*cleanup)(struct walker *);\n+\tunsigned long walked;\n+\tunsigned long got;\n+\tunsigned got_pack;\n \tint get_tree;\n \tint get_history;\n \tint get_all;\n@@ -19,7 +22,12 @@ struct walker {\n };\n \n /* Report what we got under get_verbosely */\n-void walker_say(struct walker *walker, const char *, const char *);\n+enum walker_action {\n+\tWALKER_GOT,\n+\tWALKER_WALK,\n+\tWALKER_PACK,\n+};\n+void walker_progress(struct walker *, enum walker_action);\n \n /* Load pull targets from stdin */\n int walker_targets_stdin(char ***target, const char ***write_ref);\n"},{"id":"115361","messageId":"20090603191050.GB29564@coredump.intra.peff.net","threadId":"19639","inReplyTo":"7vmy8p8947.fsf@alter.siamese.dyndns.org","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-06-03T19:10:50Z","receivedAt":"2009-06-03T19:10:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 03, 2009 at 11:28:40AM -0700, Junio C Hamano wrote:\n\n> You _could_ do something like this patch.  Instead of showing \"walk %s\"\n> and \"got %s\" lines, with occasional \"Getting pack %s\\n which contains %s\",\n> it shows and recycles a single line that shows the number of packs, walk\n> actions and got actions.\n> \n> This is a toy patch; it hiccups for too long while getting each pack, and\n> it does not cleanly restore the display after it finishes, but I'll leave\n> it to interested readers as an exercise to properly do this using the\n> progress API.\n\nThis is much better, and I had roughly the same thought when I saw the\noriginal \"stop http from spewing\" patch. The output is confusing, but we\nneed _some_ progress indicator.\n\nUnfortunately, I don't think it's possible to give a \"percent complete\"\nbecause we are fetching as we walk, meaning we don't know where the end\nis until we get there (but I might be wrong, as I have never looked too\nclosely at the http fetch code).\n\nSo I think the best we could do is probably:\n\n  1. summarize what we have fetched (N packs, N loose objects)\n  2. show what we are currently fetching (object or pack)\n  3. show the number of bytes retrieved for the current item\n  4. if the server provides content-length, show the percentage\n     completed for this object\n  5. show the current throughput\n\nThat should give the user some eye candy to show something is happening,\nand for a mostly packed repo, should come close to being an overall\nprogress indicator (since most of the time will be spent on one big\npack, that will be most of what the user sees, and its progress\nindicator will dominate the overall time).\n\nI suspect items 3-5 could mostly re-use the existing progress code, too.\n\n-Peff\n"},{"id":"115362","messageId":"20090603191555.GL3355@spearce.org","threadId":"19639","inReplyTo":"20090603191050.GB29564@coredump.intra.peff.net","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-06-03T19:15:55Z","receivedAt":"2009-06-03T19:15:55Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Jeff King <peff@peff.net> wrote:\n> On Wed, Jun 03, 2009 at 11:28:40AM -0700, Junio C Hamano wrote:\n> > This is a toy patch; it hiccups for too long while getting each pack, and\n> > it does not cleanly restore the display after it finishes, but I'll leave\n> > it to interested readers as an exercise to properly do this using the\n> > progress API.\n> \n> This is much better, and I had roughly the same thought when I saw the\n> original \"stop http from spewing\" patch. The output is confusing, but we\n> need _some_ progress indicator.\n> \n> Unfortunately, I don't think it's possible to give a \"percent complete\"\n> because we are fetching as we walk, meaning we don't know where the end\n> is until we get there (but I might be wrong, as I have never looked too\n> closely at the http fetch code).\n\nNo, you are right Peff, you can't give a \"percent complete\" because\nyou don't know how much you need to fetch.\n\nWhat we could do is try to organize the fetch queue by object type,\nget all commits, then all trees, then blobs.  The blobs are the\nbulk of the data, and by the time we hit them, we should be able\nto give some estimate on progress because we have all of the ones\nwe need to fetch in our fetch queue.  But its only a \"object count\"\nsort of thing, not a byte count.\n\nWhen fetching a .idx or a .pack though, we should be able to show\nprogress... assuming the server sent us a Content-Length header.\nIf not, in the case of a pack, we could still show receive speed\nlike index-pack does.\n \n>   1. summarize what we have fetched (N packs, N loose objects)\n>   2. show what we are currently fetching (object or pack)\n>   3. show the number of bytes retrieved for the current item\n>   4. if the server provides content-length, show the percentage\n>      completed for this object\n>   5. show the current throughput\n\nYea.  My idea of sorting by type is also a lot more work... and\nprobably doesn't buy us any better output than what you just said.\n:-)\n \n-- \nShawn.\n"},{"id":"115363","messageId":"20090603192420.GA29610@coredump.intra.peff.net","threadId":"19639","inReplyTo":"20090603191555.GL3355@spearce.org","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-06-03T19:24:20Z","receivedAt":"2009-06-03T19:24:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 03, 2009 at 12:15:55PM -0700, Shawn O. Pearce wrote:\n\n> What we could do is try to organize the fetch queue by object type,\n> get all commits, then all trees, then blobs.  The blobs are the\n> bulk of the data, and by the time we hit them, we should be able\n> to give some estimate on progress because we have all of the ones\n> we need to fetch in our fetch queue.  But its only a \"object count\"\n> sort of thing, not a byte count.\n\nThat's clever, and I think an \"object count\" would be fine (after all,\nthat is all that git:// fetching provides). However, I'm not sure how it\nwould work in practice. When we follow a walk to a commit in a pack, do\nwe really want to try to pull _just_ that commit?\n\nFor one thing, we would need the server to support partial fetches (and\nit is my assumption that we don't bother with that at all now).  I don't\nknow how widespread that is these days (and of course we would still\nneed to fall back to fetching the full pack). But even if we _could_,\nwould we get killed by http protocol overhead for each object? Certainly\nit would be no worse than fetching a totally unpacked repo, but I kind\nof assume such a fetch would be painful.\n\nOr given that the packs should be organized by type, are you proposing\nto fetch just the \"commit part\" as a single entity, then \"tree part\",\nthen the \"blob part\"? I'm a little hesitant to rely too much on what is\nbasically a performance heuristic for the pack organization (and god\nforbid packv4 ever gets finished ;) ).\n\n-Peff\n"},{"id":"115364","messageId":"20090603193205.GM3355@spearce.org","threadId":"19639","inReplyTo":"20090603192420.GA29610@coredump.intra.peff.net","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-06-03T19:32:06Z","receivedAt":"2009-06-03T19:32:06Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Jeff King <peff@peff.net> wrote:\n> On Wed, Jun 03, 2009 at 12:15:55PM -0700, Shawn O. Pearce wrote:\n> \n> > What we could do is try to organize the fetch queue by object type,\n> > get all commits, then all trees, then blobs.  The blobs are the\n> > bulk of the data, and by the time we hit them, we should be able\n> > to give some estimate on progress because we have all of the ones\n> > we need to fetch in our fetch queue.  But its only a \"object count\"\n> > sort of thing, not a byte count.\n> \n> That's clever, and I think an \"object count\" would be fine (after all,\n> that is all that git:// fetching provides). However, I'm not sure how it\n> would work in practice. When we follow a walk to a commit in a pack, do\n> we really want to try to pull _just_ that commit?\n\nNo, we pull the whole pack.  So the progress meter would have to\nswitch to do a content-length thing for the pack pull, then go back\nto the object queue.\n\nIf that means we just pulled *all* of the blobs we have queued up,\ngreat, we can probably actually whack them out of the queue once\nthe pack is down.  Actually, that's really smart to do, because\nthen we don't build up a massive list of objects when cloning a\nvery large repository like Gentoo.\n\nBy delaying trees/blobs, I meant delaying them for loose object\nfetch only, not pack based fetch.\n\n-- \nShawn.\n"},{"id":"115366","messageId":"20090603194416.GA30333@coredump.intra.peff.net","threadId":"19639","inReplyTo":"20090603193205.GM3355@spearce.org","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-06-03T19:44:16Z","receivedAt":"2009-06-03T19:44:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 03, 2009 at 12:32:06PM -0700, Shawn O. Pearce wrote:\n\n> > That's clever, and I think an \"object count\" would be fine (after all,\n> > that is all that git:// fetching provides). However, I'm not sure how it\n> > would work in practice. When we follow a walk to a commit in a pack, do\n> > we really want to try to pull _just_ that commit?\n> \n> No, we pull the whole pack.  So the progress meter would have to\n> switch to do a content-length thing for the pack pull, then go back\n> to the object queue.\n> [...]\n> By delaying trees/blobs, I meant delaying them for loose object\n> fetch only, not pack based fetch.\n\nAh, OK, I see. I wonder if that would make a big difference in practice.\nI expect repos to be fairly packed these days because of the I/O\nbenefits (and even if people aren't doing it manually, we auto-gc\nworking repos and keep pushed packs for publishing repos). So in\npractice by the time you got an accurate object count, you would be more\nor less done with the fetch (because you would have been grabbing the\nrelated blobs in packs as you grabbed commits and trees).\n\n-Peff\n"},{"id":"115367","messageId":"20090603195205.GN3355@spearce.org","threadId":"19639","inReplyTo":"20090603194416.GA30333@coredump.intra.peff.net","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-06-03T19:52:05Z","receivedAt":"2009-06-03T19:52:05Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Jeff King <peff@peff.net> wrote:\n> On Wed, Jun 03, 2009 at 12:32:06PM -0700, Shawn O. Pearce wrote:\n> \n> > > That's clever, and I think an \"object count\" would be fine (after all,\n> > > that is all that git:// fetching provides). However, I'm not sure how it\n> > > would work in practice. When we follow a walk to a commit in a pack, do\n> > > we really want to try to pull _just_ that commit?\n> > \n> > No, we pull the whole pack.  So the progress meter would have to\n> > switch to do a content-length thing for the pack pull, then go back\n> > to the object queue.\n> > [...]\n> > By delaying trees/blobs, I meant delaying them for loose object\n> > fetch only, not pack based fetch.\n> \n> Ah, OK, I see. I wonder if that would make a big difference in practice.\n> I expect repos to be fairly packed these days because of the I/O\n> benefits (and even if people aren't doing it manually, we auto-gc\n> working repos and keep pushed packs for publishing repos). So in\n> practice by the time you got an accurate object count, you would be more\n> or less done with the fetch (because you would have been grabbing the\n> related blobs in packs as you grabbed commits and trees).\n\nGood point, but not all repos are well packed.  Think of a repo that\ngets pushed to a few times a day, always under 100 objects per push,\nmaybe Linus'.  Its a lot of loose stuff to fetch.\n\nBut, you probably are right that many of them are packed... and this\ntrick to try and get a more accurate progress display wouldn't be\nworth the effort most of the time.  Likely not worth coding.\n\n-- \nShawn.\n"},{"id":"115427","messageId":"be6fef0d0906040545j7bd754e0j2c60af833e2ac4a4@mail.gmail.com","threadId":"19639","inReplyTo":"20090603191555.GL3355@spearce.org","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-06-04T12:45:28Z","receivedAt":"2009-06-04T12:45:28Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Thu, Jun 4, 2009 at 3:15 AM, Shawn O. Pearce <spearce@spearce.org> wrote:\n> No, you are right Peff, you can't give a \"percent complete\" because\n> you don't know how much you need to fetch.\n>\n> What we could do is try to organize the fetch queue by object type,\n> get all commits, then all trees, then blobs.  The blobs are the\n> bulk of the data, and by the time we hit them, we should be able\n> to give some estimate on progress because we have all of the ones\n> we need to fetch in our fetch queue.  But its only a \"object count\"\n> sort of thing, not a byte count.\n>\n> When fetching a .idx or a .pack though, we should be able to show\n> progress... assuming the server sent us a Content-Length header.\n> If not, in the case of a pack, we could still show receive speed\n> like index-pack does.\n\nI have a branch 'http-progress-indicators' at my repo\ngit://github.com/rctay/git.git. It contains a patch on walker.c that\nupdates the object total as the fetch goes along. The progress\nindicator says \"Processing targets:...\" for lack of a better name; I'm\nall for suggestions.\n\nThe branch also patches that display progress meters for the\ndownloading of .idx and .pack files. I also added a progress indicator\nfor verifying pack files in pack-check.c, because I noticed some\nsignificant time was spent doing that without informing the user about\nwhat was going on, but I'm not really sure if everyone else would\naccept it.\n\nSo far, I've used git built using that branch to fetch the git repo,\nand I also attempted the linux 2.6 kernel repo. Counting the objects\nfetched was accurate in both cases (ie. matching counts of objects\nfetched and objects to-be-fetched).\n\nI plan to clean up the patches and send it in when the http\nrefactoring patches are finalized.\n\n> Jeff King <peff@peff.net> wrote:\n>>   1. summarize what we have fetched (N packs, N loose objects)\n>>   2. show what we are currently fetching (object or pack)\n>>   3. show the number of bytes retrieved for the current item\n>>   4. if the server provides content-length, show the percentage\n>>      completed for this object\n>>   5. show the current throughput\n\nPoints 1. and 5. can probably be combined, because showing info by\ntype (packs and objects) isn't very easy to do. http-walker.c first\ntries to fetch the raw pack; if it can't be found at the url, or at\nalternate locations, it then tries to fetching packs. In other words,\nit's hard to know in advance if the object is found in unpacked or\npacked form.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"115433","messageId":"20090604160152.GA13984@sigill.intra.peff.net","threadId":"19639","inReplyTo":"be6fef0d0906040545j7bd754e0j2c60af833e2ac4a4@mail.gmail.com","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-06-04T16:01:52Z","receivedAt":"2009-06-04T16:01:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 04, 2009 at 08:45:28PM +0800, Tay Ray Chuan wrote:\n\n> I have a branch 'http-progress-indicators' at my repo\n> git://github.com/rctay/git.git. It contains a patch on walker.c that\n> updates the object total as the fetch goes along. The progress\n> indicator says \"Processing targets:...\" for lack of a better name; I'm\n> all for suggestions.\n\nThanks, I took a look at starting on a progress meter yesterday, but I\ndo think it makes sense to integrate with the work you are doing.\n\nI tried your http-progress-indicators branch. A few comments:\n\n  1. You still end up with a lot of lines of output. Some of those are\n     \"Getting pack $x\" which we can probably get rid of in non-verbose\n     mode. But we still get a different progress indicator line for each\n     fetched item, which can add up to quite a lot. I was thinking of\n     something like\n\n        Fetching %s (got %d packs, %d loose): (%d/%d)\n\n     with the substitutions:\n\n       %s = \"pack\", \"index\", or \"loose object\"\n       %d packs, %d loose = a running count of how much we've gotten\n       %d/%d = current and total byte counts for what we are getting now\n\n     and then you could keep everything on a single line. I don't think\n     is possible with the current progress code (it doesn't let you\n     restart the counter), but it should be easy with some tweaking.\n\n  2. The current progress code can also do throughput display, which\n     would be nice (see display_throughput in progress.[ch]).\n\n  3. Your implementation calls your get_http_file_size, which does a\n     separate HEAD request to get the content-length. Instead, do\n     a \"curl_easy_setopt(slot->curl, CURLOPT_PROGRESSFUNCTION, ...)\"\n     to set up a progress callback. Curl will call it with the total\n     number of bytes (from the content-length header of the actual GET\n     request) and the number of bytes currently downloaded.\n\n-Peff\n"},{"id":"115461","messageId":"200906050217.33728.jnareb@gmail.com","threadId":"19639","inReplyTo":"7vmy8p8947.fsf@alter.siamese.dyndns.org","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-06-05T00:17:32Z","receivedAt":"2009-06-05T00:17:32Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Wed, 3 Jun 2009, Junio C Hamano napisał:\n> Jakub Narebski <jnareb@gmail.com> writes:\n> \n> > Finally a question: if we turn off verbose output, do we have any kind\n> > of progress info for git-clone over http?\n[...]\n\n> You _could_ do something like this patch.  Instead of showing \"walk %s\"\n> and \"got %s\" lines, with occasional \"Getting pack %s\\n which contains %s\",\n> it shows and recycles a single line that shows the number of packs, walk\n> actions and got actions.\n> \n> This is a toy patch; it hiccups for too long while getting each pack, and\n> it does not cleanly restore the display after it finishes, but I'll leave\n> it to interested readers as an exercise to properly do this using the\n> progress API.\n\nI know it is toy patch only, but I wonder if we would want to output\ncurrent format output (walk/got chains) for '--verbose' option...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"115694","messageId":"be6fef0d0906070331y5fd596d1k67893a96a4d872ac@mail.gmail.com","threadId":"19639","inReplyTo":"20090604160152.GA13984@sigill.intra.peff.net","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-06-07T10:31:38Z","receivedAt":"2009-06-07T10:31:38Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Fri, Jun 5, 2009 at 12:01 AM, Jeff King<peff@peff.net> wrote:\n> Thanks, I took a look at starting on a progress meter yesterday, but I\n> do think it makes sense to integrate with the work you are doing.\n>\n> I tried your http-progress-indicators branch. A few comments:\n>\n>  1. You still end up with a lot of lines of output. Some of those are\n>     \"Getting pack $x\" which we can probably get rid of in non-verbose\n>     mode. But we still get a different progress indicator line for each\n>     fetched item, which can add up to quite a lot. I was thinking of\n>     something like\n>\n>        Fetching %s (got %d packs, %d loose): (%d/%d)\n>\n>     with the substitutions:\n>\n>       %s = \"pack\", \"index\", or \"loose object\"\n>       %d packs, %d loose = a running count of how much we've gotten\n>       %d/%d = current and total byte counts for what we are getting now\n>\n>     and then you could keep everything on a single line. I don't think\n>     is possible with the current progress code (it doesn't let you\n>     restart the counter), but it should be easy with some tweaking.\n\nHmm, just wondering, is this is the intended display for \"-q\" or \"-v\"?\nOr should I do isatty(), like builtin-pack-objects.c does for the\n\"Writing objects\" progress indicator?\n\n-- \nCheers,\nRay Chuan\n"},{"id":"115696","messageId":"be6fef0d0906070421j7913b0d7w6f7bb97aa7fd6814@mail.gmail.com","threadId":"19639","inReplyTo":"be6fef0d0906070331y5fd596d1k67893a96a4d872ac@mail.gmail.com","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-06-07T11:21:13Z","receivedAt":"2009-06-07T11:21:13Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Sun, Jun 7, 2009 at 6:31 PM, Tay Ray Chuan<rctay89@gmail.com> wrote:\n> On Fri, Jun 5, 2009 at 12:01 AM, Jeff King<peff@peff.net> wrote:\n>>\n>>        Fetching %s (got %d packs, %d loose): (%d/%d)\n>>\n> Hmm, just wondering, is this is the intended display for \"-q\" or \"-v\"?\n> Or should I do isatty(), like builtin-pack-objects.c does for the\n> \"Writing objects\" progress indicator?\n\nby the way, I have updated http-progress-indicators based on your suggestions.\n\nWhat I have now is:\n\n Fetching %d objects (got %d of %d, %d alt)[, and %s]: 100%\n(32077585/32077585), 30.59 MiB, done.\n\nwhere\n\n1. %d objects = number of concurrent objects being fetched, usually\naround 4-5. Since objects are fetched alongside other files like packs\nand pack indices, I separated this from (4).\n\n2. got %d of %d = a count of loose objects. I haven't done counting of\npacks yet, but it shouldn't be very hard.\n\n3. %d alt = number of alternate objects. The way I'm counting them now\nis very inaccurate; I may drop this if it's too complicated to do an\naccurate count. I added this because some people use forked repos, and\nthey may wonder why after some time, the number of objects fetched\ndoesn't increase. (The time was spent on waiting for the server, only\nfor it to return a 404).\n\n4. [, and %s] = an \"optional\" field that gets displayed when packs,\npack indices, etc. (everything except objects) are being fetched. The\nbyte counts are for this fetch, not the object fetch(es).\n\nHow about pack file verification? Some pack files are monstrous, and\ncan take some time to verify. Is it desirable to fit in pack file\nverification into the same \"Fetching...\" line? Verification is a\nper-file thing, so it should deserve the same treatment that \"Getting\npack ...\" lines got.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"115697","messageId":"be6fef0d0906070425n2be004e4y69d9d9d472ff37ac@mail.gmail.com","threadId":"19639","inReplyTo":"be6fef0d0906040545j7bd754e0j2c60af833e2ac4a4@mail.gmail.com","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-06-07T11:25:40Z","receivedAt":"2009-06-07T11:25:40Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Thu, Jun 4, 2009 at 8:45 PM, Tay Ray Chuan<rctay89@gmail.com> wrote:\n> tries to fetch the raw pack; if it can't be found at the url, or at\n\nOops, I meant \"raw object\".\n\n-- \nCheers,\nRay Chuan\n"},{"id":"115797","messageId":"20090608115431.GC13775@coredump.intra.peff.net","threadId":"19639","inReplyTo":"be6fef0d0906070331y5fd596d1k67893a96a4d872ac@mail.gmail.com","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-06-08T11:54:31Z","receivedAt":"2009-06-08T11:54:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 07, 2009 at 06:31:38PM +0800, Tay Ray Chuan wrote:\n\n> >     something like\n> >        Fetching %s (got %d packs, %d loose): (%d/%d)\n> [...]\n> Hmm, just wondering, is this is the intended display for \"-q\" or \"-v\"?\n> Or should I do isatty(), like builtin-pack-objects.c does for the\n> \"Writing objects\" progress indicator?\n\nI was imagining:\n\n  - without \"-q\", show progress if isatty(1).\n\n  - with \"-q\", never show progress\n\n  - with \"-v\", show the \"getting pack\" and \"walk\" output we show now;\n    without \"-v\", don't show it. \"-v\" has no impact on the progress\n    indicator. \n\n-Peff\n"},{"id":"115799","messageId":"20090608122430.GD13775@coredump.intra.peff.net","threadId":"19639","inReplyTo":"be6fef0d0906070421j7913b0d7w6f7bb97aa7fd6814@mail.gmail.com","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-06-08T12:24:30Z","receivedAt":"2009-06-08T12:24:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jun 07, 2009 at 07:21:13PM +0800, Tay Ray Chuan wrote:\n\n> by the way, I have updated http-progress-indicators based on your suggestions.\n\nThanks, I just looked at it (though sadly it does not merge into what is\nin 'next' right now).\n\n> What I have now is:\n> \n>  Fetching %d objects (got %d of %d, %d alt)[, and %s]: 100%\n> (32077585/32077585), 30.59 MiB, done.\n\nI tried to fetch linux-2.6, and it left me pretty confused.\n\nMy first complaint is that it is way too long. It wrapped in my\n80-column terminal, causing all sorts of visual confusion.\n\n> 1. %d objects = number of concurrent objects being fetched, usually\n> around 4-5. Since objects are fetched alongside other files like packs\n> and pack indices, I separated this from (4).\n\nThis did at times say '4' for me, but just as often it said '0' (even\nwhen stuff was obviously downloading). I hadn't thought about the fact\nthat we have concurrent downloads. That really makes things harder.\nThough it seems like we only do one pack file at a time (so maybe that\nis the reason for the '0' -- we are downloading a pack).\n\nIn fact, while watching the progress go for the linux-2.6 download, it\nis really hard to tell what is going on. The \"% completed\" number jumps\naround between multiple values, even showing what appears to be\nnonsense:\n\n  Fetching 0 objects (got 2 of 320, 0 alt) and pack:   8%\n  (241096602/302989431), 229.78 MiB | 668 KiB/s\n  ...\n  Fetching 0 objects (got 2 of 320, 0 alt) and pack:   4%\n  (270741882/302989431), 257.93 MiB | 690 KiB/s\n\nThose are two cut-and-pastes from the same fetch. You can see it\nprogressing in terms of absolute numbers, but the percentage values make\nno sense. The \"total downloaded\" and throughput numbers look roughly\ncorrect. I don't know if this is caused by multiple simultaenous\ndownloads.\n\n> 2. got %d of %d = a count of loose objects. I haven't done counting of\n> packs yet, but it shouldn't be very hard.\n\nFetching linux-2.6, I spent a very long time on \"got 2 of 320\" which\nreally wasn't all that helpful (because almost the whole thing is in\npacks). Probably a pack count would be useful there. Though I wonder if\nthere is some shorter way to summarize what is going on to keep the line\nsmaller.\n\nBut somewhat worse is that we start at '320', spend a lot of time, and\nthen magically it ends up at 1182387 at the end. So it is not all that\nuseful as a progress counter, because we don't actually know the total.\nSo we can show that we are progressing, but the end keeps getting\nfarther away. :)\n\n> 3. %d alt = number of alternate objects. The way I'm counting them now\n> is very inaccurate; I may drop this if it's too complicated to do an\n> accurate count. I added this because some people use forked repos, and\n> they may wonder why after some time, the number of objects fetched\n> doesn't increase. (The time was spent on waiting for the server, only\n> for it to return a 404).\n\nIn the name of conserving space on the line, perhaps you should just\ncount this as a \"fetched\" object and increment the fetched count by one.\nThe user doesn't have to care which were alt and which were not, as long\nas they see a counter progressing towards completion.\n\n> How about pack file verification? Some pack files are monstrous, and\n> can take some time to verify. Is it desirable to fit in pack file\n> verification into the same \"Fetching...\" line? Verification is a\n> per-file thing, so it should deserve the same treatment that \"Getting\n> pack ...\" lines got.\n\nIt does take a while on big packs, so I think it makes sense to show\nsome eye candy. You can always use the \"show only after 2 seconds\"\nfeature of the progress meter if you are concerned about too much\ninformation for quick verifications.\n\nIt probably makes sense to actually do a stop_progress() after the\nfetching and before the verifying. Otherwise you get the cruft from the\nfetch on the line, but it is not covered up by the verify. Like:\n\n  Verifying pack file: 100% (1180771/1180771), done.   0%\n  (302915802/302989431), 288.72 MiB | 752 KiB/s\n\nI wonder if you should start a newline every time we get to a new\n\"phase\". So you might see:\n\n Downloading %d loose objects: Z% (X/Y), x MiB | y KiB/s, done\n Fetching pack 1 of 2: Z% (X/Y), x MiB | y KiB/s, done\n Verifying pack 1 of 2: Z% (X/Y)\n Fetching pack 2 of 2: Z% (X/Y), x MiB | y KiB/s, done\n Verifying pack 2 of 2: Z% (X/Y)\n\nThat assumes we download packs one at a time (is that right?). It does take\na couple of lines to show what is going on, but I think most repos are\nonly going to have a couple of packs (though in theory, you could have\nmore \"loose objects\" lines interspersed with your packs).\n\n-Peff\n"},{"id":"116008","messageId":"be6fef0d0906100703j57d109d5mb38e41330caa089b@mail.gmail.com","threadId":"19639","inReplyTo":"20090608122430.GD13775@coredump.intra.peff.net","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-06-10T14:03:10Z","receivedAt":"2009-06-10T14:03:10Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Mon, Jun 8, 2009 at 8:24 PM, Jeff King<peff@peff.net> wrote:\n> Thanks, I just looked at it (though sadly it does not merge into what is\n> in 'next' right now).\n\nthanks for taking the time to do try it out (and with linux-2.6, at that).\n\n> My first complaint is that it is way too long. It wrapped in my\n> 80-column terminal, causing all sorts of visual confusion.\n\nThe byte counts can really take up alot of space. Perhaps we should\njust show the size (MiB) and completed percentage, sans byte counts?\n\n>> 1. %d objects = number of concurrent objects being fetched, usually\n>> around 4-5. Since objects are fetched alongside other files like packs\n>> and pack indices, I separated this from (4).\n>\n> This did at times say '4' for me, but just as often it said '0' (even\n> when stuff was obviously downloading). I hadn't thought about the fact\n> that we have concurrent downloads. That really makes things harder.\n> Though it seems like we only do one pack file at a time (so maybe that\n> is the reason for the '0' -- we are downloading a pack).\n\nFetching of objects and packs take place separately; it doesn't mean\nthat when '0' objects are being fetched, we're definitely fetching\nsomething else (eg. packs). Perhaps we should \"hide\" the \"Fetching 0\nobjects\" part when the number of simultaneous object fetches is 0?\n\n> In fact, while watching the progress go for the linux-2.6 download, it\n> is really hard to tell what is going on. The \"% completed\" number jumps\n> around between multiple values, even showing what appears to be\n> nonsense:\n>\n>  Fetching 0 objects (got 2 of 320, 0 alt) and pack:   8%\n>  (241096602/302989431), 229.78 MiB | 668 KiB/s\n>  ...\n>  Fetching 0 objects (got 2 of 320, 0 alt) and pack:   4%\n>  (270741882/302989431), 257.93 MiB | 690 KiB/s\n>\n> Those are two cut-and-pastes from the same fetch. You can see it\n> progressing in terms of absolute numbers, but the percentage values make\n> no sense. The \"total downloaded\" and throughput numbers look roughly\n> correct. I don't know if this is caused by multiple simultaenous\n> downloads.\n\nIt's likely that your guess (about simultaneous downloads) is causing\nthe inconsistent % completed, but then packs aren't downloaded\nsimultaneously. I'll have to check this again.\n\n> Fetching linux-2.6, I spent a very long time on \"got 2 of 320\" which\n> really wasn't all that helpful (because almost the whole thing is in\n> packs). Probably a pack count would be useful there. Though I wonder if\n> there is some shorter way to summarize what is going on to keep the line\n> smaller.\n\nWe are of course assuming that the user is fetching from a well-packed\nrepo. Again, perhaps we could cut out the \"Fetching 0 objects\" part.\n\n> But somewhat worse is that we start at '320', spend a lot of time, and\n> then magically it ends up at 1182387 at the end. So it is not all that\n> useful as a progress counter, because we don't actually know the total.\n> So we can show that we are progressing, but the end keeps getting\n> farther away. :)\n\nThe total number of objects (320) increases as we \"walk\" the commits;\nsometimes we need to fetch the \"walked\" objects, sometimes we don't\n(eg. in packs we've fetched already). There's no way to know in\nadvance the total; hence, the continually updating of the total. I\ndon't think there's it's a problem; the idea is to let the user be\nsure that git is active.\n\n>> 3. %d alt = number of alternate objects. The way I'm counting them now\n>> is very inaccurate; I may drop this if it's too complicated to do an\n>> accurate count. I added this because some people use forked repos, and\n>> they may wonder why after some time, the number of objects fetched\n>> doesn't increase. (The time was spent on waiting for the server, only\n>> for it to return a 404).\n>\n> In the name of conserving space on the line, perhaps you should just\n> count this as a \"fetched\" object and increment the fetched count by one.\n> The user doesn't have to care which were alt and which were not, as long\n> as they see a counter progressing towards completion.\n\nOk. I'll just drop this then. (The way I'm doing it right now isn't\nvery accurate: the \"alt\" count increases the moment git realises the\nobject might be found at the alternate location, not the moment the\nobject at the alternate location is fetched.)\n\n> I wonder if you should start a newline every time we get to a new\n> \"phase\". So you might see:\n>\n>  Downloading %d loose objects: Z% (X/Y), x MiB | y KiB/s, done\n>  Fetching pack 1 of 2: Z% (X/Y), x MiB | y KiB/s, done\n>  Verifying pack 1 of 2: Z% (X/Y)\n>  Fetching pack 2 of 2: Z% (X/Y), x MiB | y KiB/s, done\n>  Verifying pack 2 of 2: Z% (X/Y)\n>\n> That assumes we download packs one at a time (is that right?). It does take\n> a couple of lines to show what is going on, but I think most repos are\n> only going to have a couple of packs (though in theory, you could have\n> more \"loose objects\" lines interspersed with your packs).\n\nYeah, we do download packs one at a time (as I said above).\n\n-- \nCheers,\nRay Chuan\n"},{"id":"116009","messageId":"be6fef0d0906100707r64d489fub328a44d34927a77@mail.gmail.com","threadId":"19639","inReplyTo":"be6fef0d0906100703j57d109d5mb38e41330caa089b@mail.gmail.com","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-06-10T14:07:32Z","receivedAt":"2009-06-10T14:07:32Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Wed, Jun 10, 2009 at 10:03 PM, Tay Ray Chuan<rctay89@gmail.com> wrote:\n> On Mon, Jun 8, 2009 at 8:24 PM, Jeff King<peff@peff.net> wrote:\n>>  Fetching 0 objects (got 2 of 320, 0 alt) and pack:   8%\n>>  (241096602/302989431), 229.78 MiB | 668 KiB/s\n>>  ...\n>>  Fetching 0 objects (got 2 of 320, 0 alt) and pack:   4%\n>>  (270741882/302989431), 257.93 MiB | 690 KiB/s\n>>\n>> Those are two cut-and-pastes from the same fetch. You can see it\n>> progressing in terms of absolute numbers, but the percentage values make\n>> no sense. The \"total downloaded\" and throughput numbers look roughly\n>> correct. I don't know if this is caused by multiple simultaenous\n>> downloads.\n>\n> It's likely that your guess (about simultaneous downloads) is causing\n> the inconsistent % completed, but then packs aren't downloaded\n> simultaneously. I'll have to check this again.\n\nthe linux-2.6 repo has only 1 pack (and no http-alternates), so this is weird.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"116068","messageId":"20090611111141.GB4409@coredump.intra.peff.net","threadId":"19639","inReplyTo":"be6fef0d0906100703j57d109d5mb38e41330caa089b@mail.gmail.com","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-06-11T11:11:41Z","receivedAt":"2009-06-11T11:11:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 10, 2009 at 10:03:10PM +0800, Tay Ray Chuan wrote:\n\n> > My first complaint is that it is way too long. It wrapped in my\n> > 80-column terminal, causing all sorts of visual confusion.\n> \n> The byte counts can really take up alot of space. Perhaps we should\n> just show the size (MiB) and completed percentage, sans byte counts?\n\nI think that makes sense. Especially because the (X/Y) in git's progress\noutput usually refers to the number of _objects_, and there is nothing\nin the output to indicate that it is actually a byte count here.\n\nI think it will need some tweaking of the progress code to show the\npercentage but not the actual byte counts, but it should be a relatively\nsimple change.\n\n> Fetching of objects and packs take place separately; it doesn't mean\n> that when '0' objects are being fetched, we're definitely fetching\n> something else (eg. packs). Perhaps we should \"hide\" the \"Fetching 0\n> objects\" part when the number of simultaneous object fetches is 0?\n\nYes, though I really wonder if the \"fetching\" number is all that useful\neven when it is not zero. The _most_ important thing is to show the user\nthat something is happening, and we are waiting on the network. And I\nthink we largely show that through the \"total bytes sent\" and throughput\ncounters.\n\nSecond to that is trying to give a sense of when the task may finish.\nBut as we've discussed, we don't have a sense of the total number of\nobjects until we actually fetch them. Showing progress within packs, and\nthe total number of packs is somewhat useful there (though it can be\nmisleading -- most of the time will probably be spent on one or two of\nthe packs).\n\n> The total number of objects (320) increases as we \"walk\" the commits;\n> sometimes we need to fetch the \"walked\" objects, sometimes we don't\n> (eg. in packs we've fetched already). There's no way to know in\n> advance the total; hence, the continually updating of the total. I\n> don't think there's it's a problem; the idea is to let the user be\n> sure that git is active.\n\nRight. But I think we are better off showing simple increasing numbers\n(like bytes or objects transferred) than misleading or inaccurate\nguesses of totals. The latter creates more frustration, I think.\n\n> > I wonder if you should start a newline every time we get to a new\n> > \"phase\". So you might see:\n> >\n> >  Downloading %d loose objects: Z% (X/Y), x MiB | y KiB/s, done\n> >  Fetching pack 1 of 2: Z% (X/Y), x MiB | y KiB/s, done\n> >  Verifying pack 1 of 2: Z% (X/Y)\n> >  Fetching pack 2 of 2: Z% (X/Y), x MiB | y KiB/s, done\n> >  Verifying pack 2 of 2: Z% (X/Y)\n> >\n> > That assumes we download packs one at a time (is that right?). It does take\n> > a couple of lines to show what is going on, but I think most repos are\n> > only going to have a couple of packs (though in theory, you could have\n> > more \"loose objects\" lines interspersed with your packs).\n> \n> Yeah, we do download packs one at a time (as I said above).\n\nBut from what you wrote elsewhere in the message, it sounds like we may\nbe downloading a pack _and_ a loose object at the same time. So my\nsuggestion doesn't quite work in that case.\n\n> the linux-2.6 repo has only 1 pack (and no http-alternates), so this\n> is weird.\n\nMaybe we are fetching from different places:\n\n  $ wget 2>/dev/null -O - \\\n  http://www.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git/objects/info/packs\n  P pack-ab6a95cfd1919f6e820a8b2670403292838cfc17.pack\n  P pack-ff5d2e76c6b3d4c0a5a11efd36af43934c3744df.pack\n  P pack-588543ec42616a86eef47bb53dd04cd8a864d9b5.pack\n  P pack-93b35ef7e596e6839c020e36edfaf8206b0f78c4.pack\n  P pack-c89a5bf3b095d812bf1068cd1c84f8a07c3403c5.pack\n  P pack-2cc5038b758e40a40a4590b37a8019d1ba5a65a9.pack\n  P pack-9f36ce46120d8d9ee32e6394bb5857d7e548826b.pack\n  P pack-8bebad1b754473489d516549632e904c0b3178a2.pack\n  P pack-6554949d36a94e012da6ca6134ef62ce347b2efa.pack\n\n-Peff\n"},{"id":"116758","messageId":"be6fef0d0906220510r416d66aaoc2171bdcc61ec3a4@mail.gmail.com","threadId":"19639","inReplyTo":"20090611111141.GB4409@coredump.intra.peff.net","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-06-22T12:10:20Z","receivedAt":"2009-06-22T12:10:20Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nnote: this time, I haven't anything to show for in my repo; sorry.\n\nOn Thu, Jun 11, 2009 at 7:11 PM, Jeff King<peff@peff.net> wrote:\n> On Wed, Jun 10, 2009 at 10:03:10PM +0800, Tay Ray Chuan wrote:\n>\n>> > My first complaint is that it is way too long. It wrapped in my\n>> > 80-column terminal, causing all sorts of visual confusion.\n>>\n>> The byte counts can really take up alot of space. Perhaps we should\n>> just show the size (MiB) and completed percentage, sans byte counts?\n>\n> I think that makes sense. Especially because the (X/Y) in git's progress\n> output usually refers to the number of _objects_, and there is nothing\n> in the output to indicate that it is actually a byte count here.\n>\n> I think it will need some tweaking of the progress code to show the\n> percentage but not the actual byte counts, but it should be a relatively\n> simple change.\n\nOk.\n\n>> Fetching of objects and packs take place separately; it doesn't mean\n>> that when '0' objects are being fetched, we're definitely fetching\n>> something else (eg. packs). Perhaps we should \"hide\" the \"Fetching 0\n>> objects\" part when the number of simultaneous object fetches is 0?\n>\n> Yes, though I really wonder if the \"fetching\" number is all that useful\n> even when it is not zero. The _most_ important thing is to show the user\n> that something is happening, and we are waiting on the network. And I\n> think we largely show that through the \"total bytes sent\" and throughput\n> counters.\n\nOk also.\n\nMy understanding of this point now: if we're fetching non-objects\n*(like packs, pack indices), we wouldn't display \"Fetching X objects\"\nat all, regardless of X. But if we're not fetching any non-objects and\nwe're waiting for loose objects fetching to finish (ie. we're fetching\n1 loose object or more), then we would display \"Fetching X objects\".\n\n>> The total number of objects (320) increases as we \"walk\" the commits;\n>> sometimes we need to fetch the \"walked\" objects, sometimes we don't\n>> (eg. in packs we've fetched already). There's no way to know in\n>> advance the total; hence, the continually updating of the total. I\n>> don't think there's it's a problem; the idea is to let the user be\n>> sure that git is active.\n>\n> Right. But I think we are better off showing simple increasing numbers\n> (like bytes or objects transferred) than misleading or inaccurate\n> guesses of totals. The latter creates more frustration, I think.\n\nOk.\n\n>> Yeah, we do download packs one at a time (as I said above).\n>\n> But from what you wrote elsewhere in the message, it sounds like we may\n> be downloading a pack _and_ a loose object at the same time. So my\n> suggestion doesn't quite work in that case.\n\nIn addition to that, it isn't possible to calculate the number of pack\nindices to be fetched, because we're fetching them as we go along (see\nhttp.c::http_get_info_packs() in 'master'), so that's a potential\nproblem. I wonder if you (or anyone reading this) has any suggestions\non this?\n\nI still think we can use your idea of one-fetch-one-verify per pack,\neven though objects are fetched simultaneously.\n\nHere's what a \"git clone http://repo\" would look like, after\nincorporating points from our discussion so far:\n\n Fetching info/refs: Z%, x MiB | y KiB/s, done\n Fetching objects/info/packs: Z%, x MiB | y KiB/s, done\n Fetching pack index (1 of 3): Z%, x MiB | y KiB/s, done\n Fetching pack index (2 of 3): Z%, x MiB | y KiB/s, done\n Fetching pack index (3 of 3): Z%, x MiB | y KiB/s, done\n Fetching pack (1 of 3): Z%, x MiB | y KiB/s, done\n Verifying pack (1 of 3): Z%, (X/Y), done\n Fetching pack (2 of 3): Z%, x MiB | y KiB/s, done\n Verifying pack (2 of 3): Z%, (X/Y), done\n Fetching pack (3 of 3): Z%, x MiB | y KiB/s, done\n Verifying pack (3 of 3): Z%, (X/Y), done\n Fetching 2 objects, done.\n Checking out files, done.\n\n(I made up the last line.)\n\n>> the linux-2.6 repo has only 1 pack (and no http-alternates), so this\n>> is weird.\n>\n> Maybe we are fetching from different places:\n\nThanks. I realise the weird percentage values you saw were due to\nout-of-range numeric calculations, so actually, your point didn't\ncount, but thanks anyway.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"118323","messageId":"be6fef0d0907200824r68402229vac88e46a8e70603a@mail.gmail.com","threadId":"19639","inReplyTo":"be6fef0d0906220510r416d66aaoc2171bdcc61ec3a4@mail.gmail.com","subject":"Re: [Patch] Prevent cloning over http from spewing","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2009-07-20T15:24:04Z","receivedAt":"2009-07-20T15:24:04Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Mon, Jun 22, 2009 at 8:10 PM, Tay Ray Chuan<rctay89@gmail.com> wrote:\n> note: this time, I haven't anything to show for in my repo; sorry.\n\nI've got some stuff, as per the last email, at my git repo (branch\n'http-progress-indicators' at git://github.com/rctay/git.git).\n\n-- \nCheers,\nRay Chuan\n"}]}