[RFC PATCH 0/5] ref-filter: add new formatting options

[RFC PATCH 0/5] ref-filter: add new formatting options

[Оля Тележная]

Add formatting options %(objectsize:disk) and %(deltabase), as in
cat-file command.

I can not test %(deltabase) properly (I mean, I want to have test with
meaningful deltabase in the result - now we have only with zeros). I
tested it manually on my git repo, and I have not-null deltabases
there. We have "t/t1006-cat-file.sh" with similar case, but it is
about blobs. ref-filter does not work with blobs, I need to write test
about refs, and I feel that I can't catch the idea (and it is hard for
me to write in Shell).

Finally, I want to remove formatting logic in cat-file and use
functions from ref-filter (we are almost there, so many work was done
for this). I had an idea to make this migration in this patch (and
stop worrying about bad tests about deltabase: we already have such
test for cat-file and hopefully that could be enough). But I have
another question there. cat-file has one more formatting option:
"rest" [1]. Do we want such formatting option in ref-filter? It's
easier for me to support that in ref-filter than to leave it only
specifically for cat-file.

Thank you!

[1] https://git-scm.com/docs/git-cat-file#git-cat-file-coderestcode

[RFC PATCH 3/5] ref-filter: add deltabase option

[Olga Telezhnaya]

Add new formatting option: deltabase.
If the object is stored as a delta on-disk, this expands
to the 40-hex sha1 of the delta base object.
Otherwise, expands to the null sha1 (40 zeroes).
We have same option in cat-file command.
Hopefully, in the end I will remove formatting code from
cat-file and reuse formatting parts from ref-filter.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
---
 ref-filter.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/ref-filter.c b/ref-filter.c
index 8ba1a4e72f2c3..3cfe01a039f8b 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -246,6 +246,18 @@ static int objectsize_atom_parser(const struct ref_format *format, struct used_a
 	return 0;
 }
 
+static int deltabase_atom_parser(const struct ref_format *format, struct used_atom *atom,
+				 const char *arg, struct strbuf *err)
+{
+	if (arg)
+		return strbuf_addf_ret(err, -1, _("%%(deltabase) does not take arguments"));
+	if (*atom->name == '*')
+		oi_deref.info.delta_base_sha1 = oi_deref.delta_base_oid.hash;
+	else
+		oi.info.delta_base_sha1 = oi.delta_base_oid.hash;
+	return 0;
+}
+
 static int body_atom_parser(const struct ref_format *format, struct used_atom *atom,
 			    const char *arg, struct strbuf *err)
 {
@@ -437,6 +449,7 @@ static struct {
 	{ "objecttype", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser },
 	{ "objectsize", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser },
 	{ "objectname", SOURCE_OTHER, FIELD_STR, objectname_atom_parser },
+	{ "deltabase", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser },
 	{ "tree", SOURCE_OBJ },
 	{ "parent", SOURCE_OBJ },
 	{ "numparent", SOURCE_OBJ, FIELD_ULONG },
@@ -888,7 +901,9 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_
 		} else if (!strcmp(name, "objectsize")) {
 			v->value = oi->size;
 			v->s = xstrfmt("%lu", oi->size);
-		} else if (deref)
+		} else if (!strcmp(name, "deltabase"))
+			v->s = xstrdup(oid_to_hex(&oi->delta_base_oid));
+		else if (deref)
 			grab_objectname(name, &oi->oid, v, &used_atom[i]);
 	}
 }

--
https://github.com/git/git/pull/552

[RFC PATCH 4/5] ref-filter: add tests for deltabase

[Olga Telezhnaya]

Test new formatting option deltabase.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
---
 t/t6300-for-each-ref.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 097fdf21fe196..0ffd63071392e 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -84,6 +84,7 @@ test_atom head push:strip=-1 master
 test_atom head objecttype commit
 test_atom head objectsize 171
 test_atom head objectsize:disk 138
+test_atom head deltabase 0000000000000000000000000000000000000000
 test_atom head objectname $(git rev-parse refs/heads/master)
 test_atom head objectname:short $(git rev-parse --short refs/heads/master)
 test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
@@ -127,6 +128,8 @@ test_atom tag objecttype tag
 test_atom tag objectsize 154
 test_atom tag objectsize:disk 138
 test_atom tag '*objectsize:disk' 138
+test_atom tag deltabase 0000000000000000000000000000000000000000
+test_atom tag '*deltabase' 0000000000000000000000000000000000000000
 test_atom tag objectname $(git rev-parse refs/tags/testtag)
 test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag)
 test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)

--
https://github.com/git/git/pull/552

[RFC PATCH 2/5] ref-filter: add tests for objectsize:disk

[Olga Telezhnaya]

Test new formatting atom.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
---
 t/t6300-for-each-ref.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 97bfbee6e8d69..097fdf21fe196 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -83,6 +83,7 @@ test_atom head push:strip=1 remotes/myfork/master
 test_atom head push:strip=-1 master
 test_atom head objecttype commit
 test_atom head objectsize 171
+test_atom head objectsize:disk 138
 test_atom head objectname $(git rev-parse refs/heads/master)
 test_atom head objectname:short $(git rev-parse --short refs/heads/master)
 test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
@@ -124,6 +125,8 @@ test_atom tag upstream ''
 test_atom tag push ''
 test_atom tag objecttype tag
 test_atom tag objectsize 154
+test_atom tag objectsize:disk 138
+test_atom tag '*objectsize:disk' 138
 test_atom tag objectname $(git rev-parse refs/tags/testtag)
 test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag)
 test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)

--
https://github.com/git/git/pull/552

[RFC PATCH 5/5] ref-filter: add docs for new options

[Olga Telezhnaya]

Add documentation for formatting options objectsize:disk
and deltabase.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
---
 Documentation/git-for-each-ref.txt | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 901faef1bfdce..22d2ff88110cd 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -128,13 +128,18 @@ objecttype::
 
 objectsize::
 	The size of the object (the same as 'git cat-file -s' reports).
