{"thread":{"id":"49784","subject":"[RFC PATCH 3/5] ref-filter: add deltabase option","startedAt":"2018-11-09T07:44:04Z","lastAt":"2018-11-24T01:48:59Z","messageCount":15,"participants":["Olga Telezhnaya","Junio C Hamano","Johannes Schindelin","Jeff King","Оля Тележная"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"362781","messageId":"01020166f76d84e0-d2e0c0ad-bb3d-46ab-8834-3760546086d4-000000@eu-west-1.amazonses.com","threadId":"49784","inReplyTo":"01020166f76d845f-1a02a31e-5094-4b27-974d-a23811066c58-000000@eu-west-1.amazonses.com","subject":"[RFC PATCH 3/5] ref-filter: add deltabase option","fromName":"Olga Telezhnaya","fromEmail":"olyatelezhnaya@gmail.com","sentAt":"2018-11-09T07:44:01Z","receivedAt":"2018-11-09T07:44:04Z","isPatch":true,"sender":{"key":"olyatelezhnaya@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11246099?v=4"},"body":"Add new formatting option: deltabase.\nIf the object is stored as a delta on-disk, this expands\nto the 40-hex sha1 of the delta base object.\nOtherwise, expands to the null sha1 (40 zeroes).\nWe have same option in cat-file command.\nHopefully, in the end I will remove formatting code from\ncat-file and reuse formatting parts from ref-filter.\n\nSigned-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>\n---\n ref-filter.c | 17 ++++++++++++++++-\n 1 file changed, 16 insertions(+), 1 deletion(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 8ba1a4e72f2c3..3cfe01a039f8b 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -246,6 +246,18 @@ static int objectsize_atom_parser(const struct ref_format *format, struct used_a\n \treturn 0;\n }\n \n+static int deltabase_atom_parser(const struct ref_format *format, struct used_atom *atom,\n+\t\t\t\t const char *arg, struct strbuf *err)\n+{\n+\tif (arg)\n+\t\treturn strbuf_addf_ret(err, -1, _(\"%%(deltabase) does not take arguments\"));\n+\tif (*atom->name == '*')\n+\t\toi_deref.info.delta_base_sha1 = oi_deref.delta_base_oid.hash;\n+\telse\n+\t\toi.info.delta_base_sha1 = oi.delta_base_oid.hash;\n+\treturn 0;\n+}\n+\n static int body_atom_parser(const struct ref_format *format, struct used_atom *atom,\n \t\t\t    const char *arg, struct strbuf *err)\n {\n@@ -437,6 +449,7 @@ static struct {\n \t{ \"objecttype\", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser },\n \t{ \"objectsize\", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser },\n \t{ \"objectname\", SOURCE_OTHER, FIELD_STR, objectname_atom_parser },\n+\t{ \"deltabase\", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser },\n \t{ \"tree\", SOURCE_OBJ },\n \t{ \"parent\", SOURCE_OBJ },\n \t{ \"numparent\", SOURCE_OBJ, FIELD_ULONG },\n@@ -888,7 +901,9 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n \t\t} else if (!strcmp(name, \"objectsize\")) {\n \t\t\tv->value = oi->size;\n \t\t\tv->s = xstrfmt(\"%lu\", oi->size);\n-\t\t} else if (deref)\n+\t\t} else if (!strcmp(name, \"deltabase\"))\n+\t\t\tv->s = xstrdup(oid_to_hex(&oi->delta_base_oid));\n+\t\telse if (deref)\n \t\t\tgrab_objectname(name, &oi->oid, v, &used_atom[i]);\n \t}\n }\n\n--\nhttps://github.com/git/git/pull/552\n"},{"id":"362782","messageId":"01020166f76d8516-99637be2-ac70-4a58-a80a-8da9d9563156-000000@eu-west-1.amazonses.com","threadId":"49784","inReplyTo":"01020166f76d845f-1a02a31e-5094-4b27-974d-a23811066c58-000000@eu-west-1.amazonses.com","subject":"[RFC PATCH 4/5] ref-filter: add tests for deltabase","fromName":"Olga Telezhnaya","fromEmail":"olyatelezhnaya@gmail.com","sentAt":"2018-11-09T07:44:01Z","receivedAt":"2018-11-09T07:44:05Z","isPatch":true,"sender":{"key":"olyatelezhnaya@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11246099?v=4"},"body":"Test new formatting option deltabase.\n\nSigned-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>\n---\n t/t6300-for-each-ref.sh | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 097fdf21fe196..0ffd63071392e 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -84,6 +84,7 @@ test_atom head push:strip=-1 master\n test_atom head objecttype commit\n test_atom head objectsize 171\n test_atom head objectsize:disk 138\n+test_atom head deltabase 0000000000000000000000000000000000000000\n test_atom head objectname $(git rev-parse refs/heads/master)\n test_atom head objectname:short $(git rev-parse --short refs/heads/master)\n test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)\n@@ -127,6 +128,8 @@ test_atom tag objecttype tag\n test_atom tag objectsize 154\n test_atom tag objectsize:disk 138\n test_atom tag '*objectsize:disk' 138\n+test_atom tag deltabase 0000000000000000000000000000000000000000\n+test_atom tag '*deltabase' 0000000000000000000000000000000000000000\n test_atom tag objectname $(git rev-parse refs/tags/testtag)\n test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag)\n test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)\n\n--\nhttps://github.com/git/git/pull/552\n"},{"id":"362783","messageId":"01020166f76d84c0-b00f024c-db52-45d4-b973-3107576a3860-000000@eu-west-1.amazonses.com","threadId":"49784","inReplyTo":"01020166f76d845f-1a02a31e-5094-4b27-974d-a23811066c58-000000@eu-west-1.amazonses.com","subject":"[RFC PATCH 2/5] ref-filter: add tests for objectsize:disk","fromName":"Olga Telezhnaya","fromEmail":"olyatelezhnaya@gmail.com","sentAt":"2018-11-09T07:44:01Z","receivedAt":"2018-11-09T07:44:05Z","isPatch":true,"sender":{"key":"olyatelezhnaya@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11246099?v=4"},"body":"Test new formatting atom.\n\nSigned-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>\n---\n t/t6300-for-each-ref.sh | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 97bfbee6e8d69..097fdf21fe196 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -83,6 +83,7 @@ test_atom head push:strip=1 remotes/myfork/master\n test_atom head push:strip=-1 master\n test_atom head objecttype commit\n test_atom head objectsize 171\n+test_atom head objectsize:disk 138\n test_atom head objectname $(git rev-parse refs/heads/master)\n test_atom head objectname:short $(git rev-parse --short refs/heads/master)\n test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)\n@@ -124,6 +125,8 @@ test_atom tag upstream ''\n test_atom tag push ''\n test_atom tag objecttype tag\n test_atom tag objectsize 154\n+test_atom tag objectsize:disk 138\n+test_atom tag '*objectsize:disk' 138\n test_atom tag objectname $(git rev-parse refs/tags/testtag)\n test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag)\n test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)\n\n--\nhttps://github.com/git/git/pull/552\n"},{"id":"362784","messageId":"01020166f76d851b-3a0f5414-48ce-4172-8200-0a59fd3f66d9-000000@eu-west-1.amazonses.com","threadId":"49784","inReplyTo":"01020166f76d845f-1a02a31e-5094-4b27-974d-a23811066c58-000000@eu-west-1.amazonses.com","subject":"[RFC PATCH 5/5] ref-filter: add docs for new options","fromName":"Olga Telezhnaya","fromEmail":"olyatelezhnaya@gmail.com","sentAt":"2018-11-09T07:44:01Z","receivedAt":"2018-11-09T07:44:07Z","isPatch":true,"sender":{"key":"olyatelezhnaya@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11246099?v=4"},"body":"Add documentation for formatting options objectsize:disk\nand deltabase.\n\nSigned-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>\n---\n Documentation/git-for-each-ref.txt | 21 ++++++++++++++++++++-\n 1 file changed, 20 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 901faef1bfdce..22d2ff88110cd 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -128,13 +128,18 @@ objecttype::\n \n objectsize::\n \tThe size of the object (the same as 'git cat-file -s' reports).\n-\n+\tAppend `:disk` to get the size, in bytes, that the object takes up on\n+\tdisk. See the note about on-disk sizes in the `CAVEATS` section below.\n objectname::\n \tThe object name (aka SHA-1).\n \tFor a non-ambiguous abbreviation of the object name append `:short`.\n \tFor an abbreviation of the object name with desired length append\n \t`:short=<length>`, where the minimum length is MINIMUM_ABBREV. The\n \tlength may be exceeded to ensure unique object names.\n+deltabase::\n+\tIf the object is stored as a delta on-disk, this expands to the 40-hex\n+\tsha1 of the delta base object. Otherwise, expands to the null sha1\n+\t(40 zeroes). See `CAVEATS` section below.\n \n upstream::\n \tThe name of a local ref which can be considered ``upstream''\n@@ -361,6 +366,20 @@ This prints the authorname, if present.\n git for-each-ref --format=\"%(refname)%(if)%(authorname)%(then) Authored by: %(authorname)%(end)\"\n ------------\n \n+CAVEATS\n+-------\n+\n+Note that the sizes of objects on disk are reported accurately, but care\n+should be taken in drawing conclusions about which refs or objects are\n+responsible for disk usage. The size of a packed non-delta object may be\n+much larger than the size of objects which delta against it, but the\n+choice of which object is the base and which is the delta is arbitrary\n+and is subject to change during a repack.\n+\n+Note also that multiple copies of an object may be present in the object\n+database; in this case, it is undefined which copy's size or delta base\n+will be reported.\n+\n SEE ALSO\n --------\n linkgit:git-show-ref[1]\n\n--\nhttps://github.com/git/git/pull/552\n"},{"id":"362785","messageId":"01020166f76d845f-1a02a31e-5094-4b27-974d-a23811066c58-000000@eu-west-1.amazonses.com","threadId":"49784","inReplyTo":"CAL21BmnoZuRih3Ky66_Tk0PweD36eZ6=fbY3jGumRcSJ=Bc_pQ@mail.gmail.com","subject":"[RFC PATCH 1/5] ref-filter: add objectsize:disk option","fromName":"Olga Telezhnaya","fromEmail":"olyatelezhnaya@gmail.com","sentAt":"2018-11-09T07:44:01Z","receivedAt":"2018-11-09T07:44:09Z","isPatch":true,"sender":{"key":"olyatelezhnaya@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11246099?v=4"},"body":"Add new formatting option objectsize:disk to know\nexact size that object takes up on disk.\n\nSigned-off-by: Olga Telezhnaia <olyatelezhnaya@gmail.com>\n---\n ref-filter.c | 26 +++++++++++++++++---------\n 1 file changed, 17 insertions(+), 9 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 0c45ed9d94a4b..8ba1a4e72f2c3 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -231,12 +231,18 @@ static int objecttype_atom_parser(const struct ref_format *format, struct used_a\n static int objectsize_atom_parser(const struct ref_format *format, struct used_atom *atom,\n \t\t\t\t  const char *arg, struct strbuf *err)\n {\n-\tif (arg)\n-\t\treturn strbuf_addf_ret(err, -1, _(\"%%(objectsize) does not take arguments\"));\n-\tif (*atom->name == '*')\n-\t\toi_deref.info.sizep = &oi_deref.size;\n-\telse\n-\t\toi.info.sizep = &oi.size;\n+\tif (!arg) {\n+\t\tif (*atom->name == '*')\n+\t\t\toi_deref.info.sizep = &oi_deref.size;\n+\t\telse\n+\t\t\toi.info.sizep = &oi.size;\n+\t} else if (!strcmp(arg, \"disk\")) {\n+\t\tif (*atom->name == '*')\n+\t\t\toi_deref.info.disk_sizep = &oi_deref.disk_size;\n+\t\telse\n+\t\t\toi.info.disk_sizep = &oi.disk_size;\n+\t} else\n+\t\treturn strbuf_addf_ret(err, -1, _(\"unrecognized %%(objectsize) argument: %s\"), arg);\n \treturn 0;\n }\n \n@@ -876,11 +882,13 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n \t\t\tname++;\n \t\tif (!strcmp(name, \"objecttype\"))\n \t\t\tv->s = xstrdup(type_name(oi->type));\n-\t\telse if (!strcmp(name, \"objectsize\")) {\n+\t\telse if (!strcmp(name, \"objectsize:disk\")) {\n+\t\t\tv->value = oi->disk_size;\n+\t\t\tv->s = xstrfmt(\"%lld\", (long long)oi->disk_size);\n+\t\t} else if (!strcmp(name, \"objectsize\")) {\n \t\t\tv->value = oi->size;\n \t\t\tv->s = xstrfmt(\"%lu\", oi->size);\n-\t\t}\n-\t\telse if (deref)\n+\t\t} else if (deref)\n \t\t\tgrab_objectname(name, &oi->oid, v, &used_atom[i]);\n \t}\n }\n\n--\nhttps://github.com/git/git/pull/552\n"},{"id":"362962","messageId":"xmqqr2fq3n1j.fsf@gitster-ct.c.googlers.com","threadId":"49784","inReplyTo":"01020166f76d845f-1a02a31e-5094-4b27-974d-a23811066c58-000000@eu-west-1.amazonses.com","subject":"Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-12T05:03:20Z","receivedAt":"2018-11-12T05:03:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Olga Telezhnaya <olyatelezhnaya@gmail.com> writes:\n\n> @@ -876,11 +882,13 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n>  \t\t\tname++;\n>  \t\tif (!strcmp(name, \"objecttype\"))\n>  \t\t\tv->s = xstrdup(type_name(oi->type));\n> -\t\telse if (!strcmp(name, \"objectsize\")) {\n> +\t\telse if (!strcmp(name, \"objectsize:disk\")) {\n> +\t\t\tv->value = oi->disk_size;\n> +\t\t\tv->s = xstrfmt(\"%lld\", (long long)oi->disk_size);\n\noi.disk_size is off_t; do we know \"long long\" \n\n   (1) is available widely enough (I think it is from c99)?\n\n   (2) is sufficiently wide so that we can safely cast off_t to?\n\n   (3) will stay to be sufficiently wide as machines get larger\n       together with standard types like off_t in the future?\n\nI'd rather use intmax_t (as off_t is a signed integral type) so that\nwe do not have to worry about these questions in the first place.\n\n> +\t\t} else if (!strcmp(name, \"objectsize\")) {\n>  \t\t\tv->value = oi->size;\n>  \t\t\tv->s = xstrfmt(\"%lu\", oi->size);\n\nThis is not a suggestion but is a genuine question, but I wonder if\ntwo years down the road somebody who meets this API for the first\ntime find the asymmetry between \"objectsize\" and \"objectsize:disk\" a\ntad strange and suggest the former to have \"objectsize:real\" or some\nsynonym.  Or we can consider \"objectsize\" the primary thing (hence\nneeding no colon-plus-modifier to clarify what kind of size we are\nasking) and leave these two deliberatly asymmetric.  I am leaning\ntowards the latter myself.\n\n> -\t\t}\n> -\t\telse if (deref)\n> +\t\t} else if (deref)\n>  \t\t\tgrab_objectname(name, &oi->oid, v, &used_atom[i]);\n>  \t}\n>  }\n>\n> --\n> https://github.com/git/git/pull/552\n"},{"id":"362964","messageId":"xmqqmuqe3m1o.fsf@gitster-ct.c.googlers.com","threadId":"49784","inReplyTo":"01020166f76d851b-3a0f5414-48ce-4172-8200-0a59fd3f66d9-000000@eu-west-1.amazonses.com","subject":"Re: [RFC PATCH 5/5] ref-filter: add docs for new options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-12T05:24:51Z","receivedAt":"2018-11-12T05:24:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Olga Telezhnaya <olyatelezhnaya@gmail.com> writes:\n\n>  objectname::\n>  \tThe object name (aka SHA-1).\n>  \tFor a non-ambiguous abbreviation of the object name append `:short`.\n>  \tFor an abbreviation of the object name with desired length append\n>  \t`:short=<length>`, where the minimum length is MINIMUM_ABBREV. The\n>  \tlength may be exceeded to ensure unique object names.\n> +deltabase::\n> +\tIf the object is stored as a delta on-disk, this expands to the 40-hex\n> +\tsha1 of the delta base object. Otherwise, expands to the null sha1\n> +\t(40 zeroes). See `CAVEATS` section below.\n\nI know existing description for other things nearby still talk about\nSHA-1, but we can prepare ourselves better with something like:\n\n\tThis expands to the object name of the delta base for the\n\tgiven object, if it is stored as a delta.  Otherwise it\n\texpands to the null object name (all zeroes).\n\n> +Note that the sizes of objects on disk are reported accurately, but care\n> +should be taken in drawing conclusions about which refs or objects are\n> +responsible for disk usage. The size of a packed non-delta object may be\n> +much larger than the size of objects which delta against it, but the\n> +choice of which object is the base and which is the delta is arbitrary\n> +and is subject to change during a repack.\n> +\n> +Note also that multiple copies of an object may be present in the object\n> +database; in this case, it is undefined which copy's size or delta base\n> +will be reported.\n\nOK.\n\n\n>  SEE ALSO\n>  --------\n>  linkgit:git-show-ref[1]\n>\n> --\n> https://github.com/git/git/pull/552\n"},{"id":"362994","messageId":"nycvar.QRO.7.76.6.1811121300520.39@tvgsbejvaqbjf.bet","threadId":"49784","inReplyTo":"xmqqr2fq3n1j.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-12T12:03:25Z","receivedAt":"2018-11-12T12:03:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 12 Nov 2018, Junio C Hamano wrote:\n\n> Olga Telezhnaya <olyatelezhnaya@gmail.com> writes:\n> \n> > @@ -876,11 +882,13 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_\n> >  \t\t\tname++;\n> >  \t\tif (!strcmp(name, \"objecttype\"))\n> >  \t\t\tv->s = xstrdup(type_name(oi->type));\n> > -\t\telse if (!strcmp(name, \"objectsize\")) {\n> > +\t\telse if (!strcmp(name, \"objectsize:disk\")) {\n> > +\t\t\tv->value = oi->disk_size;\n> > +\t\t\tv->s = xstrfmt(\"%lld\", (long long)oi->disk_size);\n> \n> oi.disk_size is off_t; do we know \"long long\" \n> \n>    (1) is available widely enough (I think it is from c99)?\n> \n>    (2) is sufficiently wide so that we can safely cast off_t to?\n> \n>    (3) will stay to be sufficiently wide as machines get larger\n>        together with standard types like off_t in the future?\n> \n> I'd rather use intmax_t (as off_t is a signed integral type) so that\n> we do not have to worry about these questions in the first place.\n\nYou mean something like\n\n\t\t\tv->s = xstrfmt(\"%\"PRIdMAX, (intmax_t)oi->disk_size);\n\n\nIf so, I agree.\n\nCiao,\nDscho\n\nP.S.: I wondered whether we have precedent for PRIdMAX, as we used to use\nonly PRIuMAX, but yes: JeffH's json-writer uses PRIdMAX.\n\n> \n> > +\t\t} else if (!strcmp(name, \"objectsize\")) {\n> >  \t\t\tv->value = oi->size;\n> >  \t\t\tv->s = xstrfmt(\"%lu\", oi->size);\n> \n> This is not a suggestion but is a genuine question, but I wonder if\n> two years down the road somebody who meets this API for the first\n> time find the asymmetry between \"objectsize\" and \"objectsize:disk\" a\n> tad strange and suggest the former to have \"objectsize:real\" or some\n> synonym.  Or we can consider \"objectsize\" the primary thing (hence\n> needing no colon-plus-modifier to clarify what kind of size we are\n> asking) and leave these two deliberatly asymmetric.  I am leaning\n> towards the latter myself.\n> \n> > -\t\t}\n> > -\t\telse if (deref)\n> > +\t\t} else if (deref)\n> >  \t\t\tgrab_objectname(name, &oi->oid, v, &used_atom[i]);\n> >  \t}\n> >  }\n> >\n> > --\n> > https://github.com/git/git/pull/552\n> \n"},{"id":"363009","messageId":"20181112130525.GK3956@sigill.intra.peff.net","threadId":"49784","inReplyTo":"xmqqr2fq3n1j.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-12T13:05:25Z","receivedAt":"2018-11-12T13:05:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 12, 2018 at 02:03:20PM +0900, Junio C Hamano wrote:\n\n> > +\t\t} else if (!strcmp(name, \"objectsize\")) {\n> >  \t\t\tv->value = oi->size;\n> >  \t\t\tv->s = xstrfmt(\"%lu\", oi->size);\n> \n> This is not a suggestion but is a genuine question, but I wonder if\n> two years down the road somebody who meets this API for the first\n> time find the asymmetry between \"objectsize\" and \"objectsize:disk\" a\n> tad strange and suggest the former to have \"objectsize:real\" or some\n> synonym.  Or we can consider \"objectsize\" the primary thing (hence\n> needing no colon-plus-modifier to clarify what kind of size we are\n> asking) and leave these two deliberatly asymmetric.  I am leaning\n> towards the latter myself.\n\nI think to some degree that ship has already sailed (and is my fault!).\n\nThe ulterior motive here is to eventually unify the cat-file formatter\nwith the ref-filter formatter.  So for that we'll have to support\n%(objectsize) anyway.\n\nThat still leaves the option of having %(objectsize:real) later and\nmarking a bare %(objectsize) as a deprecated synonym. But I don't think\nthere's any advantage to trying to deal with it at this stage.\n\n-Peff\n"},{"id":"363010","messageId":"20181112131247.GL3956@sigill.intra.peff.net","threadId":"49784","inReplyTo":"nycvar.QRO.7.76.6.1811121300520.39@tvgsbejvaqbjf.bet","subject":"Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-12T13:12:47Z","receivedAt":"2018-11-12T13:12:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 12, 2018 at 01:03:25PM +0100, Johannes Schindelin wrote:\n\n> > oi.disk_size is off_t; do we know \"long long\" \n> > \n> >    (1) is available widely enough (I think it is from c99)?\n> > \n> >    (2) is sufficiently wide so that we can safely cast off_t to?\n> > \n> >    (3) will stay to be sufficiently wide as machines get larger\n> >        together with standard types like off_t in the future?\n> > \n> > I'd rather use intmax_t (as off_t is a signed integral type) so that\n> > we do not have to worry about these questions in the first place.\n> \n> You mean something like\n> \n> \t\t\tv->s = xstrfmt(\"%\"PRIdMAX, (intmax_t)oi->disk_size);\n\nI think elsewhere we simply use PRIuMAX for printing large sizes via\noff_t; we know this value isn't going to be negative.\n\nI'm not opposed to PRIdMAX, which _is_ more accurate, but...\n\n> P.S.: I wondered whether we have precedent for PRIdMAX, as we used to use\n> only PRIuMAX, but yes: JeffH's json-writer uses PRIdMAX.\n\nThat's pretty recent. I won't be surprised if we have to do some\npreprocessor trickery to handle that at some point. We have a PRIuMAX\nfallback already. That comes from c4001d92be (Use off_t when we really\nmean a file offset., 2007-03-06), but it's not clear to me if that was\nmotivated by a real platform or an over-abundance of caution.\n\nI'm OK with just using PRIdMAX as appropriate for now. It will serve as\na weather-balloon, and we can #define our way out of it later if need\nbe.\n\n-Peff\n"},{"id":"363120","messageId":"xmqqwophyc9s.fsf@gitster-ct.c.googlers.com","threadId":"49784","inReplyTo":"20181112131247.GL3956@sigill.intra.peff.net","subject":"Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-13T01:52:31Z","receivedAt":"2018-11-13T01:52:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>> You mean something like\n>> \n>> \t\t\tv->s = xstrfmt(\"%\"PRIdMAX, (intmax_t)oi->disk_size);\n>\n> I think elsewhere we simply use PRIuMAX for printing large sizes via\n> off_t; we know this value isn't going to be negative.\n>\n> I'm not opposed to PRIdMAX, which _is_ more accurate, but...\n>\n>> P.S.: I wondered whether we have precedent for PRIdMAX, as we used to use\n>> only PRIuMAX, but yes: JeffH's json-writer uses PRIdMAX.\n>\n> That's pretty recent. I won't be surprised if we have to do some\n> preprocessor trickery to handle that at some point. We have a PRIuMAX\n> fallback already. That comes from c4001d92be (Use off_t when we really\n> mean a file offset., 2007-03-06), but it's not clear to me if that was\n> motivated by a real platform or an over-abundance of caution.\n>\n> I'm OK with just using PRIdMAX as appropriate for now. It will serve as\n> a weather-balloon, and we can #define our way out of it later if need\n> be.\n\nI am OK if we avoid PRIdMAX and use PRIuMAX instead with a cast to\nthe corresponding size in this codepath, as long as we properly\nhandle negative oi.disk_size field, which may be telling some\n\"unusual\" condition to us.\n\n\n"},{"id":"363712","messageId":"CAL21BmmK_ZS=wuGXRLj=s0=dqMvUWFiLLBZWx5ymnFjyasX=sA@mail.gmail.com","threadId":"49784","inReplyTo":"xmqqwophyc9s.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option","fromName":"Оля Тележная","fromEmail":"olyatelezhnaya@gmail.com","sentAt":"2018-11-20T09:17:59Z","receivedAt":"2018-11-20T09:22:36Z","isPatch":true,"sender":{"key":"olyatelezhnaya@gmail.com","avatar":"https://avatars.githubusercontent.com/u/11246099?v=4"},"body":"вт, 13 нояб. 2018 г. в 04:52, Junio C Hamano <gitster@pobox.com>:\n>\n> Jeff King <peff@peff.net> writes:\n>\n> >> You mean something like\n> >>\n> >>                      v->s = xstrfmt(\"%\"PRIdMAX, (intmax_t)oi->disk_size);\n> >\n> > I think elsewhere we simply use PRIuMAX for printing large sizes via\n> > off_t; we know this value isn't going to be negative.\n> >\n> > I'm not opposed to PRIdMAX, which _is_ more accurate, but...\n> >\n> >> P.S.: I wondered whether we have precedent for PRIdMAX, as we used to use\n> >> only PRIuMAX, but yes: JeffH's json-writer uses PRIdMAX.\n> >\n> > That's pretty recent. I won't be surprised if we have to do some\n> > preprocessor trickery to handle that at some point. We have a PRIuMAX\n> > fallback already. That comes from c4001d92be (Use off_t when we really\n> > mean a file offset., 2007-03-06), but it's not clear to me if that was\n> > motivated by a real platform or an over-abundance of caution.\n> >\n> > I'm OK with just using PRIdMAX as appropriate for now. It will serve as\n> > a weather-balloon, and we can #define our way out of it later if need\n> > be.\n>\n> I am OK if we avoid PRIdMAX and use PRIuMAX instead with a cast to\n> the corresponding size in this codepath, as long as we properly\n> handle negative oi.disk_size field, which may be telling some\n> \"unusual\" condition to us.\n\nMaybe we want to change the type (from off_t to unsigned) directly in\nstruct object_info? That will help us not to make additional\ncheckings. Or, at least, I suggest to add check to\noid_object_info_extended() so that this function will give a guarantee\nthat the size is non-negative. That will make code cleaner (otherwise\nwe need to add checks everywhere after oid_object_info_extended()\nusage).\n\nPlease, look at this one also [1]. Thanks a lot!\n\n[1] https://public-inbox.org/git/CAL21BmnoZuRih3Ky66_Tk0PweD36eZ6=fbY3jGumRcSJ=Bc_pQ@mail.gmail.com/\n\n>\n>\n"},{"id":"363791","messageId":"xmqqa7m39swq.fsf@gitster-ct.c.googlers.com","threadId":"49784","inReplyTo":"CAL21BmmK_ZS=wuGXRLj=s0=dqMvUWFiLLBZWx5ymnFjyasX=sA@mail.gmail.com","subject":"Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-21T00:27:17Z","receivedAt":"2018-11-21T00:27:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Оля Тележная  <olyatelezhnaya@gmail.com> writes:\n\n>> I am OK if we avoid PRIdMAX and use PRIuMAX instead with a cast to\n>> the corresponding size in this codepath, as long as we properly\n>> handle negative oi.disk_size field, which may be telling some\n>> \"unusual\" condition to us.\n>\n> Maybe we want to change the type (from off_t to unsigned) directly in\n> struct object_info? That will help us not to make additional\n> checkings. Or, at least, I suggest to add check to\n> oid_object_info_extended() so that this function will give a guarantee\n> that the size is non-negative.\n\nI am fine with the approach.  The potential gain of allowing\noi.disk_size is it would allow the code to say \"I'll give these\npieces of info about the object, but the on-disk size is unknown\"\nwithout failing the whole object_info_extended() request.  And I\npersonally do not think such an ability is not all that important\nor useful.\n\n"},{"id":"363880","messageId":"nycvar.QRO.7.76.6.1811221232370.41@tvgsbejvaqbjf.bet","threadId":"49784","inReplyTo":"xmqqa7m39swq.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-11-22T11:33:52Z","receivedAt":"2018-11-22T11:34:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 21 Nov 2018, Junio C Hamano wrote:\n\n> Оля Тележная  <olyatelezhnaya@gmail.com> writes:\n> \n> >> I am OK if we avoid PRIdMAX and use PRIuMAX instead with a cast to\n> >> the corresponding size in this codepath, as long as we properly\n> >> handle negative oi.disk_size field, which may be telling some\n> >> \"unusual\" condition to us.\n> >\n> > Maybe we want to change the type (from off_t to unsigned) directly in\n> > struct object_info? That will help us not to make additional\n> > checkings. Or, at least, I suggest to add check to\n> > oid_object_info_extended() so that this function will give a guarantee\n> > that the size is non-negative.\n> \n> I am fine with the approach.  The potential gain of allowing\n> oi.disk_size is it would allow the code to say \"I'll give these\n> pieces of info about the object, but the on-disk size is unknown\"\n> without failing the whole object_info_extended() request.  And I\n> personally do not think such an ability is not all that important\n> or useful.\n\nI see that this topic advanced to `next`, which means that the Windows\nbuild of `next` is now broken.\n\nTo fix this, I prepared a GitGitGadget PR\n(https://github.com/gitgitgadget/git/pull/87) and will submit it as soon\nas I am satisfied that the build works.\n\nCiao,\nDscho"},{"id":"363978","messageId":"xmqqmupz454p.fsf@gitster-ct.c.googlers.com","threadId":"49784","inReplyTo":"nycvar.QRO.7.76.6.1811221232370.41@tvgsbejvaqbjf.bet","subject":"Re: [RFC PATCH 1/5] ref-filter: add objectsize:disk option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-24T01:48:54Z","receivedAt":"2018-11-24T01:48:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> To fix this, I prepared a GitGitGadget PR\n> (https://github.com/gitgitgadget/git/pull/87) and will submit it as soon\n> as I am satisfied that the build works.\n\nThanks.  This won't be in the upcoming release anyway, so we can fix\nit up without \"oops, let's pile another fix on it\" if we wanted to\nafter the release by kicking it back to 'pu', but in the meantime,\nkeeping the tip of 'next' free from known breakage certainly is a\nsensible thing to do.\n\n"}]}