[PATCH] count-objects: output "KiB" instead of "kilobytes"

[PATCH] count-objects: output "KiB" instead of "kilobytes"

[Mihai Capotă]

The code uses division by 1024. Also, the manual uses "KiB".

Signed-off-by: Mihai Capotă <mihai@mihaic.ro>
---
 builtin/count-objects.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 9afaa88..ecc13b0 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -124,7 +124,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
 		printf("garbage: %lu\n", garbage);
 	}
 	else
-		printf("%lu objects, %lu kilobytes\n",
+		printf("%lu objects, %lu KiB\n",
 		       loose, (unsigned long) (loose_size / 1024));
 	return 0;
 }
-- 
1.7.9.5

Re: [PATCH] count-objects: output "KiB" instead of "kilobytes"

[Junio C Hamano]

Mihai Capotă <mihai@mihaic.ro> writes:

> The code uses division by 1024. Also, the manual uses "KiB".
>
> Signed-off-by: Mihai Capotă <mihai@mihaic.ro>
> ---
>  builtin/count-objects.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/count-objects.c b/builtin/count-objects.c
> index 9afaa88..ecc13b0 100644
> --- a/builtin/count-objects.c
> +++ b/builtin/count-objects.c
> @@ -124,7 +124,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
>  		printf("garbage: %lu\n", garbage);
>  	}
>  	else
> -		printf("%lu objects, %lu kilobytes\n",
> +		printf("%lu objects, %lu KiB\n",
>  		       loose, (unsigned long) (loose_size / 1024));
>  	return 0;
>  }

I guess nobody reads this in scripts, so it should be OK.

Will queue. Thanks.

Re: [PATCH] count-objects: output "KiB" instead of "kilobytes"

[Junio C Hamano]

Mihai Capotă <mihai@mihaic.ro> writes:

> The code uses division by 1024. Also, the manual uses "KiB".
>
> Signed-off-by: Mihai Capotă <mihai@mihaic.ro>
> ---
>  builtin/count-objects.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/count-objects.c b/builtin/count-objects.c
> index 9afaa88..ecc13b0 100644
> --- a/builtin/count-objects.c
> +++ b/builtin/count-objects.c
> @@ -124,7 +124,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
>  		printf("garbage: %lu\n", garbage);
>  	}
>  	else
> -		printf("%lu objects, %lu kilobytes\n",
> +		printf("%lu objects, %lu KiB\n",
>  		       loose, (unsigned long) (loose_size / 1024));
>  	return 0;
>  }

This breaks existing tests (5301, 7408 and 5700); I noticed it too
late and wasted 20 minutes, having to re-run today's integration
cycle.

Next time, please run the testsuite before sending a patch.

Re: [PATCH] count-objects: output "KiB" instead of "kilobytes"

[Mihai Capotă]

I'm really sorry about that. I'll make sure to run the tests before
sending patches in the future.

On Wed, Apr 3, 2013 at 12:01 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Mihai Capotă <mihai@mihaic.ro> writes:
>
>> The code uses division by 1024. Also, the manual uses "KiB".
>>
>> Signed-off-by: Mihai Capotă <mihai@mihaic.ro>
>> ---
>>  builtin/count-objects.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/builtin/count-objects.c b/builtin/count-objects.c
>> index 9afaa88..ecc13b0 100644
>> --- a/builtin/count-objects.c
>> +++ b/builtin/count-objects.c
>> @@ -124,7 +124,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
>>               printf("garbage: %lu\n", garbage);
>>       }
>>       else
>> -             printf("%lu objects, %lu kilobytes\n",
>> +             printf("%lu objects, %lu KiB\n",
>>                      loose, (unsigned long) (loose_size / 1024));
>>       return 0;
>>  }
>
> This breaks existing tests (5301, 7408 and 5700); I noticed it too
> late and wasted 20 minutes, having to re-run today's integration
> cycle.
>
> Next time, please run the testsuite before sending a patch.

[PATCH v2] count-objects: output "KiB" instead of "kilobytes"

[Mihai Capotă]

The code uses division by 1024. The master branch count-objects manual also
uses "KiB".

Also updated the code that reads count-objects output (t5301, t5700, t7408, and
git-cvsimport) and the Git User's Manual.

Signed-off-by: Mihai Capotă <mihai@mihaic.ro>
---
 Documentation/user-manual.txt  |    4 ++--
 builtin/count-objects.c        |    2 +-
 git-cvsimport.perl             |    8 ++++----
 t/t5301-sliding-window.sh      |    4 ++--
 t/t5700-clone-reference.sh     |    4 ++--
 t/t7408-submodule-reference.sh |    4 ++--
 6 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index e831cc2..b61a09c 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -3175,7 +3175,7 @@ lot of objects.  Try this on an old project:
 
 ------------------------------------------------
 $ git count-objects
-6930 objects, 47620 kilobytes
+6930 objects, 47620 KiB
 ------------------------------------------------
 
 The first number is the number of objects which are kept in
@@ -3215,7 +3215,7 @@ You can verify that the loose objects are gone by looking at the
 
 ------------------------------------------------
 $ git count-objects
-0 objects, 0 kilobytes
+0 objects, 0 KiB
 ------------------------------------------------
 
 Although the object files are gone, any commands that refer to those
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 9afaa88..ecc13b0 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -124,7 +124,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
 		printf("garbage: %lu\n", garbage);
 	}
 	else
-		printf("%lu objects, %lu kilobytes\n",
+		printf("%lu objects, %lu KiB\n",
 		       loose, (unsigned long) (loose_size / 1024));
 	return 0;
 }
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 73d367c..de44e33 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -1126,12 +1126,12 @@ unless ($opt_P) {
 }
 
 # The heuristic of repacking every 1024 commits can leave a
-# lot of unpacked data.  If there is more than 1MB worth of
+# lot of unpacked data.  If there is more than 1MiB worth of
 # not-packed objects, repack once more.
 my $line = `git count-objects`;