-
+	Append `:disk` to get the size, in bytes, that the object takes up on
+	disk. See the note about on-disk sizes in the `CAVEATS` section below.
 objectname::
 	The object name (aka SHA-1).
 	For a non-ambiguous abbreviation of the object name append `:short`.
 	For an abbreviation of the object name with desired length append
 	`:short=<length>`, where the minimum length is MINIMUM_ABBREV. The
 	length may be exceeded to ensure unique object names.
+deltabase::
+	If the object is stored as a delta on-disk, this expands to the 40-hex
+	sha1 of the delta base object. Otherwise, expands to the null sha1
+	(40 zeroes). See `CAVEATS` section below.
 
 upstream::
 	The name of a local ref which can be considered ``upstream''
@@ -361,6 +366,20 @@ This prints the authorname, if present.
 git for-each-ref --format="%(refname)%(if)%(authorname)%(then) Authored by: %(authorname)%(end)"
 ------------
 
+CAVEATS
+-------
+
+Note that the sizes of objects on disk are reported accurately, but care
+should be taken in drawing conclusions about which refs or objects are
+responsible for disk usage. The size of a packed non-delta object may be
+much larger than the size of objects which delta against it, but the
+choice of which object is the base and which is the delta is arbitrary
+and is subject to change during a repack.
+
+Note also that multiple copies of an object may be present in the object
+database; in this case, it is undefined which copy's size or delta base
+will be reported.
+
 SEE ALSO
 --------
 linkgit:git-show-ref[1]

--
https://github.com/git/git/pull/552

[RFC PATCH 1/5] ref-filter: add objectsize:disk option

[Olga Telezhnaya]

Add new formatting option objectsize:disk to know
exact size that object takes up on disk.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
---
 ref-filter.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 0c45ed9d94a4b..8ba1a4e72f2c3 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -231,12 +231,18 @@ static int objecttype_atom_parser(const struct ref_format *format, struct used_a
 static int objectsize_atom_parser(const struct ref_format *format, struct used_atom *atom,
 				  const char *arg, struct strbuf *err)
 {
-	if (arg)
-		return strbuf_addf_ret(err, -1, _("%%(objectsize) does not take arguments"));
-	if (*atom->name == '*')
-		oi_deref.info.sizep = &oi_deref.size;
-	else
-		oi.info.sizep = &oi.size;
+	if (!arg) {
+		if (*atom->name == '*')
+			oi_deref.info.sizep = &oi_deref.size;
+		else
+			oi.info.sizep = &oi.size;
+	} else if (!strcmp(arg, "disk")) {
+		if (*atom->name == '*')
+			oi_deref.info.disk_sizep = &oi_deref.disk_size;
+		else
+			oi.info.disk_sizep = &oi.disk_size;
+	} else
+		return strbuf_addf_ret(err, -1, _("unrecognized %%(objectsize) argument: %s"), arg);
 	return 0;
 }
 
@@ -876,11 +882,13 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_
 			name++;
 		if (!strcmp(name, "objecttype"))
 			v->s = xstrdup(type_name(oi->type));
-		else if (!strcmp(name, "objectsize")) {
+		else if (!strcmp(name, "objectsize:disk")) {
+			v->value = oi->disk_size;
+			v->s = xstrfmt("%lld", (long long)oi->disk_size);
+		} else if (!strcmp(name, "objectsize")) {
 			v->value = oi->size;
 			v->s = xstrfmt("%lu", oi->size);
-		}
-		else if (deref)
+		} else if (deref)
 			grab_objectname(name, &oi->oid, v, &used_atom[i]);
 	}
 }

--
https://github.com/git/git/pull/552

Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option

[Junio C Hamano]

Olga Telezhnaya <olyatelezhnaya@gmail.com> writes:

> @@ -876,11 +882,13 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_
>  			name++;
>  		if (!strcmp(name, "objecttype"))
>  			v->s = xstrdup(type_name(oi->type));
> -		else if (!strcmp(name, "objectsize")) {
> +		else if (!strcmp(name, "objectsize:disk")) {
> +			v->value = oi->disk_size;
> +			v->s = xstrfmt("%lld", (long long)oi->disk_size);

oi.disk_size is off_t; do we know "long long" 

   (1) is available widely enough (I think it is from c99)?

   (2) is sufficiently wide so that we can safely cast off_t to?

   (3) will stay to be sufficiently wide as machines get larger
       together with standard types like off_t in the future?

I'd rather use intmax_t (as off_t is a signed integral type) so that
we do not have to worry about these questions in the first place.

> +		} else if (!strcmp(name, "objectsize")) {
>  			v->value = oi->size;
>  			v->s = xstrfmt("%lu", oi->size);

This is not a suggestion but is a genuine question, but I wonder if
two years down the road somebody who meets this API for the first
time find the asymmetry between "objectsize" and "objectsize:disk" a
tad strange and suggest the former to have "objectsize:real" or some
synonym.  Or we can consider "objectsize" the primary thing (hence
needing no colon-plus-modifier to clarify what kind of size we are
asking) and leave these two deliberatly asymmetric.  I am leaning
towards the latter myself.

> -		}
> -		else if (deref)
> +		} else if (deref)
>  			grab_objectname(name, &oi->oid, v, &used_atom[i]);
>  	}
>  }
>
> --
> https://github.com/git/git/pull/552

Re: [RFC PATCH 5/5] ref-filter: add docs for new options

[Junio C Hamano]

Olga Telezhnaya <olyatelezhnaya@gmail.com> writes:

>  objectname::
>  	The object name (aka SHA-1).
>  	For a non-ambiguous abbreviation of the object name append `:short`.
>  	For an abbreviation of the object name with desired length append
>  	`:short=<length>`, where the minimum length is MINIMUM_ABBREV. The
>  	length may be exceeded to ensure unique object names.
> +deltabase::
> +	If the object is stored as a delta on-disk, this expands to the 40-hex
> +	sha1 of the delta base object. Otherwise, expands to the null sha1
> +	(40 zeroes). See `CAVEATS` section below.

