Re: [PATCH v2 4/5] pack-objects: show some progress when counting kept objects
From: Duy Nguyen <hidden>
Date: 2018-03-16 19:14:55
On Mon, Mar 12, 2018 at 7:32 PM, Ævar Arnfjörð Bjarmason [off-list ref] wrote:
On Tue, Mar 06 2018, Nguyễn Thái Ngọc Duy jotted:quoted
We only show progress when there are new objects to be packed. But when --keep-pack is specified on the base pack, we will exclude most of objects. This makes 'pack-objects' stay silent for a long time while the counting phase is going. Let's show some progress whenever we visit an object instead. The number of packed objects will be shown after if it's not the same as the number of visited objects. Since the meaning of this number has changed, use another word instead of "Counting" to hint about the change.Can you elaborate on how the meaning has changed? With/without this on linux.git I get: With: Enumerating objects: 5901144, done. Getting object details: 100% (5901145/5901145), done. Delta compression using up to 8 threads. Without: Counting objects: 5901145, done. Delta compression using up to 8 threads. So now we're seemingly off-by-one but otherwise doing the same thing?
Yep, it's an off-by-one bug.
As for as user feedback goes we might as well have said "Reticulating splines", but I have some bias towards keeping the current "Counting objects..." phrasing. We ourselves have other docs referring to it that aren't changed by this patch, and there's e.g. https://githubengineering.com/counting-objects/ and lots of other 3rd party docs that refer to this.
This is why I changed the phrase. The counting is now a bit different. Documents describing this exact phrase won't apply to the new version. The old way counts objects that will be packed. The new way simply counts objects that are visited. When you keep some packs, the number of objects you visit but not pack could be very high, while in normal case the two numbers should be the same (e.g. you pack everything you visit). I would prefer to print both values (e.g. "counting objects: <packed>/<visited>") but it's not possible with the current progress code. -- Duy