-if ($line =~ /^(\d+) objects, (\d+) kilobytes$/) {
-  my ($n_objects, $kb) = ($1, $2);
-  1024 < $kb
+if ($line =~ /^(\d+) objects, (\d+) KiB$/) {
+  my ($n_objects, $kib) = ($1, $2);
+  1024 < $kib
     and system(qw(git repack -a -d));
 }
 
diff --git a/t/t5301-sliding-window.sh b/t/t5301-sliding-window.sh
index 2fc5af6..37931d2 100755
--- a/t/t5301-sliding-window.sh
+++ b/t/t5301-sliding-window.sh
@@ -20,7 +20,7 @@ test_expect_success \
      commit1=`git commit-tree $tree </dev/null` &&
      git update-ref HEAD $commit1 &&
      git repack -a -d &&
-     test "`git count-objects`" = "0 objects, 0 kilobytes" &&
+     test "`git count-objects`" = "0 objects, 0 KiB" &&
      pack1=`ls .git/objects/pack/*.pack` &&
      test -f "$pack1"'
 
@@ -46,7 +46,7 @@ test_expect_success \
      commit2=`git commit-tree $tree -p $commit1 </dev/null` &&
      git update-ref HEAD $commit2 &&
      git repack -a -d &&
-     test "`git count-objects`" = "0 objects, 0 kilobytes" &&
+     test "`git count-objects`" = "0 objects, 0 KiB" &&
      pack2=`ls .git/objects/pack/*.pack` &&
      test -f "$pack2" &&
      test "$pack1" \!= "$pack2"'
diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index c47d450..e5cfd6a 100755
--- a/t/t5700-clone-reference.sh
+++ b/t/t5700-clone-reference.sh
@@ -46,7 +46,7 @@ cd "$base_dir"
 
 test_expect_success 'that reference gets used' \
 'cd C &&
-echo "0 objects, 0 kilobytes" > expected &&
+echo "0 objects, 0 KiB" > expected &&
 git count-objects > current &&
 test_cmp expected current'
 
@@ -73,7 +73,7 @@ test_expect_success 'pulling from reference' \
 cd "$base_dir"
 
 test_expect_success 'that reference gets used' \
-'cd D && echo "0 objects, 0 kilobytes" > expected &&
+'cd D && echo "0 objects, 0 KiB" > expected &&
 git count-objects > current &&
 test_cmp expected current'
 
diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
index b770b2f..aeface6 100755
--- a/t/t7408-submodule-reference.sh
+++ b/t/t7408-submodule-reference.sh
@@ -49,7 +49,7 @@ cd "$base_dir"
 
 test_expect_success 'that reference gets used with add' \
 'cd super/sub &&
-echo "0 objects, 0 kilobytes" > expected &&
+echo "0 objects, 0 KiB" > expected &&
 git count-objects > current &&
 diff expected current'
 
@@ -72,7 +72,7 @@ cd "$base_dir"
 
 test_expect_success 'that reference gets used with update' \
 'cd super-clone/sub &&
-echo "0 objects, 0 kilobytes" > expected &&
+echo "0 objects, 0 KiB" > expected &&
 git count-objects > current &&
 diff expected current'
 
-- 
1.7.9.5

Re: [PATCH v2] count-objects: output "KiB" instead of "kilobytes"

[Junio C Hamano]

Mihai Capotă <mihai@mihaic.ro> writes:

> The code uses division by 1024. The master branch count-objects manual also
> uses "KiB".
>
> Also updated the code that reads count-objects output (t5301, t5700, t7408, and
> git-cvsimport) and the Git User's Manual.
>
> Signed-off-by: Mihai Capotă <mihai@mihaic.ro>
> ---
>  Documentation/user-manual.txt  |    4 ++--
>  builtin/count-objects.c        |    2 +-
>  git-cvsimport.perl             |    8 ++++----
>  t/t5301-sliding-window.sh      |    4 ++--
>  t/t5700-clone-reference.sh     |    4 ++--
>  t/t7408-submodule-reference.sh |    4 ++--
>  6 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
> index e831cc2..b61a09c 100644
> --- a/Documentation/user-manual.txt
> +++ b/Documentation/user-manual.txt
> @@ -3175,7 +3175,7 @@ lot of objects.  Try this on an old project:
>  
>  ------------------------------------------------
>  $ git count-objects
> -6930 objects, 47620 kilobytes
> +6930 objects, 47620 KiB
>  ------------------------------------------------
>  
>  The first number is the number of objects which are kept in
> @@ -3215,7 +3215,7 @@ You can verify that the loose objects are gone by looking at the
>  
>  ------------------------------------------------
>  $ git count-objects
> -0 objects, 0 kilobytes
> +0 objects, 0 KiB
>  ------------------------------------------------
>  
>  Although the object files are gone, any commands that refer to those

It is good to see the patch being thorough, adjusting even
documentation.

> diff --git a/git-cvsimport.perl b/git-cvsimport.perl
> index 73d367c..de44e33 100755
> --- a/git-cvsimport.perl
> +++ b/git-cvsimport.perl
> @@ -1126,12 +1126,12 @@ unless ($opt_P) {
>  }
>  
>  # The heuristic of repacking every 1024 commits can leave a
> -# lot of unpacked data.  If there is more than 1MB worth of
> +# lot of unpacked data.  If there is more than 1MiB worth of
>  # not-packed objects, repack once more.
>  my $line = `git count-objects`;
> -if ($line =~ /^(\d+) objects, (\d+) kilobytes$/) {
> -  my ($n_objects, $kb) = ($1, $2);
> -  1024 < $kb
> +if ($line =~ /^(\d+) objects, (\d+) KiB$/) {
> +  my ($n_objects, $kib) = ($1, $2);
> +  1024 < $kib
>      and system(qw(git repack -a -d));
>  }

This hunk makes me wonder if this s/kilobytes/kib/ is a good idea in
the first place.  This in-tree user was lucky enough to have been
caught and adjusted, but we don't know how many out-of-tree scripts
are broken the same way and in need of a similar treatment.

Re: [PATCH v2] count-objects: output "KiB" instead of "kilobytes"

[Mihai Capotă]

On Wed, Apr 3, 2013 at 4:38 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Mihai Capotă <mihai@mihaic.ro> writes:
>> diff --git a/git-cvsimport.perl b/git-cvsimport.perl
>> index 73d367c..de44e33 100755
>> --- a/git-cvsimport.perl
>> +++ b/git-cvsimport.perl
>> @@ -1126,12 +1126,12 @@ unless ($opt_P) {
>>  }
>>
>>  # The heuristic of repacking every 1024 commits can leave a
>> -# lot of unpacked data.  If there is more than 1MB worth of
>> +# lot of unpacked data.  If there is more than 1MiB worth of
>>  # not-packed objects, repack once more.
>>  my $line = `git count-objects`;
>> -if ($line =~ /^(\d+) objects, (\d+) kilobytes$/) {
>> -  my ($n_objects, $kb) = ($1, $2);
>> -  1024 < $kb
>> +if ($line =~ /^(\d+) objects, (\d+) KiB$/) {
>> +  my ($n_objects, $kib) = ($1, $2);
>> +  1024 < $kib
>>      and system(qw(git repack -a -d));
>>  }
>
> This hunk makes me wonder if this s/kilobytes/kib/ is a good idea in
> the first place.  This in-tree user was lucky enough to have been
> caught and adjusted, but we don't know how many out-of-tree scripts
> are broken the same way and in need of a similar treatment.

The git manual contains an explicit warning about the output of a
porcelain command changing: "The interface to Porcelain commands on
the other hand are subject to change in order to improve the end user
experience."

Re: [PATCH v2] count-objects: output "KiB" instead of "kilobytes"

[Junio C Hamano]

Mihai Capotă <mihai@mihaic.ro> writes:

> The git manual contains an explicit warning about the output of a
> porcelain command changing: "The interface to Porcelain commands on
> the other hand are subject to change in order to improve the end user
> experience."

Yeah, I know that, as I wrote it ;-)

Aside from count-object being not exactly a Porcelain, the statement
does not give us a blank check to make random changes as we see fit.
There needs to be a clear improvement.

I am just having a hard time weighing the benefit of using more
accurate kibibytes over kilobytes and the possible downside of
breaking other peoples' tools.

Perhaps it would be alright if the change was accompanied by a
warning in the Release Notes to say something like:

        If you have scripts that decide when to run "git repack" by
	parsing the output from "git count-objects", this release
	may break them.  Sorry about that.  One of the scripts
	shipped by git-core itself also had to be adjusted.  The
	command reports the total diskspace used to store loose
	objects in kibibytes, but it was labelled as "kilobytes".
	The number now is shown with "KiB", e.g. "6750 objects,
	50928 KiB".

	You may want to consider updating such scripts to always
	call "git gc --auto" to let it decide when to repack for
	you.

Also, I suspect that for the purpose of this exact output field,
nobody cares the difference between kibibytes and kilobytes.
Depending on the system, we add up either st.st_blocks or st.st_size
and the result is not that exact as "how much diskspace is
consumed".

Re: [PATCH v2] count-objects: output "KiB" instead of "kilobytes"

[Mihai Capotă]

On Thu, Apr 4, 2013 at 6:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Mihai Capotă <mihai@mihaic.ro> writes:
>
>> The git manual contains an explicit warning about the output of a
>> porcelain command changing: "The interface to Porcelain commands on
>> the other hand are subject to change in order to improve the end user
>> experience."
>
> Yeah, I know that, as I wrote it ;-)
>
> Aside from count-object being not exactly a Porcelain, the statement
> does not give us a blank check to make random changes as we see fit.
> There needs to be a clear improvement.
>
> I am just having a hard time weighing the benefit of using more
> accurate kibibytes over kilobytes and the possible downside of
> breaking other peoples' tools.
>
> Perhaps it would be alright if the change was accompanied by a
> warning in the Release Notes to say something like:
>
>         If you have scripts that decide when to run "git repack" by
>         parsing the output from "git count-objects", this release
>         may break them.  Sorry about that.  One of the scripts
>         shipped by git-core itself also had to be adjusted.  The
>         command reports the total diskspace used to store loose
>         objects in kibibytes, but it was labelled as "kilobytes".
>         The number now is shown with "KiB", e.g. "6750 objects,
>         50928 KiB".
>
>         You may want to consider updating such scripts to always
>         call "git gc --auto" to let it decide when to repack for
>         you.
>
> Also, I suspect that for the purpose of this exact output field,
> nobody cares the difference between kibibytes and kilobytes.
> Depending on the system, we add up either st.st_blocks or st.st_size
> and the result is not that exact as "how much diskspace is
> consumed".

I agree completely. I think the release notes warning is a good plan.
Just in case you decide against it, I'm also sending a completely
different patch to document the issue.

Mihai

[PATCH] count-objects doc: document use of kibibytes

[Mihai Capotă]

Document the use of kibibytes, not kilobytes, in the output of count-objects
and the reason for not correcting the output.

Also, make cvsimport comment and variable name reflect unit actually used.

Signed-off-by: Mihai Capotă <mihai@mihaic.ro>
---
 Documentation/git-count-objects.txt |    7 +++++++
 git-cvsimport.perl                  |    6 +++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt
index 23c80ce..d562dad 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -26,6 +26,13 @@ OPTIONS
 	and number of objects that can be removed by running
 	`git prune-packed`.
 
+
+BUGS
+----
+Consumed space is actually expressed in kibibytes, not kilobytes as stated in
+the output. The output is kept as it is for compatibility reasons.
+
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 73d367c..6803f04 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -1126,12 +1126,12 @@ unless ($opt_P) {
 }
 
 # The heuristic of repacking every 1024 commits can leave a
-# lot of unpacked data.  If there is more than 1MB worth of
+# lot of unpacked data.  If there is more than 1MiB worth of
 # not-packed objects, repack once more.
 my $line = `git count-objects`;
 if ($line =~ /^(\d+) objects, (\d+) kilobytes$/) {
-  my ($n_objects, $kb) = ($1, $2);
-  1024 < $kb
+  my ($n_objects, $kib) = ($1, $2);
+  1024 < $kib
     and system(qw(git repack -a -d));
 }
 
-- 
1.7.9.5

Re: [PATCH v2] count-objects: output "KiB" instead of "kilobytes"

[Antoine Pelisse]

Should we use that opportunity to implement an option like -h (for
humanize) similar to what ls(1), df(1), du(1) does ? Of course "-h" is
already used for help, so we could use -H or any other sensible
choice.
It can become tough to read the size when it gets big enough.

On Thu, Apr 4, 2013 at 6:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Mihai Capotă <mihai@mihaic.ro> writes:
>
>> The git manual contains an explicit warning about the output of a
>> porcelain command changing: "The interface to Porcelain commands on
>> the other hand are subject to change in order to improve the end user
>> experience."
>
> Yeah, I know that, as I wrote it ;-)
>
> Aside from count-object being not exactly a Porcelain, the statement
> does not give us a blank check to make random changes as we see fit.
> There needs to be a clear improvement.
>
> I am just having a hard time weighing the benefit of using more
> accurate kibibytes over kilobytes and the possible downside of
> breaking other peoples' tools.
>
> Perhaps it would be alright if the change was accompanied by a
> warning in the Release Notes to say something like:
>
>         If you have scripts that decide when to run "git repack" by
>         parsing the output from "git count-objects", this release
>         may break them.  Sorry about that.  One of the scripts
>         shipped by git-core itself also had to be adjusted.  The
>         command reports the total diskspace used to store loose
>         objects in kibibytes, but it was labelled as "kilobytes".
>         The number now is shown with "KiB", e.g. "6750 objects,
>         50928 KiB".
>
>         You may want to consider updating such scripts to always
>         call "git gc --auto" to let it decide when to repack for
>         you.
>
> Also, I suspect that for the purpose of this exact output field,
> nobody cares the difference between kibibytes and kilobytes.
> Depending on the system, we add up either st.st_blocks or st.st_size
> and the result is not that exact as "how much diskspace is
> consumed".
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] progress: create public humanize() to show sizes