I know existing description for other things nearby still talk about
SHA-1, but we can prepare ourselves better with something like:

	This expands to the object name of the delta base for the
	given object, if it is stored as a delta.  Otherwise it
	expands to the null object name (all zeroes).

> +Note that the sizes of objects on disk are reported accurately, but care
> +should be taken in drawing conclusions about which refs or objects are
> +responsible for disk usage. The size of a packed non-delta object may be
> +much larger than the size of objects which delta against it, but the
> +choice of which object is the base and which is the delta is arbitrary
> +and is subject to change during a repack.
> +
> +Note also that multiple copies of an object may be present in the object
> +database; in this case, it is undefined which copy's size or delta base
> +will be reported.

OK.


>  SEE ALSO
>  --------
>  linkgit:git-show-ref[1]
>
> --
> https://github.com/git/git/pull/552

Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option

[Johannes Schindelin]

Hi,

On Mon, 12 Nov 2018, Junio C Hamano wrote:

> Olga Telezhnaya <olyatelezhnaya@gmail.com> writes:
> 
> > @@ -876,11 +882,13 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_
> >  			name++;
> >  		if (!strcmp(name, "objecttype"))
> >  			v->s = xstrdup(type_name(oi->type));
> > -		else if (!strcmp(name, "objectsize")) {
> > +		else if (!strcmp(name, "objectsize:disk")) {
> > +			v->value = oi->disk_size;
> > +			v->s = xstrfmt("%lld", (long long)oi->disk_size);
> 
> oi.disk_size is off_t; do we know "long long" 
> 
>    (1) is available widely enough (I think it is from c99)?
> 
>    (2) is sufficiently wide so that we can safely cast off_t to?
> 
>    (3) will stay to be sufficiently wide as machines get larger
>        together with standard types like off_t in the future?
> 
> I'd rather use intmax_t (as off_t is a signed integral type) so that
> we do not have to worry about these questions in the first place.

You mean something like

			v->s = xstrfmt("%"PRIdMAX, (intmax_t)oi->disk_size);


If so, I agree.

Ciao,
Dscho

P.S.: I wondered whether we have precedent for PRIdMAX, as we used to use
only PRIuMAX, but yes: JeffH's json-writer uses PRIdMAX.

> 
> > +		} else if (!strcmp(name, "objectsize")) {
> >  			v->value = oi->size;
> >  			v->s = xstrfmt("%lu", oi->size);
> 
> This is not a suggestion but is a genuine question, but I wonder if
> two years down the road somebody who meets this API for the first
> time find the asymmetry between "objectsize" and "objectsize:disk" a
> tad strange and suggest the former to have "objectsize:real" or some
> synonym.  Or we can consider "objectsize" the primary thing (hence
> needing no colon-plus-modifier to clarify what kind of size we are
> asking) and leave these two deliberatly asymmetric.  I am leaning
> towards the latter myself.
> 
> > -		}
> > -		else if (deref)
> > +		} else if (deref)
> >  			grab_objectname(name, &oi->oid, v, &used_atom[i]);
> >  	}
> >  }
> >
> > --
> > https://github.com/git/git/pull/552
> 

Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option

[Jeff King]

On Mon, Nov 12, 2018 at 02:03:20PM +0900, Junio C Hamano wrote:

