Re: [PATCH v2] count-objects: output "KiB" instead of "kilobytes"
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 wrote: > Mihai Capotă 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"
On Thu, Apr 4, 2013 at 6:27 PM, Junio C Hamano wrote: > Mihai Capotă 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"
Mihai Capotă 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"
On Wed, Apr 3, 2013 at 4:38 PM, Junio C Hamano wrote:
> Mihai Capotă 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"
Mihai Capotă 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ă
> ---
> 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
