* [RFC PATCH 0/5] ref-filter: add new formatting options @ 2018-11-09 7:37 Оля Тележная 2018-11-09 7:44 ` [RFC PATCH 1/5] ref-filter: add objectsize:disk option Olga Telezhnaya 2018-12-24 13:16 ` [PATCH v2 0/5] ref-filter: add new formatting options Оля Тележная 0 siblings, 2 replies; 32+ messages in thread From: Оля Тележная @ 2018-11-09 7:37 UTC (permalink / raw) To: git, Christian Couder, Jeff King 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 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [RFC PATCH 1/5] ref-filter: add objectsize:disk option 2018-11-09 7:37 [RFC PATCH 0/5] ref-filter: add new formatting options Оля Тележная @ 2018-11-09 7:44 ` Olga Telezhnaya 2018-11-09 7:44 ` [RFC PATCH 5/5] ref-filter: add docs for new options Olga Telezhnaya ` (4 more replies) 2018-12-24 13:16 ` [PATCH v2 0/5] ref-filter: add new formatting options Оля Тележная 1 sibling, 5 replies; 32+ messages in thread From: Olga Telezhnaya @ 2018-11-09 7:44 UTC (permalink / raw) To: git 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [RFC PATCH 5/5] ref-filter: add docs for new options 2018-11-09 7:44 ` [RFC PATCH 1/5] ref-filter: add objectsize:disk option Olga Telezhnaya @ 2018-11-09 7:44 ` Olga Telezhnaya 2018-11-12 5:24 ` Junio C Hamano 2018-11-09 7:44 ` [RFC PATCH 3/5] ref-filter: add deltabase option Olga Telezhnaya ` (3 subsequent siblings) 4 siblings, 1 reply; 32+ messages in thread From: Olga Telezhnaya @ 2018-11-09 7:44 UTC (permalink / raw) To: git 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 5/5] ref-filter: add docs for new options 2018-11-09 7:44 ` [RFC PATCH 5/5] ref-filter: add docs for new options Olga Telezhnaya @ 2018-11-12 5:24 ` Junio C Hamano 0 siblings, 0 replies; 32+ messages in thread From: Junio C Hamano @ 2018-11-12 5:24 UTC (permalink / raw) To: Olga Telezhnaya; +Cc: git 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 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [RFC PATCH 3/5] ref-filter: add deltabase option 2018-11-09 7:44 ` [RFC PATCH 1/5] ref-filter: add objectsize:disk option Olga Telezhnaya 2018-11-09 7:44 ` [RFC PATCH 5/5] ref-filter: add docs for new options Olga Telezhnaya @ 2018-11-09 7:44 ` Olga Telezhnaya 2018-11-09 7:44 ` [RFC PATCH 4/5] ref-filter: add tests for deltabase Olga Telezhnaya ` (2 subsequent siblings) 4 siblings, 0 replies; 32+ messages in thread From: Olga Telezhnaya @ 2018-11-09 7:44 UTC (permalink / raw) To: git 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [RFC PATCH 4/5] ref-filter: add tests for deltabase 2018-11-09 7:44 ` [RFC PATCH 1/5] ref-filter: add objectsize:disk option Olga Telezhnaya 2018-11-09 7:44 ` [RFC PATCH 5/5] ref-filter: add docs for new options Olga Telezhnaya 2018-11-09 7:44 ` [RFC PATCH 3/5] ref-filter: add deltabase option Olga Telezhnaya @ 2018-11-09 7:44 ` Olga Telezhnaya 2018-11-09 7:44 ` [RFC PATCH 2/5] ref-filter: add tests for objectsize:disk Olga Telezhnaya 2018-11-12 5:03 ` [RFC PATCH 1/5] ref-filter: add objectsize:disk option Junio C Hamano 4 siblings, 0 replies; 32+ messages in thread From: Olga Telezhnaya @ 2018-11-09 7:44 UTC (permalink / raw) To: git 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [RFC PATCH 2/5] ref-filter: add tests for objectsize:disk 2018-11-09 7:44 ` [RFC PATCH 1/5] ref-filter: add objectsize:disk option Olga Telezhnaya ` (2 preceding siblings ...) 2018-11-09 7:44 ` [RFC PATCH 4/5] ref-filter: add tests for deltabase Olga Telezhnaya @ 2018-11-09 7:44 ` Olga Telezhnaya 2018-11-12 5:03 ` [RFC PATCH 1/5] ref-filter: add objectsize:disk option Junio C Hamano 4 siblings, 0 replies; 32+ messages in thread From: Olga Telezhnaya @ 2018-11-09 7:44 UTC (permalink / raw) To: git 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option 2018-11-09 7:44 ` [RFC PATCH 1/5] ref-filter: add objectsize:disk option Olga Telezhnaya ` (3 preceding siblings ...) 2018-11-09 7:44 ` [RFC PATCH 2/5] ref-filter: add tests for objectsize:disk Olga Telezhnaya @ 2018-11-12 5:03 ` Junio C Hamano 2018-11-12 12:03 ` Johannes Schindelin 2018-11-12 13:05 ` Jeff King 4 siblings, 2 replies; 32+ messages in thread From: Junio C Hamano @ 2018-11-12 5:03 UTC (permalink / raw) To: Olga Telezhnaya; +Cc: git 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 ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option 2018-11-12 5:03 ` [RFC PATCH 1/5] ref-filter: add objectsize:disk option Junio C Hamano @ 2018-11-12 12:03 ` Johannes Schindelin 2018-11-12 13:12 ` Jeff King 2018-11-12 13:05 ` Jeff King 1 sibling, 1 reply; 32+ messages in thread From: Johannes Schindelin @ 2018-11-12 12:03 UTC (permalink / raw) To: Junio C Hamano; +Cc: Olga Telezhnaya, git 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 > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option 2018-11-12 12:03 ` Johannes Schindelin @ 2018-11-12 13:12 ` Jeff King 2018-11-13 1:52 ` Junio C Hamano 0 siblings, 1 reply; 32+ messages in thread From: Jeff King @ 2018-11-12 13:12 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Junio C Hamano, Olga Telezhnaya, git 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 ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option 2018-11-12 13:12 ` Jeff King @ 2018-11-13 1:52 ` Junio C Hamano 2018-11-20 9:17 ` Оля Тележная 0 siblings, 1 reply; 32+ messages in thread From: Junio C Hamano @ 2018-11-13 1:52 UTC (permalink / raw) To: Jeff King; +Cc: Johannes Schindelin, Olga Telezhnaya, git 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. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option 2018-11-13 1:52 ` Junio C Hamano @ 2018-11-20 9:17 ` Оля Тележная 2018-11-21 0:27 ` Junio C Hamano 0 siblings, 1 reply; 32+ messages in thread From: Оля Тележная @ 2018-11-20 9:17 UTC (permalink / raw) To: Junio C Hamano, Jeff King, Christian Couder, SZEDER Gábor Cc: Johannes Schindelin, git вт, 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/ > > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option 2018-11-20 9:17 ` Оля Тележная @ 2018-11-21 0:27 ` Junio C Hamano 2018-11-22 11:33 ` Johannes Schindelin 0 siblings, 1 reply; 32+ messages in thread From: Junio C Hamano @ 2018-11-21 0:27 UTC (permalink / raw) To: Оля Тележная Cc: Jeff King, Christian Couder, SZEDER Gábor, Johannes Schindelin, git Оля Тележная <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. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option 2018-11-21 0:27 ` Junio C Hamano @ 2018-11-22 11:33 ` Johannes Schindelin 2018-11-24 1:48 ` Junio C Hamano 0 siblings, 1 reply; 32+ messages in thread From: Johannes Schindelin @ 2018-11-22 11:33 UTC (permalink / raw) To: Junio C Hamano Cc: Оля Тележная, Jeff King, Christian Couder, SZEDER Gábor, git [-- 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 ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option 2018-11-22 11:33 ` Johannes Schindelin @ 2018-11-24 1:48 ` Junio C Hamano 0 siblings, 0 replies; 32+ messages in thread From: Junio C Hamano @ 2018-11-24 1:48 UTC (permalink / raw) To: Johannes Schindelin Cc: Оля Тележная, Jeff King, Christian Couder, SZEDER Gábor, git 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. ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option 2018-11-12 5:03 ` [RFC PATCH 1/5] ref-filter: add objectsize:disk option Junio C Hamano 2018-11-12 12:03 ` Johannes Schindelin @ 2018-11-12 13:05 ` Jeff King 1 sibling, 0 replies; 32+ messages in thread From: Jeff King @ 2018-11-12 13:05 UTC (permalink / raw) To: Junio C Hamano; +Cc: Olga Telezhnaya, git 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 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 0/5] ref-filter: add new formatting options 2018-11-09 7:37 [RFC PATCH 0/5] ref-filter: add new formatting options Оля Тележная 2018-11-09 7:44 ` [RFC PATCH 1/5] ref-filter: add objectsize:disk option Olga Telezhnaya @ 2018-12-24 13:16 ` Оля Тележная 2018-12-24 13:24 ` [PATCH v2 1/6] ref-filter: add objectsize:disk option Olga Telezhnaya 2019-01-10 6:25 ` [PATCH v2 0/5] ref-filter: add new formatting options Оля Тележная 1 sibling, 2 replies; 32+ messages in thread From: Оля Тележная @ 2018-12-24 13:16 UTC (permalink / raw) To: git, Christian Couder, Jeff King пт, 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 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v2 1/6] ref-filter: add objectsize:disk option 2018-12-24 13:16 ` [PATCH v2 0/5] ref-filter: add new formatting options Оля Тележная @ 2018-12-24 13:24 ` Olga Telezhnaya 2018-12-24 13:24 ` [PATCH v2 2/6] ref-filter: add check for negative file size Olga Telezhnaya ` (5 more replies) 2019-01-10 6:25 ` [PATCH v2 0/5] ref-filter: add new formatting options Оля Тележная 1 sibling, 6 replies; 32+ messages in thread From: Olga Telezhnaya @ 2018-12-24 13:24 UTC (permalink / raw) To: git 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 2/6] ref-filter: add check for negative file size 2018-12-24 13:24 ` [PATCH v2 1/6] ref-filter: add objectsize:disk option Olga Telezhnaya @ 2018-12-24 13:24 ` Olga Telezhnaya 2018-12-24 13:24 ` [PATCH v2 5/6] ref-filter: add tests for deltabase Olga Telezhnaya ` (4 subsequent siblings) 5 siblings, 0 replies; 32+ messages in thread From: Olga Telezhnaya @ 2018-12-24 13:24 UTC (permalink / raw) To: git 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 5/6] ref-filter: add tests for deltabase 2018-12-24 13:24 ` [PATCH v2 1/6] ref-filter: add objectsize:disk option Olga Telezhnaya 2018-12-24 13:24 ` [PATCH v2 2/6] ref-filter: add check for negative file size Olga Telezhnaya @ 2018-12-24 13:24 ` Olga Telezhnaya 2018-12-24 13:24 ` [PATCH v2 4/6] ref-filter: add deltabase option Olga Telezhnaya ` (3 subsequent siblings) 5 siblings, 0 replies; 32+ messages in thread From: Olga Telezhnaya @ 2018-12-24 13:24 UTC (permalink / raw) To: git 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 4/6] ref-filter: add deltabase option 2018-12-24 13:24 ` [PATCH v2 1/6] ref-filter: add objectsize:disk option Olga Telezhnaya 2018-12-24 13:24 ` [PATCH v2 2/6] ref-filter: add check for negative file size Olga Telezhnaya 2018-12-24 13:24 ` [PATCH v2 5/6] ref-filter: add tests for deltabase Olga Telezhnaya @ 2018-12-24 13:24 ` Olga Telezhnaya 2018-12-24 13:24 ` [PATCH v2 3/6] ref-filter: add tests for objectsize:disk Olga Telezhnaya ` (2 subsequent siblings) 5 siblings, 0 replies; 32+ messages in thread From: Olga Telezhnaya @ 2018-12-24 13:24 UTC (permalink / raw) To: git 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 3/6] ref-filter: add tests for objectsize:disk 2018-12-24 13:24 ` [PATCH v2 1/6] ref-filter: add objectsize:disk option Olga Telezhnaya ` (2 preceding siblings ...) 2018-12-24 13:24 ` [PATCH v2 4/6] ref-filter: add deltabase option Olga Telezhnaya @ 2018-12-24 13:24 ` Olga Telezhnaya 2018-12-24 13:24 ` [PATCH v2 6/6] ref-filter: add docs for new options Olga Telezhnaya 2018-12-26 20:44 ` [PATCH v2 1/6] ref-filter: add objectsize:disk option Junio C Hamano 5 siblings, 0 replies; 32+ messages in thread From: Olga Telezhnaya @ 2018-12-24 13:24 UTC (permalink / raw) To: git 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v2 6/6] ref-filter: add docs for new options 2018-12-24 13:24 ` [PATCH v2 1/6] ref-filter: add objectsize:disk option Olga Telezhnaya ` (3 preceding siblings ...) 2018-12-24 13:24 ` [PATCH v2 3/6] ref-filter: add tests for objectsize:disk Olga Telezhnaya @ 2018-12-24 13:24 ` Olga Telezhnaya 2018-12-26 20:44 ` [PATCH v2 1/6] ref-filter: add objectsize:disk option Junio C Hamano 5 siblings, 0 replies; 32+ messages in thread From: Olga Telezhnaya @ 2018-12-24 13:24 UTC (permalink / raw) To: git 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v2 1/6] ref-filter: add objectsize:disk option 2018-12-24 13:24 ` [PATCH v2 1/6] ref-filter: add objectsize:disk option Olga Telezhnaya ` (4 preceding siblings ...) 2018-12-24 13:24 ` [PATCH v2 6/6] ref-filter: add docs for new options Olga Telezhnaya @ 2018-12-26 20:44 ` Junio C Hamano 5 siblings, 0 replies; 32+ messages in thread From: Junio C Hamano @ 2018-12-26 20:44 UTC (permalink / raw) To: Olga Telezhnaya; +Cc: git 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 ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH v2 0/5] ref-filter: add new formatting options 2018-12-24 13:16 ` [PATCH v2 0/5] ref-filter: add new formatting options Оля Тележная 2018-12-24 13:24 ` [PATCH v2 1/6] ref-filter: add objectsize:disk option Olga Telezhnaya @ 2019-01-10 6:25 ` Оля Тележная 2019-01-10 6:32 ` [PATCH v3 1/6] ref-filter: add objectsize:disk option Olga Telezhnaya 2019-01-10 18:17 ` [PATCH v2 0/5] ref-filter: add new formatting options Junio C Hamano 1 sibling, 2 replies; 32+ messages in thread From: Оля Тележная @ 2019-01-10 6:25 UTC (permalink / raw) To: git, Christian Couder, Jeff King, Junio C Hamano пн, 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 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH v3 1/6] ref-filter: add objectsize:disk option 2019-01-10 6:25 ` [PATCH v2 0/5] ref-filter: add new formatting options Оля Тележная @ 2019-01-10 6:32 ` Olga Telezhnaya 2019-01-10 6:32 ` [PATCH v3 5/6] ref-filter: add tests for deltabase Olga Telezhnaya ` (4 more replies) 2019-01-10 18:17 ` [PATCH v2 0/5] ref-filter: add new formatting options Junio C Hamano 1 sibling, 5 replies; 32+ messages in thread From: Olga Telezhnaya @ 2019-01-10 6:32 UTC (permalink / raw) To: git 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 5/6] ref-filter: add tests for deltabase 2019-01-10 6:32 ` [PATCH v3 1/6] ref-filter: add objectsize:disk option Olga Telezhnaya @ 2019-01-10 6:32 ` Olga Telezhnaya 2019-01-10 6:32 ` [PATCH v3 4/6] ref-filter: add deltabase option Olga Telezhnaya ` (3 subsequent siblings) 4 siblings, 0 replies; 32+ messages in thread From: Olga Telezhnaya @ 2019-01-10 6:32 UTC (permalink / raw) To: git 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 4/6] ref-filter: add deltabase option 2019-01-10 6:32 ` [PATCH v3 1/6] ref-filter: add objectsize:disk option Olga Telezhnaya 2019-01-10 6:32 ` [PATCH v3 5/6] ref-filter: add tests for deltabase Olga Telezhnaya @ 2019-01-10 6:32 ` Olga Telezhnaya 2019-01-10 6:32 ` [PATCH v3 3/6] ref-filter: add tests for objectsize:disk Olga Telezhnaya ` (2 subsequent siblings) 4 siblings, 0 replies; 32+ messages in thread From: Olga Telezhnaya @ 2019-01-10 6:32 UTC (permalink / raw) To: git 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 3/6] ref-filter: add tests for objectsize:disk 2019-01-10 6:32 ` [PATCH v3 1/6] ref-filter: add objectsize:disk option Olga Telezhnaya 2019-01-10 6:32 ` [PATCH v3 5/6] ref-filter: add tests for deltabase Olga Telezhnaya 2019-01-10 6:32 ` [PATCH v3 4/6] ref-filter: add deltabase option Olga Telezhnaya @ 2019-01-10 6:32 ` Olga Telezhnaya 2019-01-10 6:32 ` [PATCH v3 2/6] ref-filter: add check for negative file size Olga Telezhnaya 2019-01-10 6:32 ` [PATCH v3 6/6] ref-filter: add docs for new options Olga Telezhnaya 4 siblings, 0 replies; 32+ messages in thread From: Olga Telezhnaya @ 2019-01-10 6:32 UTC (permalink / raw) To: git 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 2/6] ref-filter: add check for negative file size 2019-01-10 6:32 ` [PATCH v3 1/6] ref-filter: add objectsize:disk option Olga Telezhnaya ` (2 preceding siblings ...) 2019-01-10 6:32 ` [PATCH v3 3/6] ref-filter: add tests for objectsize:disk Olga Telezhnaya @ 2019-01-10 6:32 ` Olga Telezhnaya 2019-01-10 6:32 ` [PATCH v3 6/6] ref-filter: add docs for new options Olga Telezhnaya 4 siblings, 0 replies; 32+ messages in thread From: Olga Telezhnaya @ 2019-01-10 6:32 UTC (permalink / raw) To: git 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH v3 6/6] ref-filter: add docs for new options 2019-01-10 6:32 ` [PATCH v3 1/6] ref-filter: add objectsize:disk option Olga Telezhnaya ` (3 preceding siblings ...) 2019-01-10 6:32 ` [PATCH v3 2/6] ref-filter: add check for negative file size Olga Telezhnaya @ 2019-01-10 6:32 ` Olga Telezhnaya 4 siblings, 0 replies; 32+ messages in thread From: Olga Telezhnaya @ 2019-01-10 6:32 UTC (permalink / raw) To: git 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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH v2 0/5] ref-filter: add new formatting options 2019-01-10 6:25 ` [PATCH v2 0/5] ref-filter: add new formatting options Оля Тележная 2019-01-10 6:32 ` [PATCH v3 1/6] ref-filter: add objectsize:disk option Olga Telezhnaya @ 2019-01-10 18:17 ` Junio C Hamano 1 sibling, 0 replies; 32+ messages in thread From: Junio C Hamano @ 2019-01-10 18:17 UTC (permalink / raw) To: Оля Тележная Cc: git, Christian Couder, Jeff King Оля Тележная <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 ^ permalink raw reply related [flat|nested] 32+ messages in thread
end of thread, other threads:[~2019-01-10 18:18 UTC | newest] Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-09 7:37 [RFC PATCH 0/5] ref-filter: add new formatting options Оля Тележная 2018-11-09 7:44 ` [RFC PATCH 1/5] ref-filter: add objectsize:disk option Olga Telezhnaya 2018-11-09 7:44 ` [RFC PATCH 5/5] ref-filter: add docs for new options Olga Telezhnaya 2018-11-12 5:24 ` Junio C Hamano 2018-11-09 7:44 ` [RFC PATCH 3/5] ref-filter: add deltabase option Olga Telezhnaya 2018-11-09 7:44 ` [RFC PATCH 4/5] ref-filter: add tests for deltabase Olga Telezhnaya 2018-11-09 7:44 ` [RFC PATCH 2/5] ref-filter: add tests for objectsize:disk Olga Telezhnaya 2018-11-12 5:03 ` [RFC PATCH 1/5] ref-filter: add objectsize:disk option Junio C Hamano 2018-11-12 12:03 ` Johannes Schindelin 2018-11-12 13:12 ` Jeff King 2018-11-13 1:52 ` Junio C Hamano 2018-11-20 9:17 ` Оля Тележная 2018-11-21 0:27 ` Junio C Hamano 2018-11-22 11:33 ` Johannes Schindelin 2018-11-24 1:48 ` Junio C Hamano 2018-11-12 13:05 ` Jeff King 2018-12-24 13:16 ` [PATCH v2 0/5] ref-filter: add new formatting options Оля Тележная 2018-12-24 13:24 ` [PATCH v2 1/6] ref-filter: add objectsize:disk option Olga Telezhnaya 2018-12-24 13:24 ` [PATCH v2 2/6] ref-filter: add check for negative file size Olga Telezhnaya 2018-12-24 13:24 ` [PATCH v2 5/6] ref-filter: add tests for deltabase Olga Telezhnaya 2018-12-24 13:24 ` [PATCH v2 4/6] ref-filter: add deltabase option Olga Telezhnaya 2018-12-24 13:24 ` [PATCH v2 3/6] ref-filter: add tests for objectsize:disk Olga Telezhnaya 2018-12-24 13:24 ` [PATCH v2 6/6] ref-filter: add docs for new options Olga Telezhnaya 2018-12-26 20:44 ` [PATCH v2 1/6] ref-filter: add objectsize:disk option Junio C Hamano 2019-01-10 6:25 ` [PATCH v2 0/5] ref-filter: add new formatting options Оля Тележная 2019-01-10 6:32 ` [PATCH v3 1/6] ref-filter: add objectsize:disk option Olga Telezhnaya 2019-01-10 6:32 ` [PATCH v3 5/6] ref-filter: add tests for deltabase Olga Telezhnaya 2019-01-10 6:32 ` [PATCH v3 4/6] ref-filter: add deltabase option Olga Telezhnaya 2019-01-10 6:32 ` [PATCH v3 3/6] ref-filter: add tests for objectsize:disk Olga Telezhnaya 2019-01-10 6:32 ` [PATCH v3 2/6] ref-filter: add check for negative file size Olga Telezhnaya 2019-01-10 6:32 ` [PATCH v3 6/6] ref-filter: add docs for new options Olga Telezhnaya 2019-01-10 18:17 ` [PATCH v2 0/5] ref-filter: add new formatting options Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).