> > +		} else if (!strcmp(name, "objectsize")) {
> >  			v->value = oi->size;
> >  			v->s = xstrfmt("%lu", oi->size);
> 
> This is not a suggestion but is a genuine question, but I wonder if
> two years down the road somebody who meets this API for the first
> time find the asymmetry between "objectsize" and "objectsize:disk" a
> tad strange and suggest the former to have "objectsize:real" or some
> synonym.  Or we can consider "objectsize" the primary thing (hence
> needing no colon-plus-modifier to clarify what kind of size we are
> asking) and leave these two deliberatly asymmetric.  I am leaning
> towards the latter myself.

I think to some degree that ship has already sailed (and is my fault!).

The ulterior motive here is to eventually unify the cat-file formatter
with the ref-filter formatter.  So for that we'll have to support
%(objectsize) anyway.

That still leaves the option of having %(objectsize:real) later and
marking a bare %(objectsize) as a deprecated synonym. But I don't think
there's any advantage to trying to deal with it at this stage.

-Peff

Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option

[Jeff King]

On Mon, Nov 12, 2018 at 01:03:25PM +0100, Johannes Schindelin wrote:

> > oi.disk_size is off_t; do we know "long long" 
> > 
> >    (1) is available widely enough (I think it is from c99)?
> > 
> >    (2) is sufficiently wide so that we can safely cast off_t to?
> > 
> >    (3) will stay to be sufficiently wide as machines get larger
> >        together with standard types like off_t in the future?
> > 
> > I'd rather use intmax_t (as off_t is a signed integral type) so that
> > we do not have to worry about these questions in the first place.
> 
> You mean something like
> 
> 			v->s = xstrfmt("%"PRIdMAX, (intmax_t)oi->disk_size);

I think elsewhere we simply use PRIuMAX for printing large sizes via
off_t; we know this value isn't going to be negative.

I'm not opposed to PRIdMAX, which _is_ more accurate, but...

> P.S.: I wondered whether we have precedent for PRIdMAX, as we used to use
> only PRIuMAX, but yes: JeffH's json-writer uses PRIdMAX.

That's pretty recent. I won't be surprised if we have to do some
preprocessor trickery to handle that at some point. We have a PRIuMAX
fallback already. That comes from c4001d92be (Use off_t when we really
mean a file offset., 2007-03-06), but it's not clear to me if that was
motivated by a real platform or an over-abundance of caution.

I'm OK with just using PRIdMAX as appropriate for now. It will serve as
a weather-balloon, and we can #define our way out of it later if need
be.

-Peff

Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> You mean something like
>> 
>> 			v->s = xstrfmt("%"PRIdMAX, (intmax_t)oi->disk_size);
>
> I think elsewhere we simply use PRIuMAX for printing large sizes via
> off_t; we know this value isn't going to be negative.
>
> I'm not opposed to PRIdMAX, which _is_ more accurate, but...
>
>> P.S.: I wondered whether we have precedent for PRIdMAX, as we used to use
>> only PRIuMAX, but yes: JeffH's json-writer uses PRIdMAX.
>
> That's pretty recent. I won't be surprised if we have to do some
> preprocessor trickery to handle that at some point. We have a PRIuMAX
> fallback already. That comes from c4001d92be (Use off_t when we really
> mean a file offset., 2007-03-06), but it's not clear to me if that was
> motivated by a real platform or an over-abundance of caution.
>
> I'm OK with just using PRIdMAX as appropriate for now. It will serve as
> a weather-balloon, and we can #define our way out of it later if need
> be.

I am OK if we avoid PRIdMAX and use PRIuMAX instead with a cast to
the corresponding size in this codepath, as long as we properly
handle negative oi.disk_size field, which may be telling some
"unusual" condition to us.

Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option

[Оля Тележная]

вт, 13 нояб. 2018 г. в 04:52, Junio C Hamano <gitster@pobox.com>:
>
> Jeff King <peff@peff.net> writes:
>
> >> You mean something like
> >>
> >>                      v->s = xstrfmt("%"PRIdMAX, (intmax_t)oi->disk_size);
> >
> > I think elsewhere we simply use PRIuMAX for printing large sizes via
> > off_t; we know this value isn't going to be negative.
> >
> > I'm not opposed to PRIdMAX, which _is_ more accurate, but...
> >
> >> P.S.: I wondered whether we have precedent for PRIdMAX, as we used to use
> >> only PRIuMAX, but yes: JeffH's json-writer uses PRIdMAX.
> >
> > That's pretty recent. I won't be surprised if we have to do some
> > preprocessor trickery to handle that at some point. We have a PRIuMAX
> > fallback already. That comes from c4001d92be (Use off_t when we really
> > mean a file offset., 2007-03-06), but it's not clear to me if that was
> > motivated by a real platform or an over-abundance of caution.
> >
> > I'm OK with just using PRIdMAX as appropriate for now. It will serve as
> > a weather-balloon, and we can #define our way out of it later if need
> > be.
>
> I am OK if we avoid PRIdMAX and use PRIuMAX instead with a cast to
> the corresponding size in this codepath, as long as we properly
> handle negative oi.disk_size field, which may be telling some
> "unusual" condition to us.

Maybe we want to change the type (from off_t to unsigned) directly in
struct object_info? That will help us not to make additional
checkings. Or, at least, I suggest to add check to
oid_object_info_extended() so that this function will give a guarantee
that the size is non-negative. That will make code cleaner (otherwise
we need to add checks everywhere after oid_object_info_extended()
usage).

Please, look at this one also [1]. Thanks a lot!

[1] https://public-inbox.org/git/CAL21BmnoZuRih3Ky66_Tk0PweD36eZ6=fbY3jGumRcSJ=Bc_pQ@mail.gmail.com/

>
>

Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option

[Junio C Hamano]

Оля Тележная  <olyatelezhnaya@gmail.com> writes:

>> I am OK if we avoid PRIdMAX and use PRIuMAX instead with a cast to
>> the corresponding size in this codepath, as long as we properly
>> handle negative oi.disk_size field, which may be telling some
>> "unusual" condition to us.
>
> Maybe we want to change the type (from off_t to unsigned) directly in
> struct object_info? That will help us not to make additional
> checkings. Or, at least, I suggest to add check to
> oid_object_info_extended() so that this function will give a guarantee
> that the size is non-negative.

I am fine with the approach.  The potential gain of allowing
oi.disk_size is it would allow the code to say "I'll give these
pieces of info about the object, but the on-disk size is unknown"
without failing the whole object_info_extended() request.  And I
personally do not think such an ability is not all that important
or useful.

Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option

[Johannes Schindelin]

[-- Attachment #1: Type: text/plain, Size: 1287 bytes --]

Hi Junio,

On Wed, 21 Nov 2018, Junio C Hamano wrote:

> Оля Тележная  <olyatelezhnaya@gmail.com> writes:
> 
> >> I am OK if we avoid PRIdMAX and use PRIuMAX instead with a cast to
> >> the corresponding size in this codepath, as long as we properly
> >> handle negative oi.disk_size field, which may be telling some
> >> "unusual" condition to us.
> >
> > Maybe we want to change the type (from off_t to unsigned) directly in
> > struct object_info? That will help us not to make additional
> > checkings. Or, at least, I suggest to add check to
> > oid_object_info_extended() so that this function will give a guarantee
> > that the size is non-negative.
> 
> I am fine with the approach.  The potential gain of allowing
> oi.disk_size is it would allow the code to say "I'll give these
> pieces of info about the object, but the on-disk size is unknown"
> without failing the whole object_info_extended() request.  And I
> personally do not think such an ability is not all that important
> or useful.

I see that this topic advanced to `next`, which means that the Windows
build of `next` is now broken.

To fix this, I prepared a GitGitGadget PR
(https://github.com/gitgitgadget/git/pull/87) and will submit it as soon
as I am satisfied that the build works.

Ciao,
Dscho

Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> To fix this, I prepared a GitGitGadget PR
> (https://github.com/gitgitgadget/git/pull/87) and will submit it as soon
> as I am satisfied that the build works.

Thanks.  This won't be in the upcoming release anyway, so we can fix
it up without "oops, let's pile another fix on it" if we wanted to
after the release by kicking it back to 'pu', but in the meantime,
keeping the tip of 'next' free from known breakage certainly is a
sensible thing to do.

[PATCH v2 0/5] ref-filter: add new formatting options

[Оля Тележная]

пт, 9 нояб. 2018 г. в 10:37, Оля Тележная <olyatelezhnaya@gmail.com>:
>
> Add formatting options %(objectsize:disk) and %(deltabase), as in
> cat-file command.
>
> I can not test %(deltabase) properly (I mean, I want to have test with
> meaningful deltabase in the result - now we have only with zeros). I
> tested it manually on my git repo, and I have not-null deltabases
> there. We have "t/t1006-cat-file.sh" with similar case, but it is
> about blobs. ref-filter does not work with blobs, I need to write test
> about refs, and I feel that I can't catch the idea (and it is hard for
> me to write in Shell).
>
> Finally, I want to remove formatting logic in cat-file and use
> functions from ref-filter (we are almost there, so many work was done
> for this). I had an idea to make this migration in this patch (and
> stop worrying about bad tests about deltabase: we already have such
> test for cat-file and hopefully that could be enough). But I have
> another question there. cat-file has one more formatting option:
> "rest" [1]. Do we want such formatting option in ref-filter? It's
> easier for me to support that in ref-filter than to leave it only
> specifically for cat-file.

Updates since previous version:
1. Fix type cast not to generate warnings/errors in other system
platforms (travis CI says that everything is OK now)
2. Add check for negative object size (BUG if it is negative)
3. Update documentation (thanks to Junio for better wording)

>
> Thank you!
>
> [1] https://git-scm.com/docs/git-cat-file#git-cat-file-coderestcode

[PATCH v2 6/6] ref-filter: add docs for new options

[Olga Telezhnaya]

Add documentation for formatting options objectsize:disk
and deltabase.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
---
 Documentation/git-for-each-ref.txt | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 901faef1bfdce..774cecc7ede78 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -128,13 +128,18 @@ objecttype::
 
 objectsize::
 	The size of the object (the same as 'git cat-file -s' reports).
-
+	Append `:disk` to get the size, in bytes, that the object takes up on
+	disk. See the note about on-disk sizes in the `CAVEATS` section below.
 objectname::
 	The object name (aka SHA-1).
 	For a non-ambiguous abbreviation of the object name append `:short`.
 	For an abbreviation of the object name with desired length append
 	`:short=<length>`, where the minimum length is MINIMUM_ABBREV. The
 	length may be exceeded to ensure unique object names.
+deltabase::
+	This expands to the object name of the delta base for the
+	given object, if it is stored as a delta.  Otherwise it
+	expands to the null object name (all zeroes).
 
 upstream::
 	The name of a local ref which can be considered ``upstream''
@@ -361,6 +366,20 @@ This prints the authorname, if present.
 git for-each-ref --format="%(refname)%(if)%(authorname)%(then) Authored by: %(authorname)%(end)"
 ------------
 
+CAVEATS
+-------
+
+Note that the sizes of objects on disk are reported accurately, but care
+should be taken in drawing conclusions about which refs or objects are
+responsible for disk usage. The size of a packed non-delta object may be
+much larger than the size of objects which delta against it, but the
+choice of which object is the base and which is the delta is arbitrary
+and is subject to change during a repack.
+
+Note also that multiple copies of an object may be present in the object
+database; in this case, it is undefined which copy's size or delta base
+will be reported.
+
 SEE ALSO
 --------
 linkgit:git-show-ref[1]

--
https://github.com/git/git/pull/552

[PATCH v2 4/6] ref-filter: add deltabase option

[Olga Telezhnaya]

Add new formatting option: deltabase.
If the object is stored as a delta on-disk, this expands
to the 40-hex sha1 of the delta base object.
Otherwise, expands to the null sha1 (40 zeroes).
We have same option in cat-file command.
Hopefully, in the end I will remove formatting code from
cat-file and reuse formatting parts from ref-filter.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
---
 ref-filter.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/ref-filter.c b/ref-filter.c
index 45c558bcbd521..debb8cacad067 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -246,6 +246,18 @@ static int objectsize_atom_parser(const struct ref_format *format, struct used_a
 	return 0;
 }
 
+static int deltabase_atom_parser(const struct ref_format *format, struct used_atom *atom,
+				 const char *arg, struct strbuf *err)
+{
+	if (arg)
+		return strbuf_addf_ret(err, -1, _("%%(deltabase) does not take arguments"));
+	if (*atom->name == '*')
+		oi_deref.info.delta_base_sha1 = oi_deref.delta_base_oid.hash;
+	else
+		oi.info.delta_base_sha1 = oi.delta_base_oid.hash;
+	return 0;
+}
+
 static int body_atom_parser(const struct ref_format *format, struct used_atom *atom,
 			    const char *arg, struct strbuf *err)
 {
@@ -437,6 +449,7 @@ static struct {
 	{ "objecttype", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser },
 	{ "objectsize", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser },
 	{ "objectname", SOURCE_OTHER, FIELD_STR, objectname_atom_parser },
+	{ "deltabase", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser },
 	{ "tree", SOURCE_OBJ },
 	{ "parent", SOURCE_OBJ },
 	{ "numparent", SOURCE_OBJ, FIELD_ULONG },
@@ -892,7 +905,8 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_
 		} else if (!strcmp(name, "objectsize")) {
 			v->value = oi->size;
 			v->s = xstrfmt("%"PRIuMAX , (uintmax_t)oi->size);
-		}
+		} else if (!strcmp(name, "deltabase"))
+			v->s = xstrdup(oid_to_hex(&oi->delta_base_oid));
 		else if (deref)
 			grab_objectname(name, &oi->oid, v, &used_atom[i]);
 	}

--
https://github.com/git/git/pull/552

[PATCH v2 3/6] ref-filter: add tests for objectsize:disk

[Olga Telezhnaya]

Test new formatting atom.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
---
 t/t6300-for-each-ref.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 97bfbee6e8d69..097fdf21fe196 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -83,6 +83,7 @@ test_atom head push:strip=1 remotes/myfork/master
 test_atom head push:strip=-1 master
 test_atom head objecttype commit
 test_atom head objectsize 171
+test_atom head objectsize:disk 138
 test_atom head objectname $(git rev-parse refs/heads/master)
 test_atom head objectname:short $(git rev-parse --short refs/heads/master)
 test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
@@ -124,6 +125,8 @@ test_atom tag upstream ''
 test_atom tag push ''
 test_atom tag objecttype tag
 test_atom tag objectsize 154
+test_atom tag objectsize:disk 138
+test_atom tag '*objectsize:disk' 138
 test_atom tag objectname $(git rev-parse refs/tags/testtag)
 test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag)
 test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)

