[PATCH v2 6/6] ref-filter: add docs for new options
Add documentation for formatting options objectsize:disk and deltabase. Signed-off-by: Olga Telezhnaia --- 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=`, where the minimum length is MINIMUM_ABBREV. The length may be exceeded to ensure unique object names. +deltabase:: + This expands to the object name of the delta base for the + given object, if it is stored as a delta. Otherwise it + expands to the null object name (all zeroes). upstream:: The name of a local ref which can be considered ``upstream'' @@ -361,6 +366,20 @@ This prints the authorname, if present. git for-each-ref --format="%(refname)%(if)%(authorname)%(then) Authored by: %(authorname)%(end)" +CAVEATS +--- + +Note that the sizes of objects on disk are reported accurately, but care +should be taken in drawing conclusions about which refs or objects are +responsible for disk usage. The size of a packed non-delta object may be +much larger than the size of objects which delta against it, but the +choice of which object is the base and which is the delta is arbitrary +and is subject to change during a repack. + +Note also that multiple copies of an object may be present in the object +database; in this case, it is undefined which copy's size or delta base +will be reported. + SEE ALSO linkgit:git-show-ref[1] -- https://github.com/git/git/pull/552
[PATCH v2 2/6] ref-filter: add check for negative file size
If we have negative file size, we are doing something wrong. Signed-off-by: Olga Telezhnaia --- 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
[PATCH v2 3/6] ref-filter: add tests for objectsize:disk
Test new formatting atom. Signed-off-by: Olga Telezhnaia --- t/t6300-for-each-ref.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index 97bfbee6e8d69..097fdf21fe196 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -83,6 +83,7 @@ test_atom head push:strip=1 remotes/myfork/master test_atom head push:strip=-1 master test_atom head objecttype commit test_atom head objectsize 171 +test_atom head objectsize:disk 138 test_atom head objectname $(git rev-parse refs/heads/master) test_atom head objectname:short $(git rev-parse --short refs/heads/master) test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master) @@ -124,6 +125,8 @@ test_atom tag upstream '' test_atom tag push '' test_atom tag objecttype tag test_atom tag objectsize 154 +test_atom tag objectsize:disk 138 +test_atom tag '*objectsize:disk' 138 test_atom tag objectname $(git rev-parse refs/tags/testtag) test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag) test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master) -- https://github.com/git/git/pull/552
[PATCH v2 4/6] ref-filter: add deltabase option
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 --- ref-filter.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/ref-filter.c b/ref-filter.c index 45c558bcbd521..debb8cacad067 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -246,6 +246,18 @@ static int objectsize_atom_parser(const struct ref_format *format, struct used_a return 0; } +static int deltabase_atom_parser(const struct ref_format *format, struct used_atom *atom, +const char *arg, struct strbuf *err) +{ + if (arg) + return strbuf_addf_ret(err, -1, _("%%(deltabase) does not take arguments")); + if (*atom->name == '*') + oi_deref.info.delta_base_sha1 = oi_deref.delta_base_oid.hash; + else + oi.info.delta_base_sha1 = oi.delta_base_oid.hash; + return 0; +} + static int body_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { @@ -437,6 +449,7 @@ static struct { { "objecttype", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser }, { "objectsize", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser }, { "objectname", SOURCE_OTHER, FIELD_STR, objectname_atom_parser }, + { "deltabase", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser }, { "tree", SOURCE_OBJ }, { "parent", SOURCE_OBJ }, { "numparent", SOURCE_OBJ, FIELD_ULONG }, @@ -892,7 +905,8 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_ } else if (!strcmp(name, "objectsize")) { v->value = oi->size; v->s = xstrfmt("%"PRIuMAX , (uintmax_t)oi->size); - } + } else if (!strcmp(name, "deltabase")) + v->s = xstrdup(oid_to_hex(&oi->delta_base_oid)); else if (deref) grab_objectname(name, &oi->oid, v, &used_atom[i]); } -- https://github.com/git/git/pull/552
[PATCH v2 1/6] ref-filter: add objectsize:disk option
Add new formatting option objectsize:disk to know exact size that object takes up on disk. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 23 --- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 5de616befe46e..fd95547676047 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -231,12 +231,18 @@ static int objecttype_atom_parser(const struct ref_format *format, struct used_a static int objectsize_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { - if (arg) - return strbuf_addf_ret(err, -1, _("%%(objectsize) does not take arguments")); - if (*atom->name == '*') - oi_deref.info.sizep = &oi_deref.size; - else - oi.info.sizep = &oi.size; + if (!arg) { + if (*atom->name == '*') + oi_deref.info.sizep = &oi_deref.size; + else + oi.info.sizep = &oi.size; + } else if (!strcmp(arg, "disk")) { + if (*atom->name == '*') + oi_deref.info.disk_sizep = &oi_deref.disk_size; + else + oi.info.disk_sizep = &oi.disk_size; + } else + return strbuf_addf_ret(err, -1, _("unrecognized %%(objectsize) argument: %s"), arg); return 0; } @@ -880,7 +886,10 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_ name++; if (!strcmp(name, "objecttype")) v->s = xstrdup(type_name(oi->type)); - else if (!strcmp(name, "objectsize")) { + else if (!strcmp(name, "objectsize:disk")) { + v->value = oi->disk_size; + v->s = xstrfmt("%"PRIuMAX, (intmax_t)oi->disk_size); + } else if (!strcmp(name, "objectsize")) { v->value = oi->size; v->s = xstrfmt("%"PRIuMAX , (uintmax_t)oi->size); } -- https://github.com/git/git/pull/552
[PATCH v2 5/6] ref-filter: add tests for deltabase
Test new formatting option deltabase. Signed-off-by: Olga Telezhnaia --- 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 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 +test_atom tag '*deltabase' test_atom tag objectname $(git rev-parse refs/tags/testtag) test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag) test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master) -- https://github.com/git/git/pull/552
[PATCH v3 5/6] ref-filter: add tests for deltabase
Test new formatting option deltabase. Signed-off-by: Olga Telezhnaia --- 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 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 +test_atom tag '*deltabase' test_atom tag objectname $(git rev-parse refs/tags/testtag) test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag) test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master) -- https://github.com/git/git/pull/552
[PATCH v3 3/6] ref-filter: add tests for objectsize:disk
Test new formatting atom. Signed-off-by: Olga Telezhnaia --- t/t6300-for-each-ref.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index 97bfbee6e8d69..097fdf21fe196 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -83,6 +83,7 @@ test_atom head push:strip=1 remotes/myfork/master test_atom head push:strip=-1 master test_atom head objecttype commit test_atom head objectsize 171 +test_atom head objectsize:disk 138 test_atom head objectname $(git rev-parse refs/heads/master) test_atom head objectname:short $(git rev-parse --short refs/heads/master) test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master) @@ -124,6 +125,8 @@ test_atom tag upstream '' test_atom tag push '' test_atom tag objecttype tag test_atom tag objectsize 154 +test_atom tag objectsize:disk 138 +test_atom tag '*objectsize:disk' 138 test_atom tag objectname $(git rev-parse refs/tags/testtag) test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag) test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master) -- https://github.com/git/git/pull/552
[PATCH v3 6/6] ref-filter: add docs for new options
Add documentation for formatting options objectsize:disk and deltabase. Signed-off-by: Olga Telezhnaia --- 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=`, where the minimum length is MINIMUM_ABBREV. The length may be exceeded to ensure unique object names. +deltabase:: + This expands to the object name of the delta base for the + given object, if it is stored as a delta. Otherwise it + expands to the null object name (all zeroes). upstream:: The name of a local ref which can be considered ``upstream'' @@ -361,6 +366,20 @@ This prints the authorname, if present. git for-each-ref --format="%(refname)%(if)%(authorname)%(then) Authored by: %(authorname)%(end)" +CAVEATS +--- + +Note that the sizes of objects on disk are reported accurately, but care +should be taken in drawing conclusions about which refs or objects are +responsible for disk usage. The size of a packed non-delta object may be +much larger than the size of objects which delta against it, but the +choice of which object is the base and which is the delta is arbitrary +and is subject to change during a repack. + +Note also that multiple copies of an object may be present in the object +database; in this case, it is undefined which copy's size or delta base +will be reported. + SEE ALSO linkgit:git-show-ref[1] -- https://github.com/git/git/pull/552
[PATCH v3 4/6] ref-filter: add deltabase option
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 --- ref-filter.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/ref-filter.c b/ref-filter.c index 57f3789d1040d..422a9c9ae3fd2 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -246,6 +246,18 @@ static int objectsize_atom_parser(const struct ref_format *format, struct used_a return 0; } +static int deltabase_atom_parser(const struct ref_format *format, struct used_atom *atom, +const char *arg, struct strbuf *err) +{ + if (arg) + return strbuf_addf_ret(err, -1, _("%%(deltabase) does not take arguments")); + if (*atom->name == '*') + oi_deref.info.delta_base_sha1 = oi_deref.delta_base_oid.hash; + else + oi.info.delta_base_sha1 = oi.delta_base_oid.hash; + return 0; +} + static int body_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { @@ -437,6 +449,7 @@ static struct { { "objecttype", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser }, { "objectsize", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser }, { "objectname", SOURCE_OTHER, FIELD_STR, objectname_atom_parser }, + { "deltabase", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser }, { "tree", SOURCE_OBJ }, { "parent", SOURCE_OBJ }, { "numparent", SOURCE_OBJ, FIELD_ULONG }, @@ -892,7 +905,8 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_ } else if (!strcmp(name, "objectsize")) { v->value = oi->size; v->s = xstrfmt("%"PRIuMAX , (uintmax_t)oi->size); - } + } else if (!strcmp(name, "deltabase")) + v->s = xstrdup(oid_to_hex(&oi->delta_base_oid)); else if (deref) grab_objectname(name, &oi->oid, v, &used_atom[i]); } -- https://github.com/git/git/pull/552
[PATCH v3 2/6] ref-filter: add check for negative file size
If we have negative file size, we are doing something wrong. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ref-filter.c b/ref-filter.c index ecef4b47c751c..57f3789d1040d 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1491,6 +1491,8 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj OBJECT_INFO_LOOKUP_REPLACE)) return strbuf_addf_ret(err, -1, _("missing object %s for %s"), oid_to_hex(&oi->oid), ref->refname); + if (oi->info.disk_sizep && oi->disk_size < 0) + BUG("Object size is less than zero."); if (oi->info.contentp) { *obj = parse_object_buffer(the_repository, &oi->oid, oi->type, oi->size, oi->content, &eaten); -- https://github.com/git/git/pull/552
[PATCH v3 1/6] ref-filter: add objectsize:disk option
Add new formatting option objectsize:disk to know exact size that object takes up on disk. Signed-off-by: Olga Telezhnaia --- 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
Re: [GSoC] Unify ref-filter formats with other --pretty formats
ср, 27 мар. 2019 г. в 20:01, Kapil Jain : > > > On Tue, Mar 26, 2019 at 2:48 AM Olga Telezhnaya > > wrote: > >> Kapil Jain wrote: > > > Now, the verify_ref_format function can be used inside > > > get_commit_format function, hence reusing logic. > > > Is this a correct example to work on, for this project ? > > > > Hi! Yes, in my opinion your example looks like good starting point. > > I read through the code of both functions, and I think they are different. > Please point out if I missed to see the similarity. > > or may be it seemed that way, because they both deal with different formats. > So, first should a translating function (pretty to ref-filter) be written ? > > > > > Other than this I can't find any other example, for this project in > > > pretty.* and ref-filter.* > > > Perhaps some examples could be found in command specific files, right ? > > > > Other parts of the project are about reusing other ref-filter logic. > > So, the project is not limited to reusing ref-filter logics in pretty, > it is about reusing ref-filter logic wherever possible, right ? > > > For example, we could try to reuse format_ref_array_item() from > > ref-filter.h. > > where can format_ref_array_item() be reused ? > > > I haven't dig into pretty.c logic much, but I guess it > > is possible to translate "pretty" formatting commands to ref-filter > > ones. That will allow us to remove similar logic from pretty.c. Our > > final goal is to minimise code duplication and to have one unified > > interface to extract all needed data from object and to print it > > properly. > > I looked, and yes some, but not all pretty formats are translatable. > For example: %GP, %p, %P. are not translatable to ref-filter. or is > there a workaround to translate them ? I will try to answer to all your questions here in general way. Thomas, Christian, please correct me if you disagree. Our main goal is to simplify codebase, get rid of duplication of similar logic and, as a result, simplify adding new functionality. We also need to save backward compatibility, so we can't just delete some commands, rewrite them and change their interface (unfortunately). You are free to suggest your own ideas how to achieve these goals, and you are free to choose exact tasks. Yes, you are not limited only to pretty.c, but it is a good place where to start. I was an intern in winter 2017-2018 and I was trying to get rid of formatting logic in cat-file. You may try to do same thing in pretty.c. I guess it's easier to think how to reuse ref-filter in pretty.c because ref-filter has the most general interface between all these files. But, if you are sure that you have better idea - I am strongly recommend you to share it with the mailing list. Unfortunately, I can't consult you properly about structure of pretty.c. I guess that would be your first task of the internship to dive into it and think how to improve it. By the way, you could try to make more detailed documentation and that could be one of your first contributions. It will help you to understand the system better, and other contributors will be happy to read it. > > It looks like to reuse ref-filter logic, a translator from pretty to > ref-filter needs to be built. > So, building a translator would be a starting point ? > and then second step would be to recognise places where ref-filter can > be reused, right ? > > > > what is atom ? is it a piece of a whole document ? and what is meant > > > by used atoms ? > > > > I had the same question in my beginning. Please have a look at [1]. > > Another good question - what is object. You could ensure that you > > understand this by reading [2]. > > > > [1] https://git-scm.com/docs/git-for-each-ref#_field_names > > [2] https://git-scm.com/book/en/v2/Git-Internals-Git-Objects > > Thanks, this helped.
Re: [GSoC] Unify ref-filter formats with other --pretty formats
вс, 31 мар. 2019 г. в 20:45, Kapil Jain : > > On Fri, Mar 29, 2019 at 7:23 PM Kapil Jain wrote: > > > > On Thu, Mar 28, 2019 at 11:14 PM Olga Telezhnaya > > wrote: > > > > > > Unfortunately, I can't consult you properly about structure of > > > pretty.c. I guess that would be your first task of the internship to > > > dive into it and think how to improve it. By the way, you could try to > > > make more detailed documentation and that could be one of your first > > > contributions. It will help you to understand the system better, and > > > other contributors will be happy to read it. > > > > i traced the cmd_log() to understand the point at which pretty.c could > be used, i only got to userformat_find_requirements(). > > struct userformat_want { > unsigned notes:1; > unsigned source:1; > }; > > what are notes and source flags used for ? > > olga: what approach did you follow to figure which logic in cat-file > was redundant and that ref-filter could be reused there ? > does it include picking any file, go through it entirely and then pick > places to be replaced by ref-filter logic ? I just explored how the code works. You could see my commits here [1]. Oh, that's funny, I forgot that I started from creating pretty.h. I could choose between pretty and cat-file, and I made the choice randomly. In cat-file, interface was so close to ref-filter, but the way of obtaining data was different, and formatting logic was coded twice. We decided that cat-file gets the data more efficiently, and I changed ref-filter logic, it works faster now. Then I needed to reuse formatting logic, and I am still working on that (do not worry, it must not be a reason for merge conflicts). I guess you will have another workflow: I don't know anything about how pretty gets the data, but the interface differs a lot. So you will have another tasks. If you have enough skills to debug the code, I definitely suggest you to go through all formatting process step-by-step (both for pretty and ref-filter) for different type of input, that will explain you a lot and maybe that will give you some ideas how to achieve the goals better. 1.5 years ago I didn't know how to use gdb properly, and it was much more important for me to start doing just something, that's why I used debug prints. The meaning is the same anyway. The most important advice that I can give you: think about whole process, then try to design your steps so that they could be as small as possible. I mean, it's not a good idea to make big patches (more than 3-5 commits), especially at the beginning. [1] https://github.com/git/git/commits?author=telezhnaya
Re: [GSoC] [RFC] Unify ref-filter formats with other --pretty formats
ср, 3 апр. 2019 г. в 22:54, Kapil Jain : > > Reference: > https://git.github.io/SoC-2019-Ideas/#unify-ref-filter-formats-with-other---pretty-formats > > I have spent some time with both pretty.* and ref-filter.* > > First off, we are aiming to reuse ref-filter, so avoiding any sort of > re-implementation is recommended. It is recommended, but it's normal situation to re-implement something as a middle step. > > Now, coming to pretty.* and ref-filter.* > > suppose, a function named xyz() in ref-filter.c seems like it could be > reused in pretty.c. > since ref-filter doesn't use any struct of pretty.c. The xyz() > function in its original form is not useful for pretty.c. > So now, in order for the xyz() function to be useful in pretty.c. > Function xyz() should be using structs of pretty.* > > now, if we make xyz() use the pretty.* structs, then its > re-implementation and not reusing. its like keeping two different > functions one for ref-filter and another for pretty.*. > which is what is already happening. It's OK as the middle step. Another approach is to add using ref-filter structures and continue using existing ones, so that you have 2 duplicating flows of data. Reuse ref-filter logic, and then in the end of the patch delete duplicating logic from pretty. Both these approaches could be useful, choose any of them or design your own way. I deleted so much code that I wrote, I advice you try not to afraid of it. It's OK to make something and then rewrite it several times. The only thing that matters is the final result. > > please provide any starting point for reusing ref-filter. i don't see > any in pretty.*. > reusing ref-filter specifically in pretty.* is not the motive. please > point out any file in entire code base, that you may feel can reuse > some ref-filter logic.
[RFC PATCH 5/5] ref-filter: add docs for new options
Add documentation for formatting options objectsize:disk and deltabase. Signed-off-by: Olga Telezhnaia --- 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=`, where the minimum length is MINIMUM_ABBREV. The length may be exceeded to ensure unique object names. +deltabase:: + If the object is stored as a delta on-disk, this expands to the 40-hex + sha1 of the delta base object. Otherwise, expands to the null sha1 + (40 zeroes). See `CAVEATS` section below. upstream:: The name of a local ref which can be considered ``upstream'' @@ -361,6 +366,20 @@ This prints the authorname, if present. git for-each-ref --format="%(refname)%(if)%(authorname)%(then) Authored by: %(authorname)%(end)" +CAVEATS +--- + +Note that the sizes of objects on disk are reported accurately, but care +should be taken in drawing conclusions about which refs or objects are +responsible for disk usage. The size of a packed non-delta object may be +much larger than the size of objects which delta against it, but the +choice of which object is the base and which is the delta is arbitrary +and is subject to change during a repack. + +Note also that multiple copies of an object may be present in the object +database; in this case, it is undefined which copy's size or delta base +will be reported. + SEE ALSO linkgit:git-show-ref[1] -- https://github.com/git/git/pull/552
[RFC PATCH 1/5] ref-filter: add objectsize:disk option
Add new formatting option objectsize:disk to know exact size that object takes up on disk. Signed-off-by: Olga Telezhnaia --- 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
[RFC PATCH 3/5] ref-filter: add deltabase option
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 --- ref-filter.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/ref-filter.c b/ref-filter.c index 8ba1a4e72f2c3..3cfe01a039f8b 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -246,6 +246,18 @@ static int objectsize_atom_parser(const struct ref_format *format, struct used_a return 0; } +static int deltabase_atom_parser(const struct ref_format *format, struct used_atom *atom, +const char *arg, struct strbuf *err) +{ + if (arg) + return strbuf_addf_ret(err, -1, _("%%(deltabase) does not take arguments")); + if (*atom->name == '*') + oi_deref.info.delta_base_sha1 = oi_deref.delta_base_oid.hash; + else + oi.info.delta_base_sha1 = oi.delta_base_oid.hash; + return 0; +} + static int body_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { @@ -437,6 +449,7 @@ static struct { { "objecttype", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser }, { "objectsize", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser }, { "objectname", SOURCE_OTHER, FIELD_STR, objectname_atom_parser }, + { "deltabase", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser }, { "tree", SOURCE_OBJ }, { "parent", SOURCE_OBJ }, { "numparent", SOURCE_OBJ, FIELD_ULONG }, @@ -888,7 +901,9 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_ } else if (!strcmp(name, "objectsize")) { v->value = oi->size; v->s = xstrfmt("%lu", oi->size); - } else if (deref) + } else if (!strcmp(name, "deltabase")) + v->s = xstrdup(oid_to_hex(&oi->delta_base_oid)); + else if (deref) grab_objectname(name, &oi->oid, v, &used_atom[i]); } } -- https://github.com/git/git/pull/552
[RFC PATCH 2/5] ref-filter: add tests for objectsize:disk
Test new formatting atom. Signed-off-by: Olga Telezhnaia --- t/t6300-for-each-ref.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index 97bfbee6e8d69..097fdf21fe196 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -83,6 +83,7 @@ test_atom head push:strip=1 remotes/myfork/master test_atom head push:strip=-1 master test_atom head objecttype commit test_atom head objectsize 171 +test_atom head objectsize:disk 138 test_atom head objectname $(git rev-parse refs/heads/master) test_atom head objectname:short $(git rev-parse --short refs/heads/master) test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master) @@ -124,6 +125,8 @@ test_atom tag upstream '' test_atom tag push '' test_atom tag objecttype tag test_atom tag objectsize 154 +test_atom tag objectsize:disk 138 +test_atom tag '*objectsize:disk' 138 test_atom tag objectname $(git rev-parse refs/tags/testtag) test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag) test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master) -- https://github.com/git/git/pull/552
[RFC PATCH 4/5] ref-filter: add tests for deltabase
Test new formatting option deltabase. Signed-off-by: Olga Telezhnaia --- 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 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 +test_atom tag '*deltabase' test_atom tag objectname $(git rev-parse refs/tags/testtag) test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag) test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master) -- https://github.com/git/git/pull/552
[PATCH 1/3] ref-filter: free memory from used_atom
Release memory from used_atom variable. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ref-filter.c b/ref-filter.c index e1bcb4ca8a197..1b71d08a43a84 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1996,6 +1996,9 @@ void ref_array_clear(struct ref_array *array) { int i; + for (i = 0; i < used_atom_cnt; i++) + free((char *)used_atom[i].name); + free(used_atom); for (i = 0; i < array->nr; i++) free_array_item(array->items[i]); FREE_AND_NULL(array->items); -- https://github.com/git/git/pull/538
[PATCH 2/3] ls-remote: release memory instead of UNLEAK
Use ref_array_clear() to release memory instead of UNLEAK macros. Signed-off-by: Olga Telezhnaia --- builtin/ls-remote.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c index 1a25df7ee15b4..6a0cdec30d2d7 100644 --- a/builtin/ls-remote.c +++ b/builtin/ls-remote.c @@ -151,6 +151,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix) } UNLEAK(sorting); - UNLEAK(ref_array); + ref_array_clear(&ref_array); return status; } -- https://github.com/git/git/pull/538
[PATCH 3/3] ref-filter: free item->value and item->value->s
Release item->value. Initialize item->value->s dynamically and then release its resources. Release some local variables. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 95 +--- 1 file changed, 53 insertions(+), 42 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 1b71d08a43a84..4d42c777a9b50 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -875,7 +875,7 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_ if (deref) name++; if (!strcmp(name, "objecttype")) - v->s = type_name(oi->type); + v->s = xstrdup(type_name(oi->type)); else if (!strcmp(name, "objectsize")) { v->value = oi->size; v->s = xstrfmt("%lu", oi->size); @@ -899,9 +899,9 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob if (deref) name++; if (!strcmp(name, "tag")) - v->s = tag->tag; + v->s = xstrdup(tag->tag); else if (!strcmp(name, "type") && tag->tagged) - v->s = type_name(tag->tagged->type); + v->s = xstrdup(type_name(tag->tagged->type)); else if (!strcmp(name, "object") && tag->tagged) v->s = xstrdup(oid_to_hex(&tag->tagged->oid)); } @@ -1032,7 +1032,7 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam v->value = timestamp; return; bad: - v->s = ""; + v->s = xstrdup(""); v->value = 0; } @@ -1227,7 +1227,7 @@ static void fill_missing_values(struct atom_value *val) for (i = 0; i < used_atom_cnt; i++) { struct atom_value *v = &val[i]; if (v->s == NULL) - v->s = ""; + v->s = xstrdup(""); } } @@ -1273,7 +1273,8 @@ static inline char *copy_advance(char *dst, const char *src) static const char *lstrip_ref_components(const char *refname, int len) { long remaining = len; - const char *start = refname; + const char *start = xstrdup(refname); + const char *to_free = start; if (len < 0) { int i; @@ -1294,20 +1295,24 @@ static const char *lstrip_ref_components(const char *refname, int len) while (remaining > 0) { switch (*start++) { case '\0': - return ""; + free((char *)to_free); + return xstrdup(""); case '/': remaining--; break; } } + start = xstrdup(start); + free((char *)to_free); return start; } static const char *rstrip_ref_components(const char *refname, int len) { long remaining = len; - char *start = xstrdup(refname); + const char *start = xstrdup(refname); + const char *to_free = start; if (len < 0) { int i; @@ -1327,9 +1332,10 @@ static const char *rstrip_ref_components(const char *refname, int len) while (remaining-- > 0) { char *p = strrchr(start, '/'); - if (p == NULL) - return ""; - else + if (p == NULL) { + free((char *)to_free); + return xstrdup(""); + } else p[0] = '\0'; } return start; @@ -1344,7 +1350,7 @@ static const char *show_ref(struct refname_atom *atom, const char *refname) else if (atom->option == R_RSTRIP) return rstrip_ref_components(refname, atom->rstrip); else - return refname; + return xstrdup(refname); } static void fill_remote_ref_details(struct used_atom *atom, const char *refname, @@ -1358,7 +1364,7 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname, NULL, AHEAD_BEHIND_FULL) < 0) { *s = xstrdup(msgs.gone); } else if (!num_ours && !num_theirs) - *s = ""; + *s = xstrdup(""); else if (!num_ours) *s = xstrfmt(msgs.behind, num_theirs); else if (!num_theirs) @@ -1373,36 +1379,31 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname, } } else if (atom->u.remote_ref.option == RR_TRACKSHORT) { if (stat_tracking_info(branch, &num_ours, &num_theirs, - NULL, AHEAD_BEHIND_FULL) < 0) + NULL, AHEAD_BEHIND_FULL) < 0) { + *s = xs
[PATCH v2 2/3] ls-remote: release memory instead of UNLEAK
Use ref_array_clear() to release memory instead of UNLEAK macros. Signed-off-by: Olga Telezhnaia --- builtin/ls-remote.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c index 1a25df7ee15b4..6a0cdec30d2d7 100644 --- a/builtin/ls-remote.c +++ b/builtin/ls-remote.c @@ -151,6 +151,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix) } UNLEAK(sorting); - UNLEAK(ref_array); + ref_array_clear(&ref_array); return status; } -- https://github.com/git/git/pull/538
[PATCH v2 1/3] ref-filter: free memory from used_atom
Release memory from used_atom variable for reducing number of memory leaks. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 4 1 file changed, 4 insertions(+) diff --git a/ref-filter.c b/ref-filter.c index e1bcb4ca8a197..70f1d13ab3beb 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1996,6 +1996,10 @@ void ref_array_clear(struct ref_array *array) { int i; + for (i = 0; i < used_atom_cnt; i++) + free((char *)used_atom[i].name); + FREE_AND_NULL(used_atom); + used_atom_cnt = 0; for (i = 0; i < array->nr; i++) free_array_item(array->items[i]); FREE_AND_NULL(array->items); -- https://github.com/git/git/pull/538
[PATCH v2 3/3] ref-filter: free item->value and item->value->s
Release item->value. Initialize item->value->s dynamically and then release its resources. Release some local variables. Final goal of this patch is to reduce number of memory leaks. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 96 +--- 1 file changed, 54 insertions(+), 42 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 70f1d13ab3beb..ca52ee4608c2a 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -875,7 +875,7 @@ static void grab_common_values(struct atom_value *val, int deref, struct expand_ if (deref) name++; if (!strcmp(name, "objecttype")) - v->s = type_name(oi->type); + v->s = xstrdup(type_name(oi->type)); else if (!strcmp(name, "objectsize")) { v->value = oi->size; v->s = xstrfmt("%lu", oi->size); @@ -899,9 +899,9 @@ static void grab_tag_values(struct atom_value *val, int deref, struct object *ob if (deref) name++; if (!strcmp(name, "tag")) - v->s = tag->tag; + v->s = xstrdup(tag->tag); else if (!strcmp(name, "type") && tag->tagged) - v->s = type_name(tag->tagged->type); + v->s = xstrdup(type_name(tag->tagged->type)); else if (!strcmp(name, "object") && tag->tagged) v->s = xstrdup(oid_to_hex(&tag->tagged->oid)); } @@ -1032,7 +1032,7 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam v->value = timestamp; return; bad: - v->s = ""; + v->s = xstrdup(""); v->value = 0; } @@ -1227,7 +1227,7 @@ static void fill_missing_values(struct atom_value *val) for (i = 0; i < used_atom_cnt; i++) { struct atom_value *v = &val[i]; if (v->s == NULL) - v->s = ""; + v->s = xstrdup(""); } } @@ -1273,7 +1273,8 @@ static inline char *copy_advance(char *dst, const char *src) static const char *lstrip_ref_components(const char *refname, int len) { long remaining = len; - const char *start = refname; + const char *start = xstrdup(refname); + const char *to_free = start; if (len < 0) { int i; @@ -1294,20 +1295,24 @@ static const char *lstrip_ref_components(const char *refname, int len) while (remaining > 0) { switch (*start++) { case '\0': - return ""; + free((char *)to_free); + return xstrdup(""); case '/': remaining--; break; } } + start = xstrdup(start); + free((char *)to_free); return start; } static const char *rstrip_ref_components(const char *refname, int len) { long remaining = len; - char *start = xstrdup(refname); + const char *start = xstrdup(refname); + const char *to_free = start; if (len < 0) { int i; @@ -1327,9 +1332,10 @@ static const char *rstrip_ref_components(const char *refname, int len) while (remaining-- > 0) { char *p = strrchr(start, '/'); - if (p == NULL) - return ""; - else + if (p == NULL) { + free((char *)to_free); + return xstrdup(""); + } else p[0] = '\0'; } return start; @@ -1344,7 +1350,7 @@ static const char *show_ref(struct refname_atom *atom, const char *refname) else if (atom->option == R_RSTRIP) return rstrip_ref_components(refname, atom->rstrip); else - return refname; + return xstrdup(refname); } static void fill_remote_ref_details(struct used_atom *atom, const char *refname, @@ -1358,7 +1364,7 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname, NULL, AHEAD_BEHIND_FULL) < 0) { *s = xstrdup(msgs.gone); } else if (!num_ours && !num_theirs) - *s = ""; + *s = xstrdup(""); else if (!num_ours) *s = xstrfmt(msgs.behind, num_theirs); else if (!num_theirs) @@ -1373,36 +1379,31 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname, } } else if (atom->u.remote_ref.option == RR_TRACKSHORT) { if (stat_tracking_info(branch, &num_ours, &num_theirs, - NULL, AHEAD_BEHIND_FULL) < 0) +
[PATCH 3/4] ref-filter: merge get_obj and get_object
Inline get_obj(): it would be easier to edit the code without this split. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 36 +++- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 27733ef013bed..f04169f0ea0e3 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -797,24 +797,6 @@ int verify_ref_format(struct ref_format *format) return 0; } -/* - * Given an object name, read the object data and size, and return a - * "struct object". If the object data we are returning is also borrowed - * by the "struct object" representation, set *eaten as well---it is a - * signal from parse_object_buffer to us not to free the buffer. - */ -static void *get_obj(const struct object_id *oid, struct object **obj, unsigned long *sz, int *eaten) -{ - enum object_type type; - void *buf = read_object_file(oid, &type, sz); - - if (buf) - *obj = parse_object_buffer(oid, type, *sz, buf, eaten); - else - *obj = NULL; - return buf; -} - static int grab_objectname(const char *name, const struct object_id *oid, struct atom_value *v, struct used_atom *atom) { @@ -1437,20 +1419,24 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re } static int get_object(struct ref_array_item *ref, const struct object_id *oid, - int deref, struct object **obj, struct strbuf *err) + int deref, struct object **obj, struct strbuf *err) { int eaten; int ret = 0; unsigned long size; - void *buf = get_obj(oid, obj, &size, &eaten); + enum object_type type; + void *buf = read_object_file(oid, &type, &size); if (!buf) ret = strbuf_addf_ret(err, -1, _("missing object %s for %s"), oid_to_hex(oid), ref->refname); - else if (!*obj) - ret = strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"), - oid_to_hex(oid), ref->refname); - else - grab_values(ref->value, deref, *obj, buf, size); + else { + *obj = parse_object_buffer(oid, type, size, buf, &eaten); + if (!*obj) + ret = strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"), + oid_to_hex(oid), ref->refname); + else + grab_values(ref->value, deref, *obj, buf, size); + } if (!eaten) free(buf); return ret; -- https://github.com/git/git/pull/520
[PATCH 1/4] ref-filter: add info_source to valid_atom
Add the source of object data to prevent parsing of unneeded data. The goal is to improve performance by avoiding calling expensive functions when we don't need the information they provide or when we could get it by using a cheaper function. It is stored in valid_atoms because it depends on the atoms we are interested in. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 82 +++- 1 file changed, 43 insertions(+), 39 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index fa3685d91f046..8611c24fd57d1 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -41,6 +41,7 @@ void setup_ref_filter_porcelain_msg(void) typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type; typedef enum { COMPARE_EQUAL, COMPARE_UNEQUAL, COMPARE_NONE } cmp_status; +typedef enum { SOURCE_NONE = 0, SOURCE_OBJ, SOURCE_OTHER } info_source; struct align { align_type position; @@ -73,6 +74,7 @@ struct refname_atom { static struct used_atom { const char *name; cmp_type type; + info_source source; union { char color[COLOR_MAXLEN]; struct align align; @@ -380,49 +382,50 @@ static int head_atom_parser(const struct ref_format *format, struct used_atom *a static struct { const char *name; + info_source source; cmp_type cmp_type; int (*parser)(const struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err); } valid_atom[] = { - { "refname" , FIELD_STR, refname_atom_parser }, - { "objecttype" }, - { "objectsize", FIELD_ULONG }, - { "objectname", FIELD_STR, objectname_atom_parser }, - { "tree" }, - { "parent" }, - { "numparent", FIELD_ULONG }, - { "object" }, - { "type" }, - { "tag" }, - { "author" }, - { "authorname" }, - { "authoremail" }, - { "authordate", FIELD_TIME }, - { "committer" }, - { "committername" }, - { "committeremail" }, - { "committerdate", FIELD_TIME }, - { "tagger" }, - { "taggername" }, - { "taggeremail" }, - { "taggerdate", FIELD_TIME }, - { "creator" }, - { "creatordate", FIELD_TIME }, - { "subject", FIELD_STR, subject_atom_parser }, - { "body", FIELD_STR, body_atom_parser }, - { "trailers", FIELD_STR, trailers_atom_parser }, - { "contents", FIELD_STR, contents_atom_parser }, - { "upstream", FIELD_STR, remote_ref_atom_parser }, - { "push", FIELD_STR, remote_ref_atom_parser }, - { "symref", FIELD_STR, refname_atom_parser }, - { "flag" }, - { "HEAD", FIELD_STR, head_atom_parser }, - { "color", FIELD_STR, color_atom_parser }, - { "align", FIELD_STR, align_atom_parser }, - { "end" }, - { "if", FIELD_STR, if_atom_parser }, - { "then" }, - { "else" }, + { "refname", SOURCE_NONE, FIELD_STR, refname_atom_parser }, + { "objecttype", SOURCE_OTHER }, + { "objectsize", SOURCE_OTHER, FIELD_ULONG }, + { "objectname", SOURCE_OTHER, FIELD_STR, objectname_atom_parser }, + { "tree", SOURCE_OBJ }, + { "parent", SOURCE_OBJ }, + { "numparent", SOURCE_OBJ, FIELD_ULONG }, + { "object", SOURCE_OBJ }, + { "type", SOURCE_OBJ }, + { "tag", SOURCE_OBJ }, + { "author", SOURCE_OBJ }, + { "authorname", SOURCE_OBJ }, + { "authoremail", SOURCE_OBJ }, + { "authordate", SOURCE_OBJ, FIELD_TIME }, + { "committer", SOURCE_OBJ }, + { "committername", SOURCE_OBJ }, + { "committeremail", SOURCE_OBJ }, + { "committerdate", SOURCE_OBJ, FIELD_TIME }, + { "tagger", SOURCE_OBJ }, + { "taggername", SOURCE_OBJ }, + { "taggeremail", SOURCE_OBJ }, + { "taggerdate", SOURCE_OBJ, FIELD_TIME }, + { "creator", SOURCE_OBJ }, + { "creatordate", SOURCE_OBJ, FIELD_TIME }, + { "subject", SOURCE_OBJ, FIELD_STR, subject_atom_parser }, + { "body", SOURCE_OBJ, FIELD_STR, body_atom_parser }, + { "trailers", SOURCE_OBJ, FIELD_STR, trailers_atom_parser }, + { "contents", SOURCE_OBJ, FIELD_STR, contents_atom_parser }, + { "upstream", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser }, + { "push", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser }, + { "symref", SOURCE_NONE, FIELD_STR, refname_atom_parser }, + { "flag", SOURCE_NONE }, + { "HEAD", SOURCE_NONE, FIELD_STR, head_atom_parser }, + { "color", SOURCE_NONE, FIELD_STR, color_atom_parser }, + { "align", SOURCE_NONE, FIELD_STR, align_atom_parser }, + { "end", SOURCE_NONE }, + { "if", SOURCE_NONE, FIELD_STR, if_atom_parser }, + { "then", SOURCE_NONE }, + { "else", SOURCE_NONE }, }; #define REF_FORMATTING_STATE_INIT { 0, NULL } @@ -498,6 +501,7 @@ static int parse_ref_filter_atom(const struct ref_format *format, REALLOC_ARRAY(used_atom, used_atom
[PATCH 2/4] ref-filter: add empty values to technical fields
Atoms like "align" or "end" do not have string representation. Earlier we had to go and parse whole object with a hope that we could fill their string representations. It's easier to fill them with an empty string before we start to work with whole object. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ref-filter.c b/ref-filter.c index 8611c24fd57d1..27733ef013bed 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1497,6 +1497,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) refname = get_symref(atom, ref); else if (starts_with(name, "upstream")) { const char *branch_name; + v->s = ""; /* only local branches may have an upstream */ if (!skip_prefix(ref->refname, "refs/heads/", &branch_name)) @@ -1509,6 +1510,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) continue; } else if (atom->u.remote_ref.push) { const char *branch_name; + v->s = ""; if (!skip_prefix(ref->refname, "refs/heads/", &branch_name)) continue; @@ -1549,22 +1551,26 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) continue; } else if (starts_with(name, "align")) { v->handler = align_atom_handler; + v->s = ""; continue; } else if (!strcmp(name, "end")) { v->handler = end_atom_handler; + v->s = ""; continue; } else if (starts_with(name, "if")) { const char *s; - + v->s = ""; if (skip_prefix(name, "if:", &s)) v->s = xstrdup(s); v->handler = if_atom_handler; continue; } else if (!strcmp(name, "then")) { v->handler = then_atom_handler; + v->s = ""; continue; } else if (!strcmp(name, "else")) { v->handler = else_atom_handler; + v->s = ""; continue; } else continue; -- https://github.com/git/git/pull/520
[PATCH 4/4] ref-filter: use oid_object_info() to get object
Use oid_object_info_extended() to get object info instead of read_object_file(). It will help to handle some requests faster (e.g., we do not need to parse whole object if we need to know %(objectsize)). It could also help us to add new atoms such as %(objectsize:disk) and %(deltabase). Signed-off-by: Olga Telezhnaia --- ref-filter.c | 120 +++ 1 file changed, 87 insertions(+), 33 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index f04169f0ea0e3..9bf7de4c561cb 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -61,6 +61,17 @@ struct refname_atom { int lstrip, rstrip; }; +struct expand_data { + struct object_id oid; + enum object_type type; + unsigned long size; + off_t disk_size; + struct object_id delta_base_oid; + void *content; + + struct object_info info; +} oi, oi_deref; + /* * An atom is a valid field atom listed below, possibly prefixed with * a "*" to denote deref_tag(). @@ -202,6 +213,30 @@ static int remote_ref_atom_parser(const struct ref_format *format, struct used_a return 0; } +static int objecttype_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, _("%%(objecttype) does not take arguments")); + if (*atom->name == '*') + oi_deref.info.typep = &oi_deref.type; + else + oi.info.typep = &oi.type; + return 0; +} + +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; + return 0; +} + static int body_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { @@ -388,8 +423,8 @@ static struct { const char *arg, struct strbuf *err); } valid_atom[] = { { "refname", SOURCE_NONE, FIELD_STR, refname_atom_parser }, - { "objecttype", SOURCE_OTHER }, - { "objectsize", SOURCE_OTHER, FIELD_ULONG }, + { "objecttype", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser }, + { "objectsize", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser }, { "objectname", SOURCE_OTHER, FIELD_STR, objectname_atom_parser }, { "tree", SOURCE_OBJ }, { "parent", SOURCE_OBJ }, @@ -502,6 +537,12 @@ static int parse_ref_filter_atom(const struct ref_format *format, used_atom[at].name = xmemdupz(atom, ep - atom); used_atom[at].type = valid_atom[i].cmp_type; used_atom[at].source = valid_atom[i].source; + if (used_atom[at].source == SOURCE_OBJ) { + if (*atom == '*') + oi_deref.info.contentp = &oi_deref.content; + else + oi.info.contentp = &oi.content; + } if (arg) { arg = used_atom[at].name + (arg - atom) + 1; if (!*arg) { @@ -817,7 +858,7 @@ static int grab_objectname(const char *name, const struct object_id *oid, } /* See grab_values */ -static void grab_common_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz) +static void grab_common_values(struct atom_value *val, int deref, struct expand_data *oi) { int i; @@ -829,13 +870,13 @@ static void grab_common_values(struct atom_value *val, int deref, struct object if (deref) name++; if (!strcmp(name, "objecttype")) - v->s = type_name(obj->type); + v->s = type_name(oi->type); else if (!strcmp(name, "objectsize")) { - v->value = sz; - v->s = xstrfmt("%lu", sz); + v->value = oi->size; + v->s = xstrfmt("%lu", oi->size); } else if (deref) - grab_objectname(name, &obj->oid, v, &used_atom[i]); + grab_objectname(name, &oi->oid, v, &used_atom[i]); } } @@ -1194,7 +1235,6 @@ static void fill_missing_values(struct atom_value *val) */ static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz) { - grab_common_values(val, deref, obj, buf, sz); switch (obj->type) { case OBJ_TAG: grab_tag_values(val, deref, obj, buf, sz); @@ -1418,28 +1458,35 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re return show_ref(&atom->u.refname, ref->refn
[PATCH v2 2/4] ref-filter: fill empty fields with empty values
Atoms like "align" or "end" do not have string representation. Earlier we had to go and parse whole object with a hope that we could fill their string representations. It's easier to fill them with an empty string before we start to work with whole object. It is important to mention that we fill only these atoms that must contain nothing. So, if we could not fill the atom because, for example, the object is missing, we leave it with NULL. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ref-filter.c b/ref-filter.c index 8611c24fd57d1..27733ef013bed 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1497,6 +1497,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) refname = get_symref(atom, ref); else if (starts_with(name, "upstream")) { const char *branch_name; + v->s = ""; /* only local branches may have an upstream */ if (!skip_prefix(ref->refname, "refs/heads/", &branch_name)) @@ -1509,6 +1510,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) continue; } else if (atom->u.remote_ref.push) { const char *branch_name; + v->s = ""; if (!skip_prefix(ref->refname, "refs/heads/", &branch_name)) continue; @@ -1549,22 +1551,26 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) continue; } else if (starts_with(name, "align")) { v->handler = align_atom_handler; + v->s = ""; continue; } else if (!strcmp(name, "end")) { v->handler = end_atom_handler; + v->s = ""; continue; } else if (starts_with(name, "if")) { const char *s; - + v->s = ""; if (skip_prefix(name, "if:", &s)) v->s = xstrdup(s); v->handler = if_atom_handler; continue; } else if (!strcmp(name, "then")) { v->handler = then_atom_handler; + v->s = ""; continue; } else if (!strcmp(name, "else")) { v->handler = else_atom_handler; + v->s = ""; continue; } else continue; -- https://github.com/git/git/pull/520
[PATCH v2 1/4] ref-filter: add info_source to valid_atom
Add the source of object data to prevent parsing of unneeded data. The goal is to improve performance by avoiding calling expensive functions when we don't need the information they provide or when we could get it by using a cheaper function. It is stored in valid_atoms because it depends on the atoms we are interested in. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 82 +++- 1 file changed, 43 insertions(+), 39 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index fa3685d91f046..8611c24fd57d1 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -41,6 +41,7 @@ void setup_ref_filter_porcelain_msg(void) typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type; typedef enum { COMPARE_EQUAL, COMPARE_UNEQUAL, COMPARE_NONE } cmp_status; +typedef enum { SOURCE_NONE = 0, SOURCE_OBJ, SOURCE_OTHER } info_source; struct align { align_type position; @@ -73,6 +74,7 @@ struct refname_atom { static struct used_atom { const char *name; cmp_type type; + info_source source; union { char color[COLOR_MAXLEN]; struct align align; @@ -380,49 +382,50 @@ static int head_atom_parser(const struct ref_format *format, struct used_atom *a static struct { const char *name; + info_source source; cmp_type cmp_type; int (*parser)(const struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err); } valid_atom[] = { - { "refname" , FIELD_STR, refname_atom_parser }, - { "objecttype" }, - { "objectsize", FIELD_ULONG }, - { "objectname", FIELD_STR, objectname_atom_parser }, - { "tree" }, - { "parent" }, - { "numparent", FIELD_ULONG }, - { "object" }, - { "type" }, - { "tag" }, - { "author" }, - { "authorname" }, - { "authoremail" }, - { "authordate", FIELD_TIME }, - { "committer" }, - { "committername" }, - { "committeremail" }, - { "committerdate", FIELD_TIME }, - { "tagger" }, - { "taggername" }, - { "taggeremail" }, - { "taggerdate", FIELD_TIME }, - { "creator" }, - { "creatordate", FIELD_TIME }, - { "subject", FIELD_STR, subject_atom_parser }, - { "body", FIELD_STR, body_atom_parser }, - { "trailers", FIELD_STR, trailers_atom_parser }, - { "contents", FIELD_STR, contents_atom_parser }, - { "upstream", FIELD_STR, remote_ref_atom_parser }, - { "push", FIELD_STR, remote_ref_atom_parser }, - { "symref", FIELD_STR, refname_atom_parser }, - { "flag" }, - { "HEAD", FIELD_STR, head_atom_parser }, - { "color", FIELD_STR, color_atom_parser }, - { "align", FIELD_STR, align_atom_parser }, - { "end" }, - { "if", FIELD_STR, if_atom_parser }, - { "then" }, - { "else" }, + { "refname", SOURCE_NONE, FIELD_STR, refname_atom_parser }, + { "objecttype", SOURCE_OTHER }, + { "objectsize", SOURCE_OTHER, FIELD_ULONG }, + { "objectname", SOURCE_OTHER, FIELD_STR, objectname_atom_parser }, + { "tree", SOURCE_OBJ }, + { "parent", SOURCE_OBJ }, + { "numparent", SOURCE_OBJ, FIELD_ULONG }, + { "object", SOURCE_OBJ }, + { "type", SOURCE_OBJ }, + { "tag", SOURCE_OBJ }, + { "author", SOURCE_OBJ }, + { "authorname", SOURCE_OBJ }, + { "authoremail", SOURCE_OBJ }, + { "authordate", SOURCE_OBJ, FIELD_TIME }, + { "committer", SOURCE_OBJ }, + { "committername", SOURCE_OBJ }, + { "committeremail", SOURCE_OBJ }, + { "committerdate", SOURCE_OBJ, FIELD_TIME }, + { "tagger", SOURCE_OBJ }, + { "taggername", SOURCE_OBJ }, + { "taggeremail", SOURCE_OBJ }, + { "taggerdate", SOURCE_OBJ, FIELD_TIME }, + { "creator", SOURCE_OBJ }, + { "creatordate", SOURCE_OBJ, FIELD_TIME }, + { "subject", SOURCE_OBJ, FIELD_STR, subject_atom_parser }, + { "body", SOURCE_OBJ, FIELD_STR, body_atom_parser }, + { "trailers", SOURCE_OBJ, FIELD_STR, trailers_atom_parser }, + { "contents", SOURCE_OBJ, FIELD_STR, contents_atom_parser }, + { "upstream", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser }, + { "push", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser }, + { "symref", SOURCE_NONE, FIELD_STR, refname_atom_parser }, + { "flag", SOURCE_NONE }, + { "HEAD", SOURCE_NONE, FIELD_STR, head_atom_parser }, + { "color", SOURCE_NONE, FIELD_STR, color_atom_parser }, + { "align", SOURCE_NONE, FIELD_STR, align_atom_parser }, + { "end", SOURCE_NONE }, + { "if", SOURCE_NONE, FIELD_STR, if_atom_parser }, + { "then", SOURCE_NONE }, + { "else", SOURCE_NONE }, }; #define REF_FORMATTING_STATE_INIT { 0, NULL } @@ -498,6 +501,7 @@ static int parse_ref_filter_atom(const struct ref_format *format, REALLOC_ARRAY(used_atom, used_atom
[PATCH v2 3/4] ref-filter: merge get_obj and get_object
Inline get_obj(): it would be easier to edit the code without this split. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 36 +++- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 27733ef013bed..f04169f0ea0e3 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -797,24 +797,6 @@ int verify_ref_format(struct ref_format *format) return 0; } -/* - * Given an object name, read the object data and size, and return a - * "struct object". If the object data we are returning is also borrowed - * by the "struct object" representation, set *eaten as well---it is a - * signal from parse_object_buffer to us not to free the buffer. - */ -static void *get_obj(const struct object_id *oid, struct object **obj, unsigned long *sz, int *eaten) -{ - enum object_type type; - void *buf = read_object_file(oid, &type, sz); - - if (buf) - *obj = parse_object_buffer(oid, type, *sz, buf, eaten); - else - *obj = NULL; - return buf; -} - static int grab_objectname(const char *name, const struct object_id *oid, struct atom_value *v, struct used_atom *atom) { @@ -1437,20 +1419,24 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re } static int get_object(struct ref_array_item *ref, const struct object_id *oid, - int deref, struct object **obj, struct strbuf *err) + int deref, struct object **obj, struct strbuf *err) { int eaten; int ret = 0; unsigned long size; - void *buf = get_obj(oid, obj, &size, &eaten); + enum object_type type; + void *buf = read_object_file(oid, &type, &size); if (!buf) ret = strbuf_addf_ret(err, -1, _("missing object %s for %s"), oid_to_hex(oid), ref->refname); - else if (!*obj) - ret = strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"), - oid_to_hex(oid), ref->refname); - else - grab_values(ref->value, deref, *obj, buf, size); + else { + *obj = parse_object_buffer(oid, type, size, buf, &eaten); + if (!*obj) + ret = strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"), + oid_to_hex(oid), ref->refname); + else + grab_values(ref->value, deref, *obj, buf, size); + } if (!eaten) free(buf); return ret; -- https://github.com/git/git/pull/520
[PATCH v2 4/4] ref-filter: use oid_object_info() to get object
Use oid_object_info_extended() to get object info instead of read_object_file(). It will help to handle some requests faster (e.g., we do not need to parse whole object if we need to know %(objectsize)). It could also help us to add new atoms such as %(objectsize:disk) and %(deltabase). Signed-off-by: Olga Telezhnaia --- ref-filter.c | 123 ++- 1 file changed, 89 insertions(+), 34 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index f04169f0ea0e3..112955f006648 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -61,6 +61,17 @@ struct refname_atom { int lstrip, rstrip; }; +static struct expand_data { + struct object_id oid; + enum object_type type; + unsigned long size; + off_t disk_size; + struct object_id delta_base_oid; + void *content; + + struct object_info info; +} oi, oi_deref; + /* * An atom is a valid field atom listed below, possibly prefixed with * a "*" to denote deref_tag(). @@ -202,6 +213,30 @@ static int remote_ref_atom_parser(const struct ref_format *format, struct used_a return 0; } +static int objecttype_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, _("%%(objecttype) does not take arguments")); + if (*atom->name == '*') + oi_deref.info.typep = &oi_deref.type; + else + oi.info.typep = &oi.type; + return 0; +} + +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; + return 0; +} + static int body_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { @@ -388,8 +423,8 @@ static struct { const char *arg, struct strbuf *err); } valid_atom[] = { { "refname", SOURCE_NONE, FIELD_STR, refname_atom_parser }, - { "objecttype", SOURCE_OTHER }, - { "objectsize", SOURCE_OTHER, FIELD_ULONG }, + { "objecttype", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser }, + { "objectsize", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser }, { "objectname", SOURCE_OTHER, FIELD_STR, objectname_atom_parser }, { "tree", SOURCE_OBJ }, { "parent", SOURCE_OBJ }, @@ -502,6 +537,12 @@ static int parse_ref_filter_atom(const struct ref_format *format, used_atom[at].name = xmemdupz(atom, ep - atom); used_atom[at].type = valid_atom[i].cmp_type; used_atom[at].source = valid_atom[i].source; + if (used_atom[at].source == SOURCE_OBJ) { + if (*atom == '*') + oi_deref.info.contentp = &oi_deref.content; + else + oi.info.contentp = &oi.content; + } if (arg) { arg = used_atom[at].name + (arg - atom) + 1; if (!*arg) { @@ -817,7 +858,7 @@ static int grab_objectname(const char *name, const struct object_id *oid, } /* See grab_values */ -static void grab_common_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz) +static void grab_common_values(struct atom_value *val, int deref, struct expand_data *oi) { int i; @@ -829,13 +870,13 @@ static void grab_common_values(struct atom_value *val, int deref, struct object if (deref) name++; if (!strcmp(name, "objecttype")) - v->s = type_name(obj->type); + v->s = type_name(oi->type); else if (!strcmp(name, "objectsize")) { - v->value = sz; - v->s = xstrfmt("%lu", sz); + v->value = oi->size; + v->s = xstrfmt("%lu", oi->size); } else if (deref) - grab_objectname(name, &obj->oid, v, &used_atom[i]); + grab_objectname(name, &oi->oid, v, &used_atom[i]); } } @@ -1194,7 +1235,6 @@ static void fill_missing_values(struct atom_value *val) */ static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz) { - grab_common_values(val, deref, obj, buf, sz); switch (obj->type) { case OBJ_TAG: grab_tag_values(val, deref, obj, buf, sz); @@ -1418,28 +1458,36 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re return show_ref(&atom->u.refname, re
[PATCH v3 2/5] ref-filter: fill empty fields with empty values
Atoms like "align" or "end" do not have string representation. Earlier we had to go and parse whole object with a hope that we could fill their string representations. It's easier to fill them with an empty string before we start to work with whole object. It is important to mention that we fill only these atoms that must contain nothing. So, if we could not fill the atom because, for example, the object is missing, we leave it with NULL. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ref-filter.c b/ref-filter.c index 8611c24fd57d1..27733ef013bed 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1497,6 +1497,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) refname = get_symref(atom, ref); else if (starts_with(name, "upstream")) { const char *branch_name; + v->s = ""; /* only local branches may have an upstream */ if (!skip_prefix(ref->refname, "refs/heads/", &branch_name)) @@ -1509,6 +1510,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) continue; } else if (atom->u.remote_ref.push) { const char *branch_name; + v->s = ""; if (!skip_prefix(ref->refname, "refs/heads/", &branch_name)) continue; @@ -1549,22 +1551,26 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) continue; } else if (starts_with(name, "align")) { v->handler = align_atom_handler; + v->s = ""; continue; } else if (!strcmp(name, "end")) { v->handler = end_atom_handler; + v->s = ""; continue; } else if (starts_with(name, "if")) { const char *s; - + v->s = ""; if (skip_prefix(name, "if:", &s)) v->s = xstrdup(s); v->handler = if_atom_handler; continue; } else if (!strcmp(name, "then")) { v->handler = then_atom_handler; + v->s = ""; continue; } else if (!strcmp(name, "else")) { v->handler = else_atom_handler; + v->s = ""; continue; } else continue; -- https://github.com/git/git/pull/520
[PATCH v3 4/5] ref-filter: merge get_obj and get_object
Inline get_obj(): it would be easier to edit the code without this split. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 36 +++- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 8db7ca95b12c0..2b401a17c4689 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -797,24 +797,6 @@ int verify_ref_format(struct ref_format *format) return 0; } -/* - * Given an object name, read the object data and size, and return a - * "struct object". If the object data we are returning is also borrowed - * by the "struct object" representation, set *eaten as well---it is a - * signal from parse_object_buffer to us not to free the buffer. - */ -static void *get_obj(const struct object_id *oid, struct object **obj, unsigned long *sz, int *eaten) -{ - enum object_type type; - void *buf = read_object_file(oid, &type, sz); - - if (buf) - *obj = parse_object_buffer(oid, type, *sz, buf, eaten); - else - *obj = NULL; - return buf; -} - static int grab_objectname(const char *name, const struct object_id *oid, struct atom_value *v, struct used_atom *atom) { @@ -1437,21 +1419,25 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re } static int get_object(struct ref_array_item *ref, const struct object_id *oid, - int deref, struct object **obj, struct strbuf *err) + int deref, struct object **obj, struct strbuf *err) { /* parse_object_buffer() will set eaten to 0 if free() will be needed */ int eaten = 1; int ret = 0; unsigned long size; - void *buf = get_obj(oid, obj, &size, &eaten); + enum object_type type; + void *buf = read_object_file(oid, &type, &size); if (!buf) ret = strbuf_addf_ret(err, -1, _("missing object %s for %s"), oid_to_hex(oid), ref->refname); - else if (!*obj) - ret = strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"), - oid_to_hex(oid), ref->refname); - else - grab_values(ref->value, deref, *obj, buf, size); + else { + *obj = parse_object_buffer(oid, type, size, buf, &eaten); + if (!*obj) + ret = strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"), + oid_to_hex(oid), ref->refname); + else + grab_values(ref->value, deref, *obj, buf, size); + } if (!eaten) free(buf); return ret; -- https://github.com/git/git/pull/520
[PATCH v3 1/5] ref-filter: add info_source to valid_atom
Add the source of object data to prevent parsing of unneeded data. The goal is to improve performance by avoiding calling expensive functions when we don't need the information they provide or when we could get it by using a cheaper function. It is stored in valid_atoms because it depends on the atoms we are interested in. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 82 +++- 1 file changed, 43 insertions(+), 39 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index fa3685d91f046..8611c24fd57d1 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -41,6 +41,7 @@ void setup_ref_filter_porcelain_msg(void) typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type; typedef enum { COMPARE_EQUAL, COMPARE_UNEQUAL, COMPARE_NONE } cmp_status; +typedef enum { SOURCE_NONE = 0, SOURCE_OBJ, SOURCE_OTHER } info_source; struct align { align_type position; @@ -73,6 +74,7 @@ struct refname_atom { static struct used_atom { const char *name; cmp_type type; + info_source source; union { char color[COLOR_MAXLEN]; struct align align; @@ -380,49 +382,50 @@ static int head_atom_parser(const struct ref_format *format, struct used_atom *a static struct { const char *name; + info_source source; cmp_type cmp_type; int (*parser)(const struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err); } valid_atom[] = { - { "refname" , FIELD_STR, refname_atom_parser }, - { "objecttype" }, - { "objectsize", FIELD_ULONG }, - { "objectname", FIELD_STR, objectname_atom_parser }, - { "tree" }, - { "parent" }, - { "numparent", FIELD_ULONG }, - { "object" }, - { "type" }, - { "tag" }, - { "author" }, - { "authorname" }, - { "authoremail" }, - { "authordate", FIELD_TIME }, - { "committer" }, - { "committername" }, - { "committeremail" }, - { "committerdate", FIELD_TIME }, - { "tagger" }, - { "taggername" }, - { "taggeremail" }, - { "taggerdate", FIELD_TIME }, - { "creator" }, - { "creatordate", FIELD_TIME }, - { "subject", FIELD_STR, subject_atom_parser }, - { "body", FIELD_STR, body_atom_parser }, - { "trailers", FIELD_STR, trailers_atom_parser }, - { "contents", FIELD_STR, contents_atom_parser }, - { "upstream", FIELD_STR, remote_ref_atom_parser }, - { "push", FIELD_STR, remote_ref_atom_parser }, - { "symref", FIELD_STR, refname_atom_parser }, - { "flag" }, - { "HEAD", FIELD_STR, head_atom_parser }, - { "color", FIELD_STR, color_atom_parser }, - { "align", FIELD_STR, align_atom_parser }, - { "end" }, - { "if", FIELD_STR, if_atom_parser }, - { "then" }, - { "else" }, + { "refname", SOURCE_NONE, FIELD_STR, refname_atom_parser }, + { "objecttype", SOURCE_OTHER }, + { "objectsize", SOURCE_OTHER, FIELD_ULONG }, + { "objectname", SOURCE_OTHER, FIELD_STR, objectname_atom_parser }, + { "tree", SOURCE_OBJ }, + { "parent", SOURCE_OBJ }, + { "numparent", SOURCE_OBJ, FIELD_ULONG }, + { "object", SOURCE_OBJ }, + { "type", SOURCE_OBJ }, + { "tag", SOURCE_OBJ }, + { "author", SOURCE_OBJ }, + { "authorname", SOURCE_OBJ }, + { "authoremail", SOURCE_OBJ }, + { "authordate", SOURCE_OBJ, FIELD_TIME }, + { "committer", SOURCE_OBJ }, + { "committername", SOURCE_OBJ }, + { "committeremail", SOURCE_OBJ }, + { "committerdate", SOURCE_OBJ, FIELD_TIME }, + { "tagger", SOURCE_OBJ }, + { "taggername", SOURCE_OBJ }, + { "taggeremail", SOURCE_OBJ }, + { "taggerdate", SOURCE_OBJ, FIELD_TIME }, + { "creator", SOURCE_OBJ }, + { "creatordate", SOURCE_OBJ, FIELD_TIME }, + { "subject", SOURCE_OBJ, FIELD_STR, subject_atom_parser }, + { "body", SOURCE_OBJ, FIELD_STR, body_atom_parser }, + { "trailers", SOURCE_OBJ, FIELD_STR, trailers_atom_parser }, + { "contents", SOURCE_OBJ, FIELD_STR, contents_atom_parser }, + { "upstream", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser }, + { "push", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser }, + { "symref", SOURCE_NONE, FIELD_STR, refname_atom_parser }, + { "flag", SOURCE_NONE }, + { "HEAD", SOURCE_NONE, FIELD_STR, head_atom_parser }, + { "color", SOURCE_NONE, FIELD_STR, color_atom_parser }, + { "align", SOURCE_NONE, FIELD_STR, align_atom_parser }, + { "end", SOURCE_NONE }, + { "if", SOURCE_NONE, FIELD_STR, if_atom_parser }, + { "then", SOURCE_NONE }, + { "else", SOURCE_NONE }, }; #define REF_FORMATTING_STATE_INIT { 0, NULL } @@ -498,6 +501,7 @@ static int parse_ref_filter_atom(const struct ref_format *format, REALLOC_ARRAY(used_atom, used_atom
[PATCH v3 5/5] ref-filter: use oid_object_info() to get object
Use oid_object_info_extended() to get object info instead of read_object_file(). It will help to handle some requests faster (e.g., we do not need to parse whole object if we need to know %(objectsize)). It could also help us to add new atoms such as %(objectsize:disk) and %(deltabase). Signed-off-by: Olga Telezhnaia --- ref-filter.c | 120 +++ 1 file changed, 87 insertions(+), 33 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 2b401a17c4689..112955f006648 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -61,6 +61,17 @@ struct refname_atom { int lstrip, rstrip; }; +static struct expand_data { + struct object_id oid; + enum object_type type; + unsigned long size; + off_t disk_size; + struct object_id delta_base_oid; + void *content; + + struct object_info info; +} oi, oi_deref; + /* * An atom is a valid field atom listed below, possibly prefixed with * a "*" to denote deref_tag(). @@ -202,6 +213,30 @@ static int remote_ref_atom_parser(const struct ref_format *format, struct used_a return 0; } +static int objecttype_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, _("%%(objecttype) does not take arguments")); + if (*atom->name == '*') + oi_deref.info.typep = &oi_deref.type; + else + oi.info.typep = &oi.type; + return 0; +} + +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; + return 0; +} + static int body_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { @@ -388,8 +423,8 @@ static struct { const char *arg, struct strbuf *err); } valid_atom[] = { { "refname", SOURCE_NONE, FIELD_STR, refname_atom_parser }, - { "objecttype", SOURCE_OTHER }, - { "objectsize", SOURCE_OTHER, FIELD_ULONG }, + { "objecttype", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser }, + { "objectsize", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser }, { "objectname", SOURCE_OTHER, FIELD_STR, objectname_atom_parser }, { "tree", SOURCE_OBJ }, { "parent", SOURCE_OBJ }, @@ -502,6 +537,12 @@ static int parse_ref_filter_atom(const struct ref_format *format, used_atom[at].name = xmemdupz(atom, ep - atom); used_atom[at].type = valid_atom[i].cmp_type; used_atom[at].source = valid_atom[i].source; + if (used_atom[at].source == SOURCE_OBJ) { + if (*atom == '*') + oi_deref.info.contentp = &oi_deref.content; + else + oi.info.contentp = &oi.content; + } if (arg) { arg = used_atom[at].name + (arg - atom) + 1; if (!*arg) { @@ -817,7 +858,7 @@ static int grab_objectname(const char *name, const struct object_id *oid, } /* See grab_values */ -static void grab_common_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz) +static void grab_common_values(struct atom_value *val, int deref, struct expand_data *oi) { int i; @@ -829,13 +870,13 @@ static void grab_common_values(struct atom_value *val, int deref, struct object if (deref) name++; if (!strcmp(name, "objecttype")) - v->s = type_name(obj->type); + v->s = type_name(oi->type); else if (!strcmp(name, "objectsize")) { - v->value = sz; - v->s = xstrfmt("%lu", sz); + v->value = oi->size; + v->s = xstrfmt("%lu", oi->size); } else if (deref) - grab_objectname(name, &obj->oid, v, &used_atom[i]); + grab_objectname(name, &oi->oid, v, &used_atom[i]); } } @@ -1194,7 +1235,6 @@ static void fill_missing_values(struct atom_value *val) */ static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz) { - grab_common_values(val, deref, obj, buf, sz); switch (obj->type) { case OBJ_TAG: grab_tag_values(val, deref, obj, buf, sz); @@ -1418,29 +1458,36 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re return show_ref(&atom->u.refname, re
[PATCH v3 3/5] ref-filter: initialize eaten variable
Initialize variable `eaten` before its using. We may initialize it in parse_object_buffer(), but there are cases when we do not reach this invocation. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ref-filter.c b/ref-filter.c index 27733ef013bed..8db7ca95b12c0 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1439,7 +1439,8 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re static int get_object(struct ref_array_item *ref, const struct object_id *oid, int deref, struct object **obj, struct strbuf *err) { - int eaten; + /* parse_object_buffer() will set eaten to 0 if free() will be needed */ + int eaten = 1; int ret = 0; unsigned long size; void *buf = get_obj(oid, obj, &size, &eaten); -- https://github.com/git/git/pull/520
[PATCH RFC 0/20] cat-file: start using formatting logic from ref-filter
Hi everyone, It was a long way for me, I got older (by 1 year) and smarter (hopefully), and maybe I will finish my Outreachy Internship task for now. (I am doing it just for one year and a half, that's OK) If serious: In this patch we remove cat-file formatting logic and reuse ref-filter logic there. As a positive side effect, cat-file now has many new formatting tokens (all from ref-filter formatting), including deref (like %(*objectsize:disk)). I have already tried to do this task one year ago, and it was bad attempt. I feel that today's patch is much better. In my opinion, it still has some issues. I mentioned all of them in TODOs in comments. All of them considered to be separate tasks for other patches. Some of them sound easy and could be great tasks for newbies. I also have a question about site https://git-scm.com/docs/ I thought it is updated automatically based on Documentation folder in the project, but it is not true. I edited docs for for-each-ref in December, I still see my patch in master, but for-each-ref docs in git-csm is outdated. Is it OK? Thank you! Olga
[PATCH RFC 19/20] cat-file: tests for new atoms added
Add some tests for new formatting atoms from ref-filter. Some of new atoms are supported automatically, some of them are expanded into empty string (because they are useless for some types of objects). Signed-off-by: Olga Telezhnaia --- t/t1006-cat-file.sh | 48 + 1 file changed, 48 insertions(+) diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index 43c4be1e5ef55..3c848f2773bbb 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -20,6 +20,19 @@ maybe_remove_timestamp () { fi } +test_atom () { +name=$1 +sha1=$2 +atoms=$3 +expected=$4 + +test_expect_success "$name" ' + echo "$expected" >expect && + echo $sha1 | git cat-file --batch-check="$atoms" >actual && + test_cmp expect actual +' +} + run_tests () { type=$1 sha1=$2 @@ -119,6 +132,13 @@ $content" maybe_remove_timestamp "$(cat actual.full)" $no_ts >actual && test_cmp expect actual ' + +for atom in refname parent body trailers upstream push symref flag +do + test_atom "Check %($atom) gives empty output" "$sha1" "%($atom)" "" +done + +test_atom "Check %(HEAD) gives only one space as output" "$sha1" '%(HEAD)' ' ' } hello_content="Hello World" @@ -140,6 +160,12 @@ test_expect_success '--batch-check without %(rest) considers whole line' ' test_cmp expect actual ' +shortname=`echo $hello_sha1 | sed 's/^.\{0\}\(.\{7\}\).*/\1/'` +test_atom 'Check format option %(objectname:short) works' "$hello_sha1" '%(objectname:short)' "$shortname" + +test_atom 'Check format option %(align) is not broken' \ +"$hello_sha1" "%(align:8)%(objecttype)%(end)%(objectname)" "blob $hello_sha1" + test_oid_init tree_sha1=$(git write-tree) @@ -159,6 +185,17 @@ $commit_message" run_tests 'commit' $commit_sha1 $commit_size "$commit_content" "$commit_content" 1 +test_atom "Check format option %(if) is not broken" "$commit_sha1" \ +"%(if)%(author)%(then)%(objectname)%(end)" "$commit_sha1" +test_atom "Check %(tree) works for commit" "$commit_sha1" "%(tree)" "$tree_sha1" +test_atom "Check %(numparent) works for commit" "$commit_sha1" "%(numparent)" "0" +test_atom "Check %(authorname) works for commit" "$commit_sha1" "%(authorname)" "$GIT_AUTHOR_NAME" +test_atom "Check %(authoremail) works for commit" "$commit_sha1" "%(authoremail)" "<$GIT_AUTHOR_EMAIL>" +test_atom "Check %(committername) works for commit" "$commit_sha1" "%(committername)" "$GIT_COMMITTER_NAME" +test_atom "Check %(committeremail) works for commit" "$commit_sha1" "%(committeremail)" "<$GIT_COMMITTER_EMAIL>" +test_atom "Check %(subject) works for commit" "$commit_sha1" "%(subject)" "$commit_message" +test_atom "Check %(contents) works for commit" "$commit_sha1" "%(contents)" "$commit_message" + tag_header_without_timestamp="object $hello_sha1 type blob tag hellotag @@ -173,6 +210,17 @@ tag_size=$(strlen "$tag_content") run_tests 'tag' $tag_sha1 $tag_size "$tag_content" "$tag_content" 1 +test_atom "Check %(object) works for tag" "$tag_sha1" "%(object)" "$hello_sha1" +test_atom "Check %(type) works for tag" "$tag_sha1" "%(type)" "blob" +test_atom "Check %(tag) works for tag" "$tag_sha1" "%(tag)" "hellotag" +test_atom "Check %(taggername) works for tag" "$tag_sha1" "%(taggername)" "$GIT_COMMITTER_NAME" +test_atom "Check %(taggeremail) works for tag" "$tag_sha1" "%(taggeremail)" "<$GIT_COMMITTER_EMAIL>" +test_atom "Check %(subject) works for tag" "$tag_sha1" "%(subject)" "$tag_description" +test_atom "Check %(contents) works for tag" "$tag_sha1" "%(contents)" "$tag_description" + +test_atom "Check %(color) gives no additional output" "$sha1" \ +"%(objectname) %(color:green) %(objecttype)" "$sha1 $type" + test_expect_success \ "Reach a blob from a tag pointing to it" \ "test '$hello_content' = \"\$(git cat-file blob $tag_sha1)\"" -- https://github.com/git/git/pull/568
[PATCH RFC 20/20] cat-file: update docs
Update the docs for cat-file command. Some new formatting atoms added because of reusing ref-filter code. Actually, %(rest) is supported for all ref-filter commands, but it has the meaning only for cat-file, that's why I decided to leave it here. Signed-off-by: Olga Telezhnaia --- Documentation/git-cat-file.txt | 38 ++ 1 file changed, 2 insertions(+), 36 deletions(-) diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt index 8eca671b8278c..b32fcde802ca1 100644 --- a/Documentation/git-cat-file.txt +++ b/Documentation/git-cat-file.txt @@ -195,28 +195,8 @@ the whole line is considered as an object, as if it were fed to linkgit:git-rev-parse[1]. You can specify the information shown for each object by using a custom -``. The `` is copied literally to stdout for each -object, with placeholders of the form `%(atom)` expanded, followed by a -newline. The available atoms are: - -`objectname`:: - The 40-hex object name of the object. - -`objecttype`:: - The type of the object (the same as `cat-file -t` reports). - -`objectsize`:: - The size, in bytes, of the object (the same as `cat-file -s` - reports). - -`objectsize:disk`:: - The size, in bytes, that the object takes up on disk. See the - note about on-disk sizes in the `CAVEATS` section below. - -`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` below. +``. The format is the same as that of linkgit:git-for-each-ref[1], +with one additional option: `rest`:: If this atom is used in the output string, input lines are split @@ -300,20 +280,6 @@ notdir SP LF is printed when, during symlink resolution, a file is used as a directory name. -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. - GIT --- Part of the linkgit:git[1] suite -- https://github.com/git/git/pull/568
[PATCH RFC 18/20] cat-file: get rid of expand_data
Clean up cat-file after moving all formatting logic to ref-filter. We do not need to use struct expand_data anymore. Signed-off-by: Olga Telezhnaia --- builtin/cat-file.c | 43 +++ ref-filter.c | 11 ++- ref-filter.h | 12 3 files changed, 33 insertions(+), 33 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 6fa100d1bea72..ee7557e1e0975 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -28,8 +28,6 @@ struct batch_options { }; static const char *force_path; -/* global rest will be deleted at the end of this patch */ -static const char *rest; static int cat_one_file(int opt, const char *exp_type, const char *obj_name, int unknown_type) @@ -170,16 +168,19 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name, static void batch_object_write(const char *obj_name, struct strbuf *scratch, struct batch_options *opt, - struct expand_data *data) + struct ref_array_item *item) { struct strbuf err = STRBUF_INIT; - struct ref_array_item item = { data->oid }; - item.request_rest = rest; - item.check_obj = 1; + /* +* TODO: get rid of memory leak. The best way is to reuse ref_array +* in batch_objects and then call ref_array_clear. +*/ + item->value = 0; + item->check_obj = 1; strbuf_reset(scratch); - if (format_ref_array_item(&item, &opt->format, scratch, &err)) { - printf("%s missing\n", obj_name ? obj_name : oid_to_hex(&item.oid)); + if (format_ref_array_item(item, &opt->format, scratch, &err)) { + printf("%s missing\n", obj_name ? obj_name : oid_to_hex(&item->oid)); fflush(stdout); return; } @@ -189,7 +190,7 @@ static void batch_object_write(const char *obj_name, strbuf_release(&err); if (opt->print_contents) { - print_raw_object_or_die(&item, opt->cmdmode, opt->buffer_output); + print_raw_object_or_die(item, opt->cmdmode, opt->buffer_output); write_or_die(1, "\n", 1); } } @@ -197,14 +198,14 @@ static void batch_object_write(const char *obj_name, static void batch_one_object(const char *obj_name, struct strbuf *scratch, struct batch_options *opt, -struct expand_data *data) +struct ref_array_item *item) { struct object_context ctx; int flags = opt->follow_symlinks ? GET_OID_FOLLOW_SYMLINKS : 0; enum get_oid_result result; result = get_oid_with_context(the_repository, obj_name, - flags, &data->oid, &ctx); + flags, &item->oid, &ctx); if (result != FOUND) { switch (result) { case MISSING_OBJECT: @@ -242,12 +243,12 @@ static void batch_one_object(const char *obj_name, return; } - batch_object_write(obj_name, scratch, opt, data); + batch_object_write(obj_name, scratch, opt, item); } struct object_cb_data { struct batch_options *opt; - struct expand_data *expand; + struct ref_array_item *item; struct oidset *seen; struct strbuf *scratch; }; @@ -255,8 +256,8 @@ struct object_cb_data { static int batch_object_cb(const struct object_id *oid, void *vdata) { struct object_cb_data *data = vdata; - oidcpy(&data->expand->oid, oid); - batch_object_write(NULL, data->scratch, data->opt, data->expand); + oidcpy(&data->item->oid, oid); + batch_object_write(NULL, data->scratch, data->opt, data->item); return 0; } @@ -306,20 +307,20 @@ static int batch_objects(struct batch_options *opt) { struct strbuf input = STRBUF_INIT; struct strbuf output = STRBUF_INIT; - struct expand_data data; int save_warning; int retval = 0; int is_rest = strstr(opt->format.format, "%(rest)") != NULL || opt->cmdmode; - memset(&data, 0, sizeof(data)); if (opt->all_objects) { struct object_cb_data cb; + struct ref_array_item item; + memset(&item, 0, sizeof(item)); if (repository_format_partial_clone) warning("This repository has extensions.partialClone set. Some objects may not be loaded."); cb.opt = opt; - cb.expand = &data; + cb.item = &item; cb.scratch = &output; if (opt->unordered) { @@ -358,6 +359,8 @@ static int batch_objects(struct batch_options *opt) warn_on_object_refname_ambiguity = 0; while (strbuf_getline(&input, stdin)
[PATCH RFC 13/20] cat-file: rewrite print_object_or_die
In the next commit I will move function print_object_or_die to ref-filter, and I decided to rewrite it a little so that it becomes much more flatten and a little bit shorter. I also changed input parameters, it allows me to move it to ref-filter, ref-filter knows nothing about batch_options. The logic of the function remains the same. Signed-off-by: Olga Telezhnaia --- builtin/cat-file.c | 72 +- 1 file changed, 33 insertions(+), 39 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index a4e56762f9e56..2066ff1e697e4 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -226,50 +226,17 @@ static size_t expand_format(struct strbuf *sb, const char *start, void *data) return end - start + 1; } -static void print_object_or_die(struct batch_options *opt, struct expand_data *data) +static void print_object_or_die(struct expand_data *data, int cmdmode, + int buffered, const char *rest) { const struct object_id *oid = &data->oid; + unsigned long size; + char *contents; assert(data->info.typep); - if (data->type == OBJ_BLOB) { - if (opt->buffer_output) - fflush(stdout); - if (opt->cmdmode) { - char *contents; - unsigned long size; - - if (!rest) - die("missing path for '%s'", oid_to_hex(oid)); - - if (opt->cmdmode == 'w') { - if (filter_object(rest, 0100644, oid, - &contents, &size)) - die("could not convert '%s' %s", - oid_to_hex(oid), rest); - } else if (opt->cmdmode == 'c') { - enum object_type type; - if (!textconv_object(the_repository, -rest, 0100644, oid, -1, &contents, &size)) - contents = read_object_file(oid, - &type, - &size); - if (!contents) - die("could not convert '%s' %s", - oid_to_hex(oid), rest); - } else - BUG("invalid cmdmode: %c", opt->cmdmode); - write_or_die(1, contents, size); - free(contents); - } else if (stream_blob_to_fd(1, oid, NULL, 0)) - die("unable to stream %s to stdout", oid_to_hex(oid)); - } - else { + if (data->type != OBJ_BLOB) { enum object_type type; - unsigned long size; - void *contents; - contents = read_object_file(oid, &type, &size); if (!contents) die("object %s disappeared", oid_to_hex(oid)); @@ -280,7 +247,34 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d write_or_die(1, contents, size); free(contents); + return; + } + + if (buffered) + fflush(stdout); + if (!cmdmode) { + if (stream_blob_to_fd(1, oid, NULL, 0)) + die("unable to stream %s to stdout", oid_to_hex(oid)); + return; } + + if (!rest) + die("missing path for '%s'", oid_to_hex(oid)); + + if (cmdmode == 'w') { + if (filter_object(rest, 0100644, oid, &contents, &size)) + die("could not convert '%s' %s", oid_to_hex(oid), rest); + } else if (cmdmode == 'c') { + enum object_type type; + if (!textconv_object(the_repository, rest, 0100644, oid, 1, +&contents, &size)) + contents = read_object_file(oid, &type, &size); + if (!contents) + die("could not convert '%s' %s", oid_to_hex(oid), rest); + } else + BUG("invalid cmdmode: %c", cmdmode); + write_or_die(1, contents, size); + free(contents); } static void batch_object_write(const char *obj_name, @@ -303,7 +297,7 @@ static void batch_object_write(const char *obj_name, write_or_die(1, scratch->buf, scratch->len); if (opt->print_contents) { - print_object_or_die(opt, data); + print_object_or_die(data, opt->cmdmode, opt->buffer_output, rest); write_or_die(1, "\n", 1); } } -- https://github.com/git/git/pull/568
[PATCH RFC 08/20] cat-file: remove rest from expand_data
Get rid of rest field in struct expand_data. expand_data may be global further as we use it in ref-filter also, so we need to remove cat-file specific fields from it. All globals that I add through this patch will be deleted in the end, so treat it just as the middle step. Signed-off-by: Olga Telezhnaia --- builtin/cat-file.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index f6470380f55b3..e52646c0e6b5b 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -29,9 +29,10 @@ struct batch_options { }; static const char *force_path; -/* Next 2 vars will be deleted at the end of this patch */ +/* Next 3 vars will be deleted at the end of this patch */ static int mark_query; static int skip_object_info; +static const char *rest; static int filter_object(const char *path, unsigned mode, const struct object_id *oid, @@ -197,7 +198,6 @@ struct expand_data { enum object_type type; unsigned long size; off_t disk_size; - const char *rest; struct object_id delta_base_oid; /* @@ -238,8 +238,8 @@ static void expand_atom(struct strbuf *sb, const char *atom, int len, else strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)data->disk_size); } else if (is_atom("rest", atom, len)) { - if (data->rest) - strbuf_addstr(sb, data->rest); + if (rest) + strbuf_addstr(sb, rest); } else if (is_atom("deltabase", atom, len)) { if (mark_query) data->info.delta_base_sha1 = data->delta_base_oid.hash; @@ -287,25 +287,25 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d char *contents; unsigned long size; - if (!data->rest) + if (!rest) die("missing path for '%s'", oid_to_hex(oid)); if (opt->cmdmode == 'w') { - if (filter_object(data->rest, 0100644, oid, + if (filter_object(rest, 0100644, oid, &contents, &size)) die("could not convert '%s' %s", - oid_to_hex(oid), data->rest); + oid_to_hex(oid), rest); } else if (opt->cmdmode == 'c') { enum object_type type; if (!textconv_object(the_repository, -data->rest, 0100644, oid, +rest, 0100644, oid, 1, &contents, &size)) contents = read_object_file(oid, &type, &size); if (!contents) die("could not convert '%s' %s", - oid_to_hex(oid), data->rest); + oid_to_hex(oid), rest); } else BUG("invalid cmdmode: %c", opt->cmdmode); batch_write(opt, contents, size); @@ -555,7 +555,7 @@ static int batch_objects(struct batch_options *opt) while (*p && strchr(" \t", *p)) *p++ = '\0'; } - data.rest = p; + rest = p; } batch_one_object(input.buf, &output, opt, &data); -- https://github.com/git/git/pull/568
[PATCH RFC 10/20] cat-file: inline stream_blob
Inline function stream_blob, it simplifies further migrating process. Signed-off-by: Olga Telezhnaia --- builtin/cat-file.c | 26 -- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index edf45f078b919..cd9a4447c8da9 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -56,13 +56,6 @@ static int filter_object(const char *path, unsigned mode, return 0; } -static int stream_blob(const struct object_id *oid) -{ - if (stream_blob_to_fd(1, oid, NULL, 0)) - die("unable to stream %s to stdout", oid_to_hex(oid)); - return 0; -} - static int cat_one_file(int opt, const char *exp_type, const char *obj_name, int unknown_type) { @@ -145,8 +138,11 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name, return cmd_ls_tree(2, ls_args, NULL); } - if (type == OBJ_BLOB) - return stream_blob(&oid); + if (type == OBJ_BLOB) { + if (stream_blob_to_fd(1, &oid, NULL, 0)) + die("unable to stream %s to stdout", oid_to_hex(&oid)); + return 0; + } buf = read_object_file(&oid, &type, &size); if (!buf) die("Cannot read object %s", obj_name); @@ -168,8 +164,11 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name, } else oidcpy(&blob_oid, &oid); - if (oid_object_info(the_repository, &blob_oid, NULL) == OBJ_BLOB) - return stream_blob(&blob_oid); + if (oid_object_info(the_repository, &blob_oid, NULL) == OBJ_BLOB) { + if (stream_blob_to_fd(1, &blob_oid, NULL, 0)) + die("unable to stream %s to stdout", oid_to_hex(&blob_oid)); + return 0; + } /* * we attempted to dereference a tag to a blob * and failed; there may be new dereference @@ -295,9 +294,8 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d BUG("invalid cmdmode: %c", opt->cmdmode); batch_write(opt, contents, size); free(contents); - } else { - stream_blob(oid); - } + } else if (stream_blob_to_fd(1, oid, NULL, 0)) + die("unable to stream %s to stdout", oid_to_hex(oid)); } else { enum object_type type; -- https://github.com/git/git/pull/568
[PATCH RFC 06/20] cat-file: remove mark_query from expand_data
Get rid of mark_query field in struct expand_data. expand_data may be global further as we use it in ref-filter also, so we need to remove cat-file specific fields from it. All globals that I add through this patch will be deleted in the end, so treat it just as the middle step. Signed-off-by: Olga Telezhnaia --- builtin/cat-file.c | 22 +- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 60f3839b06f8c..9bcb02fad1f0d 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -29,6 +29,8 @@ struct batch_options { }; static const char *force_path; +/* Will be deleted at the end of this patch */ +static int mark_query; static int filter_object(const char *path, unsigned mode, const struct object_id *oid, @@ -197,12 +199,6 @@ struct expand_data { const char *rest; struct object_id delta_base_oid; - /* -* If mark_query is true, we do not expand anything, but rather -* just mark the object_info with items we wish to query. -*/ - int mark_query; - /* * After a mark_query run, this object_info is set up to be * passed to oid_object_info_extended. It will point to the data @@ -230,20 +226,20 @@ static void expand_atom(struct strbuf *sb, const char *atom, int len, struct expand_data *data = vdata; if (is_atom("objectname", atom, len)) { - if (!data->mark_query) + if (!mark_query) strbuf_addstr(sb, oid_to_hex(&data->oid)); } else if (is_atom("objecttype", atom, len)) { - if (data->mark_query) + if (mark_query) data->info.typep = &data->type; else strbuf_addstr(sb, type_name(data->type)); } else if (is_atom("objectsize", atom, len)) { - if (data->mark_query) + if (mark_query) data->info.sizep = &data->size; else strbuf_addf(sb, "%"PRIuMAX , (uintmax_t)data->size); } else if (is_atom("objectsize:disk", atom, len)) { - if (data->mark_query) + if (mark_query) data->info.disk_sizep = &data->disk_size; else strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)data->disk_size); @@ -251,7 +247,7 @@ static void expand_atom(struct strbuf *sb, const char *atom, int len, if (data->rest) strbuf_addstr(sb, data->rest); } else if (is_atom("deltabase", atom, len)) { - if (data->mark_query) + if (mark_query) data->info.delta_base_sha1 = data->delta_base_oid.hash; else strbuf_addstr(sb, @@ -490,9 +486,9 @@ static int batch_objects(struct batch_options *opt) * object. */ memset(&data, 0, sizeof(data)); - data.mark_query = 1; + mark_query = 1; strbuf_expand(&output, opt->format.format, expand_format, &data); - data.mark_query = 0; + mark_query = 0; strbuf_release(&output); if (opt->all_objects) { -- https://github.com/git/git/pull/568
[PATCH RFC 05/20] cat-file: remove split_on_whitespace
Get rid of split_on_whitespace field in struct expand_data. expand_data may be global further as we use it in ref-filter also, so we need to remove cat-file specific fields from it. Signed-off-by: Olga Telezhnaia --- builtin/cat-file.c | 16 +++- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index e5de596611800..60f3839b06f8c 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -203,13 +203,6 @@ struct expand_data { */ int mark_query; - /* -* Whether to split the input on whitespace before feeding it to -* get_sha1; this is decided during the mark_query phase based on -* whether we have a %(rest) token in our format. -*/ - int split_on_whitespace; - /* * After a mark_query run, this object_info is set up to be * passed to oid_object_info_extended. It will point to the data @@ -255,9 +248,7 @@ static void expand_atom(struct strbuf *sb, const char *atom, int len, else strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)data->disk_size); } else if (is_atom("rest", atom, len)) { - if (data->mark_query) - data->split_on_whitespace = 1; - else if (data->rest) + if (data->rest) strbuf_addstr(sb, data->rest); } else if (is_atom("deltabase", atom, len)) { if (data->mark_query) @@ -491,6 +482,7 @@ static int batch_objects(struct batch_options *opt) struct expand_data data; int save_warning; int retval = 0; + int is_rest = strstr(opt->format.format, "%(rest)") != NULL || opt->cmdmode; /* * Expand once with our special mark_query flag, which will prime the @@ -502,8 +494,6 @@ static int batch_objects(struct batch_options *opt) strbuf_expand(&output, opt->format.format, expand_format, &data); data.mark_query = 0; strbuf_release(&output); - if (opt->cmdmode) - data.split_on_whitespace = 1; if (opt->all_objects) { struct object_info empty = OBJECT_INFO_INIT; @@ -564,7 +554,7 @@ static int batch_objects(struct batch_options *opt) warn_on_object_refname_ambiguity = 0; while (strbuf_getline(&input, stdin) != EOF) { - if (data.split_on_whitespace) { + if (is_rest) { /* * Split at first whitespace, tying off the beginning * of the string and saving the remainder (or NULL) in -- https://github.com/git/git/pull/568
[PATCH RFC 17/20] cat-file: reuse ref-filter formatting logic
Start using general ref-filter formatting logic in cat-file. Signed-off-by: Olga Telezhnaia --- builtin/cat-file.c | 111 - ref-filter.c | 39 +++- ref-filter.h | 4 +- 3 files changed, 49 insertions(+), 105 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 6c0cbf71f0f0c..6fa100d1bea72 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -28,9 +28,7 @@ struct batch_options { }; static const char *force_path; -/* Next 3 vars will be deleted at the end of this patch */ -static int mark_query; -static int skip_object_info; +/* global rest will be deleted at the end of this patch */ static const char *rest; static int cat_one_file(int opt, const char *exp_type, const char *obj_name, @@ -169,84 +167,29 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name, return 0; } -static int is_atom(const char *atom, const char *s, int slen) -{ - int alen = strlen(atom); - return alen == slen && !memcmp(atom, s, alen); -} - -static void expand_atom(struct strbuf *sb, const char *atom, int len, - void *vdata) -{ - struct expand_data *data = vdata; - - if (is_atom("objectname", atom, len)) { - if (!mark_query) - strbuf_addstr(sb, oid_to_hex(&data->oid)); - } else if (is_atom("objecttype", atom, len)) { - if (mark_query) - data->info.typep = &data->type; - else - strbuf_addstr(sb, type_name(data->type)); - } else if (is_atom("objectsize", atom, len)) { - if (mark_query) - data->info.sizep = &data->size; - else - strbuf_addf(sb, "%"PRIuMAX , (uintmax_t)data->size); - } else if (is_atom("objectsize:disk", atom, len)) { - if (mark_query) - data->info.disk_sizep = &data->disk_size; - else - strbuf_addf(sb, "%"PRIuMAX, (uintmax_t)data->disk_size); - } else if (is_atom("rest", atom, len)) { - if (rest) - strbuf_addstr(sb, rest); - } else if (is_atom("deltabase", atom, len)) { - if (mark_query) - data->info.delta_base_sha1 = data->delta_base_oid.hash; - else - strbuf_addstr(sb, - oid_to_hex(&data->delta_base_oid)); - } else - die("unknown format element: %.*s", len, atom); -} - -static size_t expand_format(struct strbuf *sb, const char *start, void *data) -{ - const char *end; - - if (*start != '(') - return 0; - end = strchr(start + 1, ')'); - if (!end) - die("format element '%s' does not end in ')'", start); - - expand_atom(sb, start + 1, end - start - 1, data); - - return end - start + 1; -} - static void batch_object_write(const char *obj_name, struct strbuf *scratch, struct batch_options *opt, struct expand_data *data) { - if (!skip_object_info && - oid_object_info_extended(the_repository, &data->oid, &data->info, -OBJECT_INFO_LOOKUP_REPLACE) < 0) { - printf("%s missing\n", - obj_name ? obj_name : oid_to_hex(&data->oid)); + struct strbuf err = STRBUF_INIT; + struct ref_array_item item = { data->oid }; + item.request_rest = rest; + item.check_obj = 1; + strbuf_reset(scratch); + + if (format_ref_array_item(&item, &opt->format, scratch, &err)) { + printf("%s missing\n", obj_name ? obj_name : oid_to_hex(&item.oid)); fflush(stdout); return; } - strbuf_reset(scratch); - strbuf_expand(scratch, opt->format.format, expand_format, data); strbuf_addch(scratch, '\n'); write_or_die(1, scratch->buf, scratch->len); + strbuf_release(&err); if (opt->print_contents) { - print_object_or_die(data, opt->cmdmode, opt->buffer_output, rest); + print_raw_object_or_die(&item, opt->cmdmode, opt->buffer_output); write_or_die(1, "\n", 1); } } @@ -367,30 +310,7 @@ static int batch_objects(struct batch_options *opt) int save_warning; int retval = 0; int is_rest = strstr(opt->format.format, "%(rest)") != NULL || opt->cmdmode; - - /* -* Expand once with our special mark_query flag, which will prime the -* object_info to be handed to oid_object_info_extended for each -* object. -*/ memset(&data, 0, sizeof(data)); - mark_query = 1; - strbuf_expand(&output, opt->format.format, expand_format, &data); - mark_query = 0;
[PATCH RFC 09/20] ref-filter: make expand_data global
Put struct expand_data into global scope to reuse it in cat-file. Signed-off-by: Olga Telezhnaia --- builtin/cat-file.c | 15 --- ref-filter.c | 11 +-- ref-filter.h | 12 3 files changed, 13 insertions(+), 25 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index e52646c0e6b5b..edf45f078b919 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -193,21 +193,6 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name, return 0; } -struct expand_data { - struct object_id oid; - enum object_type type; - unsigned long size; - off_t disk_size; - struct object_id delta_base_oid; - - /* -* After a mark_query run, this object_info is set up to be -* passed to oid_object_info_extended. It will point to the data -* elements above, so you can retrieve the response from there. -*/ - struct object_info info; -}; - static int is_atom(const char *atom, const char *s, int slen) { int alen = strlen(atom); diff --git a/ref-filter.c b/ref-filter.c index 46bf89b3330de..65b94ea21e54f 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -64,16 +64,7 @@ struct refname_atom { int lstrip, rstrip; }; -static struct expand_data { - struct object_id oid; - enum object_type type; - unsigned long size; - off_t disk_size; - struct object_id delta_base_oid; - void *content; - - struct object_info info; -} oi, oi_deref; +static struct expand_data oi, oi_deref; /* * An atom is a valid field atom listed below, possibly prefixed with diff --git a/ref-filter.h b/ref-filter.h index aaeda9f324f5c..fc61457d4d660 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -5,6 +5,7 @@ #include "refs.h" #include "commit.h" #include "parse-options.h" +#include "object-store.h" /* Quoting styles */ #define QUOTE_NONE 0 @@ -73,6 +74,17 @@ struct ref_filter { verbose; }; +struct expand_data { + struct object_id oid; + enum object_type type; + unsigned long size; + off_t disk_size; + struct object_id delta_base_oid; + void *content; + + struct object_info info; +}; + struct ref_format { /* * Set these to define the format; make sure you call -- https://github.com/git/git/pull/568
[PATCH RFC 02/20] ref-filter: rename field in ref_array_item stuct
Rename objectname field to oid in struct ref_array_item. We usually use objectname word for string representation of object id, so oid explains the content better. Signed-off-by: Olga Telezhnaia --- builtin/ls-remote.c | 2 +- ref-filter.c| 8 ref-filter.h| 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c index 1d7f1f5ce2783..ce79aede726c7 100644 --- a/builtin/ls-remote.c +++ b/builtin/ls-remote.c @@ -143,7 +143,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix) const struct ref_array_item *ref = ref_array.items[i]; if (show_symref_target && ref->symref) printf("ref: %s\t%s\n", ref->symref, ref->refname); - printf("%s\t%s\n", oid_to_hex(&ref->objectname), ref->refname); + printf("%s\t%s\n", oid_to_hex(&ref->oid), ref->refname); status = 0; /* we found something */ } diff --git a/ref-filter.c b/ref-filter.c index 422a9c9ae3fd2..736e1f9cc38fc 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1615,7 +1615,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) v->s = xstrdup(buf + 1); } continue; - } else if (!deref && grab_objectname(name, &ref->objectname, v, atom)) { + } else if (!deref && grab_objectname(name, &ref->oid, v, atom)) { continue; } else if (!strcmp(name, "HEAD")) { if (atom->u.head && !strcmp(ref->refname, atom->u.head)) @@ -1661,7 +1661,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) struct atom_value *v = &ref->value[i]; if (v->s == NULL && used_atom[i].source == SOURCE_NONE) return strbuf_addf_ret(err, -1, _("missing object %s for %s"), - oid_to_hex(&ref->objectname), ref->refname); + oid_to_hex(&ref->oid), ref->refname); } if (need_tagged) @@ -1671,7 +1671,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) return 0; - oi.oid = ref->objectname; + oi.oid = ref->oid; if (get_object(ref, 0, &obj, &oi, err)) return -1; @@ -1898,7 +1898,7 @@ static struct ref_array_item *new_ref_array_item(const char *refname, struct ref_array_item *ref; FLEX_ALLOC_STR(ref, refname, refname); - oidcpy(&ref->objectname, oid); + oidcpy(&ref->oid, oid); return ref; } diff --git a/ref-filter.h b/ref-filter.h index 85c8ebc3b904e..4d7d36e9f522d 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -34,7 +34,7 @@ struct ref_sorting { }; struct ref_array_item { - struct object_id objectname; + struct object_id oid; int flag; unsigned int kind; const char *symref; -- https://github.com/git/git/pull/568
[PATCH RFC 04/20] for-each-ref: tests for new atom %(rest) added
Add tests for new formatting atom %(rest). We need this atom for cat-file command. Signed-off-by: Olga Telezhnaia --- t/t6300-for-each-ref.sh | 6 ++ 1 file changed, 6 insertions(+) diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index 0ffd63071392e..fb361369a037c 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -322,6 +322,12 @@ test_expect_success 'exercise strftime with odd fields' ' test_cmp expected actual ' +test_expect_success 'Check format %(rest) gives empty output ' ' + echo >expected && + git for-each-ref --format="%(rest)" refs/heads >actual && + test_cmp expected actual +' + cat >expected <<\EOF refs/heads/master refs/remotes/origin/master -- https://github.com/git/git/pull/568
[PATCH RFC 12/20] cat-file: remove batch_write function
Correct me if I am wrong, but it was not really good idea to implement batch_write in cmd_cat_file. Maybe it's a good task for newbies to add flag (whether we accept batch write or not) to write_or_die? Signed-off-by: Olga Telezhnaia --- builtin/cat-file.c | 17 - 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 41f333b73d851..a4e56762f9e56 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -226,15 +226,6 @@ static size_t expand_format(struct strbuf *sb, const char *start, void *data) return end - start + 1; } -static void batch_write(struct batch_options *opt, const void *data, int len) -{ - if (opt->buffer_output) { - if (fwrite(data, 1, len, stdout) != len) - die_errno("unable to write to stdout"); - } else - write_or_die(1, data, len); -} - static void print_object_or_die(struct batch_options *opt, struct expand_data *data) { const struct object_id *oid = &data->oid; @@ -269,7 +260,7 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d oid_to_hex(oid), rest); } else BUG("invalid cmdmode: %c", opt->cmdmode); - batch_write(opt, contents, size); + write_or_die(1, contents, size); free(contents); } else if (stream_blob_to_fd(1, oid, NULL, 0)) die("unable to stream %s to stdout", oid_to_hex(oid)); @@ -287,7 +278,7 @@ static void print_object_or_die(struct batch_options *opt, struct expand_data *d if (data->info.sizep && size != data->size) die("object %s changed size!?", oid_to_hex(oid)); - batch_write(opt, contents, size); + write_or_die(1, contents, size); free(contents); } } @@ -309,11 +300,11 @@ static void batch_object_write(const char *obj_name, strbuf_reset(scratch); strbuf_expand(scratch, opt->format.format, expand_format, data); strbuf_addch(scratch, '\n'); - batch_write(opt, scratch->buf, scratch->len); + write_or_die(1, scratch->buf, scratch->len); if (opt->print_contents) { print_object_or_die(opt, data); - batch_write(opt, "\n", 1); + write_or_die(1, "\n", 1); } } -- https://github.com/git/git/pull/568
[PATCH RFC 15/20] ref-filter: add raw formatting option
Add new formatting option %(raw), it means that we want to print all the file without any changes. It will help further to migrate all cat-file formatting logic from cat-file to ref-filter. For now, we just treat it as the empty string. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 13 + 1 file changed, 13 insertions(+) diff --git a/ref-filter.c b/ref-filter.c index 68d9741a56468..bb963a4110fb2 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -208,6 +208,15 @@ static int remote_ref_atom_parser(const struct ref_format *format, struct used_a return 0; } +static int raw_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, _("%%(raw) does not take arguments")); + oi.info.typep = &oi.type; + return 0; +} + static int objecttype_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg, struct strbuf *err) { @@ -478,6 +487,7 @@ static struct { { "then", SOURCE_NONE }, { "else", SOURCE_NONE }, { "rest", SOURCE_NONE }, + { "raw", SOURCE_NONE, FIELD_STR, raw_atom_parser }, }; #define REF_FORMATTING_STATE_INIT { 0, NULL } @@ -1619,6 +1629,9 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) } else if (starts_with(name, "rest")) { v->s = xstrdup(ref->request_rest ? ref->request_rest : ""); continue; + } else if (!strcmp(name, "raw")) { + v->s = xstrdup(""); + continue; } else if (starts_with(name, "align")) { v->handler = align_atom_handler; v->s = xstrdup(""); -- https://github.com/git/git/pull/568
[PATCH RFC 07/20] cat-file: remove skip_object_info
Get rid of skip_object_info field in struct expand_data. expand_data may be global further as we use it in ref-filter also, so we need to remove cat-file specific fields from it. All globals that I add through this patch will be deleted in the end, so treat it just as the middle step. Signed-off-by: Olga Telezhnaia --- builtin/cat-file.c | 14 -- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 9bcb02fad1f0d..f6470380f55b3 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -29,8 +29,9 @@ struct batch_options { }; static const char *force_path; -/* Will be deleted at the end of this patch */ +/* Next 2 vars will be deleted at the end of this patch */ static int mark_query; +static int skip_object_info; static int filter_object(const char *path, unsigned mode, const struct object_id *oid, @@ -205,13 +206,6 @@ struct expand_data { * elements above, so you can retrieve the response from there. */ struct object_info info; - - /* -* This flag will be true if the requested batch format and options -* don't require us to call oid_object_info, which can then be -* optimized out. -*/ - unsigned skip_object_info : 1; }; static int is_atom(const char *atom, const char *s, int slen) @@ -343,7 +337,7 @@ static void batch_object_write(const char *obj_name, struct batch_options *opt, struct expand_data *data) { - if (!data->skip_object_info && + if (!skip_object_info && oid_object_info_extended(the_repository, &data->oid, &data->info, OBJECT_INFO_LOOKUP_REPLACE) < 0) { printf("%s missing\n", @@ -494,7 +488,7 @@ static int batch_objects(struct batch_options *opt) if (opt->all_objects) { struct object_info empty = OBJECT_INFO_INIT; if (!memcmp(&data.info, &empty, sizeof(empty))) - data.skip_object_info = 1; + skip_object_info = 1; } /* -- https://github.com/git/git/pull/568
[PATCH RFC 16/20] for-each-ref: tests for new atom %(raw) added
Add tests for new formatting atom %(raw). We need this atom for cat-file command. Signed-off-by: Olga Telezhnaia --- t/t6300-for-each-ref.sh | 6 ++ 1 file changed, 6 insertions(+) diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index fb361369a037c..6a5626d537f35 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -328,6 +328,12 @@ test_expect_success 'Check format %(rest) gives empty output ' ' test_cmp expected actual ' +test_expect_success 'Check format %(raw) gives empty output ' ' + echo >expected && + git for-each-ref --format="%(raw)" refs/heads >actual && + test_cmp expected actual +' + cat >expected <<\EOF refs/heads/master refs/remotes/origin/master -- https://github.com/git/git/pull/568
[PATCH RFC 11/20] cat-file: move filter_object to diff.c
Move function filter_object to diff.c, like it is done with function textconv_object. Signed-off-by: Olga Telezhnaia --- builtin/cat-file.c | 23 --- diff.c | 23 +++ diff.h | 4 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index cd9a4447c8da9..41f333b73d851 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -3,7 +3,6 @@ * * Copyright (C) Linus Torvalds, 2005 */ -#define USE_THE_INDEX_COMPATIBILITY_MACROS #include "cache.h" #include "config.h" #include "builtin.h" @@ -34,28 +33,6 @@ static int mark_query; static int skip_object_info; static const char *rest; -static int filter_object(const char *path, unsigned mode, -const struct object_id *oid, -char **buf, unsigned long *size) -{ - enum object_type type; - - *buf = read_object_file(oid, &type, size); - if (!*buf) - return error(_("cannot read object %s '%s'"), -oid_to_hex(oid), path); - if ((type == OBJ_BLOB) && S_ISREG(mode)) { - struct strbuf strbuf = STRBUF_INIT; - if (convert_to_working_tree(&the_index, path, *buf, *size, &strbuf)) { - free(*buf); - *size = strbuf.len; - *buf = strbuf_detach(&strbuf, NULL); - } - } - - return 0; -} - static int cat_one_file(int opt, const char *exp_type, const char *obj_name, int unknown_type) { diff --git a/diff.c b/diff.c index 5306c48652db5..fe7160c86755d 100644 --- a/diff.c +++ b/diff.c @@ -1,6 +1,7 @@ /* * Copyright (C) 2005 Junio C Hamano */ +#define USE_THE_INDEX_COMPATIBILITY_MACROS #include "cache.h" #include "config.h" #include "tempfile.h" @@ -6524,6 +6525,28 @@ int textconv_object(struct repository *r, return 1; } +int filter_object(const char *path, unsigned mode, + const struct object_id *oid, + char **buf, unsigned long *size) +{ + enum object_type type; + + *buf = read_object_file(oid, &type, size); + if (!*buf) + return error(_("cannot read object %s '%s'"), +oid_to_hex(oid), path); + if ((type == OBJ_BLOB) && S_ISREG(mode)) { + struct strbuf strbuf = STRBUF_INIT; + if (convert_to_working_tree(&the_index, path, *buf, *size, &strbuf)) { + free(*buf); + *size = strbuf.len; + *buf = strbuf_detach(&strbuf, NULL); + } + } + + return 0; +} + void setup_diff_pager(struct diff_options *opt) { /* diff --git a/diff.h b/diff.h index b512d0477ac3a..c3709e611870a 100644 --- a/diff.h +++ b/diff.h @@ -476,6 +476,10 @@ int textconv_object(struct repository *repo, const struct object_id *oid, int oid_valid, char **buf, unsigned long *buf_size); +int filter_object(const char *path, unsigned mode, + const struct object_id *oid, + char **buf, unsigned long *size); + int parse_rename_score(const char **cp_p); long parse_algorithm_value(const char *value); -- https://github.com/git/git/pull/568
[PATCH RFC 03/20] ref-filter: add rest formatting option
Add rest option that allows to add string into ref_array_item and then put it into specific place of the output. We are using it now in cat-file command: user could put anything in the input after objectname, and it will appear in the output in place of %(rest). Signed-off-by: Olga Telezhnaia --- ref-filter.c | 4 ref-filter.h | 1 + 2 files changed, 5 insertions(+) diff --git a/ref-filter.c b/ref-filter.c index 736e1f9cc38fc..46bf89b3330de 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -485,6 +485,7 @@ static struct { { "if", SOURCE_NONE, FIELD_STR, if_atom_parser }, { "then", SOURCE_NONE }, { "else", SOURCE_NONE }, + { "rest", SOURCE_NONE }, }; #define REF_FORMATTING_STATE_INIT { 0, NULL } @@ -1623,6 +1624,9 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) else v->s = xstrdup(" "); continue; + } else if (starts_with(name, "rest")) { + v->s = xstrdup(ref->request_rest ? ref->request_rest : ""); + continue; } else if (starts_with(name, "align")) { v->handler = align_atom_handler; v->s = xstrdup(""); diff --git a/ref-filter.h b/ref-filter.h index 4d7d36e9f522d..aaeda9f324f5c 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -40,6 +40,7 @@ struct ref_array_item { const char *symref; struct commit *commit; struct atom_value *value; + const char *request_rest; char refname[FLEX_ARRAY]; }; -- https://github.com/git/git/pull/568
[PATCH RFC 01/20] cat-file: reuse struct ref_format
Start using ref_format struct instead of simple char*. Need that for further reusing of formatting logic from ref-filter. Signed-off-by: Olga Telezhnaia --- builtin/cat-file.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 0f092382e175c..e5de596611800 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -15,8 +15,10 @@ #include "sha1-array.h" #include "packfile.h" #include "object-store.h" +#include "ref-filter.h" struct batch_options { + struct ref_format format; int enabled; int follow_symlinks; int print_contents; @@ -24,7 +26,6 @@ struct batch_options { int all_objects; int unordered; int cmdmode; /* may be 'w' or 'c' for --filters or --textconv */ - const char *format; }; static const char *force_path; @@ -365,7 +366,7 @@ static void batch_object_write(const char *obj_name, } strbuf_reset(scratch); - strbuf_expand(scratch, opt->format, expand_format, data); + strbuf_expand(scratch, opt->format.format, expand_format, data); strbuf_addch(scratch, '\n'); batch_write(opt, scratch->buf, scratch->len); @@ -491,9 +492,6 @@ static int batch_objects(struct batch_options *opt) int save_warning; int retval = 0; - if (!opt->format) - opt->format = "%(objectname) %(objecttype) %(objectsize)"; - /* * Expand once with our special mark_query flag, which will prime the * object_info to be handed to oid_object_info_extended for each @@ -501,7 +499,7 @@ static int batch_objects(struct batch_options *opt) */ memset(&data, 0, sizeof(data)); data.mark_query = 1; - strbuf_expand(&output, opt->format, expand_format, &data); + strbuf_expand(&output, opt->format.format, expand_format, &data); data.mark_query = 0; strbuf_release(&output); if (opt->cmdmode) @@ -617,7 +615,7 @@ static int batch_option_callback(const struct option *opt, bo->enabled = 1; bo->print_contents = !strcmp(opt->long_name, "batch"); - bo->format = arg; + bo->format.format = arg; return 0; } @@ -626,7 +624,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix) { int opt = 0; const char *exp_type = NULL, *obj_name = NULL; - struct batch_options batch = {0}; + struct batch_options batch = { REF_FORMAT_INIT }; int unknown_type = 0; const struct option options[] = { @@ -707,6 +705,9 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix) if (batch.buffer_output < 0) batch.buffer_output = batch.all_objects; + if (!batch.format.format) + batch.format.format = "%(objectname) %(objecttype) %(objectsize)"; + if (batch.enabled) return batch_objects(&batch); -- https://github.com/git/git/pull/568
[PATCH RFC 14/20] cat-file: move print_object_or_die to ref-filter
Move printing function to ref-filter, it is logical because we move all formatting/printing logic to ref-filter. It could be much better if we embed this logic into current flows in ref-filter, but it looks like the task for another patch. Signed-off-by: Olga Telezhnaia --- builtin/cat-file.c | 51 - ref-filter.c | 52 ++ ref-filter.h | 3 +++ 3 files changed, 55 insertions(+), 51 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 2066ff1e697e4..6c0cbf71f0f0c 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -226,57 +226,6 @@ static size_t expand_format(struct strbuf *sb, const char *start, void *data) return end - start + 1; } -static void print_object_or_die(struct expand_data *data, int cmdmode, - int buffered, const char *rest) -{ - const struct object_id *oid = &data->oid; - unsigned long size; - char *contents; - - assert(data->info.typep); - - if (data->type != OBJ_BLOB) { - enum object_type type; - contents = read_object_file(oid, &type, &size); - if (!contents) - die("object %s disappeared", oid_to_hex(oid)); - if (type != data->type) - die("object %s changed type!?", oid_to_hex(oid)); - if (data->info.sizep && size != data->size) - die("object %s changed size!?", oid_to_hex(oid)); - - write_or_die(1, contents, size); - free(contents); - return; - } - - if (buffered) - fflush(stdout); - if (!cmdmode) { - if (stream_blob_to_fd(1, oid, NULL, 0)) - die("unable to stream %s to stdout", oid_to_hex(oid)); - return; - } - - if (!rest) - die("missing path for '%s'", oid_to_hex(oid)); - - if (cmdmode == 'w') { - if (filter_object(rest, 0100644, oid, &contents, &size)) - die("could not convert '%s' %s", oid_to_hex(oid), rest); - } else if (cmdmode == 'c') { - enum object_type type; - if (!textconv_object(the_repository, rest, 0100644, oid, 1, -&contents, &size)) - contents = read_object_file(oid, &type, &size); - if (!contents) - die("could not convert '%s' %s", oid_to_hex(oid), rest); - } else - BUG("invalid cmdmode: %c", cmdmode); - write_or_die(1, contents, size); - free(contents); -} - static void batch_object_write(const char *obj_name, struct strbuf *scratch, struct batch_options *opt, diff --git a/ref-filter.c b/ref-filter.c index 65b94ea21e54f..68d9741a56468 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -20,6 +20,7 @@ #include "commit-slab.h" #include "commit-graph.h" #include "commit-reach.h" +#include "streaming.h" static struct ref_msg { const char *gone; @@ -2366,3 +2367,54 @@ int parse_opt_merge_filter(const struct option *opt, const char *arg, int unset) return 0; } + +void print_object_or_die(struct expand_data *data, int cmdmode, +int buffered, const char *rest) +{ + const struct object_id *oid = &data->oid; + unsigned long size; + char *contents; + + assert(data->info.typep); + + if (data->type != OBJ_BLOB) { + enum object_type type; + contents = read_object_file(oid, &type, &size); + if (!contents) + die("object %s disappeared", oid_to_hex(oid)); + if (type != data->type) + die("object %s changed type!?", oid_to_hex(oid)); + if (data->info.sizep && size != data->size) + die("object %s changed size!?", oid_to_hex(oid)); + + write_or_die(1, contents, size); + free(contents); + return; + } + + if (buffered) + fflush(stdout); + if (!cmdmode) { + if (stream_blob_to_fd(1, oid, NULL, 0)) + die("unable to stream %s to stdout", oid_to_hex(oid)); + return; + } + + if (!rest) + die("missing path for '%s'", oid_to_hex(oid)); + + if (cmdmode == 'w') { + if (filter_object(rest, 0100644, oid, &contents, &size)) + die("could not convert '%s' %s", oid_to_hex(oid), rest); + } else if (cmdmode == 'c') { + enum object_type type; + if (!textconv_object(the_repository, rest, 0100644, oid, 1, +&contents, &size)) + contents = read_object_file(oid, &type, &size); +
Re: [PATCH RFC 0/20] cat-file: start using formatting logic from ref-filter
пт, 22 февр. 2019 г. в 19:09, Eric Sunshine : > > On Fri, Feb 22, 2019 at 10:58 AM Olga Telezhnaya > wrote: > > I also have a question about site https://git-scm.com/docs/ > > I thought it is updated automatically based on Documentation folder in > > the project, but it is not true. I edited docs for for-each-ref in > > December, I still see my patch in master, but for-each-ref docs in > > git-csm is outdated. Is it OK? > > If you look at https://git-scm.com/docs/git-for-each-ref, you'll find > a pop-up control at the top of the page which allows you to select > documentation for a particular release of Git (say, 2.19.1). Your > change to git-for-each-ref.txt may be in "master" but is not yet in > any official final release. It will be in 2.21.0, but that release is > still in the RC stage, thus doesn't appear at > https://git-scm.com/docs/git-for-each-ref. Oh, thank you, I missed that.
Re: [PATCH RFC 04/20] for-each-ref: tests for new atom %(rest) added
пт, 1 мар. 2019 г. в 00:11, Jeff King : > > On Fri, Feb 22, 2019 at 04:05:45PM +, Olga Telezhnaya wrote: > > > Add tests for new formatting atom %(rest). > > We need this atom for cat-file command. > > While I do normally encourage splitting up commits, in this case I think > it would make sense to squash this together with patch 3. There's > nothing to say here about what %(rest) is that isn't already said in > that commit message. Agree, will squash. > > That said, I'm still not sure that for-each-ref should be supporting > %(rest) at all. We should hopefully already have coverage of cat-file > using "%(rest)" (and if not, we should add some to make sure it's not > regressed by the conversion). If we want to use ref-filter formatting logic in cat-file, we have to add this atom in ref-filter. I agree that we do not need it in ref-filter, and that's why I left %(rest) in cat-file docs (it's in the end of the patch). But in the code, I am not sure we want to make one more array with specific cat-file atoms (or atoms for other command). > > -Peff
Re: [PATCH RFC 0/20] cat-file: start using formatting logic from ref-filter
пт, 1 мар. 2019 г. в 00:41, Jeff King : > > On Fri, Feb 22, 2019 at 06:50:06PM +0300, Olga Telezhnaya wrote: > > > It was a long way for me, I got older (by 1 year) and smarter > > (hopefully), and maybe I will finish my Outreachy Internship task for > > now. (I am doing it just for one year and a half, that's OK) > > Welcome back! > > Sorry to be a bit slow on the review. I've read through and commented on > patch 10. Some of my comments were "I'll have to see how this plays out > later in the series", so you may want to hold off on responding until I > read the rest. :) > > > If serious: > > In this patch we remove cat-file formatting logic and reuse ref-filter > > logic there. As a positive side effect, cat-file now has many new > > formatting tokens (all from ref-filter formatting), including deref > > (like %(*objectsize:disk)). I have already tried to do this task one > > year ago, and it was bad attempt. I feel that today's patch is much > > better. > > I'm still concerned that this is going to regress the performance of > cat-file noticeably without some big cleanups in ref-filter. Here are > timings on linux.git before and after your patches: > > [before] > $ time git cat-file --unordered --batch-all-objects --batch-check >/dev/null > real 0m16.602s > user 0m15.545s > sys 0m0.495s > > [after] > $ time git cat-file --unordered --batch-all-objects --batch-check >/dev/null > real 0m27.301s > user 0m24.549s > sys 0m2.752s > > I don't think that's anything particularly wrong with your patches. It's > the existing strategy of ref-filter (in particular how it is very eager > to allocate lots of separate strings). And it may be too early to switch > cat-file over to it. I have a guess that we need to add batch printing argument to our general printing functions, that could make my version faster. > > > I also have a question about site https://git-scm.com/docs/ > > I thought it is updated automatically based on Documentation folder in > > the project, but it is not true. I edited docs for for-each-ref in > > December, I still see my patch in master, but for-each-ref docs in > > git-csm is outdated. Is it OK? > > Yeah, as Eric noted, we only build docs for the tagged releases. In > theory it would be easy to just build the tip of master nightly, but the > data model for the site would need quite a bit of adjustment. > > -Peff
Re: [PATCH RFC 0/20] cat-file: start using formatting logic from ref-filter
пт, 1 мар. 2019 г. в 00:43, Jeff King : > > On Fri, Feb 22, 2019 at 06:50:06PM +0300, Olga Telezhnaya wrote: > > > In my opinion, it still has some issues. I mentioned all of them in > > TODOs in comments. All of them considered to be separate tasks for > > other patches. Some of them sound easy and could be great tasks for > > newbies. > > One other thing I forgot to mention: your patches ended up on the list > in jumbled order. How do you send them? Usually `send-email` would add 1 > second to the timestamp of each, so that threading mail readers sort > them as you'd expect (even if they arrive out of order due to the > vagaries of SMTP servers). Oh, that's one more bug in submitgit, I guess. I will not use it anymore, OK, it's time to change the habits. > > -Peff
Re: [GSoC] Unify ref-filter formats with other --pretty formats
пн, 25 мар. 2019 г. в 22:27, Kapil Jain : > > Hi, > > Below are some two queries concerning > https://git.github.io/SoC-2019-Ideas/#unify-ref-filter-formats-with-other---pretty-formats > > Q1) > > In pretty.h & pretty.c: > void get_commit_format(const char *arg, struct rev_info *); > This function Parses given arguments from "arg", checks it for > correctness and * fill struct rev_info. > > In ref-filter.h & ref-filter.c: > int verify_ref_format(struct ref_format *format); > This function is Used to verify if the given format is correct and to > parse out the used atoms. > > Now, the verify_ref_format function can be used inside > get_commit_format function, hence reusing logic. > Is this a correct example to work on, for this project ? Hi! Yes, in my opinion your example looks like good starting point. > If not, please point out an example so as to understand the problem > statement better. > > Other than this I can't find any other example, for this project in > pretty.* and ref-filter.* > Perhaps some examples could be found in command specific files, right ? Other parts of the project are about reusing other ref-filter logic. For example, we could try to reuse format_ref_array_item() from ref-filter.h. I haven't dig into pretty.c logic much, but I guess it is possible to translate "pretty" formatting commands to ref-filter ones. That will allow us to remove similar logic from pretty.c. Our final goal is to minimise code duplication and to have one unified interface to extract all needed data from object and to print it properly. > > Q2) > About a recurring term 'atom' in ref-filter and pretty: > what is atom ? is it a piece of a whole document ? and what is meant > by used atoms ? I had the same question in my beginning. Please have a look at [1]. Another good question - what is object. You could ensure that you understand this by reading [2]. > > Thanks. [1] https://git-scm.com/docs/git-for-each-ref#_field_names [2] https://git-scm.com/book/en/v2/Git-Internals-Git-Objects
Re: [GSoC] My project blog
вт, 20 авг. 2019 г. в 07:59, Matheus Tavares Bernardino : > > Hi, everyone > > I just posted the penultimate report on my project: > https://matheustavares.gitlab.io/posts/going-for-a-too-big-step This > week I’ve been working on a v2 of threaded git-grep w/ parallel > inflation, to allow threads when grepping submodules. I also tried > some more optimizations along the way. Thank you for great blog post! You have done so many things, it is impressive. I absolutely agree with your plans to structure the job and split it to smaller pieces, it should definitely help. Moreover, if you want, you can make something like a documentation and delegate some parts. I hope you enjoyed this summer :) Thank you for your readiness to continue contributing after GSoC. It is not mandatory, you do not have to finish the project, but if you want - all the community will be so glad to see you as the contributor. Thank you so much, Olga > > As always, any comments/suggestions will be highly appreciated. > > Thanks, > Matheus
Re: Git in Outreachy December 2019?
сб, 31 авг. 2019 г. в 10:58, Christian Couder : > > On Tue, Aug 27, 2019 at 7:17 AM Jeff King wrote: > > > > Do we have interested mentors for the next round of Outreachy? > > I am interested to co-mentor. I am not ready to give the answer right now, but I will definitely think about the project proposals, I want to help with that part. By the way, we can take some projects (with description) from this summer round of GSoC. 3 of 4 projects were not selected. > > > The deadline for Git to apply to the program is September 5th. The > > deadline for mentors to have submitted project descriptions is September > > 24th. Intern applications would start on October 1st. > > > > If there are mentors who want to participate, I can handle the project > > application and can start asking around for funding. > > That would be really nice, thank you! Thank you!
Re: [RFC] Post/tutorial for newcomers
вт, 3 сент. 2019 г. в 05:32, Matheus Tavares Bernardino : > > Hi, everyone > > I've been writing a blog post based on what I learned during GSoC to > help other students here at FLUSP[1] start contributing as well. > > In the meantime `Documentation/MyFirstContribution.txt` was released, > so I've pointed my interested colleagues to it. But since I was > already pretty close to finishing the post, I decided to conclude it > anyway. > > Here is the end result: > https://matheustavares.gitlab.io/posts/first-steps-contributing-to-git > If you have any comments or suggestions for it, please, let me know :) This is amazing! Thank you for such painstaking work. I wish I had this doc before doing my first steps at Git :) > > Thanks, > Matheus > > [1]: https://flusp.ime.usp.br
[RFC 2/4] ref-filter: add return value && strbuf to handlers
Continue removing any printing from ref-filter formatting logic, so that it could be more general. Change the signature of handlers by adding return value and strbuf parameter for errors. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 71 1 file changed, 48 insertions(+), 23 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 54fae00bdd410..07bedc636398c 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -387,7 +387,8 @@ struct ref_formatting_state { struct atom_value { const char *s; - void (*handler)(struct atom_value *atomv, struct ref_formatting_state *state); + int (*handler)(struct atom_value *atomv, struct ref_formatting_state *state, + struct strbuf *err); uintmax_t value; /* used for sorting when not FIELD_STR */ struct used_atom *atom; }; @@ -481,7 +482,8 @@ static void quote_formatting(struct strbuf *s, const char *str, int quote_style) } } -static void append_atom(struct atom_value *v, struct ref_formatting_state *state) +static int append_atom(struct atom_value *v, struct ref_formatting_state *state, + struct strbuf *err) { /* * Quote formatting is only done when the stack has a single @@ -493,6 +495,7 @@ static void append_atom(struct atom_value *v, struct ref_formatting_state *state quote_formatting(&state->stack->output, v->s, state->quote_style); else strbuf_addstr(&state->stack->output, v->s); + return 0; } static void push_stack_element(struct ref_formatting_stack **stack) @@ -527,7 +530,8 @@ static void end_align_handler(struct ref_formatting_stack **stack) strbuf_release(&s); } -static void align_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state) +static int align_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, + struct strbuf *err) { struct ref_formatting_stack *new_stack; @@ -535,6 +539,7 @@ static void align_atom_handler(struct atom_value *atomv, struct ref_formatting_s new_stack = state->stack; new_stack->at_end = end_align_handler; new_stack->at_end_data = &atomv->atom->u.align; + return 0; } static void if_then_else_handler(struct ref_formatting_stack **stack) @@ -572,7 +577,8 @@ static void if_then_else_handler(struct ref_formatting_stack **stack) free(if_then_else); } -static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state) +static int if_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, + struct strbuf *err) { struct ref_formatting_stack *new_stack; struct if_then_else *if_then_else = xcalloc(sizeof(struct if_then_else), 1); @@ -584,6 +590,7 @@ static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_stat new_stack = state->stack; new_stack->at_end = if_then_else_handler; new_stack->at_end_data = if_then_else; + return 0; } static int is_empty(const char *s) @@ -596,19 +603,24 @@ static int is_empty(const char *s) return 1; } -static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state) +static int then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, +struct strbuf *err) { struct ref_formatting_stack *cur = state->stack; struct if_then_else *if_then_else = NULL; if (cur->at_end == if_then_else_handler) if_then_else = (struct if_then_else *)cur->at_end_data; - if (!if_then_else) - die(_("format: %%(then) atom used without an %%(if) atom")); - if (if_then_else->then_atom_seen) - die(_("format: %%(then) atom used more than once")); - if (if_then_else->else_atom_seen) - die(_("format: %%(then) atom used after %%(else)")); + if (!if_then_else) { + strbuf_addstr(err, _("format: %(then) atom used without an %(if) atom")); + return -1; + } else if (if_then_else->then_atom_seen) { + strbuf_addstr(err, _("format: %(then) atom used more than once")); + return -1; + } else if (if_then_else->else_atom_seen) { + strbuf_addstr(err, _("format: %(then) atom used after %(else)")); + return -1; + } if_then_else->then_atom_seen = 1; /* * If the 'equals' or 'notequals' attribute is used then @@ -624,34 +636,44 @@ static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_st } else if (cur->output.len && !is_empty(cur->output.buf)) if_then_else->condition_satisfied = 1; strbuf_reset(&cur->output); + return 0; } -static void else_atom_handler(struct atom_value *atomv,
[RFC 4/4] ref-filter: add return value to parsers
Continue removing any printing from ref-filter formatting logic, so that it could be more general. Change the signature of parsers by adding return value and strbuf parameter for error message. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 177 +++ 1 file changed, 118 insertions(+), 59 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index e146215bf1e64..06eb95e7c2c07 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -101,22 +101,28 @@ static struct used_atom { } *used_atom; static int used_atom_cnt, need_tagged, need_symref; -static void color_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *color_value) +static int color_atom_parser(const struct ref_format *format, struct used_atom *atom, +const char *color_value, struct strbuf *err) { - if (!color_value) - die(_("expected format: %%(color:)")); - if (color_parse(color_value, atom->u.color) < 0) - die(_("unrecognized color: %%(color:%s)"), color_value); + if (!color_value) { + strbuf_addstr(err, _("expected format: %(color:)")); + return -1; + } + if (color_parse(color_value, atom->u.color) < 0) { + strbuf_addf(err, _("unrecognized color: %%(color:%s)"), color_value); + return -1; + } /* * We check this after we've parsed the color, which lets us complain * about syntactically bogus color names even if they won't be used. */ if (!want_color(format->use_color)) color_parse("", atom->u.color); + return 0; } -static void refname_atom_parser_internal(struct refname_atom *atom, -const char *arg, const char *name) +static int refname_atom_parser_internal(struct refname_atom *atom, const char *arg, +const char *name, struct strbuf *err) { if (!arg) atom->option = R_NORMAL; @@ -125,17 +131,25 @@ static void refname_atom_parser_internal(struct refname_atom *atom, else if (skip_prefix(arg, "lstrip=", &arg) || skip_prefix(arg, "strip=", &arg)) { atom->option = R_LSTRIP; - if (strtol_i(arg, 10, &atom->lstrip)) - die(_("Integer value expected refname:lstrip=%s"), arg); + if (strtol_i(arg, 10, &atom->lstrip)) { + strbuf_addf(err, _("Integer value expected refname:lstrip=%s"), arg); + return -1; + } } else if (skip_prefix(arg, "rstrip=", &arg)) { atom->option = R_RSTRIP; - if (strtol_i(arg, 10, &atom->rstrip)) - die(_("Integer value expected refname:rstrip=%s"), arg); - } else - die(_("unrecognized %%(%s) argument: %s"), name, arg); + if (strtol_i(arg, 10, &atom->rstrip)) { + strbuf_addf(err, _("Integer value expected refname:rstrip=%s"), arg); + return -1; + } + } else { + strbuf_addf(err, _("unrecognized %%(%s) argument: %s"), name, arg); + return -1; + } + return 0; } -static void remote_ref_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg) +static int remote_ref_atom_parser(const struct ref_format *format, struct used_atom *atom, + const char *arg, struct strbuf *err) { struct string_list params = STRING_LIST_INIT_DUP; int i; @@ -145,9 +159,8 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_ if (!arg) { atom->u.remote_ref.option = RR_REF; - refname_atom_parser_internal(&atom->u.remote_ref.refname, -arg, atom->name); - return; + return refname_atom_parser_internal(&atom->u.remote_ref.refname, + arg, atom->name, err); } atom->u.remote_ref.nobracket = 0; @@ -170,29 +183,40 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_ atom->u.remote_ref.push_remote = 1; } else { atom->u.remote_ref.option = RR_REF; - refname_atom_parser_internal(&atom->u.remote_ref.refname, -arg, atom->name); + if (refname_atom_parser_internal(&atom->u.remote_ref.refname, +arg, atom->name, err)) + return -1; } } string_list_clear(¶ms, 0); + return 0; } -static void body_atom_parser(const struct ref_format *format,
[RFC 3/4] ref-filter: change parsing function error handling
Continue removing any printing from ref-filter formatting logic, so that it could be more general. Change the signature of parse_ref_filter_atom() by changing return value, adding previous return value to function parameter and also adding strbuf parameter for error message. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 45 - 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 07bedc636398c..e146215bf1e64 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -397,7 +397,8 @@ struct atom_value { * Used to parse format string and sort specifiers */ static int parse_ref_filter_atom(const struct ref_format *format, -const char *atom, const char *ep) +const char *atom, const char *ep, int *res, +struct strbuf *err) { const char *sp; const char *arg; @@ -406,14 +407,18 @@ static int parse_ref_filter_atom(const struct ref_format *format, sp = atom; if (*sp == '*' && sp < ep) sp++; /* deref */ - if (ep <= sp) - die(_("malformed field name: %.*s"), (int)(ep-atom), atom); + if (ep <= sp) { + strbuf_addf(err, _("malformed field name: %.*s"), (int)(ep-atom), atom); + return -1; + } /* Do we have the atom already used elsewhere? */ for (i = 0; i < used_atom_cnt; i++) { int len = strlen(used_atom[i].name); - if (len == ep - atom && !memcmp(used_atom[i].name, atom, len)) - return i; + if (len == ep - atom && !memcmp(used_atom[i].name, atom, len)) { + *res = i; + return 0; + } } /* @@ -432,8 +437,10 @@ static int parse_ref_filter_atom(const struct ref_format *format, break; } - if (ARRAY_SIZE(valid_atom) <= i) - die(_("unknown field name: %.*s"), (int)(ep-atom), atom); + if (ARRAY_SIZE(valid_atom) <= i) { + strbuf_addf(err, _("unknown field name: %.*s"), (int)(ep-atom), atom); + return -1; + } /* Add it in, including the deref prefix */ at = used_atom_cnt; @@ -458,7 +465,8 @@ static int parse_ref_filter_atom(const struct ref_format *format, need_tagged = 1; if (!strcmp(valid_atom[i].name, "symref")) need_symref = 1; - return at; + *res = at; + return 0; } static void quote_formatting(struct strbuf *s, const char *str, int quote_style) @@ -725,17 +733,20 @@ int verify_ref_format(struct ref_format *format) format->need_color_reset_at_eol = 0; for (cp = format->format; *cp && (sp = find_next(cp)); ) { + struct strbuf err = STRBUF_INIT; const char *color, *ep = strchr(sp, ')'); int at; if (!ep) return error(_("malformed format string %s"), sp); /* sp points at "%(" and ep points at the closing ")" */ - at = parse_ref_filter_atom(format, sp + 2, ep); + if (parse_ref_filter_atom(format, sp + 2, ep, &at, &err)) + die("%s", err.buf); cp = ep + 1; if (skip_prefix(used_atom[at].name, "color:", &color)) format->need_color_reset_at_eol = !!strcmp(color, "reset"); + strbuf_release(&err); } if (format->need_color_reset_at_eol && !want_color(format->use_color)) format->need_color_reset_at_eol = 0; @@ -2154,15 +2165,18 @@ int format_ref_array_item(struct ref_array_item *info, for (cp = format->format; *cp && (sp = find_next(cp)); cp = ep + 1) { struct atom_value *atomv; + struct strbuf err = STRBUF_INIT; + int pos; ep = strchr(sp, ')'); if (cp < sp) append_literal(cp, sp, &state); - get_ref_atom_value(info, - parse_ref_filter_atom(format, sp + 2, ep), - &atomv); + if (parse_ref_filter_atom(format, sp + 2, ep, &pos, &err)) + return -1; + get_ref_atom_value(info, pos, &atomv); if (atomv->handler(atomv, &state, error_buf)) return -1; + strbuf_release(&err); } if (*cp) { sp = cp + strlen(cp); @@ -2215,7 +2229,12 @@ static int parse_sorting_atom(const char *atom) */ struct ref_format dummy = REF_FORMAT_INIT; const char *end = atom + strlen(atom); - return parse_ref_filter_atom(&dummy, atom, end); + struct strbuf err = STRBUF_INIT; + int res; + if (parse_ref_filter_atom(&d
[RFC 1/4] ref-filter: start adding strbufs with errors
This is a first step in removing any printing from ref-filter formatting logic, so that it could be more general. Everything would be the same for show_ref_array_item() users. But, if you want to deal with errors by your own, you could invoke format_ref_array_item(). It means that you need to print everything (the result and errors) on your side. This commit changes signature of format_ref_array_item() by adding return value and strbuf parameter for errors, and fixes its callers. Signed-off-by: Olga Telezhnaia --- builtin/branch.c | 7 +-- ref-filter.c | 17 - ref-filter.h | 7 --- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 8dcc2ed058be6..f86709ca42d5e 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -391,7 +391,6 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin struct ref_array array; int maxwidth = 0; const char *remote_prefix = ""; - struct strbuf out = STRBUF_INIT; char *to_free = NULL; /* @@ -419,7 +418,10 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin ref_array_sort(sorting, &array); for (i = 0; i < array.nr; i++) { - format_ref_array_item(array.items[i], format, &out); + struct strbuf out = STRBUF_INIT; + struct strbuf err = STRBUF_INIT; + if (format_ref_array_item(array.items[i], format, &out, &err)) + die("%s", err.buf); if (column_active(colopts)) { assert(!filter->verbose && "--column and --verbose are incompatible"); /* format to a string_list to let print_columns() do its job */ @@ -428,6 +430,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin fwrite(out.buf, 1, out.len, stdout); putchar('\n'); } + strbuf_release(&err); strbuf_release(&out); } diff --git a/ref-filter.c b/ref-filter.c index 45fc56216aaa8..54fae00bdd410 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2118,9 +2118,10 @@ static void append_literal(const char *cp, const char *ep, struct ref_formatting } } -void format_ref_array_item(struct ref_array_item *info, +int format_ref_array_item(struct ref_array_item *info, const struct ref_format *format, - struct strbuf *final_buf) + struct strbuf *final_buf, + struct strbuf *error_buf) { const char *cp, *sp, *ep; struct ref_formatting_state state = REF_FORMATTING_STATE_INIT; @@ -2148,19 +2149,25 @@ void format_ref_array_item(struct ref_array_item *info, resetv.s = GIT_COLOR_RESET; append_atom(&resetv, &state); } - if (state.stack->prev) - die(_("format: %%(end) atom missing")); + if (state.stack->prev) { + strbuf_addstr(error_buf, _("format: %(end) atom missing")); + return -1; + } strbuf_addbuf(final_buf, &state.stack->output); pop_stack_element(&state.stack); + return 0; } void show_ref_array_item(struct ref_array_item *info, const struct ref_format *format) { struct strbuf final_buf = STRBUF_INIT; + struct strbuf error_buf = STRBUF_INIT; - format_ref_array_item(info, format, &final_buf); + if (format_ref_array_item(info, format, &final_buf, &error_buf)) + die("%s", error_buf.buf); fwrite(final_buf.buf, 1, final_buf.len, stdout); + strbuf_release(&error_buf); strbuf_release(&final_buf); putchar('\n'); } diff --git a/ref-filter.h b/ref-filter.h index 0d98342b34319..e13f8e6f8721a 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -110,9 +110,10 @@ int verify_ref_format(struct ref_format *format); /* Sort the given ref_array as per the ref_sorting provided */ void ref_array_sort(struct ref_sorting *sort, struct ref_array *array); /* Based on the given format and quote_style, fill the strbuf */ -void format_ref_array_item(struct ref_array_item *info, - const struct ref_format *format, - struct strbuf *final_buf); +int format_ref_array_item(struct ref_array_item *info, + const struct ref_format *format, + struct strbuf *final_buf, + struct strbuf *error_buf); /* Print the ref using the given format and quote_style */ void show_ref_array_item(struct ref_array_item *info, const struct ref_format *format); /* Parse a single sort specifier and add it to the list */ -- https://github.com/git/git/pull/466
[PATCH v2 5/5] ref-filter: get_ref_atom_value() error handling
Finish removing any printing from ref-filter formatting logic, so that it could be more general. Change the signature of get_ref_atom_value() and underlying functions by adding return value and strbuf parameter for error message. It's important to mention that grab_objectname() returned 1 if it gets objectname atom and 0 otherwise. Now this logic changed: we return 0 if we have no error, -1 otherwise. If someone needs to know whether it's objectname atom or not, he/she could use starts_with() function. It duplicates this checking but it does not sound like a really big overhead. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 109 +-- 1 file changed, 69 insertions(+), 40 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 62ea4adcd0ff1..3f0c3924273d5 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -831,26 +831,27 @@ static void *get_obj(const struct object_id *oid, struct object **obj, unsigned } static int grab_objectname(const char *name, const unsigned char *sha1, - struct atom_value *v, struct used_atom *atom) + struct atom_value *v, struct used_atom *atom, + struct strbuf *err) { if (starts_with(name, "objectname")) { if (atom->u.objectname.option == O_SHORT) { v->s = xstrdup(find_unique_abbrev(sha1, DEFAULT_ABBREV)); - return 1; } else if (atom->u.objectname.option == O_FULL) { v->s = xstrdup(sha1_to_hex(sha1)); - return 1; } else if (atom->u.objectname.option == O_LENGTH) { v->s = xstrdup(find_unique_abbrev(sha1, atom->u.objectname.length)); - return 1; - } else - die("BUG: unknown %%(objectname) option"); + } else { + strbuf_addstr(err, "BUG: unknown %(objectname) option"); + return -1; + } } return 0; } /* See grab_values */ -static void grab_common_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz) +static int grab_common_values(struct atom_value *val, int deref, struct object *obj, + void *buf, unsigned long sz, struct strbuf *err) { int i; @@ -868,8 +869,10 @@ static void grab_common_values(struct atom_value *val, int deref, struct object v->s = xstrfmt("%lu", sz); } else if (deref) - grab_objectname(name, obj->oid.hash, v, &used_atom[i]); + if (grab_objectname(name, obj->oid.hash, v, &used_atom[i], err)) + return -1; } + return 0; } /* See grab_values */ @@ -1225,9 +1228,11 @@ static void fill_missing_values(struct atom_value *val) * pointed at by the ref itself; otherwise it is the object the * ref (which is a tag) refers to. */ -static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz) +static int grab_values(struct atom_value *val, int deref, struct object *obj, + void *buf, unsigned long sz, struct strbuf *err) { - grab_common_values(val, deref, obj, buf, sz); + if (grab_common_values(val, deref, obj, buf, sz, err)) + return -1; switch (obj->type) { case OBJ_TAG: grab_tag_values(val, deref, obj, buf, sz); @@ -1247,8 +1252,10 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, v /* grab_blob_values(val, deref, obj, buf, sz); */ break; default: - die("Eh? Object of type %d?", obj->type); + strbuf_addf(err, "Eh? Object of type %d?", obj->type); + return -1; } + return 0; } static inline char *copy_advance(char *dst, const char *src) @@ -1335,8 +1342,9 @@ static const char *show_ref(struct refname_atom *atom, const char *refname) return refname; } -static void fill_remote_ref_details(struct used_atom *atom, const char *refname, - struct branch *branch, const char **s) +static int fill_remote_ref_details(struct used_atom *atom, const char *refname, + struct branch *branch, const char **s, + struct strbuf *err) { int num_ours, num_theirs; if (atom->u.remote_ref.option == RR_REF) @@ -1362,7 +1370,7 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname, } else if (atom->u.remote_ref.option == RR_TRACKSHORT) { if (stat_tracking_info(branch, &num_ours, &num_theirs, NULL, AHEAD_BEHIND_FULL) < 0) -
[PATCH v2 3/5] ref-filter: change parsing function error handling
Continue removing any printing from ref-filter formatting logic, so that it could be more general. Change the signature of parse_ref_filter_atom() by changing return value, adding previous return value to function parameter and also adding strbuf parameter for error message. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 43 ++- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index d120360104806..dd83ef326511d 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -397,7 +397,8 @@ struct atom_value { * Used to parse format string and sort specifiers */ static int parse_ref_filter_atom(const struct ref_format *format, -const char *atom, const char *ep) +const char *atom, const char *ep, int *res, +struct strbuf *err) { const char *sp; const char *arg; @@ -406,14 +407,18 @@ static int parse_ref_filter_atom(const struct ref_format *format, sp = atom; if (*sp == '*' && sp < ep) sp++; /* deref */ - if (ep <= sp) - die(_("malformed field name: %.*s"), (int)(ep-atom), atom); + if (ep <= sp) { + strbuf_addf(err, _("malformed field name: %.*s"), (int)(ep-atom), atom); + return -1; + } /* Do we have the atom already used elsewhere? */ for (i = 0; i < used_atom_cnt; i++) { int len = strlen(used_atom[i].name); - if (len == ep - atom && !memcmp(used_atom[i].name, atom, len)) - return i; + if (len == ep - atom && !memcmp(used_atom[i].name, atom, len)) { + *res = i; + return 0; + } } /* @@ -432,8 +437,10 @@ static int parse_ref_filter_atom(const struct ref_format *format, break; } - if (ARRAY_SIZE(valid_atom) <= i) - die(_("unknown field name: %.*s"), (int)(ep-atom), atom); + if (ARRAY_SIZE(valid_atom) <= i) { + strbuf_addf(err, _("unknown field name: %.*s"), (int)(ep-atom), atom); + return -1; + } /* Add it in, including the deref prefix */ at = used_atom_cnt; @@ -458,7 +465,8 @@ static int parse_ref_filter_atom(const struct ref_format *format, need_tagged = 1; if (!strcmp(valid_atom[i].name, "symref")) need_symref = 1; - return at; + *res = at; + return 0; } static void quote_formatting(struct strbuf *s, const char *str, int quote_style) @@ -725,17 +733,20 @@ int verify_ref_format(struct ref_format *format) format->need_color_reset_at_eol = 0; for (cp = format->format; *cp && (sp = find_next(cp)); ) { + struct strbuf err = STRBUF_INIT; const char *color, *ep = strchr(sp, ')'); int at; if (!ep) return error(_("malformed format string %s"), sp); /* sp points at "%(" and ep points at the closing ")" */ - at = parse_ref_filter_atom(format, sp + 2, ep); + if (parse_ref_filter_atom(format, sp + 2, ep, &at, &err)) + die("%s", err.buf); cp = ep + 1; if (skip_prefix(used_atom[at].name, "color:", &color)) format->need_color_reset_at_eol = !!strcmp(color, "reset"); + strbuf_release(&err); } if (format->need_color_reset_at_eol && !want_color(format->use_color)) format->need_color_reset_at_eol = 0; @@ -2154,13 +2165,14 @@ int format_ref_array_item(struct ref_array_item *info, for (cp = format->format; *cp && (sp = find_next(cp)); cp = ep + 1) { struct atom_value *atomv; + int pos; ep = strchr(sp, ')'); if (cp < sp) append_literal(cp, sp, &state); - get_ref_atom_value(info, - parse_ref_filter_atom(format, sp + 2, ep), - &atomv); + if (parse_ref_filter_atom(format, sp + 2, ep, &pos, error_buf)) + return -1; + get_ref_atom_value(info, pos, &atomv); if (atomv->handler(atomv, &state, error_buf)) return -1; } @@ -2215,7 +2227,12 @@ static int parse_sorting_atom(const char *atom) */ struct ref_format dummy = REF_FORMAT_INIT; const char *end = atom + strlen(atom); - return parse_ref_filter_atom(&dummy, atom, end); + struct strbuf err = STRBUF_INIT; + int res; + if (parse_ref_filter_atom(&dummy, atom, end, &res, &err)) + die("%s", err.buf); + strbuf_release(&err); + return res; } /* If no sorting o
[PATCH v2 1/5] ref-filter: start adding strbufs with errors
This is a first step in removing any printing from ref-filter formatting logic, so that it could be more general. Everything would be the same for show_ref_array_item() users. But, if you want to deal with errors by your own, you could invoke format_ref_array_item(). It means that you need to print everything (the result and errors) on your side. This commit changes signature of format_ref_array_item() by adding return value and strbuf parameter for errors, and adjusts its callers. While at it, reduce the scope of the out-variable. Signed-off-by: Olga Telezhnaia --- builtin/branch.c | 7 +-- ref-filter.c | 17 - ref-filter.h | 7 --- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 8dcc2ed058be6..f86709ca42d5e 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -391,7 +391,6 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin struct ref_array array; int maxwidth = 0; const char *remote_prefix = ""; - struct strbuf out = STRBUF_INIT; char *to_free = NULL; /* @@ -419,7 +418,10 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin ref_array_sort(sorting, &array); for (i = 0; i < array.nr; i++) { - format_ref_array_item(array.items[i], format, &out); + struct strbuf out = STRBUF_INIT; + struct strbuf err = STRBUF_INIT; + if (format_ref_array_item(array.items[i], format, &out, &err)) + die("%s", err.buf); if (column_active(colopts)) { assert(!filter->verbose && "--column and --verbose are incompatible"); /* format to a string_list to let print_columns() do its job */ @@ -428,6 +430,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin fwrite(out.buf, 1, out.len, stdout); putchar('\n'); } + strbuf_release(&err); strbuf_release(&out); } diff --git a/ref-filter.c b/ref-filter.c index 45fc56216aaa8..54fae00bdd410 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2118,9 +2118,10 @@ static void append_literal(const char *cp, const char *ep, struct ref_formatting } } -void format_ref_array_item(struct ref_array_item *info, +int format_ref_array_item(struct ref_array_item *info, const struct ref_format *format, - struct strbuf *final_buf) + struct strbuf *final_buf, + struct strbuf *error_buf) { const char *cp, *sp, *ep; struct ref_formatting_state state = REF_FORMATTING_STATE_INIT; @@ -2148,19 +2149,25 @@ void format_ref_array_item(struct ref_array_item *info, resetv.s = GIT_COLOR_RESET; append_atom(&resetv, &state); } - if (state.stack->prev) - die(_("format: %%(end) atom missing")); + if (state.stack->prev) { + strbuf_addstr(error_buf, _("format: %(end) atom missing")); + return -1; + } strbuf_addbuf(final_buf, &state.stack->output); pop_stack_element(&state.stack); + return 0; } void show_ref_array_item(struct ref_array_item *info, const struct ref_format *format) { struct strbuf final_buf = STRBUF_INIT; + struct strbuf error_buf = STRBUF_INIT; - format_ref_array_item(info, format, &final_buf); + if (format_ref_array_item(info, format, &final_buf, &error_buf)) + die("%s", error_buf.buf); fwrite(final_buf.buf, 1, final_buf.len, stdout); + strbuf_release(&error_buf); strbuf_release(&final_buf); putchar('\n'); } diff --git a/ref-filter.h b/ref-filter.h index 0d98342b34319..e13f8e6f8721a 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -110,9 +110,10 @@ int verify_ref_format(struct ref_format *format); /* Sort the given ref_array as per the ref_sorting provided */ void ref_array_sort(struct ref_sorting *sort, struct ref_array *array); /* Based on the given format and quote_style, fill the strbuf */ -void format_ref_array_item(struct ref_array_item *info, - const struct ref_format *format, - struct strbuf *final_buf); +int format_ref_array_item(struct ref_array_item *info, + const struct ref_format *format, + struct strbuf *final_buf, + struct strbuf *error_buf); /* Print the ref using the given format and quote_style */ void show_ref_array_item(struct ref_array_item *info, const struct ref_format *format); /* Parse a single sort specifier and add it to the list */ -- https://github.com/git/git/pull/466
[PATCH v2 2/5] ref-filter: add return value && strbuf to handlers
Continue removing any printing from ref-filter formatting logic, so that it could be more general. Change the signature of handlers by adding return value and strbuf parameter for errors. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 71 1 file changed, 48 insertions(+), 23 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 54fae00bdd410..d120360104806 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -387,7 +387,8 @@ struct ref_formatting_state { struct atom_value { const char *s; - void (*handler)(struct atom_value *atomv, struct ref_formatting_state *state); + int (*handler)(struct atom_value *atomv, struct ref_formatting_state *state, + struct strbuf *err); uintmax_t value; /* used for sorting when not FIELD_STR */ struct used_atom *atom; }; @@ -481,7 +482,8 @@ static void quote_formatting(struct strbuf *s, const char *str, int quote_style) } } -static void append_atom(struct atom_value *v, struct ref_formatting_state *state) +static int append_atom(struct atom_value *v, struct ref_formatting_state *state, + struct strbuf *unused_err) { /* * Quote formatting is only done when the stack has a single @@ -493,6 +495,7 @@ static void append_atom(struct atom_value *v, struct ref_formatting_state *state quote_formatting(&state->stack->output, v->s, state->quote_style); else strbuf_addstr(&state->stack->output, v->s); + return 0; } static void push_stack_element(struct ref_formatting_stack **stack) @@ -527,7 +530,8 @@ static void end_align_handler(struct ref_formatting_stack **stack) strbuf_release(&s); } -static void align_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state) +static int align_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, + struct strbuf *unused_err) { struct ref_formatting_stack *new_stack; @@ -535,6 +539,7 @@ static void align_atom_handler(struct atom_value *atomv, struct ref_formatting_s new_stack = state->stack; new_stack->at_end = end_align_handler; new_stack->at_end_data = &atomv->atom->u.align; + return 0; } static void if_then_else_handler(struct ref_formatting_stack **stack) @@ -572,7 +577,8 @@ static void if_then_else_handler(struct ref_formatting_stack **stack) free(if_then_else); } -static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state) +static int if_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, + struct strbuf *unused_err) { struct ref_formatting_stack *new_stack; struct if_then_else *if_then_else = xcalloc(sizeof(struct if_then_else), 1); @@ -584,6 +590,7 @@ static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_stat new_stack = state->stack; new_stack->at_end = if_then_else_handler; new_stack->at_end_data = if_then_else; + return 0; } static int is_empty(const char *s) @@ -596,19 +603,24 @@ static int is_empty(const char *s) return 1; } -static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state) +static int then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, +struct strbuf *err) { struct ref_formatting_stack *cur = state->stack; struct if_then_else *if_then_else = NULL; if (cur->at_end == if_then_else_handler) if_then_else = (struct if_then_else *)cur->at_end_data; - if (!if_then_else) - die(_("format: %%(then) atom used without an %%(if) atom")); - if (if_then_else->then_atom_seen) - die(_("format: %%(then) atom used more than once")); - if (if_then_else->else_atom_seen) - die(_("format: %%(then) atom used after %%(else)")); + if (!if_then_else) { + strbuf_addstr(err, _("format: %(then) atom used without an %(if) atom")); + return -1; + } else if (if_then_else->then_atom_seen) { + strbuf_addstr(err, _("format: %(then) atom used more than once")); + return -1; + } else if (if_then_else->else_atom_seen) { + strbuf_addstr(err, _("format: %(then) atom used after %(else)")); + return -1; + } if_then_else->then_atom_seen = 1; /* * If the 'equals' or 'notequals' attribute is used then @@ -624,34 +636,44 @@ static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_st } else if (cur->output.len && !is_empty(cur->output.buf)) if_then_else->condition_satisfied = 1; strbuf_reset(&cur->output); + return 0; } -static void else_atom_handler(stru
[PATCH v2 4/5] ref-filter: add return value to parsers
Continue removing any printing from ref-filter formatting logic, so that it could be more general. Change the signature of parsers by adding return value and strbuf parameter for error message. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 177 +++ 1 file changed, 118 insertions(+), 59 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index dd83ef326511d..62ea4adcd0ff1 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -101,22 +101,28 @@ static struct used_atom { } *used_atom; static int used_atom_cnt, need_tagged, need_symref; -static void color_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *color_value) +static int color_atom_parser(const struct ref_format *format, struct used_atom *atom, +const char *color_value, struct strbuf *err) { - if (!color_value) - die(_("expected format: %%(color:)")); - if (color_parse(color_value, atom->u.color) < 0) - die(_("unrecognized color: %%(color:%s)"), color_value); + if (!color_value) { + strbuf_addstr(err, _("expected format: %(color:)")); + return -1; + } + if (color_parse(color_value, atom->u.color) < 0) { + strbuf_addf(err, _("unrecognized color: %%(color:%s)"), color_value); + return -1; + } /* * We check this after we've parsed the color, which lets us complain * about syntactically bogus color names even if they won't be used. */ if (!want_color(format->use_color)) color_parse("", atom->u.color); + return 0; } -static void refname_atom_parser_internal(struct refname_atom *atom, -const char *arg, const char *name) +static int refname_atom_parser_internal(struct refname_atom *atom, const char *arg, +const char *name, struct strbuf *err) { if (!arg) atom->option = R_NORMAL; @@ -125,17 +131,25 @@ static void refname_atom_parser_internal(struct refname_atom *atom, else if (skip_prefix(arg, "lstrip=", &arg) || skip_prefix(arg, "strip=", &arg)) { atom->option = R_LSTRIP; - if (strtol_i(arg, 10, &atom->lstrip)) - die(_("Integer value expected refname:lstrip=%s"), arg); + if (strtol_i(arg, 10, &atom->lstrip)) { + strbuf_addf(err, _("Integer value expected refname:lstrip=%s"), arg); + return -1; + } } else if (skip_prefix(arg, "rstrip=", &arg)) { atom->option = R_RSTRIP; - if (strtol_i(arg, 10, &atom->rstrip)) - die(_("Integer value expected refname:rstrip=%s"), arg); - } else - die(_("unrecognized %%(%s) argument: %s"), name, arg); + if (strtol_i(arg, 10, &atom->rstrip)) { + strbuf_addf(err, _("Integer value expected refname:rstrip=%s"), arg); + return -1; + } + } else { + strbuf_addf(err, _("unrecognized %%(%s) argument: %s"), name, arg); + return -1; + } + return 0; } -static void remote_ref_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg) +static int remote_ref_atom_parser(const struct ref_format *format, struct used_atom *atom, + const char *arg, struct strbuf *err) { struct string_list params = STRING_LIST_INIT_DUP; int i; @@ -145,9 +159,8 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_ if (!arg) { atom->u.remote_ref.option = RR_REF; - refname_atom_parser_internal(&atom->u.remote_ref.refname, -arg, atom->name); - return; + return refname_atom_parser_internal(&atom->u.remote_ref.refname, + arg, atom->name, err); } atom->u.remote_ref.nobracket = 0; @@ -170,29 +183,40 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_ atom->u.remote_ref.push_remote = 1; } else { atom->u.remote_ref.option = RR_REF; - refname_atom_parser_internal(&atom->u.remote_ref.refname, -arg, atom->name); + if (refname_atom_parser_internal(&atom->u.remote_ref.refname, +arg, atom->name, err)) + return -1; } } string_list_clear(¶ms, 0); + return 0; } -static void body_atom_parser(const struct ref_format *format,
[PATCH v3 1/5] ref-filter: start adding strbufs with errors
This is a first step in removing any printing from ref-filter formatting logic, so that it could be more general. Everything would be the same for show_ref_array_item() users. But, if you want to deal with errors by your own, you could invoke format_ref_array_item(). It means that you need to print everything (the result and errors) on your side. This commit changes signature of format_ref_array_item() by adding return value and strbuf parameter for errors, and adjusts its callers. While at it, reduce the scope of the out-variable. Signed-off-by: Olga Telezhnaia --- builtin/branch.c | 7 +-- ref-filter.c | 17 - ref-filter.h | 7 --- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 6d0cea9d4bcc4..c21e5a04a0177 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -391,7 +391,6 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin struct ref_array array; int maxwidth = 0; const char *remote_prefix = ""; - struct strbuf out = STRBUF_INIT; char *to_free = NULL; /* @@ -419,7 +418,10 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin ref_array_sort(sorting, &array); for (i = 0; i < array.nr; i++) { - format_ref_array_item(array.items[i], format, &out); + struct strbuf out = STRBUF_INIT; + struct strbuf err = STRBUF_INIT; + if (format_ref_array_item(array.items[i], format, &out, &err)) + die("%s", err.buf); if (column_active(colopts)) { assert(!filter->verbose && "--column and --verbose are incompatible"); /* format to a string_list to let print_columns() do its job */ @@ -428,6 +430,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin fwrite(out.buf, 1, out.len, stdout); putchar('\n'); } + strbuf_release(&err); strbuf_release(&out); } diff --git a/ref-filter.c b/ref-filter.c index 45fc56216aaa8..54fae00bdd410 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2118,9 +2118,10 @@ static void append_literal(const char *cp, const char *ep, struct ref_formatting } } -void format_ref_array_item(struct ref_array_item *info, +int format_ref_array_item(struct ref_array_item *info, const struct ref_format *format, - struct strbuf *final_buf) + struct strbuf *final_buf, + struct strbuf *error_buf) { const char *cp, *sp, *ep; struct ref_formatting_state state = REF_FORMATTING_STATE_INIT; @@ -2148,19 +2149,25 @@ void format_ref_array_item(struct ref_array_item *info, resetv.s = GIT_COLOR_RESET; append_atom(&resetv, &state); } - if (state.stack->prev) - die(_("format: %%(end) atom missing")); + if (state.stack->prev) { + strbuf_addstr(error_buf, _("format: %(end) atom missing")); + return -1; + } strbuf_addbuf(final_buf, &state.stack->output); pop_stack_element(&state.stack); + return 0; } void show_ref_array_item(struct ref_array_item *info, const struct ref_format *format) { struct strbuf final_buf = STRBUF_INIT; + struct strbuf error_buf = STRBUF_INIT; - format_ref_array_item(info, format, &final_buf); + if (format_ref_array_item(info, format, &final_buf, &error_buf)) + die("%s", error_buf.buf); fwrite(final_buf.buf, 1, final_buf.len, stdout); + strbuf_release(&error_buf); strbuf_release(&final_buf); putchar('\n'); } diff --git a/ref-filter.h b/ref-filter.h index 0d98342b34319..e13f8e6f8721a 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -110,9 +110,10 @@ int verify_ref_format(struct ref_format *format); /* Sort the given ref_array as per the ref_sorting provided */ void ref_array_sort(struct ref_sorting *sort, struct ref_array *array); /* Based on the given format and quote_style, fill the strbuf */ -void format_ref_array_item(struct ref_array_item *info, - const struct ref_format *format, - struct strbuf *final_buf); +int format_ref_array_item(struct ref_array_item *info, + const struct ref_format *format, + struct strbuf *final_buf, + struct strbuf *error_buf); /* Print the ref using the given format and quote_style */ void show_ref_array_item(struct ref_array_item *info, const struct ref_format *format); /* Parse a single sort specifier and add it to the list */ -- https://github.com/git/git/pull/466
[PATCH v3 5/5] ref-filter: get_ref_atom_value() error handling
Finish removing any printing from ref-filter formatting logic, so that it could be more general. Change the signature of get_ref_atom_value() and underlying functions by adding return value and strbuf parameter for error message. Some die() calls are left; all of them are not for users, but for Git developers. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 55 --- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index b90cec1056954..85f971663a4aa 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1445,28 +1445,36 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re return show_ref(&atom->u.refname, ref->refname); } -static void get_object(struct ref_array_item *ref, const struct object_id *oid, - int deref, struct object **obj) +static int get_object(struct ref_array_item *ref, const struct object_id *oid, + int deref, struct object **obj, struct strbuf *err) { int eaten; unsigned long size; void *buf = get_obj(oid, obj, &size, &eaten); - if (!buf) - die(_("missing object %s for %s"), - oid_to_hex(oid), ref->refname); - if (!*obj) - die(_("parse_object_buffer failed on %s for %s"), - oid_to_hex(oid), ref->refname); - + if (!buf) { + strbuf_addf(err, _("missing object %s for %s"), oid_to_hex(oid), + ref->refname); + if (!eaten) + free(buf); + return -1; + } + if (!*obj) { + strbuf_addf(err, _("parse_object_buffer failed on %s for %s"), + oid_to_hex(oid), ref->refname); + if (!eaten) + free(buf); + return -1; + } grab_values(ref->value, deref, *obj, buf, size); if (!eaten) free(buf); + return 0; } /* * Parse the object referred by ref, and grab needed value. */ -static void populate_value(struct ref_array_item *ref) +static int populate_value(struct ref_array_item *ref, struct strbuf *err) { struct object *obj; int i; @@ -1588,16 +1596,17 @@ static void populate_value(struct ref_array_item *ref) break; } if (used_atom_cnt <= i) - return; + return 0; - get_object(ref, &ref->objectname, 0, &obj); + if (get_object(ref, &ref->objectname, 0, &obj, err)) + return -1; /* * If there is no atom that wants to know about tagged * object, we are done. */ if (!need_tagged || (obj->type != OBJ_TAG)) - return; + return 0; /* * If it is a tag object, see if we use a value that derefs @@ -1611,20 +1620,23 @@ static void populate_value(struct ref_array_item *ref) * is not consistent with what deref_tag() does * which peels the onion to the core. */ - get_object(ref, tagged, 1, &obj); + return get_object(ref, tagged, 1, &obj, err); } /* * Given a ref, return the value for the atom. This lazily gets value * out of the object by calling populate value. */ -static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom_value **v) +static int get_ref_atom_value(struct ref_array_item *ref, int atom, + struct atom_value **v, struct strbuf *err) { if (!ref->value) { - populate_value(ref); + if (populate_value(ref, err)) + return -1; fill_missing_values(ref->value); } *v = &ref->value[atom]; + return 0; } /* @@ -2148,9 +2160,13 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru int cmp; cmp_type cmp_type = used_atom[s->atom].type; int (*cmp_fn)(const char *, const char *); + struct strbuf err = STRBUF_INIT; - get_ref_atom_value(a, s->atom, &va); - get_ref_atom_value(b, s->atom, &vb); + if (get_ref_atom_value(a, s->atom, &va, &err)) + die("%s", err.buf); + if (get_ref_atom_value(b, s->atom, &vb, &err)) + die("%s", err.buf); + strbuf_release(&err); cmp_fn = s->ignore_case ? strcasecmp : strcmp; if (s->version) cmp = versioncmp(va->s, vb->s); @@ -2230,7 +2246,8 @@ int format_ref_array_item(struct ref_array_item *info, pos = parse_ref_filter_atom(format, sp + 2, ep, error_buf); if (pos < 0) return -1; - get_ref_atom_value(info, pos, &atomv); + if (get_ref_atom_value(info, pos, &atomv, error_buf)) + return -1; if (atomv->handler(atomv, &state, e
[PATCH v3 2/5] ref-filter: add return value && strbuf to handlers
Continue removing any printing from ref-filter formatting logic, so that it could be more general. Change the signature of handlers by adding return value and strbuf parameter for errors. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 71 1 file changed, 48 insertions(+), 23 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 54fae00bdd410..d120360104806 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -387,7 +387,8 @@ struct ref_formatting_state { struct atom_value { const char *s; - void (*handler)(struct atom_value *atomv, struct ref_formatting_state *state); + int (*handler)(struct atom_value *atomv, struct ref_formatting_state *state, + struct strbuf *err); uintmax_t value; /* used for sorting when not FIELD_STR */ struct used_atom *atom; }; @@ -481,7 +482,8 @@ static void quote_formatting(struct strbuf *s, const char *str, int quote_style) } } -static void append_atom(struct atom_value *v, struct ref_formatting_state *state) +static int append_atom(struct atom_value *v, struct ref_formatting_state *state, + struct strbuf *unused_err) { /* * Quote formatting is only done when the stack has a single @@ -493,6 +495,7 @@ static void append_atom(struct atom_value *v, struct ref_formatting_state *state quote_formatting(&state->stack->output, v->s, state->quote_style); else strbuf_addstr(&state->stack->output, v->s); + return 0; } static void push_stack_element(struct ref_formatting_stack **stack) @@ -527,7 +530,8 @@ static void end_align_handler(struct ref_formatting_stack **stack) strbuf_release(&s); } -static void align_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state) +static int align_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, + struct strbuf *unused_err) { struct ref_formatting_stack *new_stack; @@ -535,6 +539,7 @@ static void align_atom_handler(struct atom_value *atomv, struct ref_formatting_s new_stack = state->stack; new_stack->at_end = end_align_handler; new_stack->at_end_data = &atomv->atom->u.align; + return 0; } static void if_then_else_handler(struct ref_formatting_stack **stack) @@ -572,7 +577,8 @@ static void if_then_else_handler(struct ref_formatting_stack **stack) free(if_then_else); } -static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state) +static int if_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, + struct strbuf *unused_err) { struct ref_formatting_stack *new_stack; struct if_then_else *if_then_else = xcalloc(sizeof(struct if_then_else), 1); @@ -584,6 +590,7 @@ static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_stat new_stack = state->stack; new_stack->at_end = if_then_else_handler; new_stack->at_end_data = if_then_else; + return 0; } static int is_empty(const char *s) @@ -596,19 +603,24 @@ static int is_empty(const char *s) return 1; } -static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state) +static int then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, +struct strbuf *err) { struct ref_formatting_stack *cur = state->stack; struct if_then_else *if_then_else = NULL; if (cur->at_end == if_then_else_handler) if_then_else = (struct if_then_else *)cur->at_end_data; - if (!if_then_else) - die(_("format: %%(then) atom used without an %%(if) atom")); - if (if_then_else->then_atom_seen) - die(_("format: %%(then) atom used more than once")); - if (if_then_else->else_atom_seen) - die(_("format: %%(then) atom used after %%(else)")); + if (!if_then_else) { + strbuf_addstr(err, _("format: %(then) atom used without an %(if) atom")); + return -1; + } else if (if_then_else->then_atom_seen) { + strbuf_addstr(err, _("format: %(then) atom used more than once")); + return -1; + } else if (if_then_else->else_atom_seen) { + strbuf_addstr(err, _("format: %(then) atom used after %(else)")); + return -1; + } if_then_else->then_atom_seen = 1; /* * If the 'equals' or 'notequals' attribute is used then @@ -624,34 +636,44 @@ static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_st } else if (cur->output.len && !is_empty(cur->output.buf)) if_then_else->condition_satisfied = 1; strbuf_reset(&cur->output); + return 0; } -static void else_atom_handler(stru
[PATCH v3 3/5] ref-filter: change parsing function error handling
Continue removing any printing from ref-filter formatting logic, so that it could be more general. Change the signature of parse_ref_filter_atom() by adding strbuf parameter for error message. Return value means the same except negative values: they indicate errors (previous version could return only non-negative value). Signed-off-by: Olga Telezhnaia --- ref-filter.c | 36 ++-- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index d120360104806..2313a33f0baa4 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -397,7 +397,8 @@ struct atom_value { * Used to parse format string and sort specifiers */ static int parse_ref_filter_atom(const struct ref_format *format, -const char *atom, const char *ep) +const char *atom, const char *ep, +struct strbuf *err) { const char *sp; const char *arg; @@ -406,8 +407,10 @@ static int parse_ref_filter_atom(const struct ref_format *format, sp = atom; if (*sp == '*' && sp < ep) sp++; /* deref */ - if (ep <= sp) - die(_("malformed field name: %.*s"), (int)(ep-atom), atom); + if (ep <= sp) { + strbuf_addf(err, _("malformed field name: %.*s"), (int)(ep-atom), atom); + return -1; + } /* Do we have the atom already used elsewhere? */ for (i = 0; i < used_atom_cnt; i++) { @@ -432,8 +435,10 @@ static int parse_ref_filter_atom(const struct ref_format *format, break; } - if (ARRAY_SIZE(valid_atom) <= i) - die(_("unknown field name: %.*s"), (int)(ep-atom), atom); + if (ARRAY_SIZE(valid_atom) <= i) { + strbuf_addf(err, _("unknown field name: %.*s"), (int)(ep-atom), atom); + return -1; + } /* Add it in, including the deref prefix */ at = used_atom_cnt; @@ -725,17 +730,21 @@ int verify_ref_format(struct ref_format *format) format->need_color_reset_at_eol = 0; for (cp = format->format; *cp && (sp = find_next(cp)); ) { + struct strbuf err = STRBUF_INIT; const char *color, *ep = strchr(sp, ')'); int at; if (!ep) return error(_("malformed format string %s"), sp); /* sp points at "%(" and ep points at the closing ")" */ - at = parse_ref_filter_atom(format, sp + 2, ep); + at = parse_ref_filter_atom(format, sp + 2, ep, &err); + if (at < 0) + die("%s", err.buf); cp = ep + 1; if (skip_prefix(used_atom[at].name, "color:", &color)) format->need_color_reset_at_eol = !!strcmp(color, "reset"); + strbuf_release(&err); } if (format->need_color_reset_at_eol && !want_color(format->use_color)) format->need_color_reset_at_eol = 0; @@ -2154,13 +2163,15 @@ int format_ref_array_item(struct ref_array_item *info, for (cp = format->format; *cp && (sp = find_next(cp)); cp = ep + 1) { struct atom_value *atomv; + int pos; ep = strchr(sp, ')'); if (cp < sp) append_literal(cp, sp, &state); - get_ref_atom_value(info, - parse_ref_filter_atom(format, sp + 2, ep), - &atomv); + pos = parse_ref_filter_atom(format, sp + 2, ep, error_buf); + if (pos < 0) + return -1; + get_ref_atom_value(info, pos, &atomv); if (atomv->handler(atomv, &state, error_buf)) return -1; } @@ -2215,7 +2226,12 @@ static int parse_sorting_atom(const char *atom) */ struct ref_format dummy = REF_FORMAT_INIT; const char *end = atom + strlen(atom); - return parse_ref_filter_atom(&dummy, atom, end); + struct strbuf err = STRBUF_INIT; + int res = parse_ref_filter_atom(&dummy, atom, end, &err); + if (res < 0) + die("%s", err.buf); + strbuf_release(&err); + return res; } /* If no sorting option is given, use refname to sort as default */ -- https://github.com/git/git/pull/466
[PATCH v3 4/5] ref-filter: add return value to parsers
Continue removing any printing from ref-filter formatting logic, so that it could be more general. Change the signature of parsers by adding return value and strbuf parameter for error message. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 177 +++ 1 file changed, 118 insertions(+), 59 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 2313a33f0baa4..b90cec1056954 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -101,22 +101,28 @@ static struct used_atom { } *used_atom; static int used_atom_cnt, need_tagged, need_symref; -static void color_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *color_value) +static int color_atom_parser(const struct ref_format *format, struct used_atom *atom, +const char *color_value, struct strbuf *err) { - if (!color_value) - die(_("expected format: %%(color:)")); - if (color_parse(color_value, atom->u.color) < 0) - die(_("unrecognized color: %%(color:%s)"), color_value); + if (!color_value) { + strbuf_addstr(err, _("expected format: %(color:)")); + return -1; + } + if (color_parse(color_value, atom->u.color) < 0) { + strbuf_addf(err, _("unrecognized color: %%(color:%s)"), color_value); + return -1; + } /* * We check this after we've parsed the color, which lets us complain * about syntactically bogus color names even if they won't be used. */ if (!want_color(format->use_color)) color_parse("", atom->u.color); + return 0; } -static void refname_atom_parser_internal(struct refname_atom *atom, -const char *arg, const char *name) +static int refname_atom_parser_internal(struct refname_atom *atom, const char *arg, +const char *name, struct strbuf *err) { if (!arg) atom->option = R_NORMAL; @@ -125,17 +131,25 @@ static void refname_atom_parser_internal(struct refname_atom *atom, else if (skip_prefix(arg, "lstrip=", &arg) || skip_prefix(arg, "strip=", &arg)) { atom->option = R_LSTRIP; - if (strtol_i(arg, 10, &atom->lstrip)) - die(_("Integer value expected refname:lstrip=%s"), arg); + if (strtol_i(arg, 10, &atom->lstrip)) { + strbuf_addf(err, _("Integer value expected refname:lstrip=%s"), arg); + return -1; + } } else if (skip_prefix(arg, "rstrip=", &arg)) { atom->option = R_RSTRIP; - if (strtol_i(arg, 10, &atom->rstrip)) - die(_("Integer value expected refname:rstrip=%s"), arg); - } else - die(_("unrecognized %%(%s) argument: %s"), name, arg); + if (strtol_i(arg, 10, &atom->rstrip)) { + strbuf_addf(err, _("Integer value expected refname:rstrip=%s"), arg); + return -1; + } + } else { + strbuf_addf(err, _("unrecognized %%(%s) argument: %s"), name, arg); + return -1; + } + return 0; } -static void remote_ref_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg) +static int remote_ref_atom_parser(const struct ref_format *format, struct used_atom *atom, + const char *arg, struct strbuf *err) { struct string_list params = STRING_LIST_INIT_DUP; int i; @@ -145,9 +159,8 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_ if (!arg) { atom->u.remote_ref.option = RR_REF; - refname_atom_parser_internal(&atom->u.remote_ref.refname, -arg, atom->name); - return; + return refname_atom_parser_internal(&atom->u.remote_ref.refname, + arg, atom->name, err); } atom->u.remote_ref.nobracket = 0; @@ -170,29 +183,40 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_ atom->u.remote_ref.push_remote = 1; } else { atom->u.remote_ref.option = RR_REF; - refname_atom_parser_internal(&atom->u.remote_ref.refname, -arg, atom->name); + if (refname_atom_parser_internal(&atom->u.remote_ref.refname, +arg, atom->name, err)) + return -1; } } string_list_clear(¶ms, 0); + return 0; } -static void body_atom_parser(const struct ref_format *format,
[PATCH v4 5/5] ref-filter: get_ref_atom_value() error handling
Finish removing die() calls from ref-filter formatting logic, so that it could be used by other commands. Change the signature of get_ref_atom_value() and underlying functions by adding return value and strbuf parameter for error message. Return value equals 0 upon success and -1 upon failure (there could be more error codes further if necessary). Upon failure, error message is appended to the strbuf. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 55 --- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index b90cec1056954..85f971663a4aa 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1445,28 +1445,36 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re return show_ref(&atom->u.refname, ref->refname); } -static void get_object(struct ref_array_item *ref, const struct object_id *oid, - int deref, struct object **obj) +static int get_object(struct ref_array_item *ref, const struct object_id *oid, + int deref, struct object **obj, struct strbuf *err) { int eaten; unsigned long size; void *buf = get_obj(oid, obj, &size, &eaten); - if (!buf) - die(_("missing object %s for %s"), - oid_to_hex(oid), ref->refname); - if (!*obj) - die(_("parse_object_buffer failed on %s for %s"), - oid_to_hex(oid), ref->refname); - + if (!buf) { + strbuf_addf(err, _("missing object %s for %s"), oid_to_hex(oid), + ref->refname); + if (!eaten) + free(buf); + return -1; + } + if (!*obj) { + strbuf_addf(err, _("parse_object_buffer failed on %s for %s"), + oid_to_hex(oid), ref->refname); + if (!eaten) + free(buf); + return -1; + } grab_values(ref->value, deref, *obj, buf, size); if (!eaten) free(buf); + return 0; } /* * Parse the object referred by ref, and grab needed value. */ -static void populate_value(struct ref_array_item *ref) +static int populate_value(struct ref_array_item *ref, struct strbuf *err) { struct object *obj; int i; @@ -1588,16 +1596,17 @@ static void populate_value(struct ref_array_item *ref) break; } if (used_atom_cnt <= i) - return; + return 0; - get_object(ref, &ref->objectname, 0, &obj); + if (get_object(ref, &ref->objectname, 0, &obj, err)) + return -1; /* * If there is no atom that wants to know about tagged * object, we are done. */ if (!need_tagged || (obj->type != OBJ_TAG)) - return; + return 0; /* * If it is a tag object, see if we use a value that derefs @@ -1611,20 +1620,23 @@ static void populate_value(struct ref_array_item *ref) * is not consistent with what deref_tag() does * which peels the onion to the core. */ - get_object(ref, tagged, 1, &obj); + return get_object(ref, tagged, 1, &obj, err); } /* * Given a ref, return the value for the atom. This lazily gets value * out of the object by calling populate value. */ -static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom_value **v) +static int get_ref_atom_value(struct ref_array_item *ref, int atom, + struct atom_value **v, struct strbuf *err) { if (!ref->value) { - populate_value(ref); + if (populate_value(ref, err)) + return -1; fill_missing_values(ref->value); } *v = &ref->value[atom]; + return 0; } /* @@ -2148,9 +2160,13 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru int cmp; cmp_type cmp_type = used_atom[s->atom].type; int (*cmp_fn)(const char *, const char *); + struct strbuf err = STRBUF_INIT; - get_ref_atom_value(a, s->atom, &va); - get_ref_atom_value(b, s->atom, &vb); + if (get_ref_atom_value(a, s->atom, &va, &err)) + die("%s", err.buf); + if (get_ref_atom_value(b, s->atom, &vb, &err)) + die("%s", err.buf); + strbuf_release(&err); cmp_fn = s->ignore_case ? strcasecmp : strcmp; if (s->version) cmp = versioncmp(va->s, vb->s); @@ -2230,7 +2246,8 @@ int format_ref_array_item(struct ref_array_item *info, pos = parse_ref_filter_atom(format, sp + 2, ep, error_buf); if (pos < 0) return -1; - get_ref_atom_value(info, pos, &atomv); + if (get_ref_atom_value(info, pos, &atomv, error_
[PATCH v4 1/5] ref-filter: start adding strbufs with errors
This is a first step in removing die() calls from ref-filter formatting logic, so that it could be used by other commands that do not want to die during formatting process. die() calls related to bugs in code will not be touched in this patch. Everything would be the same for show_ref_array_item() users. But, if you want to deal with errors by your own, you could invoke format_ref_array_item(). It means that you need to print everything (the result and errors) on your side. This commit changes signature of format_ref_array_item() by adding return value and strbuf parameter for errors, and adjusts its callers. While at it, reduce the scope of the out-variable. Signed-off-by: Olga Telezhnaia --- builtin/branch.c | 7 +-- ref-filter.c | 17 - ref-filter.h | 7 --- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 6d0cea9d4bcc4..c21e5a04a0177 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -391,7 +391,6 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin struct ref_array array; int maxwidth = 0; const char *remote_prefix = ""; - struct strbuf out = STRBUF_INIT; char *to_free = NULL; /* @@ -419,7 +418,10 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin ref_array_sort(sorting, &array); for (i = 0; i < array.nr; i++) { - format_ref_array_item(array.items[i], format, &out); + struct strbuf out = STRBUF_INIT; + struct strbuf err = STRBUF_INIT; + if (format_ref_array_item(array.items[i], format, &out, &err)) + die("%s", err.buf); if (column_active(colopts)) { assert(!filter->verbose && "--column and --verbose are incompatible"); /* format to a string_list to let print_columns() do its job */ @@ -428,6 +430,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin fwrite(out.buf, 1, out.len, stdout); putchar('\n'); } + strbuf_release(&err); strbuf_release(&out); } diff --git a/ref-filter.c b/ref-filter.c index 45fc56216aaa8..54fae00bdd410 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2118,9 +2118,10 @@ static void append_literal(const char *cp, const char *ep, struct ref_formatting } } -void format_ref_array_item(struct ref_array_item *info, +int format_ref_array_item(struct ref_array_item *info, const struct ref_format *format, - struct strbuf *final_buf) + struct strbuf *final_buf, + struct strbuf *error_buf) { const char *cp, *sp, *ep; struct ref_formatting_state state = REF_FORMATTING_STATE_INIT; @@ -2148,19 +2149,25 @@ void format_ref_array_item(struct ref_array_item *info, resetv.s = GIT_COLOR_RESET; append_atom(&resetv, &state); } - if (state.stack->prev) - die(_("format: %%(end) atom missing")); + if (state.stack->prev) { + strbuf_addstr(error_buf, _("format: %(end) atom missing")); + return -1; + } strbuf_addbuf(final_buf, &state.stack->output); pop_stack_element(&state.stack); + return 0; } void show_ref_array_item(struct ref_array_item *info, const struct ref_format *format) { struct strbuf final_buf = STRBUF_INIT; + struct strbuf error_buf = STRBUF_INIT; - format_ref_array_item(info, format, &final_buf); + if (format_ref_array_item(info, format, &final_buf, &error_buf)) + die("%s", error_buf.buf); fwrite(final_buf.buf, 1, final_buf.len, stdout); + strbuf_release(&error_buf); strbuf_release(&final_buf); putchar('\n'); } diff --git a/ref-filter.h b/ref-filter.h index 0d98342b34319..e13f8e6f8721a 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -110,9 +110,10 @@ int verify_ref_format(struct ref_format *format); /* Sort the given ref_array as per the ref_sorting provided */ void ref_array_sort(struct ref_sorting *sort, struct ref_array *array); /* Based on the given format and quote_style, fill the strbuf */ -void format_ref_array_item(struct ref_array_item *info, - const struct ref_format *format, - struct strbuf *final_buf); +int format_ref_array_item(struct ref_array_item *info, + const struct ref_format *format, + struct strbuf *final_buf, + struct strbuf *error_buf); /* Print the ref using the given format and quote_style */ void show_ref_array_item(struct ref_array_item *info, const struct ref_format *format); /* P
[PATCH v4 3/5] ref-filter: change parsing function error handling
Continue removing die() calls from ref-filter formatting logic, so that it could be used by other commands. Change the signature of parse_ref_filter_atom() by adding strbuf parameter for error message. The function returns the position in the used_atom[] array (as before) for the given atom, or -1 to signal an error (there could be more negative error codes further if necessary). Upon failure, error message is appended to the strbuf. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 36 ++-- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index d120360104806..2313a33f0baa4 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -397,7 +397,8 @@ struct atom_value { * Used to parse format string and sort specifiers */ static int parse_ref_filter_atom(const struct ref_format *format, -const char *atom, const char *ep) +const char *atom, const char *ep, +struct strbuf *err) { const char *sp; const char *arg; @@ -406,8 +407,10 @@ static int parse_ref_filter_atom(const struct ref_format *format, sp = atom; if (*sp == '*' && sp < ep) sp++; /* deref */ - if (ep <= sp) - die(_("malformed field name: %.*s"), (int)(ep-atom), atom); + if (ep <= sp) { + strbuf_addf(err, _("malformed field name: %.*s"), (int)(ep-atom), atom); + return -1; + } /* Do we have the atom already used elsewhere? */ for (i = 0; i < used_atom_cnt; i++) { @@ -432,8 +435,10 @@ static int parse_ref_filter_atom(const struct ref_format *format, break; } - if (ARRAY_SIZE(valid_atom) <= i) - die(_("unknown field name: %.*s"), (int)(ep-atom), atom); + if (ARRAY_SIZE(valid_atom) <= i) { + strbuf_addf(err, _("unknown field name: %.*s"), (int)(ep-atom), atom); + return -1; + } /* Add it in, including the deref prefix */ at = used_atom_cnt; @@ -725,17 +730,21 @@ int verify_ref_format(struct ref_format *format) format->need_color_reset_at_eol = 0; for (cp = format->format; *cp && (sp = find_next(cp)); ) { + struct strbuf err = STRBUF_INIT; const char *color, *ep = strchr(sp, ')'); int at; if (!ep) return error(_("malformed format string %s"), sp); /* sp points at "%(" and ep points at the closing ")" */ - at = parse_ref_filter_atom(format, sp + 2, ep); + at = parse_ref_filter_atom(format, sp + 2, ep, &err); + if (at < 0) + die("%s", err.buf); cp = ep + 1; if (skip_prefix(used_atom[at].name, "color:", &color)) format->need_color_reset_at_eol = !!strcmp(color, "reset"); + strbuf_release(&err); } if (format->need_color_reset_at_eol && !want_color(format->use_color)) format->need_color_reset_at_eol = 0; @@ -2154,13 +2163,15 @@ int format_ref_array_item(struct ref_array_item *info, for (cp = format->format; *cp && (sp = find_next(cp)); cp = ep + 1) { struct atom_value *atomv; + int pos; ep = strchr(sp, ')'); if (cp < sp) append_literal(cp, sp, &state); - get_ref_atom_value(info, - parse_ref_filter_atom(format, sp + 2, ep), - &atomv); + pos = parse_ref_filter_atom(format, sp + 2, ep, error_buf); + if (pos < 0) + return -1; + get_ref_atom_value(info, pos, &atomv); if (atomv->handler(atomv, &state, error_buf)) return -1; } @@ -2215,7 +2226,12 @@ static int parse_sorting_atom(const char *atom) */ struct ref_format dummy = REF_FORMAT_INIT; const char *end = atom + strlen(atom); - return parse_ref_filter_atom(&dummy, atom, end); + struct strbuf err = STRBUF_INIT; + int res = parse_ref_filter_atom(&dummy, atom, end, &err); + if (res < 0) + die("%s", err.buf); + strbuf_release(&err); + return res; } /* If no sorting option is given, use refname to sort as default */ -- https://github.com/git/git/pull/466
[PATCH v4 4/5] ref-filter: add return value to parsers
Continue removing die() calls from ref-filter formatting logic, so that it could be used by other commands. Change the signature of parsers by adding return value and strbuf parameter for error message. Return value equals 0 upon success and -1 upon failure (there could be more error codes further if necessary). Upon failure, error message is appended to the strbuf. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 177 +++ 1 file changed, 118 insertions(+), 59 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 2313a33f0baa4..b90cec1056954 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -101,22 +101,28 @@ static struct used_atom { } *used_atom; static int used_atom_cnt, need_tagged, need_symref; -static void color_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *color_value) +static int color_atom_parser(const struct ref_format *format, struct used_atom *atom, +const char *color_value, struct strbuf *err) { - if (!color_value) - die(_("expected format: %%(color:)")); - if (color_parse(color_value, atom->u.color) < 0) - die(_("unrecognized color: %%(color:%s)"), color_value); + if (!color_value) { + strbuf_addstr(err, _("expected format: %(color:)")); + return -1; + } + if (color_parse(color_value, atom->u.color) < 0) { + strbuf_addf(err, _("unrecognized color: %%(color:%s)"), color_value); + return -1; + } /* * We check this after we've parsed the color, which lets us complain * about syntactically bogus color names even if they won't be used. */ if (!want_color(format->use_color)) color_parse("", atom->u.color); + return 0; } -static void refname_atom_parser_internal(struct refname_atom *atom, -const char *arg, const char *name) +static int refname_atom_parser_internal(struct refname_atom *atom, const char *arg, +const char *name, struct strbuf *err) { if (!arg) atom->option = R_NORMAL; @@ -125,17 +131,25 @@ static void refname_atom_parser_internal(struct refname_atom *atom, else if (skip_prefix(arg, "lstrip=", &arg) || skip_prefix(arg, "strip=", &arg)) { atom->option = R_LSTRIP; - if (strtol_i(arg, 10, &atom->lstrip)) - die(_("Integer value expected refname:lstrip=%s"), arg); + if (strtol_i(arg, 10, &atom->lstrip)) { + strbuf_addf(err, _("Integer value expected refname:lstrip=%s"), arg); + return -1; + } } else if (skip_prefix(arg, "rstrip=", &arg)) { atom->option = R_RSTRIP; - if (strtol_i(arg, 10, &atom->rstrip)) - die(_("Integer value expected refname:rstrip=%s"), arg); - } else - die(_("unrecognized %%(%s) argument: %s"), name, arg); + if (strtol_i(arg, 10, &atom->rstrip)) { + strbuf_addf(err, _("Integer value expected refname:rstrip=%s"), arg); + return -1; + } + } else { + strbuf_addf(err, _("unrecognized %%(%s) argument: %s"), name, arg); + return -1; + } + return 0; } -static void remote_ref_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg) +static int remote_ref_atom_parser(const struct ref_format *format, struct used_atom *atom, + const char *arg, struct strbuf *err) { struct string_list params = STRING_LIST_INIT_DUP; int i; @@ -145,9 +159,8 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_ if (!arg) { atom->u.remote_ref.option = RR_REF; - refname_atom_parser_internal(&atom->u.remote_ref.refname, -arg, atom->name); - return; + return refname_atom_parser_internal(&atom->u.remote_ref.refname, + arg, atom->name, err); } atom->u.remote_ref.nobracket = 0; @@ -170,29 +183,40 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_ atom->u.remote_ref.push_remote = 1; } else { atom->u.remote_ref.option = RR_REF; - refname_atom_parser_internal(&atom->u.remote_ref.refname, -arg, atom->name); + if (refname_atom_parser_internal(&atom->u.remote_ref.refname, +arg, atom->name, err)) +
[PATCH v4 2/5] ref-filter: add return value && strbuf to handlers
Continue removing die() calls from ref-filter formatting logic, so that it could be used by other commands. Change the signature of handlers by adding return value and strbuf parameter for errors. Return value equals 0 upon success and -1 upon failure (there could be more error codes further if necessary). Upon failure, error message is appended to the strbuf. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 71 1 file changed, 48 insertions(+), 23 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 54fae00bdd410..d120360104806 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -387,7 +387,8 @@ struct ref_formatting_state { struct atom_value { const char *s; - void (*handler)(struct atom_value *atomv, struct ref_formatting_state *state); + int (*handler)(struct atom_value *atomv, struct ref_formatting_state *state, + struct strbuf *err); uintmax_t value; /* used for sorting when not FIELD_STR */ struct used_atom *atom; }; @@ -481,7 +482,8 @@ static void quote_formatting(struct strbuf *s, const char *str, int quote_style) } } -static void append_atom(struct atom_value *v, struct ref_formatting_state *state) +static int append_atom(struct atom_value *v, struct ref_formatting_state *state, + struct strbuf *unused_err) { /* * Quote formatting is only done when the stack has a single @@ -493,6 +495,7 @@ static void append_atom(struct atom_value *v, struct ref_formatting_state *state quote_formatting(&state->stack->output, v->s, state->quote_style); else strbuf_addstr(&state->stack->output, v->s); + return 0; } static void push_stack_element(struct ref_formatting_stack **stack) @@ -527,7 +530,8 @@ static void end_align_handler(struct ref_formatting_stack **stack) strbuf_release(&s); } -static void align_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state) +static int align_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, + struct strbuf *unused_err) { struct ref_formatting_stack *new_stack; @@ -535,6 +539,7 @@ static void align_atom_handler(struct atom_value *atomv, struct ref_formatting_s new_stack = state->stack; new_stack->at_end = end_align_handler; new_stack->at_end_data = &atomv->atom->u.align; + return 0; } static void if_then_else_handler(struct ref_formatting_stack **stack) @@ -572,7 +577,8 @@ static void if_then_else_handler(struct ref_formatting_stack **stack) free(if_then_else); } -static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state) +static int if_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, + struct strbuf *unused_err) { struct ref_formatting_stack *new_stack; struct if_then_else *if_then_else = xcalloc(sizeof(struct if_then_else), 1); @@ -584,6 +590,7 @@ static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_stat new_stack = state->stack; new_stack->at_end = if_then_else_handler; new_stack->at_end_data = if_then_else; + return 0; } static int is_empty(const char *s) @@ -596,19 +603,24 @@ static int is_empty(const char *s) return 1; } -static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state) +static int then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, +struct strbuf *err) { struct ref_formatting_stack *cur = state->stack; struct if_then_else *if_then_else = NULL; if (cur->at_end == if_then_else_handler) if_then_else = (struct if_then_else *)cur->at_end_data; - if (!if_then_else) - die(_("format: %%(then) atom used without an %%(if) atom")); - if (if_then_else->then_atom_seen) - die(_("format: %%(then) atom used more than once")); - if (if_then_else->else_atom_seen) - die(_("format: %%(then) atom used after %%(else)")); + if (!if_then_else) { + strbuf_addstr(err, _("format: %(then) atom used without an %(if) atom")); + return -1; + } else if (if_then_else->then_atom_seen) { + strbuf_addstr(err, _("format: %(then) atom used more than once")); + return -1; + } else if (if_then_else->else_atom_seen) { + strbuf_addstr(err, _("format: %(then) atom used after %(else)")); + return -1; + } if_then_else->then_atom_seen = 1; /* * If the 'equals' or 'notequals' attribute is used then @@ -624,34 +636,44 @@ static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_st } else if (cur->output.len && !i
[PATCH v5 3/6] ref-filter: add return value && strbuf to handlers
Continue removing die() calls from ref-filter formatting logic, so that it could be used by other commands. Change the signature of handlers by adding return value and strbuf parameter for errors. Return value equals 0 upon success and -1 upon failure. Upon failure, error message is appended to the strbuf. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 47 +++ 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 2c3fb2f003708..f79c8c477f1dc 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -387,7 +387,8 @@ struct ref_formatting_state { struct atom_value { const char *s; - void (*handler)(struct atom_value *atomv, struct ref_formatting_state *state); + int (*handler)(struct atom_value *atomv, struct ref_formatting_state *state, + struct strbuf *err); uintmax_t value; /* used for sorting when not FIELD_STR */ struct used_atom *atom; }; @@ -481,7 +482,8 @@ static void quote_formatting(struct strbuf *s, const char *str, int quote_style) } } -static void append_atom(struct atom_value *v, struct ref_formatting_state *state) +static int append_atom(struct atom_value *v, struct ref_formatting_state *state, + struct strbuf *unused_err) { /* * Quote formatting is only done when the stack has a single @@ -493,6 +495,7 @@ static void append_atom(struct atom_value *v, struct ref_formatting_state *state quote_formatting(&state->stack->output, v->s, state->quote_style); else strbuf_addstr(&state->stack->output, v->s); + return 0; } static void push_stack_element(struct ref_formatting_stack **stack) @@ -527,7 +530,8 @@ static void end_align_handler(struct ref_formatting_stack **stack) strbuf_release(&s); } -static void align_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state) +static int align_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, + struct strbuf *unused_err) { struct ref_formatting_stack *new_stack; @@ -535,6 +539,7 @@ static void align_atom_handler(struct atom_value *atomv, struct ref_formatting_s new_stack = state->stack; new_stack->at_end = end_align_handler; new_stack->at_end_data = &atomv->atom->u.align; + return 0; } static void if_then_else_handler(struct ref_formatting_stack **stack) @@ -572,7 +577,8 @@ static void if_then_else_handler(struct ref_formatting_stack **stack) free(if_then_else); } -static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state) +static int if_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, + struct strbuf *unused_err) { struct ref_formatting_stack *new_stack; struct if_then_else *if_then_else = xcalloc(sizeof(struct if_then_else), 1); @@ -584,6 +590,7 @@ static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_stat new_stack = state->stack; new_stack->at_end = if_then_else_handler; new_stack->at_end_data = if_then_else; + return 0; } static int is_empty(const char *s) @@ -596,7 +603,8 @@ static int is_empty(const char *s) return 1; } -static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state) +static int then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, +struct strbuf *err) { struct ref_formatting_stack *cur = state->stack; struct if_then_else *if_then_else = NULL; @@ -604,11 +612,11 @@ static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_st if (cur->at_end == if_then_else_handler) if_then_else = (struct if_then_else *)cur->at_end_data; if (!if_then_else) - die(_("format: %%(then) atom used without an %%(if) atom")); + return strbuf_error(err, -1, _("format: %%(then) atom used without an %%(if) atom")); if (if_then_else->then_atom_seen) - die(_("format: %%(then) atom used more than once")); + return strbuf_error(err, -1, _("format: %%(then) atom used more than once")); if (if_then_else->else_atom_seen) - die(_("format: %%(then) atom used after %%(else)")); + return strbuf_error(err, -1, _("format: %%(then) atom used after %%(else)")); if_then_else->then_atom_seen = 1; /* * If the 'equals' or 'notequals' attribute is used then @@ -624,9 +632,11 @@ static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_st } else if (cur->output.len && !is_empty(cur->output.buf)) if_then_else->condition_satisfied = 1; strbuf_reset(&cur->output); + return 0; } -static void els
[PATCH v5 6/6] ref-filter: libify get_ref_atom_value()
Finish removing die() calls from ref-filter formatting logic, so that it could be used by other commands. Change the signature of get_ref_atom_value() and underlying functions by adding return value and strbuf parameter for error message. Return value equals 0 upon success and -1 upon failure. Upon failure, error message is appended to the strbuf. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 49 ++--- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 0ef7386b5fd20..52bb84398dc8d 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1398,28 +1398,30 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re return show_ref(&atom->u.refname, ref->refname); } -static void get_object(struct ref_array_item *ref, const struct object_id *oid, - int deref, struct object **obj) +static int get_object(struct ref_array_item *ref, const struct object_id *oid, + int deref, struct object **obj, struct strbuf *err) { int eaten; + int ret = 0; unsigned long size; void *buf = get_obj(oid, obj, &size, &eaten); if (!buf) - die(_("missing object %s for %s"), - oid_to_hex(oid), ref->refname); - if (!*obj) - die(_("parse_object_buffer failed on %s for %s"), - oid_to_hex(oid), ref->refname); - - grab_values(ref->value, deref, *obj, buf, size); + ret = strbuf_error(err, -1, _("missing object %s for %s"), + oid_to_hex(oid), ref->refname); + else if (!*obj) + ret = strbuf_error(err, -1, _("parse_object_buffer failed on %s for %s"), + oid_to_hex(oid), ref->refname); + else + grab_values(ref->value, deref, *obj, buf, size); if (!eaten) free(buf); + return ret; } /* * Parse the object referred by ref, and grab needed value. */ -static void populate_value(struct ref_array_item *ref) +static int populate_value(struct ref_array_item *ref, struct strbuf *err) { struct object *obj; int i; @@ -1541,16 +1543,17 @@ static void populate_value(struct ref_array_item *ref) break; } if (used_atom_cnt <= i) - return; + return 0; - get_object(ref, &ref->objectname, 0, &obj); + if (get_object(ref, &ref->objectname, 0, &obj, err)) + return -1; /* * If there is no atom that wants to know about tagged * object, we are done. */ if (!need_tagged || (obj->type != OBJ_TAG)) - return; + return 0; /* * If it is a tag object, see if we use a value that derefs @@ -1564,20 +1567,23 @@ static void populate_value(struct ref_array_item *ref) * is not consistent with what deref_tag() does * which peels the onion to the core. */ - get_object(ref, tagged, 1, &obj); + return get_object(ref, tagged, 1, &obj, err); } /* * Given a ref, return the value for the atom. This lazily gets value * out of the object by calling populate value. */ -static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom_value **v) +static int get_ref_atom_value(struct ref_array_item *ref, int atom, + struct atom_value **v, struct strbuf *err) { if (!ref->value) { - populate_value(ref); + if (populate_value(ref, err)) + return -1; fill_missing_values(ref->value); } *v = &ref->value[atom]; + return 0; } /* @@ -2101,9 +2107,13 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru int cmp; cmp_type cmp_type = used_atom[s->atom].type; int (*cmp_fn)(const char *, const char *); + struct strbuf err = STRBUF_INIT; - get_ref_atom_value(a, s->atom, &va); - get_ref_atom_value(b, s->atom, &vb); + if (get_ref_atom_value(a, s->atom, &va, &err)) + die("%s", err.buf); + if (get_ref_atom_value(b, s->atom, &vb, &err)) + die("%s", err.buf); + strbuf_release(&err); cmp_fn = s->ignore_case ? strcasecmp : strcmp; if (s->version) cmp = versioncmp(va->s, vb->s); @@ -2183,7 +2193,8 @@ int format_ref_array_item(struct ref_array_item *info, pos = parse_ref_filter_atom(format, sp + 2, ep, error_buf); if (pos < 0) return -1; - get_ref_atom_value(info, pos, &atomv); + if (get_ref_atom_value(info, pos, &atomv, error_buf)) + return -1; if (atomv->handler(atomv, &state, error_buf)) return -1;
[PATCH v5 1/6] strbuf: add shortcut to work with error messages
Add function strbuf_error() that helps to save few lines of code. Function expands fmt with placeholders, append resulting error message to strbuf *err, and return error code ret. Signed-off-by: Olga Telezhnaia --- strbuf.h | 13 + 1 file changed, 13 insertions(+) diff --git a/strbuf.h b/strbuf.h index e6cae5f4398c8..fa66d4835f1a7 100644 --- a/strbuf.h +++ b/strbuf.h @@ -620,4 +620,17 @@ char *xstrvfmt(const char *fmt, va_list ap); __attribute__((format (printf, 1, 2))) char *xstrfmt(const char *fmt, ...); +/* + * Expand error message, append it to strbuf *err, then return error code ret. + * Allow to save few lines of code. + */ +static inline int strbuf_error(struct strbuf *err, int ret, const char *fmt, ...) +{ + va_list ap; + va_start(ap, fmt); + strbuf_vaddf(err, fmt, ap); + va_end(ap); + return ret; +} + #endif /* STRBUF_H */ -- https://github.com/git/git/pull/466
[PATCH v5 5/6] ref-filter: add return value to parsers
Continue removing die() calls from ref-filter formatting logic, so that it could be used by other commands. Change the signature of parsers by adding return value and strbuf parameter for error message. Return value equals 0 upon success and -1 upon failure. Upon failure, error message is appended to the strbuf. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 112 --- 1 file changed, 68 insertions(+), 44 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index ca38728b4c998..0ef7386b5fd20 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -101,22 +101,25 @@ static struct used_atom { } *used_atom; static int used_atom_cnt, need_tagged, need_symref; -static void color_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *color_value) +static int color_atom_parser(const struct ref_format *format, struct used_atom *atom, +const char *color_value, struct strbuf *err) { if (!color_value) - die(_("expected format: %%(color:)")); + return strbuf_error(err, -1, _("expected format: %%(color:)")); if (color_parse(color_value, atom->u.color) < 0) - die(_("unrecognized color: %%(color:%s)"), color_value); + return strbuf_error(err, -1, _("unrecognized color: %%(color:%s)"), + color_value); /* * We check this after we've parsed the color, which lets us complain * about syntactically bogus color names even if they won't be used. */ if (!want_color(format->use_color)) color_parse("", atom->u.color); + return 0; } -static void refname_atom_parser_internal(struct refname_atom *atom, -const char *arg, const char *name) +static int refname_atom_parser_internal(struct refname_atom *atom, const char *arg, +const char *name, struct strbuf *err) { if (!arg) atom->option = R_NORMAL; @@ -126,16 +129,18 @@ static void refname_atom_parser_internal(struct refname_atom *atom, skip_prefix(arg, "strip=", &arg)) { atom->option = R_LSTRIP; if (strtol_i(arg, 10, &atom->lstrip)) - die(_("Integer value expected refname:lstrip=%s"), arg); + return strbuf_error(err, -1, _("Integer value expected refname:lstrip=%s"), arg); } else if (skip_prefix(arg, "rstrip=", &arg)) { atom->option = R_RSTRIP; if (strtol_i(arg, 10, &atom->rstrip)) - die(_("Integer value expected refname:rstrip=%s"), arg); + return strbuf_error(err, -1, _("Integer value expected refname:rstrip=%s"), arg); } else - die(_("unrecognized %%(%s) argument: %s"), name, arg); + return strbuf_error(err, -1, _("unrecognized %%(%s) argument: %s"), name, arg); + return 0; } -static void remote_ref_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg) +static int remote_ref_atom_parser(const struct ref_format *format, struct used_atom *atom, + const char *arg, struct strbuf *err) { struct string_list params = STRING_LIST_INIT_DUP; int i; @@ -145,9 +150,8 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_ if (!arg) { atom->u.remote_ref.option = RR_REF; - refname_atom_parser_internal(&atom->u.remote_ref.refname, -arg, atom->name); - return; + return refname_atom_parser_internal(&atom->u.remote_ref.refname, + arg, atom->name, err); } atom->u.remote_ref.nobracket = 0; @@ -170,29 +174,36 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_ atom->u.remote_ref.push_remote = 1; } else { atom->u.remote_ref.option = RR_REF; - refname_atom_parser_internal(&atom->u.remote_ref.refname, -arg, atom->name); + if (refname_atom_parser_internal(&atom->u.remote_ref.refname, +arg, atom->name, err)) + return -1; } } string_list_clear(¶ms, 0); + return 0; } -static void body_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg) +static int body_atom_parser(const struct ref_format *format, struct used_atom *atom, + const char *arg, struct strbuf *err) { if (arg) - die(_("%%(body) does not take arguments")
[PATCH v5 4/6] ref-filter: change parsing function error handling
Continue removing die() calls from ref-filter formatting logic, so that it could be used by other commands. Change the signature of parse_ref_filter_atom() by adding strbuf parameter for error message. The function returns the position in the used_atom[] array (as before) for the given atom, or -1 to signal an error. Upon failure, error message is appended to the strbuf. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 30 ++ 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index f79c8c477f1dc..ca38728b4c998 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -397,7 +397,8 @@ struct atom_value { * Used to parse format string and sort specifiers */ static int parse_ref_filter_atom(const struct ref_format *format, -const char *atom, const char *ep) +const char *atom, const char *ep, +struct strbuf *err) { const char *sp; const char *arg; @@ -407,7 +408,8 @@ static int parse_ref_filter_atom(const struct ref_format *format, if (*sp == '*' && sp < ep) sp++; /* deref */ if (ep <= sp) - die(_("malformed field name: %.*s"), (int)(ep-atom), atom); + return strbuf_error(err, -1, _("malformed field name: %.*s"), + (int)(ep-atom), atom); /* Do we have the atom already used elsewhere? */ for (i = 0; i < used_atom_cnt; i++) { @@ -433,7 +435,8 @@ static int parse_ref_filter_atom(const struct ref_format *format, } if (ARRAY_SIZE(valid_atom) <= i) - die(_("unknown field name: %.*s"), (int)(ep-atom), atom); + return strbuf_error(err, -1, _("unknown field name: %.*s"), + (int)(ep-atom), atom); /* Add it in, including the deref prefix */ at = used_atom_cnt; @@ -715,17 +718,21 @@ int verify_ref_format(struct ref_format *format) format->need_color_reset_at_eol = 0; for (cp = format->format; *cp && (sp = find_next(cp)); ) { + struct strbuf err = STRBUF_INIT; const char *color, *ep = strchr(sp, ')'); int at; if (!ep) return error(_("malformed format string %s"), sp); /* sp points at "%(" and ep points at the closing ")" */ - at = parse_ref_filter_atom(format, sp + 2, ep); + at = parse_ref_filter_atom(format, sp + 2, ep, &err); + if (at < 0) + die("%s", err.buf); cp = ep + 1; if (skip_prefix(used_atom[at].name, "color:", &color)) format->need_color_reset_at_eol = !!strcmp(color, "reset"); + strbuf_release(&err); } if (format->need_color_reset_at_eol && !want_color(format->use_color)) format->need_color_reset_at_eol = 0; @@ -2144,13 +2151,15 @@ int format_ref_array_item(struct ref_array_item *info, for (cp = format->format; *cp && (sp = find_next(cp)); cp = ep + 1) { struct atom_value *atomv; + int pos; ep = strchr(sp, ')'); if (cp < sp) append_literal(cp, sp, &state); - get_ref_atom_value(info, - parse_ref_filter_atom(format, sp + 2, ep), - &atomv); + pos = parse_ref_filter_atom(format, sp + 2, ep, error_buf); + if (pos < 0) + return -1; + get_ref_atom_value(info, pos, &atomv); if (atomv->handler(atomv, &state, error_buf)) return -1; } @@ -2203,7 +2212,12 @@ static int parse_sorting_atom(const char *atom) */ struct ref_format dummy = REF_FORMAT_INIT; const char *end = atom + strlen(atom); - return parse_ref_filter_atom(&dummy, atom, end); + struct strbuf err = STRBUF_INIT; + int res = parse_ref_filter_atom(&dummy, atom, end, &err); + if (res < 0) + die("%s", err.buf); + strbuf_release(&err); + return res; } /* If no sorting option is given, use refname to sort as default */ -- https://github.com/git/git/pull/466
[PATCH v5 2/6] ref-filter: start adding strbufs with errors
This is a first step in removing die() calls from ref-filter formatting logic, so that it could be used by other commands that do not want to die during formatting process. die() calls related to bugs in code will not be touched in this patch. Everything would be the same for show_ref_array_item() users. But, if you want to deal with errors by your own, you could invoke format_ref_array_item(). It means that you need to print everything (the result and errors) on your side. This commit changes signature of format_ref_array_item() by adding return value and strbuf parameter for errors, and adjusts its callers. While at it, reduce the scope of the out-variable. Signed-off-by: Olga Telezhnaia --- builtin/branch.c | 7 +-- ref-filter.c | 13 + ref-filter.h | 7 --- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 6d0cea9d4bcc4..c21e5a04a0177 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -391,7 +391,6 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin struct ref_array array; int maxwidth = 0; const char *remote_prefix = ""; - struct strbuf out = STRBUF_INIT; char *to_free = NULL; /* @@ -419,7 +418,10 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin ref_array_sort(sorting, &array); for (i = 0; i < array.nr; i++) { - format_ref_array_item(array.items[i], format, &out); + struct strbuf out = STRBUF_INIT; + struct strbuf err = STRBUF_INIT; + if (format_ref_array_item(array.items[i], format, &out, &err)) + die("%s", err.buf); if (column_active(colopts)) { assert(!filter->verbose && "--column and --verbose are incompatible"); /* format to a string_list to let print_columns() do its job */ @@ -428,6 +430,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin fwrite(out.buf, 1, out.len, stdout); putchar('\n'); } + strbuf_release(&err); strbuf_release(&out); } diff --git a/ref-filter.c b/ref-filter.c index 45fc56216aaa8..2c3fb2f003708 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2118,9 +2118,10 @@ static void append_literal(const char *cp, const char *ep, struct ref_formatting } } -void format_ref_array_item(struct ref_array_item *info, +int format_ref_array_item(struct ref_array_item *info, const struct ref_format *format, - struct strbuf *final_buf) + struct strbuf *final_buf, + struct strbuf *error_buf) { const char *cp, *sp, *ep; struct ref_formatting_state state = REF_FORMATTING_STATE_INIT; @@ -2149,18 +2150,22 @@ void format_ref_array_item(struct ref_array_item *info, append_atom(&resetv, &state); } if (state.stack->prev) - die(_("format: %%(end) atom missing")); + return strbuf_error(error_buf, -1, _("format: %%(end) atom missing")); strbuf_addbuf(final_buf, &state.stack->output); pop_stack_element(&state.stack); + return 0; } void show_ref_array_item(struct ref_array_item *info, const struct ref_format *format) { struct strbuf final_buf = STRBUF_INIT; + struct strbuf error_buf = STRBUF_INIT; - format_ref_array_item(info, format, &final_buf); + if (format_ref_array_item(info, format, &final_buf, &error_buf)) + die("%s", error_buf.buf); fwrite(final_buf.buf, 1, final_buf.len, stdout); + strbuf_release(&error_buf); strbuf_release(&final_buf); putchar('\n'); } diff --git a/ref-filter.h b/ref-filter.h index 0d98342b34319..e13f8e6f8721a 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -110,9 +110,10 @@ int verify_ref_format(struct ref_format *format); /* Sort the given ref_array as per the ref_sorting provided */ void ref_array_sort(struct ref_sorting *sort, struct ref_array *array); /* Based on the given format and quote_style, fill the strbuf */ -void format_ref_array_item(struct ref_array_item *info, - const struct ref_format *format, - struct strbuf *final_buf); +int format_ref_array_item(struct ref_array_item *info, + const struct ref_format *format, + struct strbuf *final_buf, + struct strbuf *error_buf); /* Print the ref using the given format and quote_style */ void show_ref_array_item(struct ref_array_item *info, const struct ref_format *format); /* Parse a single sort specifier and add it to the list */ -- https://github.com/git/git/pull/466
[PATCH v6 5/6] ref-filter: add return value to parsers
Continue removing die() calls from ref-filter formatting logic, so that it could be used by other commands. Change the signature of parsers by adding return value and strbuf parameter for error message. Return value equals 0 upon success and -1 upon failure. Upon failure, error message is appended to the strbuf. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 138 ++- 1 file changed, 89 insertions(+), 49 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 93fa6b4e5e63d..3f85ef64267d9 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -114,22 +114,25 @@ static int strbuf_addf_ret(struct strbuf *sb, int ret, const char *fmt, ...) return ret; } -static void color_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *color_value) +static int color_atom_parser(const struct ref_format *format, struct used_atom *atom, +const char *color_value, struct strbuf *err) { if (!color_value) - die(_("expected format: %%(color:)")); + return strbuf_addf_ret(err, -1, _("expected format: %%(color:)")); if (color_parse(color_value, atom->u.color) < 0) - die(_("unrecognized color: %%(color:%s)"), color_value); + return strbuf_addf_ret(err, -1, _("unrecognized color: %%(color:%s)"), + color_value); /* * We check this after we've parsed the color, which lets us complain * about syntactically bogus color names even if they won't be used. */ if (!want_color(format->use_color)) color_parse("", atom->u.color); + return 0; } -static void refname_atom_parser_internal(struct refname_atom *atom, -const char *arg, const char *name) +static int refname_atom_parser_internal(struct refname_atom *atom, const char *arg, +const char *name, struct strbuf *err) { if (!arg) atom->option = R_NORMAL; @@ -139,16 +142,18 @@ static void refname_atom_parser_internal(struct refname_atom *atom, skip_prefix(arg, "strip=", &arg)) { atom->option = R_LSTRIP; if (strtol_i(arg, 10, &atom->lstrip)) - die(_("Integer value expected refname:lstrip=%s"), arg); + return strbuf_addf_ret(err, -1, _("Integer value expected refname:lstrip=%s"), arg); } else if (skip_prefix(arg, "rstrip=", &arg)) { atom->option = R_RSTRIP; if (strtol_i(arg, 10, &atom->rstrip)) - die(_("Integer value expected refname:rstrip=%s"), arg); + return strbuf_addf_ret(err, -1, _("Integer value expected refname:rstrip=%s"), arg); } else - die(_("unrecognized %%(%s) argument: %s"), name, arg); + return strbuf_addf_ret(err, -1, _("unrecognized %%(%s) argument: %s"), name, arg); + return 0; } -static void remote_ref_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg) +static int remote_ref_atom_parser(const struct ref_format *format, struct used_atom *atom, + const char *arg, struct strbuf *err) { struct string_list params = STRING_LIST_INIT_DUP; int i; @@ -158,9 +163,8 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_ if (!arg) { atom->u.remote_ref.option = RR_REF; - refname_atom_parser_internal(&atom->u.remote_ref.refname, -arg, atom->name); - return; + return refname_atom_parser_internal(&atom->u.remote_ref.refname, + arg, atom->name, err); } atom->u.remote_ref.nobracket = 0; @@ -183,29 +187,38 @@ static void remote_ref_atom_parser(const struct ref_format *format, struct used_ atom->u.remote_ref.push_remote = 1; } else { atom->u.remote_ref.option = RR_REF; - refname_atom_parser_internal(&atom->u.remote_ref.refname, -arg, atom->name); + if (refname_atom_parser_internal(&atom->u.remote_ref.refname, +arg, atom->name, err)) { + string_list_clear(¶ms, 0); + return -1; + } } } string_list_clear(¶ms, 0); + return 0; } -static void body_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg) +static int body_atom_parser(const struct ref_format *format, struct used_atom *atom, + c
[PATCH v6 6/6] ref-filter: libify get_ref_atom_value()
Finish removing die() calls from ref-filter formatting logic, so that it could be used by other commands. Change the signature of get_ref_atom_value() and underlying functions by adding return value and strbuf parameter for error message. Return value equals 0 upon success and -1 upon failure. Upon failure, error message is appended to the strbuf. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 54 ++ 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 3f85ef64267d9..3bc65e49358ee 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1427,28 +1427,30 @@ static const char *get_refname(struct used_atom *atom, struct ref_array_item *re return show_ref(&atom->u.refname, ref->refname); } -static void get_object(struct ref_array_item *ref, const struct object_id *oid, - int deref, struct object **obj) +static int get_object(struct ref_array_item *ref, const struct object_id *oid, + int deref, struct object **obj, struct strbuf *err) { int eaten; + int ret = 0; unsigned long size; void *buf = get_obj(oid, obj, &size, &eaten); if (!buf) - die(_("missing object %s for %s"), - oid_to_hex(oid), ref->refname); - if (!*obj) - die(_("parse_object_buffer failed on %s for %s"), - oid_to_hex(oid), ref->refname); - - grab_values(ref->value, deref, *obj, buf, size); + ret = strbuf_addf_ret(err, -1, _("missing object %s for %s"), + oid_to_hex(oid), ref->refname); + else if (!*obj) + ret = strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"), + oid_to_hex(oid), ref->refname); + else + grab_values(ref->value, deref, *obj, buf, size); if (!eaten) free(buf); + return ret; } /* * Parse the object referred by ref, and grab needed value. */ -static void populate_value(struct ref_array_item *ref) +static int populate_value(struct ref_array_item *ref, struct strbuf *err) { struct object *obj; int i; @@ -1570,16 +1572,17 @@ static void populate_value(struct ref_array_item *ref) break; } if (used_atom_cnt <= i) - return; + return 0; - get_object(ref, &ref->objectname, 0, &obj); + if (get_object(ref, &ref->objectname, 0, &obj, err)) + return -1; /* * If there is no atom that wants to know about tagged * object, we are done. */ if (!need_tagged || (obj->type != OBJ_TAG)) - return; + return 0; /* * If it is a tag object, see if we use a value that derefs @@ -1593,20 +1596,23 @@ static void populate_value(struct ref_array_item *ref) * is not consistent with what deref_tag() does * which peels the onion to the core. */ - get_object(ref, tagged, 1, &obj); + return get_object(ref, tagged, 1, &obj, err); } /* * Given a ref, return the value for the atom. This lazily gets value * out of the object by calling populate value. */ -static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom_value **v) +static int get_ref_atom_value(struct ref_array_item *ref, int atom, + struct atom_value **v, struct strbuf *err) { if (!ref->value) { - populate_value(ref); + if (populate_value(ref, err)) + return -1; fill_missing_values(ref->value); } *v = &ref->value[atom]; + return 0; } /* @@ -2130,9 +2136,13 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru int cmp; cmp_type cmp_type = used_atom[s->atom].type; int (*cmp_fn)(const char *, const char *); + struct strbuf err = STRBUF_INIT; - get_ref_atom_value(a, s->atom, &va); - get_ref_atom_value(b, s->atom, &vb); + if (get_ref_atom_value(a, s->atom, &va, &err)) + die("%s", err.buf); + if (get_ref_atom_value(b, s->atom, &vb, &err)) + die("%s", err.buf); + strbuf_release(&err); cmp_fn = s->ignore_case ? strcasecmp : strcmp; if (s->version) cmp = versioncmp(va->s, vb->s); @@ -2210,12 +2220,8 @@ int format_ref_array_item(struct ref_array_item *info, if (cp < sp) append_literal(cp, sp, &state); pos = parse_ref_filter_atom(format, sp + 2, ep, error_buf); - if (pos < 0) { - pop_stack_element(&state.stack); - return -1; - } - get_ref_atom_value(info, pos, &atomv); - if (atom
[PATCH v6 3/6] ref-filter: add return value && strbuf to handlers
Continue removing die() calls from ref-filter formatting logic, so that it could be used by other commands. Change the signature of handlers by adding return value and strbuf parameter for errors. Return value equals 0 upon success and -1 upon failure. Upon failure, error message is appended to the strbuf. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 51 +++ 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 9833709dbefe3..a18c86961f08c 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -400,7 +400,8 @@ struct ref_formatting_state { struct atom_value { const char *s; - void (*handler)(struct atom_value *atomv, struct ref_formatting_state *state); + int (*handler)(struct atom_value *atomv, struct ref_formatting_state *state, + struct strbuf *err); uintmax_t value; /* used for sorting when not FIELD_STR */ struct used_atom *atom; }; @@ -494,7 +495,8 @@ static void quote_formatting(struct strbuf *s, const char *str, int quote_style) } } -static void append_atom(struct atom_value *v, struct ref_formatting_state *state) +static int append_atom(struct atom_value *v, struct ref_formatting_state *state, + struct strbuf *unused_err) { /* * Quote formatting is only done when the stack has a single @@ -506,6 +508,7 @@ static void append_atom(struct atom_value *v, struct ref_formatting_state *state quote_formatting(&state->stack->output, v->s, state->quote_style); else strbuf_addstr(&state->stack->output, v->s); + return 0; } static void push_stack_element(struct ref_formatting_stack **stack) @@ -540,7 +543,8 @@ static void end_align_handler(struct ref_formatting_stack **stack) strbuf_release(&s); } -static void align_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state) +static int align_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, + struct strbuf *unused_err) { struct ref_formatting_stack *new_stack; @@ -548,6 +552,7 @@ static void align_atom_handler(struct atom_value *atomv, struct ref_formatting_s new_stack = state->stack; new_stack->at_end = end_align_handler; new_stack->at_end_data = &atomv->atom->u.align; + return 0; } static void if_then_else_handler(struct ref_formatting_stack **stack) @@ -585,7 +590,8 @@ static void if_then_else_handler(struct ref_formatting_stack **stack) free(if_then_else); } -static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state) +static int if_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, + struct strbuf *unused_err) { struct ref_formatting_stack *new_stack; struct if_then_else *if_then_else = xcalloc(sizeof(struct if_then_else), 1); @@ -597,6 +603,7 @@ static void if_atom_handler(struct atom_value *atomv, struct ref_formatting_stat new_stack = state->stack; new_stack->at_end = if_then_else_handler; new_stack->at_end_data = if_then_else; + return 0; } static int is_empty(const char *s) @@ -609,7 +616,8 @@ static int is_empty(const char *s) return 1; } -static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state) +static int then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, +struct strbuf *err) { struct ref_formatting_stack *cur = state->stack; struct if_then_else *if_then_else = NULL; @@ -617,11 +625,11 @@ static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_st if (cur->at_end == if_then_else_handler) if_then_else = (struct if_then_else *)cur->at_end_data; if (!if_then_else) - die(_("format: %%(then) atom used without an %%(if) atom")); + return strbuf_addf_ret(err, -1, _("format: %%(then) atom used without an %%(if) atom")); if (if_then_else->then_atom_seen) - die(_("format: %%(then) atom used more than once")); + return strbuf_addf_ret(err, -1, _("format: %%(then) atom used more than once")); if (if_then_else->else_atom_seen) - die(_("format: %%(then) atom used after %%(else)")); + return strbuf_addf_ret(err, -1, _("format: %%(then) atom used after %%(else)")); if_then_else->then_atom_seen = 1; /* * If the 'equals' or 'notequals' attribute is used then @@ -637,9 +645,11 @@ static void then_atom_handler(struct atom_value *atomv, struct ref_formatting_st } else if (cur->output.len && !is_empty(cur->output.buf)) if_then_else->condition_satisfied = 1; strbuf_reset(&cur->output); + return 0; } -st
[PATCH v6 2/6] ref-filter: start adding strbufs with errors
This is a first step in removing die() calls from ref-filter formatting logic, so that it could be used by other commands that do not want to die during formatting process. die() calls related to bugs in code will not be touched in this patch. Everything would be the same for show_ref_array_item() users. But, if you want to deal with errors by your own, you could invoke format_ref_array_item(). It means that you need to print everything (the result and errors) on your side. This commit changes signature of format_ref_array_item() by adding return value and strbuf parameter for errors, and adjusts its callers. While at it, reduce the scope of the out-variable. Signed-off-by: Olga Telezhnaia --- builtin/branch.c | 7 +-- ref-filter.c | 17 - ref-filter.h | 7 --- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/builtin/branch.c b/builtin/branch.c index 6d0cea9d4bcc4..c21e5a04a0177 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -391,7 +391,6 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin struct ref_array array; int maxwidth = 0; const char *remote_prefix = ""; - struct strbuf out = STRBUF_INIT; char *to_free = NULL; /* @@ -419,7 +418,10 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin ref_array_sort(sorting, &array); for (i = 0; i < array.nr; i++) { - format_ref_array_item(array.items[i], format, &out); + struct strbuf out = STRBUF_INIT; + struct strbuf err = STRBUF_INIT; + if (format_ref_array_item(array.items[i], format, &out, &err)) + die("%s", err.buf); if (column_active(colopts)) { assert(!filter->verbose && "--column and --verbose are incompatible"); /* format to a string_list to let print_columns() do its job */ @@ -428,6 +430,7 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin fwrite(out.buf, 1, out.len, stdout); putchar('\n'); } + strbuf_release(&err); strbuf_release(&out); } diff --git a/ref-filter.c b/ref-filter.c index 0c8d1589cf316..9833709dbefe3 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2131,9 +2131,10 @@ static void append_literal(const char *cp, const char *ep, struct ref_formatting } } -void format_ref_array_item(struct ref_array_item *info, +int format_ref_array_item(struct ref_array_item *info, const struct ref_format *format, - struct strbuf *final_buf) + struct strbuf *final_buf, + struct strbuf *error_buf) { const char *cp, *sp, *ep; struct ref_formatting_state state = REF_FORMATTING_STATE_INIT; @@ -2161,19 +2162,25 @@ void format_ref_array_item(struct ref_array_item *info, resetv.s = GIT_COLOR_RESET; append_atom(&resetv, &state); } - if (state.stack->prev) - die(_("format: %%(end) atom missing")); + if (state.stack->prev) { + pop_stack_element(&state.stack); + return strbuf_addf_ret(error_buf, -1, _("format: %%(end) atom missing")); + } strbuf_addbuf(final_buf, &state.stack->output); pop_stack_element(&state.stack); + return 0; } void show_ref_array_item(struct ref_array_item *info, const struct ref_format *format) { struct strbuf final_buf = STRBUF_INIT; + struct strbuf error_buf = STRBUF_INIT; - format_ref_array_item(info, format, &final_buf); + if (format_ref_array_item(info, format, &final_buf, &error_buf)) + die("%s", error_buf.buf); fwrite(final_buf.buf, 1, final_buf.len, stdout); + strbuf_release(&error_buf); strbuf_release(&final_buf); putchar('\n'); } diff --git a/ref-filter.h b/ref-filter.h index 0d98342b34319..e13f8e6f8721a 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -110,9 +110,10 @@ int verify_ref_format(struct ref_format *format); /* Sort the given ref_array as per the ref_sorting provided */ void ref_array_sort(struct ref_sorting *sort, struct ref_array *array); /* Based on the given format and quote_style, fill the strbuf */ -void format_ref_array_item(struct ref_array_item *info, - const struct ref_format *format, - struct strbuf *final_buf); +int format_ref_array_item(struct ref_array_item *info, + const struct ref_format *format, + struct strbuf *final_buf, + struct strbuf *error_buf); /* Print the ref using the given format and quote_style */ void show_ref_array_item(struct ref_array_item *info, cons
[PATCH v6 4/6] ref-filter: change parsing function error handling
Continue removing die() calls from ref-filter formatting logic, so that it could be used by other commands. Change the signature of parse_ref_filter_atom() by adding strbuf parameter for error message. The function returns the position in the used_atom[] array (as before) for the given atom, or -1 to signal an error. Upon failure, error message is appended to the strbuf. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 32 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index a18c86961f08c..93fa6b4e5e63d 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -410,7 +410,8 @@ struct atom_value { * Used to parse format string and sort specifiers */ static int parse_ref_filter_atom(const struct ref_format *format, -const char *atom, const char *ep) +const char *atom, const char *ep, +struct strbuf *err) { const char *sp; const char *arg; @@ -420,7 +421,8 @@ static int parse_ref_filter_atom(const struct ref_format *format, if (*sp == '*' && sp < ep) sp++; /* deref */ if (ep <= sp) - die(_("malformed field name: %.*s"), (int)(ep-atom), atom); + return strbuf_addf_ret(err, -1, _("malformed field name: %.*s"), + (int)(ep-atom), atom); /* Do we have the atom already used elsewhere? */ for (i = 0; i < used_atom_cnt; i++) { @@ -446,7 +448,8 @@ static int parse_ref_filter_atom(const struct ref_format *format, } if (ARRAY_SIZE(valid_atom) <= i) - die(_("unknown field name: %.*s"), (int)(ep-atom), atom); + return strbuf_addf_ret(err, -1, _("unknown field name: %.*s"), + (int)(ep-atom), atom); /* Add it in, including the deref prefix */ at = used_atom_cnt; @@ -728,17 +731,21 @@ int verify_ref_format(struct ref_format *format) format->need_color_reset_at_eol = 0; for (cp = format->format; *cp && (sp = find_next(cp)); ) { + struct strbuf err = STRBUF_INIT; const char *color, *ep = strchr(sp, ')'); int at; if (!ep) return error(_("malformed format string %s"), sp); /* sp points at "%(" and ep points at the closing ")" */ - at = parse_ref_filter_atom(format, sp + 2, ep); + at = parse_ref_filter_atom(format, sp + 2, ep, &err); + if (at < 0) + die("%s", err.buf); cp = ep + 1; if (skip_prefix(used_atom[at].name, "color:", &color)) format->need_color_reset_at_eol = !!strcmp(color, "reset"); + strbuf_release(&err); } if (format->need_color_reset_at_eol && !want_color(format->use_color)) format->need_color_reset_at_eol = 0; @@ -2157,13 +2164,17 @@ int format_ref_array_item(struct ref_array_item *info, for (cp = format->format; *cp && (sp = find_next(cp)); cp = ep + 1) { struct atom_value *atomv; + int pos; ep = strchr(sp, ')'); if (cp < sp) append_literal(cp, sp, &state); - get_ref_atom_value(info, - parse_ref_filter_atom(format, sp + 2, ep), - &atomv); + pos = parse_ref_filter_atom(format, sp + 2, ep, error_buf); + if (pos < 0) { + pop_stack_element(&state.stack); + return -1; + } + get_ref_atom_value(info, pos, &atomv); if (atomv->handler(atomv, &state, error_buf)) { pop_stack_element(&state.stack); return -1; @@ -,7 +2233,12 @@ static int parse_sorting_atom(const char *atom) */ struct ref_format dummy = REF_FORMAT_INIT; const char *end = atom + strlen(atom); - return parse_ref_filter_atom(&dummy, atom, end); + struct strbuf err = STRBUF_INIT; + int res = parse_ref_filter_atom(&dummy, atom, end, &err); + if (res < 0) + die("%s", err.buf); + strbuf_release(&err); + return res; } /* If no sorting option is given, use refname to sort as default */ -- https://github.com/git/git/pull/466
[PATCH v6 1/6] ref-filter: add shortcut to work with strbufs
Add function strbuf_addf_ret() that helps to save a few lines of code. Function expands fmt with placeholders, append resulting message to strbuf *sb, and return error code ret. Signed-off-by: Olga Telezhnaia --- ref-filter.c | 13 + 1 file changed, 13 insertions(+) diff --git a/ref-filter.c b/ref-filter.c index 45fc56216aaa8..0c8d1589cf316 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -101,6 +101,19 @@ static struct used_atom { } *used_atom; static int used_atom_cnt, need_tagged, need_symref; +/* + * Expand string, append it to strbuf *sb, then return error code ret. + * Allow to save few lines of code. + */ +static int strbuf_addf_ret(struct strbuf *sb, int ret, const char *fmt, ...) +{ + va_list ap; + va_start(ap, fmt); + strbuf_vaddf(sb, fmt, ap); + va_end(ap); + return ret; +} + static void color_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *color_value) { if (!color_value) -- https://github.com/git/git/pull/466
[PATCH Outreachy 1/2] format: create pretty.h file
Create header for pretty.c to make formatting interface more structured. This is a middle point, this file would be merged futher with other files which contain formatting stuff. Signed-off-by: Olga Telezhnaia Mentored-by: Christian Couder Mentored by: Jeff King --- archive.c | 1 + builtin/notes.c | 2 +- builtin/reset.c | 2 +- builtin/show-branch.c | 2 +- combine-diff.c| 1 + commit.c | 1 + commit.h | 80 -- diffcore-pickaxe.c| 1 + grep.c| 1 + log-tree.c| 1 + notes-cache.c | 1 + pretty.h | 87 +++ revision.h| 2 +- sequencer.c | 1 + sha1_name.c | 1 + submodule.c | 1 + 16 files changed, 101 insertions(+), 84 deletions(-) create mode 100644 pretty.h diff --git a/archive.c b/archive.c index 0b7b62af0c3ec..60607e8c00857 100644 --- a/archive.c +++ b/archive.c @@ -2,6 +2,7 @@ #include "config.h" #include "refs.h" #include "commit.h" +#include "pretty.h" #include "tree-walk.h" #include "attr.h" #include "archive.h" diff --git a/builtin/notes.c b/builtin/notes.c index 1a2c7d92ad7e7..7c8176164561b 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -12,7 +12,7 @@ #include "builtin.h" #include "notes.h" #include "blob.h" -#include "commit.h" +#include "pretty.h" #include "refs.h" #include "exec_cmd.h" #include "run-command.h" diff --git a/builtin/reset.c b/builtin/reset.c index 906e541658230..e15f595799c40 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -12,7 +12,7 @@ #include "lockfile.h" #include "tag.h" #include "object.h" -#include "commit.h" +#include "pretty.h" #include "run-command.h" #include "refs.h" #include "diff.h" diff --git a/builtin/show-branch.c b/builtin/show-branch.c index 2e24b5c330e8e..e8a4aa40cb4b6 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -1,6 +1,6 @@ #include "cache.h" #include "config.h" -#include "commit.h" +#include "pretty.h" #include "refs.h" #include "builtin.h" #include "color.h" diff --git a/combine-diff.c b/combine-diff.c index 2505de119a2be..01ba1b03a06d2 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -1,5 +1,6 @@ #include "cache.h" #include "commit.h" +#include "pretty.h" #include "blob.h" #include "diff.h" #include "diffcore.h" diff --git a/commit.c b/commit.c index cab8d4455bdbd..ac17a27a4ab0a 100644 --- a/commit.c +++ b/commit.c @@ -1,6 +1,7 @@ #include "cache.h" #include "tag.h" #include "commit.h" +#include "pretty.h" #include "pkt-line.h" #include "utf8.h" #include "diff.h" diff --git a/commit.h b/commit.h index 99a3fea68d3f6..41a2067809444 100644 --- a/commit.h +++ b/commit.h @@ -121,93 +121,13 @@ struct commit_list *copy_commit_list(struct commit_list *list); void free_commit_list(struct commit_list *list); -/* Commit formats */ -enum cmit_fmt { - CMIT_FMT_RAW, - CMIT_FMT_MEDIUM, - CMIT_FMT_DEFAULT = CMIT_FMT_MEDIUM, - CMIT_FMT_SHORT, - CMIT_FMT_FULL, - CMIT_FMT_FULLER, - CMIT_FMT_ONELINE, - CMIT_FMT_EMAIL, - CMIT_FMT_MBOXRD, - CMIT_FMT_USERFORMAT, - - CMIT_FMT_UNSPECIFIED -}; - -static inline int cmit_fmt_is_mail(enum cmit_fmt fmt) -{ - return (fmt == CMIT_FMT_EMAIL || fmt == CMIT_FMT_MBOXRD); -} - struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */ -struct pretty_print_context { - /* -* Callers should tweak these to change the behavior of pp_* functions. -*/ - enum cmit_fmt fmt; - int abbrev; - const char *after_subject; - int preserve_subject; - struct date_mode date_mode; - unsigned date_mode_explicit:1; - int print_email_subject; - int expand_tabs_in_log; - int need_8bit_cte; - char *notes_message; - struct reflog_walk_info *reflog_info; - struct rev_info *rev; - const char *output_encoding; - struct string_list *mailmap; - int color; - struct ident_split *from_ident; - - /* -* Fields below here are manipulated internally by pp_* functions and -* should not be counted on by callers. -*/ - struct string_list in_body_headers; - int graph_width; -}; - -struct userformat_want { - unsigned notes:1; -}; - extern int has_non_ascii(const char *text); extern const char *logmsg_reencode(const struct commit *commit, char **commit_encoding, const char *output_encoding); -extern void get_commit_format(const char *arg, struct rev_info *); -extern const char *format_subject(struct strbuf *sb, const char *msg, - const char *line_separator); -extern void userformat_find_requirements(const char *fmt, struct userformat_want *w); -extern int commit_format_is_empty(enum cmit_fmt); ex