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

2013-04-05 Thread Mihai Capotă
On Thu, Apr 4, 2013 at 6:27 PM, Junio C Hamano gits...@pobox.com wrote:
 Mihai Capotă mi...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-05 Thread 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 gits...@pobox.com wrote:
 Mihai Capotă mi...@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 majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-04 Thread Mihai Capotă
On Wed, Apr 3, 2013 at 4:38 PM, Junio C Hamano gits...@pobox.com wrote:
 Mihai Capotă mi...@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.

Mihai
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-04 Thread Junio C Hamano
Mihai Capotă mi...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-03 Thread Junio C Hamano
Mihai Capotă mi...@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ă mi...@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.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html