--
https://github.com/git/git/pull/552

[PATCH v2 5/6] ref-filter: add tests for deltabase

[Olga Telezhnaya]

Test new formatting option deltabase.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
---
 t/t6300-for-each-ref.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 097fdf21fe196..0ffd63071392e 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -84,6 +84,7 @@ test_atom head push:strip=-1 master
 test_atom head objecttype commit
 test_atom head objectsize 171
 test_atom head objectsize:disk 138
+test_atom head deltabase 0000000000000000000000000000000000000000
 test_atom head objectname $(git rev-parse refs/heads/master)
 test_atom head objectname:short $(git rev-parse --short refs/heads/master)
 test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
@@ -127,6 +128,8 @@ test_atom tag objecttype tag
 test_atom tag objectsize 154
 test_atom tag objectsize:disk 138
 test_atom tag '*objectsize:disk' 138
+test_atom tag deltabase 0000000000000000000000000000000000000000
+test_atom tag '*deltabase' 0000000000000000000000000000000000000000
 test_atom tag objectname $(git rev-parse refs/tags/testtag)
 test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag)
 test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)

--
https://github.com/git/git/pull/552

[PATCH v2 1/6] ref-filter: add objectsize:disk option

[Olga Telezhnaya]

Add new formatting option objectsize:disk to know
exact size that object takes up on disk.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
---
 ref-filter.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 5de616befe46e..fd95547676047 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -231,12 +231,18 @@ static int objecttype_atom_parser(const struct ref_format *format, struct used_a
 static int objectsize_atom_parser(const struct ref_format *format, struct used_atom *atom,
 				  const char *arg, struct strbuf *err)
 {
-	if (arg)
-		return strbuf_addf_ret(err, -1, _("%%(objectsize) does not take arguments"));
-	if (*atom->name == '*')
-		oi_deref.info.sizep = &oi_deref.size;
-	else
-		oi.info.sizep = &oi.size;
+	if (!arg) {
+		if (*atom->name == '*')
+			oi_deref.info.sizep = &oi_deref.size;
+		else
+			oi.info.sizep = &oi.size;
+	} else if (!strcmp(arg, "disk")) {
+		if (*atom->name == '*')
+			oi_deref.info.disk_sizep = &oi_deref.disk_size;
+		else
+			oi.info.disk_sizep = &oi.disk_size;
+	} else
+		return strbuf_addf_ret(err, -1, _("unrecognized %%(objectsize) argument: %s"), arg);
 	return 0;
 }
 
@@ -880,7 +886,10 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_
 			name++;
 		if (!strcmp(name, "objecttype"))
 			v->s = xstrdup(type_name(oi->type));
-		else if (!strcmp(name, "objectsize")) {
+		else if (!strcmp(name, "objectsize:disk")) {
+			v->value = oi->disk_size;
+			v->s = xstrfmt("%"PRIuMAX, (intmax_t)oi->disk_size);
+		} else if (!strcmp(name, "objectsize")) {
 			v->value = oi->size;
 			v->s = xstrfmt("%"PRIuMAX , (uintmax_t)oi->size);
 		}

--
https://github.com/git/git/pull/552

[PATCH v2 2/6] ref-filter: add check for negative file size

[Olga Telezhnaya]

If we have negative file size, we are doing something wrong.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
---
 ref-filter.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ref-filter.c b/ref-filter.c
index fd95547676047..45c558bcbd521 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1491,6 +1491,8 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj
 				     OBJECT_INFO_LOOKUP_REPLACE))
 		return strbuf_addf_ret(err, -1, _("missing object %s for %s"),
 				       oid_to_hex(&oi->oid), ref->refname);
