Re: [Patch] Prevent cloning over http from spewing

On Sun, Jun 07, 2009 at 07:21:13PM +0800, Tay Ray Chuan wrote:

Thanks, I just looked at it (though sadly it does not merge into what is in 'next' right now).

I tried to fetch linux-2.6, and it left me pretty confused.

My first complaint is that it is way too long. It wrapped in my 80-column terminal, causing all sorts of visual confusion.

This did at times say '4' for me, but just as often it said '0' (even when stuff was obviously downloading). I hadn't thought about the fact that we have concurrent downloads. That really makes things harder. Though it seems like we only do one pack file at a time (so maybe that is the reason for the '0' -- we are downloading a pack).

In fact, while watching the progress go for the linux-2.6 download, it is really hard to tell what is going on. The "% completed" number jumps around between multiple values, even showing what appears to be nonsense:

  Fetching 0 objects (got 2 of 320, 0 alt) and pack:   8%
  (241096602/302989431), 229.78 MiB | 668 KiB/s
  ...
  Fetching 0 objects (got 2 of 320, 0 alt) and pack:   4%
  (270741882/302989431), 257.93 MiB | 690 KiB/s

Those are two cut-and-pastes from the same fetch. You can see it progressing in terms of absolute numbers, but the percentage values make no sense. The "total downloaded" and throughput numbers look roughly correct. I don't know if this is caused by multiple simultaenous downloads.

Fetching linux-2.6, I spent a very long time on "got 2 of 320" which really wasn't all that helpful (because almost the whole thing is in packs). Probably a pack count would be useful there. Though I wonder if there is some shorter way to summarize what is going on to keep the line smaller.

But somewhat worse is that we start at '320', spend a lot of time, and then magically it ends up at 1182387 at the end. So it is not all that useful as a progress counter, because we don't actually know the total. So we can show that we are progressing, but the end keeps getting farther away. :)

In the name of conserving space on the line, perhaps you should just count this as a "fetched" object and increment the fetched count by one. The user doesn't have to care which were alt and which were not, as long as they see a counter progressing towards completion.

It does take a while on big packs, so I think it makes sense to show some eye candy. You can always use the "show only after 2 seconds" feature of the progress meter if you are concerned about too much information for quick verifications.

It probably makes sense to actually do a stop_progress() after the fetching and before the verifying. Otherwise you get the cruft from the fetch on the line, but it is not covered up by the verify. Like:

  Verifying pack file: 100% (1180771/1180771), done.   0%
  (302915802/302989431), 288.72 MiB | 752 KiB/s

I wonder if you should start a newline every time we get to a new "phase". So you might see:

 Downloading %d loose objects: Z% (X/Y), x MiB | y KiB/s, done
 Fetching pack 1 of 2: Z% (X/Y), x MiB | y KiB/s, done
 Verifying pack 1 of 2: Z% (X/Y)
 Fetching pack 2 of 2: Z% (X/Y), x MiB | y KiB/s, done
 Verifying pack 2 of 2: Z% (X/Y)

That assumes we download packs one at a time (is that right?). It does take a couple of lines to show what is going on, but I think most repos are only going to have a couple of packs (though in theory, you could have more "loose objects" lines interspersed with your packs).

-Peff