[Antoine Pelisse]

Currently, humanization of downloaded size is done in the same function
as text formatting. This is an issue if anyone else wants to use this.

Separate text formatting from size simplification and make the function
public so that it can easily be used by other clients.

We now can use humanize() for both downloaded size and download speed
calculation. One of the drawbacks is that speed will no look like this
when download is stalled: "0 bytes/s" instead of "0 KiB/s".

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 progress.c |   60 ++++++++++++++++++++++++++++++++++--------------------------
 progress.h |    2 ++
 2 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/progress.c b/progress.c
index 3971f49..76c1e42 100644
--- a/progress.c
+++ b/progress.c
@@ -8,8 +8,11 @@
  * published by the Free Software Foundation.
  */
 
+#include <string.h>
+
 #include "git-compat-util.h"
 #include "progress.h"
+#include "strbuf.h"
 
 #define TP_IDX_MAX      8
 
@@ -112,34 +115,33 @@ static int display(struct progress *progress, unsigned n, const char *done)
 	return 0;
 }
 
-static void throughput_string(struct throughput *tp, off_t total,
-			      unsigned int rate)
+void humanize(struct strbuf *buf, off_t bytes)
 {
-	int l = sizeof(tp->display);
-	if (total > 1 << 30) {
-		l -= snprintf(tp->display, l, ", %u.%2.2u GiB",
-			      (int)(total >> 30),
-			      (int)(total & ((1 << 30) - 1)) / 10737419);
-	} else if (total > 1 << 20) {
-		int x = total + 5243;  /* for rounding */
-		l -= snprintf(tp->display, l, ", %u.%2.2u MiB",
-			      x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);
-	} else if (total > 1 << 10) {
-		int x = total + 5;  /* for rounding */
-		l -= snprintf(tp->display, l, ", %u.%2.2u KiB",
-			      x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
+	if (bytes > 1 << 30) {
+		strbuf_addf(buf, "%u.%2.2u GiB",
+			    (int)(bytes >> 30),
+			    (int)(bytes & ((1 << 30) - 1)) / 10737419);
+	} else if (bytes > 1 << 20) {
+		int x = bytes + 5243;  /* for rounding */
+		strbuf_addf(buf, "%u.%2.2u MiB",
+			    x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);
+	} else if (bytes > 1 << 10) {
+		int x = bytes + 5;  /* for rounding */
+		strbuf_addf(buf, "%u.%2.2u KiB",
+			    x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
 	} else {
-		l -= snprintf(tp->display, l, ", %u bytes", (int)total);
+		strbuf_addf(buf, "%u bytes", (int)bytes);
 	}
+}
 
-	if (rate > 1 << 10) {
-		int x = rate + 5;  /* for rounding */
-		snprintf(tp->display + sizeof(tp->display) - l, l,
-			 " | %u.%2.2u MiB/s",
-			 x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
-	} else if (rate)
-		snprintf(tp->display + sizeof(tp->display) - l, l,
-			 " | %u KiB/s", rate);
+static void throughput_string(struct strbuf *buf, off_t total,
+			      unsigned int rate)
+{
+	strbuf_addstr(buf, ", ");
+	humanize(buf, total);
+	strbuf_addstr(buf, " | ");
+	humanize(buf, rate * 1024);
+	strbuf_addstr(buf, "/s");
 }
 
 void display_throughput(struct progress *progress, off_t total)
@@ -183,6 +185,7 @@ void display_throughput(struct progress *progress, off_t total)
 	misecs += (int)(tv.tv_usec - tp->prev_tv.tv_usec) / 977;
 
 	if (misecs > 512) {
+		struct strbuf buf = STRBUF_INIT;
 		unsigned int count, rate;
 
 		count = total - tp->prev_total;
@@ -197,7 +200,9 @@ void display_throughput(struct progress *progress, off_t total)
 		tp->last_misecs[tp->idx] = misecs;
 		tp->idx = (tp->idx + 1) % TP_IDX_MAX;
 
-		throughput_string(tp, total, rate);
+		throughput_string(&buf, total, rate);
+		strncpy(tp->display, buf.buf, sizeof(tp->display));
+		strbuf_release(&buf);
 		if (progress->last_value != -1 && progress_update)
 			display(progress, progress->last_value, NULL);
 	}
@@ -253,9 +258,12 @@ void stop_progress_msg(struct progress **p_progress, const char *msg)
 
 		bufp = (len < sizeof(buf)) ? buf : xmalloc(len + 1);
 		if (tp) {
+			struct strbuf strbuf = STRBUF_INIT;
 			unsigned int rate = !tp->avg_misecs ? 0 :
 					tp->avg_bytes / tp->avg_misecs;
-			throughput_string(tp, tp->curr_total, rate);
+			throughput_string(&strbuf, tp->curr_total, rate);
+			strncpy(tp->display, strbuf.buf, sizeof(tp->display));
+			strbuf_release(&strbuf);
 		}
 		progress_update = 1;
 		sprintf(bufp, ", %s.\n", msg);
diff --git a/progress.h b/progress.h
index 611e4c4..0e70f55 100644
--- a/progress.h
+++ b/progress.h
@@ -2,7 +2,9 @@
 #define PROGRESS_H
 
 struct progress;
+struct strbuf;
 
+void humanize(struct strbuf *buf, off_t bytes);
 void display_throughput(struct progress *progress, off_t total);
 int display_progress(struct progress *progress, unsigned n);
 struct progress *start_progress(const char *title, unsigned total);
-- 
1.7.9.5

[PATCH 2/2] count-objects: add -H option to humanize sizes

[Antoine Pelisse]

Use the new humanize() function to print loose objects size, pack size,
and garbage size in verbose mode, or loose objects size in regular mode.
This patch doesn't change the way anything is displayed when the option
is not used.

Also update the documentation.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 Documentation/git-count-objects.txt |   14 ++++++++---
 builtin/count-objects.c             |   47 +++++++++++++++++++++++++++++------
 2 files changed, 49 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt
index da6e72e..b300e84 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -8,7 +8,7 @@ git-count-objects - Count unpacked number of objects and their disk consumption
 SYNOPSIS
 --------
 [verse]
-'git count-objects' [-v]
+'git count-objects' [-v] [-H | --human-readable]
 
 DESCRIPTION
 -----------
@@ -24,11 +24,11 @@ OPTIONS
 +
 count: the number of loose objects
 +
-size: disk space consumed by loose objects, in KiB
+size: disk space consumed by loose objects, in KiB (unless -H is specified)
 +
 in-pack: the number of in-pack objects
 +
-size-pack: disk space consumed by the packs, in KiB
+size-pack: disk space consumed by the packs, in KiB (unless -H is specified)
 +
 prune-packable: the number of loose objects that are also present in
 the packs. These objects could be pruned using `git prune-packed`.
@@ -36,7 +36,13 @@ the packs. These objects could be pruned using `git prune-packed`.
 garbage: the number of files in object database that are not valid
 loose objects nor valid packs
 +
-size-garbage: disk space consumed by garbage files, in KiB
+size-garbage: disk space consumed by garbage files, in KiB (unless -H is
+specified)
+
+-H::
+--human-readable::
+
+Print sizes in human readable format
 
 GIT
 ---
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 0343e35..9836f6a 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -8,6 +8,7 @@
 #include "dir.h"
 #include "builtin.h"
 #include "parse-options.h"
+#include "progress.h"
 
 static unsigned long garbage;
 static off_t size_garbage;
@@ -79,13 +80,13 @@ static void count_objects(DIR *d, char *path, int len, int verbose,
 }
 
 static char const * const count_objects_usage[] = {
-	N_("git count-objects [-v]"),
+	N_("git count-objects [-v] [-H | --human-readable]"),
 	NULL
 };
 
 int cmd_count_objects(int argc, const char **argv, const char *prefix)
 {
-	int i, verbose = 0;
+	int i, verbose = 0, human_readable = 0;
 	const char *objdir = get_object_directory();
 	int len = strlen(objdir);
 	char *path = xmalloc(len + 50);
@@ -93,6 +94,8 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
 	off_t loose_size = 0;
 	struct option opts[] = {
 		OPT__VERBOSE(&verbose, N_("be verbose")),
+		OPT_BOOLEAN('H', "human-readable", &human_readable,
+			    N_("print sizes in human readable format")),
 		OPT_END(),
 	};
 
@@ -119,6 +122,9 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
 		struct packed_git *p;
 		unsigned long num_pack = 0;
 		off_t size_pack = 0;
+		struct strbuf loose_buf = STRBUF_INIT;
+		struct strbuf pack_buf = STRBUF_INIT;
+		struct strbuf garbage_buf = STRBUF_INIT;
 		if (!packed_git)
 			prepare_packed_git();
 		for (p = packed_git; p; p = p->next) {
@@ -130,17 +136,42 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
 			size_pack += p->pack_size + p->index_size;
 			num_pack++;
 		}
+
+		if (human_readable) {
+			humanize(&loose_buf, loose_size);
+			humanize(&pack_buf, size_pack);
+			humanize(&garbage_buf, size_garbage);
+		}
+		else {
+			strbuf_addf(&loose_buf, "%lu",
+				    (unsigned long)(loose_size / 1024));
+			strbuf_addf(&pack_buf, "%lu",
+				    (unsigned long)(size_pack / 1024));
+			strbuf_addf(&garbage_buf, "%lu",
+				    (unsigned long)(size_garbage / 1024));
+		}
+
 		printf("count: %lu\n", loose);
-		printf("size: %lu\n", (unsigned long) (loose_size / 1024));
+		printf("size: %s\n", loose_buf.buf);
 		printf("in-pack: %lu\n", packed);
 		printf("packs: %lu\n", num_pack);
-		printf("size-pack: %lu\n", (unsigned long) (size_pack / 1024));
+		printf("size-pack: %s\n", pack_buf.buf);
 		printf("prune-packable: %lu\n", packed_loose);
 		printf("garbage: %lu\n", garbage);
-		printf("size-garbage: %lu\n", (unsigned long) (size_garbage / 1024));
+		printf("size-garbage: %s\n", garbage_buf.buf);
+		strbuf_release(&loose_buf);
+		strbuf_release(&pack_buf);
+		strbuf_release(&garbage_buf);
+	}
+	else {
+		struct strbuf buf = STRBUF_INIT;
+		if (human_readable)
+			humanize(&buf, loose_size);
+		else
+			strbuf_addf(&buf, "%lu KiB",
+				    (unsigned long)(loose_size / 1024));
+		printf("%lu objects, %s\n", loose, buf.buf);
+		strbuf_release(&buf);
 	}
-	else
-		printf("%lu objects, %lu KiB\n",
-		       loose, (unsigned long) (loose_size / 1024));
 	return 0;
 }
-- 
1.7.9.5

Re: [PATCH 1/2] progress: create public humanize() to show sizes

[Junio C Hamano]

Antoine Pelisse <apelisse@gmail.com> writes:

> Currently, humanization of downloaded size is done in the same function
> as text formatting. This is an issue if anyone else wants to use this.
>
> Separate text formatting from size simplification and make the function
> public so that it can easily be used by other clients.
>
> We now can use humanize() for both downloaded size and download speed
> calculation. One of the drawbacks is that speed will no look like this
> when download is stalled: "0 bytes/s" instead of "0 KiB/s".
>
> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
> ---

Sounds good, but I think this helper function should live in
strbuf.[ch], where many other text/string helpers that take a strbuf
as their first parameter live.

>  progress.c |   60 ++++++++++++++++++++++++++++++++++--------------------------
>  progress.h |    2 ++
>  2 files changed, 36 insertions(+), 26 deletions(-)
>
> diff --git a/progress.c b/progress.c
> index 3971f49..76c1e42 100644
> --- a/progress.c
> +++ b/progress.c
> @@ -8,8 +8,11 @@
>   * published by the Free Software Foundation.
>   */
>  
> +#include <string.h>
> +

Please do not do this.

If you somehow need to have <string.h>, a suitable place should be
found in git-compat-util.h; various platforms seem to have quirks
with the ordering of system header files, and git-compat-util.h is
meant to encapsulate them.

In fact, git-compat-util.h should already include it.

>  #include "git-compat-util.h"
>  #include "progress.h"
> +#include "strbuf.h"
>  
>  #define TP_IDX_MAX      8
>  
> @@ -112,34 +115,33 @@ static int display(struct progress *progress, unsigned n, const char *done)
>  	return 0;
>  }
>  
> -static void throughput_string(struct throughput *tp, off_t total,
> -			      unsigned int rate)
> +void humanize(struct strbuf *buf, off_t bytes)
>  {
> -	int l = sizeof(tp->display);
> -	if (total > 1 << 30) {
> -		l -= snprintf(tp->display, l, ", %u.%2.2u GiB",
> -			      (int)(total >> 30),
> -			      (int)(total & ((1 << 30) - 1)) / 10737419);
> -	} else if (total > 1 << 20) {
> -		int x = total + 5243;  /* for rounding */
> -		l -= snprintf(tp->display, l, ", %u.%2.2u MiB",
> -			      x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);
> -	} else if (total > 1 << 10) {
> -		int x = total + 5;  /* for rounding */
> -		l -= snprintf(tp->display, l, ", %u.%2.2u KiB",
> -			      x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
> +	if (bytes > 1 << 30) {
> +		strbuf_addf(buf, "%u.%2.2u GiB",
> +			    (int)(bytes >> 30),
> +			    (int)(bytes & ((1 << 30) - 1)) / 10737419);
> +	} else if (bytes > 1 << 20) {
> +		int x = bytes + 5243;  /* for rounding */
> +		strbuf_addf(buf, "%u.%2.2u MiB",
> +			    x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);
> +	} else if (bytes > 1 << 10) {
> +		int x = bytes + 5;  /* for rounding */
> +		strbuf_addf(buf, "%u.%2.2u KiB",
> +			    x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
>  	} else {
> -		l -= snprintf(tp->display, l, ", %u bytes", (int)total);
> +		strbuf_addf(buf, "%u bytes", (int)bytes);
>  	}
> +}
>  
> -	if (rate > 1 << 10) {
> -		int x = rate + 5;  /* for rounding */
> -		snprintf(tp->display + sizeof(tp->display) - l, l,
> -			 " | %u.%2.2u MiB/s",
> -			 x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
> -	} else if (rate)
> -		snprintf(tp->display + sizeof(tp->display) - l, l,
> -			 " | %u KiB/s", rate);
> +static void throughput_string(struct strbuf *buf, off_t total,
> +			      unsigned int rate)
> +{
> +	strbuf_addstr(buf, ", ");
> +	humanize(buf, total);
> +	strbuf_addstr(buf, " | ");
> +	humanize(buf, rate * 1024);
> +	strbuf_addstr(buf, "/s");
>  }
>  
>  void display_throughput(struct progress *progress, off_t total)
> @@ -183,6 +185,7 @@ void display_throughput(struct progress *progress, off_t total)
>  	misecs += (int)(tv.tv_usec - tp->prev_tv.tv_usec) / 977;
>  
>  	if (misecs > 512) {
> +		struct strbuf buf = STRBUF_INIT;
>  		unsigned int count, rate;
>  
>  		count = total - tp->prev_total;
> @@ -197,7 +200,9 @@ void display_throughput(struct progress *progress, off_t total)
>  		tp->last_misecs[tp->idx] = misecs;
>  		tp->idx = (tp->idx + 1) % TP_IDX_MAX;
>  
> -		throughput_string(tp, total, rate);
> +		throughput_string(&buf, total, rate);
> +		strncpy(tp->display, buf.buf, sizeof(tp->display));
> +		strbuf_release(&buf);
>  		if (progress->last_value != -1 && progress_update)
>  			display(progress, progress->last_value, NULL);
>  	}
> @@ -253,9 +258,12 @@ void stop_progress_msg(struct progress **p_progress, const char *msg)
>  
>  		bufp = (len < sizeof(buf)) ? buf : xmalloc(len + 1);
>  		if (tp) {
> +			struct strbuf strbuf = STRBUF_INIT;
>  			unsigned int rate = !tp->avg_misecs ? 0 :
>  					tp->avg_bytes / tp->avg_misecs;
> -			throughput_string(tp, tp->curr_total, rate);
> +			throughput_string(&strbuf, tp->curr_total, rate);
> +			strncpy(tp->display, strbuf.buf, sizeof(tp->display));
> +			strbuf_release(&strbuf);
>  		}
>  		progress_update = 1;
>  		sprintf(bufp, ", %s.\n", msg);
> diff --git a/progress.h b/progress.h
> index 611e4c4..0e70f55 100644
> --- a/progress.h
> +++ b/progress.h
> @@ -2,7 +2,9 @@
>  #define PROGRESS_H
>  
>  struct progress;
> +struct strbuf;
>  
> +void humanize(struct strbuf *buf, off_t bytes);
>  void display_throughput(struct progress *progress, off_t total);
>  int display_progress(struct progress *progress, unsigned n);
>  struct progress *start_progress(const char *title, unsigned total);

[PATCH 1/2] strbuf: create strbuf_humanize() to show byte sizes

[Antoine Pelisse]

Currently, humanization of downloaded size is done in the same
function as text formatting in 'process.c'. This is an issue if anyone
else wants to use this.

Separate text formatting from size simplification and make the function
public in strbuf so that it can easily be used by other clients.

We now can use strbuf_humanize() for both downloaded size and download
speed calculation. One of the drawbacks is that speed will now look like
this when download is stalled: "0 bytes/s" instead of "0 KiB/s".

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 Documentation/technical/api-strbuf.txt |    5 ++++
 progress.c                             |   43 +++++++++++---------------------
 strbuf.c                               |   19 ++++++++++++++
 strbuf.h                               |    1 +
 4 files changed, 40 insertions(+), 28 deletions(-)

diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt
index 2c59cb2..7b6ecda 100644
--- a/Documentation/technical/api-strbuf.txt
+++ b/Documentation/technical/api-strbuf.txt
@@ -230,6 +230,11 @@ which can be used by the programmer of the callback as she sees fit.
 	destination. This is useful for literal data to be fed to either
 	strbuf_expand or to the *printf family of functions.
 
+`strbuf_humanize`::
+
+	Append the given byte size as a human-readable string (i.e. 12.23 KiB,
+	3.50 MiB).
+
 `strbuf_addf`::
 
 	Add a formatted string to the buffer.
diff --git a/progress.c b/progress.c
index 3971f49..8e09058 100644
--- a/progress.c
+++ b/progress.c
@@ -10,6 +10,7 @@
 
 #include "git-compat-util.h"
 #include "progress.h"
+#include "strbuf.h"
 
 #define TP_IDX_MAX      8
 
@@ -112,34 +113,14 @@ static int display(struct progress *progress, unsigned n, const char *done)
 	return 0;
 }
 
-static void throughput_string(struct throughput *tp, off_t total,
+static void throughput_string(struct strbuf *buf, off_t total,
 			      unsigned int rate)
 {
-	int l = sizeof(tp->display);
-	if (total > 1 << 30) {
-		l -= snprintf(tp->display, l, ", %u.%2.2u GiB",
-			      (int)(total >> 30),
-			      (int)(total & ((1 << 30) - 1)) / 10737419);
-	} else if (total > 1 << 20) {
-		int x = total + 5243;  /* for rounding */
-		l -= snprintf(tp->display, l, ", %u.%2.2u MiB",
-			      x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);
-	} else if (total > 1 << 10) {
-		int x = total + 5;  /* for rounding */
-		l -= snprintf(tp->display, l, ", %u.%2.2u KiB",
-			      x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
-	} else {
-		l -= snprintf(tp->display, l, ", %u bytes", (int)total);
-	}
-
-	if (rate > 1 << 10) {
-		int x = rate + 5;  /* for rounding */
-		snprintf(tp->display + sizeof(tp->display) - l, l,
-			 " | %u.%2.2u MiB/s",
-			 x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
-	} else if (rate)
-		snprintf(tp->display + sizeof(tp->display) - l, l,
-			 " | %u KiB/s", rate);
+	strbuf_addstr(buf, ", ");
+	strbuf_humanize(buf, total);
+	strbuf_addstr(buf, " | ");
+	strbuf_humanize(buf, rate * 1024);
+	strbuf_addstr(buf, "/s");
 }
 
 void display_throughput(struct progress *progress, off_t total)
@@ -183,6 +164,7 @@ void display_throughput(struct progress *progress, off_t total)
 	misecs += (int)(tv.tv_usec - tp->prev_tv.tv_usec) / 977;
 
 	if (misecs > 512) {
+		struct strbuf buf = STRBUF_INIT;
 		unsigned int count, rate;
 
 		count = total - tp->prev_total;
@@ -197,7 +179,9 @@ void display_throughput(struct progress *progress, off_t total)
 		tp->last_misecs[tp->idx] = misecs;
 		tp->idx = (tp->idx + 1) % TP_IDX_MAX;
 
-		throughput_string(tp, total, rate);
+		throughput_string(&buf, total, rate);
+		strncpy(tp->display, buf.buf, sizeof(tp->display));
+		strbuf_release(&buf);
 		if (progress->last_value != -1 && progress_update)
 			display(progress, progress->last_value, NULL);
 	}
@@ -253,9 +237,12 @@ void stop_progress_msg(struct progress **p_progress, const char *msg)
 
 		bufp = (len < sizeof(buf)) ? buf : xmalloc(len + 1);
 		if (tp) {
+			struct strbuf strbuf = STRBUF_INIT;
 			unsigned int rate = !tp->avg_misecs ? 0 :
 					tp->avg_bytes / tp->avg_misecs;
-			throughput_string(tp, tp->curr_total, rate);
+			throughput_string(&strbuf, tp->curr_total, rate);
+			strncpy(tp->display, strbuf.buf, sizeof(tp->display));
+			strbuf_release(&strbuf);
 		}
 		progress_update = 1;
 		sprintf(bufp, ", %s.\n", msg);
diff --git a/strbuf.c b/strbuf.c
index 48e9abb..8a50e66 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -528,6 +528,25 @@ void strbuf_addstr_urlencode(struct strbuf *sb, const char *s,
 	strbuf_add_urlencode(sb, s, strlen(s), reserved);
 }
 
+void strbuf_humanize(struct strbuf *buf, off_t bytes)
+{
+	if (bytes > 1 << 30) {
+		strbuf_addf(buf, "%u.%2.2u GiB",
+			    (int)(bytes >> 30),
+			    (int)(bytes & ((1 << 30) - 1)) / 10737419);
+	} else if (bytes > 1 << 20) {
+		int x = bytes + 5243;  /* for rounding */
+		strbuf_addf(buf, "%u.%2.2u MiB",
+			    x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);
+	} else if (bytes > 1 << 10) {
+		int x = bytes + 5;  /* for rounding */
+		strbuf_addf(buf, "%u.%2.2u KiB",
+			    x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
+	} else {
+		strbuf_addf(buf, "%u bytes", (int)bytes);
+	}
+}
+
 int printf_ln(const char *fmt, ...)
 {
 	int ret;
diff --git a/strbuf.h b/strbuf.h
index 958822c..317c5a8 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -170,6 +170,7 @@ extern int strbuf_check_branch_ref(struct strbuf *sb, const char *name);
 
 extern void strbuf_addstr_urlencode(struct strbuf *, const char *,
 				    int reserved);
+extern void strbuf_humanize(struct strbuf *buf, off_t bytes);
 
 __attribute__((format (printf,1,2)))
 extern int printf_ln(const char *fmt, ...);
-- 
1.7.9.5

[PATCH 2/2] count-objects: add -H option to humanize sizes

[Antoine Pelisse]

Use the new humanize() function to print loose objects size, pack size,
and garbage size in verbose mode, or loose objects size in regular mode.
This patch doesn't change the way anything is displayed when the option
is not used.

Also update the documentation.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
 Documentation/git-count-objects.txt |   14 ++++++++---
 builtin/count-objects.c             |   46 +++++++++++++++++++++++++++++------
 2 files changed, 48 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt
index da6e72e..b300e84 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -8,7 +8,7 @@ git-count-objects - Count unpacked number of objects and their disk consumption
 SYNOPSIS
 --------
 [verse]
-'git count-objects' [-v]
+'git count-objects' [-v] [-H | --human-readable]
 
 DESCRIPTION
 -----------
@@ -24,11 +24,11 @@ OPTIONS
 +
 count: the number of loose objects
 +
-size: disk space consumed by loose objects, in KiB
+size: disk space consumed by loose objects, in KiB (unless -H is specified)
 +
 in-pack: the number of in-pack objects
 +
-size-pack: disk space consumed by the packs, in KiB
+size-pack: disk space consumed by the packs, in KiB (unless -H is specified)
 +
 prune-packable: the number of loose objects that are also present in
 the packs. These objects could be pruned using `git prune-packed`.
@@ -36,7 +36,13 @@ the packs. These objects could be pruned using `git prune-packed`.
 garbage: the number of files in object database that are not valid
 loose objects nor valid packs
 +
-size-garbage: disk space consumed by garbage files, in KiB
+size-garbage: disk space consumed by garbage files, in KiB (unless -H is
+specified)
+
+-H::
+--human-readable::
+
+Print sizes in human readable format
 
 GIT
 ---
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 0343e35..935ad9e 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -79,13 +79,13 @@ static void count_objects(DIR *d, char *path, int len, int verbose,
 }
 
 static char const * const count_objects_usage[] = {
-	N_("git count-objects [-v]"),
+	N_("git count-objects [-v] [-H | --human-readable]"),
 	NULL
 };
 
 int cmd_count_objects(int argc, const char **argv, const char *prefix)
 {
-	int i, verbose = 0;
+	int i, verbose = 0, human_readable = 0;
 	const char *objdir = get_object_directory();
 	int len = strlen(objdir);
 	char *path = xmalloc(len + 50);
@@ -93,6 +93,8 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
 	off_t loose_size = 0;
 	struct option opts[] = {
 		OPT__VERBOSE(&verbose, N_("be verbose")),
+		OPT_BOOLEAN('H', "human-readable", &human_readable,
+			    N_("print sizes in human readable format")),
 		OPT_END(),
 	};
 
@@ -119,6 +121,9 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
 		struct packed_git *p;
 		unsigned long num_pack = 0;
 		off_t size_pack = 0;
+		struct strbuf loose_buf = STRBUF_INIT;
+		struct strbuf pack_buf = STRBUF_INIT;
+		struct strbuf garbage_buf = STRBUF_INIT;
 		if (!packed_git)
 			prepare_packed_git();
 		for (p = packed_git; p; p = p->next) {
@@ -130,17 +135,42 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
 			size_pack += p->pack_size + p->index_size;
 			num_pack++;
 		}
+
+		if (human_readable) {
+			strbuf_humanize(&loose_buf, loose_size);
+			strbuf_humanize(&pack_buf, size_pack);
+			strbuf_humanize(&garbage_buf, size_garbage);
+		}
+		else {
+			strbuf_addf(&loose_buf, "%lu",
+				    (unsigned long)(loose_size / 1024));
+			strbuf_addf(&pack_buf, "%lu",
+				    (unsigned long)(size_pack / 1024));
+			strbuf_addf(&garbage_buf, "%lu",
+				    (unsigned long)(size_garbage / 1024));
+		}
+
 		printf("count: %lu\n", loose);
-		printf("size: %lu\n", (unsigned long) (loose_size / 1024));
+		printf("size: %s\n", loose_buf.buf);
 		printf("in-pack: %lu\n", packed);
 		printf("packs: %lu\n", num_pack);
-		printf("size-pack: %lu\n", (unsigned long) (size_pack / 1024));
+		printf("size-pack: %s\n", pack_buf.buf);
 		printf("prune-packable: %lu\n", packed_loose);
 		printf("garbage: %lu\n", garbage);
-		printf("size-garbage: %lu\n", (unsigned long) (size_garbage / 1024));
+		printf("size-garbage: %s\n", garbage_buf.buf);
+		strbuf_release(&loose_buf);
+		strbuf_release(&pack_buf);
+		strbuf_release(&garbage_buf);
+	}
+	else {
+		struct strbuf buf = STRBUF_INIT;
+		if (human_readable)
+			strbuf_humanize(&buf, loose_size);
+		else
+			strbuf_addf(&buf, "%lu KiB",
+				    (unsigned long)(loose_size / 1024));
+		printf("%lu objects, %s\n", loose, buf.buf);
+		strbuf_release(&buf);
 	}
-	else
-		printf("%lu objects, %lu KiB\n",
-		       loose, (unsigned long) (loose_size / 1024));
 	return 0;
 }
-- 
1.7.9.5

Re: [PATCH 1/2] strbuf: create strbuf_humanize() to show byte sizes

[Jonathan Nieder]

Antoine Pelisse wrote:

> Separate text formatting from size simplification and make the function
> public in strbuf so that it can easily be used by other clients.
>
> We now can use strbuf_humanize() for both downloaded size and download
> speed calculation.

Sounds like a good thing to do.

>                    One of the drawbacks is that speed will now look like
> this when download is stalled: "0 bytes/s" instead of "0 KiB/s".

At first glance that is neither obviously a benefit nor obviously a
drawback.  Can you spell this out more?

> --- a/Documentation/technical/api-strbuf.txt
> +++ b/Documentation/technical/api-strbuf.txt
> @@ -230,6 +230,11 @@ which can be used by the programmer of the callback as she sees fit.
>  	destination. This is useful for literal data to be fed to either
>  	strbuf_expand or to the *printf family of functions.
>  
> +`strbuf_humanize`::
> +
> +	Append the given byte size as a human-readable string (i.e. 12.23 KiB,
> +	3.50 MiB).

Based on the function name alone, it is not easy to guess what it will
do (e.g., maybe it will paraphrase 3 to "three" and 10000000 to
"enormous").  How about something like strbuf_filesize?

If I understand the code correctly, this jumps units each time it
exceeds 1.0 of the next unit (bytes, KiB, MiB, GiB), which sounds like
a fine behavior.

Hope that helps,
Jonathan

Re: [PATCH 1/2] strbuf: create strbuf_humanize() to show byte sizes

[Antoine Pelisse]

On Wed, Apr 10, 2013 at 9:43 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Antoine Pelisse wrote:
>>                    One of the drawbacks is that speed will now look like
>> this when download is stalled: "0 bytes/s" instead of "0 KiB/s".
>
> At first glance that is neither obviously a benefit nor obviously a
> drawback.  Can you spell this out more?

The drawback to me is that it changes the user experience with no reason.
But that's really a minor change, I agree. (maybe I should have put it
as a comment/question after ---)

>> --- a/Documentation/technical/api-strbuf.txt
>> +++ b/Documentation/technical/api-strbuf.txt
>> @@ -230,6 +230,11 @@ which can be used by the programmer of the callback as she sees fit.
>>       destination. This is useful for literal data to be fed to either
>>       strbuf_expand or to the *printf family of functions.
>>
>> +`strbuf_humanize`::
>> +
>> +     Append the given byte size as a human-readable string (i.e. 12.23 KiB,
>> +     3.50 MiB).
>
> Based on the function name alone, it is not easy to guess what it will
> do (e.g., maybe it will paraphrase 3 to "three" and 10000000 to
> "enormous").  How about something like strbuf_filesize?

I think the suggestion from Junio makes more sense, as it can be used
for download speed.

> If I understand the code correctly, this jumps units each time it
> exceeds 1.0 of the next unit (bytes, KiB, MiB, GiB), which sounds like
> a fine behavior.

The code has simply been extracted from the former function and kept unmodified.

Thanks for the help !
Antoine,

Re: [PATCH 1/2] strbuf: create strbuf_humanize() to show byte sizes

[Junio C Hamano]

Antoine Pelisse <apelisse@gmail.com> writes:

> Currently, humanization of downloaded size is done in the same
> function as text formatting in 'process.c'. This is an issue if anyone
> else wants to use this.
>
> Separate text formatting from size simplification and make the function
> public in strbuf so that it can easily be used by other clients.
>
> We now can use strbuf_humanize() for both downloaded size and download
> speed calculation. One of the drawbacks is that speed will now look like
> this when download is stalled: "0 bytes/s" instead of "0 KiB/s".

Personally, I do not think the "drawback" is so big an issue.  If
the caller really cares, we could always add another parameter to
this formatter to tell it the minimum unit we care about (e.g. pass
1024 to say "Don't bother showing scale lower than kibi").

This is a bit late response, but if we ever want to count something
in a dimention other than "bytes", like time (e.g. "kiloseconds") or
number of commits (e.g. "centicommits"), etc., we cannot reuse this
formatter very easily.  We may want to have "byte" somewhere in its
name for now to make sure the callers understand its limitation.

I'll tentatively rename it to "strbuf_humanize_bytes()" while queuing.

Thanks.

> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
> ---
>  Documentation/technical/api-strbuf.txt |    5 ++++
>  progress.c                             |   43 +++++++++++---------------------
>  strbuf.c                               |   19 ++++++++++++++
>  strbuf.h                               |    1 +
>  4 files changed, 40 insertions(+), 28 deletions(-)
>
> diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt
> index 2c59cb2..7b6ecda 100644
> --- a/Documentation/technical/api-strbuf.txt
> +++ b/Documentation/technical/api-strbuf.txt
> @@ -230,6 +230,11 @@ which can be used by the programmer of the callback as she sees fit.
>  	destination. This is useful for literal data to be fed to either
>  	strbuf_expand or to the *printf family of functions.
>  
> +`strbuf_humanize`::
> +
> +	Append the given byte size as a human-readable string (i.e. 12.23 KiB,
> +	3.50 MiB).
> +
>  `strbuf_addf`::
>  
>  	Add a formatted string to the buffer.
> diff --git a/progress.c b/progress.c
> index 3971f49..8e09058 100644
> --- a/progress.c
> +++ b/progress.c
> @@ -10,6 +10,7 @@
>  
>  #include "git-compat-util.h"
>  #include "progress.h"
> +#include "strbuf.h"
>  
>  #define TP_IDX_MAX      8
>  
> @@ -112,34 +113,14 @@ static int display(struct progress *progress, unsigned n, const char *done)
>  	return 0;
>  }
>  
> -static void throughput_string(struct throughput *tp, off_t total,
> +static void throughput_string(struct strbuf *buf, off_t total,
>  			      unsigned int rate)
>  {
> -	int l = sizeof(tp->display);
> -	if (total > 1 << 30) {
> -		l -= snprintf(tp->display, l, ", %u.%2.2u GiB",
> -			      (int)(total >> 30),
> -			      (int)(total & ((1 << 30) - 1)) / 10737419);
> -	} else if (total > 1 << 20) {
> -		int x = total + 5243;  /* for rounding */
> -		l -= snprintf(tp->display, l, ", %u.%2.2u MiB",
> -			      x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);
> -	} else if (total > 1 << 10) {
> -		int x = total + 5;  /* for rounding */
> -		l -= snprintf(tp->display, l, ", %u.%2.2u KiB",
> -			      x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
> -	} else {
> -		l -= snprintf(tp->display, l, ", %u bytes", (int)total);
> -	}
> -
> -	if (rate > 1 << 10) {
> -		int x = rate + 5;  /* for rounding */
> -		snprintf(tp->display + sizeof(tp->display) - l, l,
> -			 " | %u.%2.2u MiB/s",
> -			 x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
> -	} else if (rate)
> -		snprintf(tp->display + sizeof(tp->display) - l, l,
> -			 " | %u KiB/s", rate);
> +	strbuf_addstr(buf, ", ");
> +	strbuf_humanize(buf, total);
> +	strbuf_addstr(buf, " | ");
> +	strbuf_humanize(buf, rate * 1024);
> +	strbuf_addstr(buf, "/s");
>  }
>  
>  void display_throughput(struct progress *progress, off_t total)
> @@ -183,6 +164,7 @@ void display_throughput(struct progress *progress, off_t total)
>  	misecs += (int)(tv.tv_usec - tp->prev_tv.tv_usec) / 977;
>  
>  	if (misecs > 512) {
> +		struct strbuf buf = STRBUF_INIT;
>  		unsigned int count, rate;
>  
>  		count = total - tp->prev_total;
> @@ -197,7 +179,9 @@ void display_throughput(struct progress *progress, off_t total)
>  		tp->last_misecs[tp->idx] = misecs;
>  		tp->idx = (tp->idx + 1) % TP_IDX_MAX;
>  
> -		throughput_string(tp, total, rate);
> +		throughput_string(&buf, total, rate);
> +		strncpy(tp->display, buf.buf, sizeof(tp->display));
> +		strbuf_release(&buf);
>  		if (progress->last_value != -1 && progress_update)
>  			display(progress, progress->last_value, NULL);
>  	}
> @@ -253,9 +237,12 @@ void stop_progress_msg(struct progress **p_progress, const char *msg)
>  
>  		bufp = (len < sizeof(buf)) ? buf : xmalloc(len + 1);
>  		if (tp) {
> +			struct strbuf strbuf = STRBUF_INIT;
>  			unsigned int rate = !tp->avg_misecs ? 0 :
>  					tp->avg_bytes / tp->avg_misecs;
> -			throughput_string(tp, tp->curr_total, rate);
> +			throughput_string(&strbuf, tp->curr_total, rate);
> +			strncpy(tp->display, strbuf.buf, sizeof(tp->display));
> +			strbuf_release(&strbuf);
>  		}
>  		progress_update = 1;
>  		sprintf(bufp, ", %s.\n", msg);
> diff --git a/strbuf.c b/strbuf.c
> index 48e9abb..8a50e66 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -528,6 +528,25 @@ void strbuf_addstr_urlencode(struct strbuf *sb, const char *s,
>  	strbuf_add_urlencode(sb, s, strlen(s), reserved);
>  }
>  
> +void strbuf_humanize(struct strbuf *buf, off_t bytes)
> +{
> +	if (bytes > 1 << 30) {
> +		strbuf_addf(buf, "%u.%2.2u GiB",
> +			    (int)(bytes >> 30),
> +			    (int)(bytes & ((1 << 30) - 1)) / 10737419);
> +	} else if (bytes > 1 << 20) {
> +		int x = bytes + 5243;  /* for rounding */
> +		strbuf_addf(buf, "%u.%2.2u MiB",
> +			    x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);
> +	} else if (bytes > 1 << 10) {
> +		int x = bytes + 5;  /* for rounding */
> +		strbuf_addf(buf, "%u.%2.2u KiB",
> +			    x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
> +	} else {
> +		strbuf_addf(buf, "%u bytes", (int)bytes);
> +	}
> +}
> +
>  int printf_ln(const char *fmt, ...)
>  {
>  	int ret;
> diff --git a/strbuf.h b/strbuf.h
> index 958822c..317c5a8 100644
> --- a/strbuf.h
> +++ b/strbuf.h
> @@ -170,6 +170,7 @@ extern int strbuf_check_branch_ref(struct strbuf *sb, const char *name);
>  
>  extern void strbuf_addstr_urlencode(struct strbuf *, const char *,
>  				    int reserved);
> +extern void strbuf_humanize(struct strbuf *buf, off_t bytes);
>  
>  __attribute__((format (printf,1,2)))
>  extern int printf_ln(const char *fmt, ...);

Re: [PATCH 1/2] strbuf: create strbuf_humanize() to show byte sizes

[Antoine Pelisse]

On Wed, Apr 10, 2013 at 9:57 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Antoine Pelisse <apelisse@gmail.com> writes:
>
>> Currently, humanization of downloaded size is done in the same
>> function as text formatting in 'process.c'. This is an issue if anyone
>> else wants to use this.
>>
>> Separate text formatting from size simplification and make the function
>> public in strbuf so that it can easily be used by other clients.
>>
>> We now can use strbuf_humanize() for both downloaded size and download
>> speed calculation. One of the drawbacks is that speed will now look like
>> this when download is stalled: "0 bytes/s" instead of "0 KiB/s".
>
> Personally, I do not think the "drawback" is so big an issue.  If
> the caller really cares, we could always add another parameter to
> this formatter to tell it the minimum unit we care about (e.g. pass
> 1024 to say "Don't bother showing scale lower than kibi").

I thought about that, but decided it was not worth it (at least for the moment)

> This is a bit late response, but if we ever want to count something
> in a dimention other than "bytes", like time (e.g. "kiloseconds") or
> number of commits (e.g. "centicommits"), etc., we cannot reuse this
> formatter very easily.  We may want to have "byte" somewhere in its
> name for now to make sure the callers understand its limitation.

I'm not in a hurry.
But it look tough to make it generic: one is binary, another is
sexagesimal, and the last is decimal

> I'll tentatively rename it to "strbuf_humanize_bytes()" while queuing.

I like the idea,
Thanks,

Re: [PATCH 1/2] progress: create public humanize() to show sizes

[Eric Sunshine]

On Mon, Apr 8, 2013 at 2:18 PM, Antoine Pelisse <apelisse@gmail.com> wrote:
> Currently, humanization of downloaded size is done in the same function
> as text formatting. This is an issue if anyone else wants to use this.
>
> Separate text formatting from size simplification and make the function
> public so that it can easily be used by other clients.
>
> We now can use humanize() for both downloaded size and download speed
> calculation. One of the drawbacks is that speed will no look like this

s/no/now/

> when download is stalled: "0 bytes/s" instead of "0 KiB/s".
>
> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>