+	if (oi->info.disk_sizep && oi->disk_size < 0)
+		BUG("Object size is less than zero.");
 
 	if (oi->info.contentp) {
 		*obj = parse_object_buffer(the_repository, &oi->oid, oi->type, oi->size, oi->content, &eaten);

--
https://github.com/git/git/pull/552

Re: [PATCH v2 1/6] ref-filter: add objectsize:disk option

[Junio C Hamano]

Olga Telezhnaya <olyatelezhnaya@gmail.com> writes:

> @@ -880,7 +886,10 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_
>  			name++;
>  		if (!strcmp(name, "objecttype"))
>  			v->s = xstrdup(type_name(oi->type));
> -		else if (!strcmp(name, "objectsize")) {
> +		else if (!strcmp(name, "objectsize:disk")) {
> +			v->value = oi->disk_size;
> +			v->s = xstrfmt("%"PRIuMAX, (intmax_t)oi->disk_size);

Shouldn't this cast the field to (uintmax_t) type, as we'd format
with %PRIuMAX and we know the size on-disk is not negative?

Other than that, looks good.

Let me rewind the tip of 'next' and replace the previous round with
this iteration.

Thanks.

> +		} else if (!strcmp(name, "objectsize")) {
>  			v->value = oi->size;
>  			v->s = xstrfmt("%"PRIuMAX , (uintmax_t)oi->size);
>  		}
>
> --
> https://github.com/git/git/pull/552

Re: [PATCH v2 0/5] ref-filter: add new formatting options

[Оля Тележная]

пн, 24 дек. 2018 г. в 16:16, Оля Тележная <olyatelezhnaya@gmail.com>:
>
> пт, 9 нояб. 2018 г. в 10:37, Оля Тележная <olyatelezhnaya@gmail.com>:
> >
> > Add formatting options %(objectsize:disk) and %(deltabase), as in
> > cat-file command.
> >
> > I can not test %(deltabase) properly (I mean, I want to have test with
> > meaningful deltabase in the result - now we have only with zeros). I
> > tested it manually on my git repo, and I have not-null deltabases
> > there. We have "t/t1006-cat-file.sh" with similar case, but it is
> > about blobs. ref-filter does not work with blobs, I need to write test
> > about refs, and I feel that I can't catch the idea (and it is hard for
> > me to write in Shell).
> >
> > Finally, I want to remove formatting logic in cat-file and use
> > functions from ref-filter (we are almost there, so many work was done
> > for this). I had an idea to make this migration in this patch (and
> > stop worrying about bad tests about deltabase: we already have such
> > test for cat-file and hopefully that could be enough). But I have
> > another question there. cat-file has one more formatting option:
> > "rest" [1]. Do we want such formatting option in ref-filter? It's
> > easier for me to support that in ref-filter than to leave it only
> > specifically for cat-file.
>
> Updates since previous version:
> 1. Fix type cast not to generate warnings/errors in other system
> platforms (travis CI says that everything is OK now)
> 2. Add check for negative object size (BUG if it is negative)
> 3. Update documentation (thanks to Junio for better wording)

Just fixed 1 cast from (intmax_t) to (uintmax_t).

>
> >
> > Thank you!
> >
> > [1] https://git-scm.com/docs/git-cat-file#git-cat-file-coderestcode

[PATCH v3 1/6] ref-filter: add objectsize:disk option

[Olga Telezhnaya]

Add new formatting option objectsize:disk to know
exact size that object takes up on disk.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
---
 ref-filter.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/ref-filter.c b/ref-filter.c
index 61d75d5c86c64..ecef4b47c751c 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -231,12 +231,18 @@ static int objecttype_atom_parser(const struct ref_format *format, struct used_a
 static int objectsize_atom_parser(const struct ref_format *format, struct used_atom *atom,
 				  const char *arg, struct strbuf *err)
 {
-	if (arg)
-		return strbuf_addf_ret(err, -1, _("%%(objectsize) does not take arguments"));
-	if (*atom->name == '*')
-		oi_deref.info.sizep = &oi_deref.size;
-	else
-		oi.info.sizep = &oi.size;
+	if (!arg) {
+		if (*atom->name == '*')
+			oi_deref.info.sizep = &oi_deref.size;
+		else
+			oi.info.sizep = &oi.size;
+	} else if (!strcmp(arg, "disk")) {
+		if (*atom->name == '*')
+			oi_deref.info.disk_sizep = &oi_deref.disk_size;
+		else
+			oi.info.disk_sizep = &oi.disk_size;
+	} else
+		return strbuf_addf_ret(err, -1, _("unrecognized %%(objectsize) argument: %s"), arg);
 	return 0;
 }
 
@@ -880,7 +886,10 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_
 			name++;
 		if (!strcmp(name, "objecttype"))
 			v->s = xstrdup(type_name(oi->type));
-		else if (!strcmp(name, "objectsize")) {
+		else if (!strcmp(name, "objectsize:disk")) {
+			v->value = oi->disk_size;
+			v->s = xstrfmt("%"PRIuMAX, (uintmax_t)oi->disk_size);
+		} else if (!strcmp(name, "objectsize")) {
 			v->value = oi->size;
 			v->s = xstrfmt("%"PRIuMAX , (uintmax_t)oi->size);
 		}

--
https://github.com/git/git/pull/552

[PATCH v3 5/6] ref-filter: add tests for deltabase

[Olga Telezhnaya]

Test new formatting option deltabase.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
---
 t/t6300-for-each-ref.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 097fdf21fe196..0ffd63071392e 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -84,6 +84,7 @@ test_atom head push:strip=-1 master
 test_atom head objecttype commit
 test_atom head objectsize 171
 test_atom head objectsize:disk 138
+test_atom head deltabase 0000000000000000000000000000000000000000
 test_atom head objectname $(git rev-parse refs/heads/master)
 test_atom head objectname:short $(git rev-parse --short refs/heads/master)
 test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
@@ -127,6 +128,8 @@ test_atom tag objecttype tag
 test_atom tag objectsize 154
 test_atom tag objectsize:disk 138
 test_atom tag '*objectsize:disk' 138
+test_atom tag deltabase 0000000000000000000000000000000000000000
+test_atom tag '*deltabase' 0000000000000000000000000000000000000000
 test_atom tag objectname $(git rev-parse refs/tags/testtag)
 test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag)
 test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)

--
https://github.com/git/git/pull/552

[PATCH v3 4/6] ref-filter: add deltabase option

[Olga Telezhnaya]

Add new formatting option: deltabase.
If the object is stored as a delta on-disk, this expands
to the 40-hex sha1 of the delta base object.
Otherwise, expands to the null sha1 (40 zeroes).
We have same option in cat-file command.
Hopefully, in the end I will remove formatting code from
cat-file and reuse formatting parts from ref-filter.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
---
 ref-filter.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/ref-filter.c b/ref-filter.c
index 57f3789d1040d..422a9c9ae3fd2 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -246,6 +246,18 @@ static int objectsize_atom_parser(const struct ref_format *format, struct used_a
 	return 0;
 }
 
+static int deltabase_atom_parser(const struct ref_format *format, struct used_atom *atom,
+				 const char *arg, struct strbuf *err)
+{
+	if (arg)
+		return strbuf_addf_ret(err, -1, _("%%(deltabase) does not take arguments"));
+	if (*atom->name == '*')
+		oi_deref.info.delta_base_sha1 = oi_deref.delta_base_oid.hash;
+	else
+		oi.info.delta_base_sha1 = oi.delta_base_oid.hash;
+	return 0;
+}
+
 static int body_atom_parser(const struct ref_format *format, struct used_atom *atom,
 			    const char *arg, struct strbuf *err)
 {
@@ -437,6 +449,7 @@ static struct {
 	{ "objecttype", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser },
 	{ "objectsize", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser },
 	{ "objectname", SOURCE_OTHER, FIELD_STR, objectname_atom_parser },
+	{ "deltabase", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser },
 	{ "tree", SOURCE_OBJ },
 	{ "parent", SOURCE_OBJ },
 	{ "numparent", SOURCE_OBJ, FIELD_ULONG },
@@ -892,7 +905,8 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_
 		} else if (!strcmp(name, "objectsize")) {
 			v->value = oi->size;
 			v->s = xstrfmt("%"PRIuMAX , (uintmax_t)oi->size);
-		}
+		} else if (!strcmp(name, "deltabase"))
+			v->s = xstrdup(oid_to_hex(&oi->delta_base_oid));
 		else if (deref)
 			grab_objectname(name, &oi->oid, v, &used_atom[i]);
 	}

--
https://github.com/git/git/pull/552

[PATCH v3 2/6] ref-filter: add check for negative file size

[Olga Telezhnaya]

If we have negative file size, we are doing something wrong.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
---
 ref-filter.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ref-filter.c b/ref-filter.c
index ecef4b47c751c..57f3789d1040d 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1491,6 +1491,8 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj
 				     OBJECT_INFO_LOOKUP_REPLACE))
 		return strbuf_addf_ret(err, -1, _("missing object %s for %s"),
 				       oid_to_hex(&oi->oid), ref->refname);
+	if (oi->info.disk_sizep && oi->disk_size < 0)
+		BUG("Object size is less than zero.");
 
 	if (oi->info.contentp) {
 		*obj = parse_object_buffer(the_repository, &oi->oid, oi->type, oi->size, oi->content, &eaten);

--
https://github.com/git/git/pull/552

[PATCH v3 6/6] ref-filter: add docs for new options

[Olga Telezhnaya]

Add documentation for formatting options objectsize:disk
and deltabase.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
---
 Documentation/git-for-each-ref.txt | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 901faef1bfdce..774cecc7ede78 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -128,13 +128,18 @@ objecttype::
 
 objectsize::
 	The size of the object (the same as 'git cat-file -s' reports).
-
+	Append `:disk` to get the size, in bytes, that the object takes up on
+	disk. See the note about on-disk sizes in the `CAVEATS` section below.
 objectname::
 	The object name (aka SHA-1).
 	For a non-ambiguous abbreviation of the object name append `:short`.
 	For an abbreviation of the object name with desired length append
 	`:short=<length>`, where the minimum length is MINIMUM_ABBREV. The
 	length may be exceeded to ensure unique object names.
+deltabase::
+	This expands to the object name of the delta base for the
+	given object, if it is stored as a delta.  Otherwise it
+	expands to the null object name (all zeroes).
 
 upstream::
 	The name of a local ref which can be considered ``upstream''
@@ -361,6 +366,20 @@ This prints the authorname, if present.
 git for-each-ref --format="%(refname)%(if)%(authorname)%(then) Authored by: %(authorname)%(end)"
 ------------
 
+CAVEATS
+-------
+
+Note that the sizes of objects on disk are reported accurately, but care
+should be taken in drawing conclusions about which refs or objects are
+responsible for disk usage. The size of a packed non-delta object may be
+much larger than the size of objects which delta against it, but the
+choice of which object is the base and which is the delta is arbitrary
+and is subject to change during a repack.
+
+Note also that multiple copies of an object may be present in the object
+database; in this case, it is undefined which copy's size or delta base
+will be reported.
+
 SEE ALSO
 --------
 linkgit:git-show-ref[1]

--
https://github.com/git/git/pull/552

[PATCH v3 3/6] ref-filter: add tests for objectsize:disk

[Olga Telezhnaya]

Test new formatting atom.

Signed-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>
---
 t/t6300-for-each-ref.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 97bfbee6e8d69..097fdf21fe196 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -83,6 +83,7 @@ test_atom head push:strip=1 remotes/myfork/master
 test_atom head push:strip=-1 master
 test_atom head objecttype commit
 test_atom head objectsize 171
+test_atom head objectsize:disk 138
 test_atom head objectname $(git rev-parse refs/heads/master)
 test_atom head objectname:short $(git rev-parse --short refs/heads/master)
 test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
@@ -124,6 +125,8 @@ test_atom tag upstream ''
 test_atom tag push ''
 test_atom tag objecttype tag
 test_atom tag objectsize 154
+test_atom tag objectsize:disk 138
+test_atom tag '*objectsize:disk' 138
 test_atom tag objectname $(git rev-parse refs/tags/testtag)
 test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag)
 test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)

--
https://github.com/git/git/pull/552

Re: [PATCH v2 0/5] ref-filter: add new formatting options

[Junio C Hamano]

Оля Тележная  <olyatelezhnaya@gmail.com> writes:

> Just fixed 1 cast from (intmax_t) to (uintmax_t).

Thanks.

As the previous one already is in 'next', let's queue this on top of
it instead.

-- >8 --
Subject: [PATCH] ref-filter: give uintmax_t to format with %PRIuMAX

As long as we are casting to a wider type, we should cast to the one
with the correct signed-ness.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 ref-filter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ref-filter.c b/ref-filter.c
index d8d3718abb..b22cab133e 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -897,7 +897,7 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_
 			v->s = xstrdup(type_name(oi->type));
 		else if (!strcmp(name, "objectsize:disk")) {
 			v->value = oi->disk_size;
-			v->s = xstrfmt("%"PRIuMAX, (intmax_t)oi->disk_size);
+			v->s = xstrfmt("%"PRIuMAX, (uintmax_t)oi->disk_size);
 		} else if (!strcmp(name, "objectsize")) {
 			v->value = oi->size;
 			v->s = xstrfmt("%lu", oi->size);
-- 
2.20.1-98-gecbdaf0899