From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-12 11:47:09
From: ZheNing Hu <redacted>
There are two "if (opt->all_objects)" blocks next
to each other, merge them into one to provide better
readability.
Helped-by: Jeff King [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
Acked-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
builtin/cat-file.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
@@ -520,14 +520,11 @@ static int batch_objects(struct batch_options *opt)data.info.typep=&data.type;if(opt->all_objects){+structobject_cb_datacb;structobject_infoempty=OBJECT_INFO_INIT;if(!memcmp(&data.info,&empty,sizeof(empty)))data.skip_object_info=1;-}--if(opt->all_objects){-structobject_cb_datacb;if(has_promisor_remote())warning("This repository uses promisor remotes. Some objects may not be loaded.");
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-12 11:47:09
From: ZheNing Hu <redacted>
The --batch code to print an object assumes we found out the type of
the object from calling oid_object_info_extended(). This is true for
the default format, but even in a custom format, we manually modify
the object_info struct to ask for the type.
This assumption was broken by 845de33a5b (cat-file: avoid noop calls
to sha1_object_info_extended, 2016-05-18). That commit skips the call
to oid_object_info_extended() entirely when --batch-all-objects is in
use, and the custom format does not include any placeholders that
require calling it.
Or when the custom format only include placeholders like %(objectname) or
%(rest), oid_object_info_extended() will not get the type of the object.
This results in an error when we try to confirm that the type didn't
change:
$ git cat-file --batch=batman --batch-all-objects
batman
fatal: object 000023961a0c02d6e21dc51ea3484ff71abf1c74 changed type!?
and also has other subtle effects (e.g., we'd fail to stream a blob,
since we don't realize it's a blob in the first place).
We can fix this by flipping the order of the setup. The check for "do
we need to get the object info" must come _after_ we've decided
whether we need to look up the type.
Helped-by: Jeff King [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
Acked-by: Jeff King <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
builtin/cat-file.c | 13 +++++++------
t/t1006-cat-file.sh | 22 ++++++++++++++++++++++
2 files changed, 29 insertions(+), 6 deletions(-)
@@ -586,4 +586,26 @@ test_expect_success 'cat-file --unordered works' 'test_cmpexpectactual'+test_expect_success'set up object list for --batch-all-objects tests''+git-Call-twocat-file--batch-all-objects--batch-check="%(objectname)">objects+'++test_expect_success'cat-file --batch="%(objectname)" with --batch-all-objects will work''+git-Call-twocat-file--batch="%(objectname)"<objects>expect&&+git-Call-twocat-file--batch-all-objects--batch="%(objectname)">actual&&+cmpexpectactual+'++test_expect_success'cat-file --batch="%(rest)" with --batch-all-objects will work''+git-Call-twocat-file--batch="%(rest)"<objects>expect&&+git-Call-twocat-file--batch-all-objects--batch="%(rest)">actual&&+cmpexpectactual+'++test_expect_success'cat-file --batch="batman" with --batch-all-objects will work''+git-Call-twocat-file--batch="batman"<objects>expect&&+git-Call-twocat-file--batch-all-objects--batch="batman">actual&&+cmpexpectactual+'+ test_done
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-12 11:47:09
From: ZheNing Hu <redacted>
Only tag and commit objects use `grab_sub_body_contents()` to grab
object contents in the current codebase. We want to teach the
function to also handle blobs and trees to get their raw data,
without parsing a blob (whose contents looks like a commit or a tag)
incorrectly as a commit or a tag.
Skip the block of code that is specific to handling commits and tags
early when the given object is of a wrong type to help later
addition to handle other types of objects in this function.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Helped-by: Junio C Hamano [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
ref-filter.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
@@ -1678,7 +1684,7 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **objreturnstrbuf_addf_ret(err,-1,_("parse_object_buffer failed on %s for %s"),oid_to_hex(&oi->oid),ref->refname);}-grab_values(ref->value,deref,*obj,oi->content);+grab_values(ref->value,deref,*obj,oi);}grab_common_values(ref->value,deref,oi);
@@ -559,6 +560,15 @@ static int if_atom_parser(struct ref_format *format, struct used_atom *atom,return0;}+staticintrest_atom_parser(structref_format*format,structused_atom*atom,+constchar*arg,structstrbuf*err)+{+if(arg)+returnstrbuf_addf_ret(err,-1,_("%%(rest) does not take arguments"));+format->use_rest=1;+return0;+}+staticinthead_atom_parser(structref_format*format,structused_atom*atom,constchar*arg,structstrbuf*unused_err){
@@ -1998,6 +1998,10 @@ test_expect_success '--format should list tags as per format given' 'test_cmpexpectactual'+test_expect_success'git tag -l with --format="%(rest)" must fail''+test_must_failgittag-l--format="%(rest)""v1*"+'+ test_expect_success"set up color tests"'echo"<RED>v1.0<RESET>">expect.color&&echo"v1.0">expect.bare&&
@@ -194,6 +194,10 @@ test_expect_success GPG 'verifying tag with --format' 'test_cmpexpectactual'+test_expect_successGPG'verifying tag with --format="%(rest)" must fail''+test_must_failgitverify-tag--format="%(rest)""fourth-signed"+'+ test_expect_successGPG'verifying a forged tag with --format should fail silently''test_must_failgitverify-tag--format="tagname : %(tag)"$(catforged1.tag)>actual-forged&&test_must_be_emptyactual-forged
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-12 11:47:12
From: ZheNing Hu <redacted>
Because in the refactor of `git cat-file --batch` later,
oid_object_info_extended() in get_object() will be used to obtain
the info of an object with it's oid. When the object cannot be
obtained in the git repository, `cat-file --batch` expects to output
"<oid> missing" and continue the next oid query instead of letting
Git exit. In other error conditions, Git should exit normally. So we
can achieve this function by passing the return value of get_object().
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Helped-by: Ævar Arnfjörð Bjarmason [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
ref-filter.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-12 11:47:12
From: ZheNing Hu <redacted>
Use non-const ref_format in *_atom_parser(), which can help us
modify the members of ref_format in *_atom_parser().
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
builtin/tag.c | 2 +-
ref-filter.c | 44 ++++++++++++++++++++++----------------------
ref-filter.h | 4 ++--
3 files changed, 25 insertions(+), 25 deletions(-)
@@ -116,7 +116,7 @@ void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);voidref_sorting_set_sort_flags_all(structref_sorting*sorting,unsignedintmask,inton);/* Based on the given format and quote_style, fill the strbuf */intformat_ref_array_item(structref_array_item*info,-conststructref_format*format,+structref_format*format,structstrbuf*final_buf,structstrbuf*error_buf);/* Parse a single sort specifier and add it to the list */
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-12 11:47:14
From: ZheNing Hu <redacted>
Add new formatting option `%(raw)`, which will print the raw
object data without any changes. It will help further to migrate
all cat-file formatting logic from cat-file to ref-filter.
The raw data of blob, tree objects may contain '\0', but most of
the logic in `ref-filter` depends on the output of the atom being
text (specifically, no embedded NULs in it).
E.g. `quote_formatting()` use `strbuf_addstr()` or `*._quote_buf()`
add the data to the buffer. The raw data of a tree object is
`100644 one\0...`, only the `100644 one` will be added to the buffer,
which is incorrect.
Therefore, we need to find a way to record the length of the
atom_value's member `s`. Although strbuf can already record the
string and its length, if we want to replace the type of atom_value's
member `s` with strbuf, many places in ref-filter that are filled
with dynamically allocated mermory in `v->s` are not easy to replace.
At the same time, we need to check if `v->s == NULL` in
populate_value(), and strbuf cannot easily distinguish NULL and empty
strings, but c-style "const char *" can do it. So add a new member in
`struct atom_value`: `s_size`, which can record raw object size, it
can help us add raw object data to the buffer or compare two buffers
which contain raw object data.
Note that `--format=%(raw)` cannot be used with `--python`, `--shell`,
`--tcl`, and `--perl` because if the binary raw data is passed to a
variable in such languages, these may not support arbitrary binary data
in their string variable type.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Helped-by: Bagas Sanjaya [off-list ref]
Helped-by: Ævar Arnfjörð Bjarmason [off-list ref]
Helped-by: Felipe Contreras [off-list ref]
Helped-by: Phillip Wood [off-list ref]
Helped-by: Junio C Hamano [off-list ref]
Based-on-patch-by: Olga Telezhnaya [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
Documentation/git-for-each-ref.txt | 9 ++
ref-filter.c | 140 +++++++++++++++----
t/t6300-for-each-ref.sh | 216 +++++++++++++++++++++++++++++
3 files changed, 338 insertions(+), 27 deletions(-)
@@ -235,6 +235,15 @@ and `date` to extract the named component. For email fields (`authoremail`, without angle brackets, and `:localpart` to get the part before the `@` symbol out of the trimmed email.+The raw data in an object is `raw`.++raw:size::+ The raw data size of the object.++Note that `--format=%(raw)` can not be used with `--python`, `--shell`, `--tcl`,+`--perl` because such language may not support arbitrary binary data in their+string variable type.+ The message in a commit or a tag object is `contents`, from which `contents:<part>` can be used to extract various parts out of:
@@ -620,12 +637,15 @@ struct ref_formatting_state {structatom_value{constchar*s;+size_ts_size;int(*handler)(structatom_value*atomv,structref_formatting_state*state,structstrbuf*err);uintmax_tvalue;/* used for sorting when not FIELD_STR */structused_atom*atom;};+#define ATOM_VALUE_S_SIZE_INIT (-1)+/**Usedtoparseformatstringandsortspecifiers*/
@@ -644,13 +664,6 @@ static int parse_ref_filter_atom(const struct ref_format *format,returnstrbuf_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++){-intlen=strlen(used_atom[i].name);-if(len==ep-atom&&!memcmp(used_atom[i].name,atom,len))-returni;-}-/**Iftheatomnamehasacolon,stripitandeverythingafter*itoff-itspecifiestheformatforthisentry,and
@@ -660,6 +673,13 @@ static int parse_ref_filter_atom(const struct ref_format *format,arg=memchr(sp,':',ep-sp);atom_len=(arg?arg:ep)-sp;+/* Do we have the atom already used elsewhere? */+for(i=0;i<used_atom_cnt;i++){+intlen=strlen(used_atom[i].name);+if(len==ep-atom&&!memcmp(used_atom[i].name,atom,len))+returni;+}+/* Is the atom a valid one? */for(i=0;i<ARRAY_SIZE(valid_atom);i++){intlen=strlen(valid_atom[i].name);
@@ -709,11 +729,14 @@ static int parse_ref_filter_atom(const struct ref_format *format,returnat;}-staticvoidquote_formatting(structstrbuf*s,constchar*str,intquote_style)+staticvoidquote_formatting(structstrbuf*s,constchar*str,size_tlen,intquote_style){switch(quote_style){caseQUOTE_NONE:-strbuf_addstr(s,str);+if(len!=ATOM_VALUE_S_SIZE_INIT)+strbuf_add(s,str,len);+else+strbuf_addstr(s,str);break;caseQUOTE_SHELL:sq_quote_buf(s,str);
@@ -867,18 +895,22 @@ static int then_atom_handler(struct atom_value *atomv, struct ref_formatting_staif(if_then_else->else_atom_seen)returnstrbuf_addf_ret(err,-1,_("format: %%(then) atom used after %%(else)"));if_then_else->then_atom_seen=1;+if(if_then_else->str)+str_len=strlen(if_then_else->str);/**Ifthe'equals'or'notequals'attributeisusedthen*performtherequiredcomparison.Ifnot,onlynon-empty*stringssatisfythe'if'condition.*/if(if_then_else->cmp_status==COMPARE_EQUAL){-if(!strcmp(if_then_else->str,cur->output.buf))+if(str_len==cur->output.len&&+!memcmp(if_then_else->str,cur->output.buf,cur->output.len))if_then_else->condition_satisfied=1;}elseif(if_then_else->cmp_status==COMPARE_UNEQUAL){-if(strcmp(if_then_else->str,cur->output.buf))+if(str_len!=cur->output.len||+memcmp(if_then_else->str,cur->output.buf,cur->output.len))if_then_else->condition_satisfied=1;-}elseif(cur->output.len&&!is_empty(cur->output.buf))+}elseif(cur->output.len&&!is_empty(&cur->output))if_then_else->condition_satisfied=1;strbuf_reset(&cur->output);return0;
@@ -974,6 +1006,10 @@ int verify_ref_format(struct ref_format *format)at=parse_ref_filter_atom(format,sp+2,ep,&err);if(at<0)die("%s",err.buf);+if(format->quote_style&&used_atom[at].atom_type==ATOM_RAW&&+used_atom[at].u.raw_data.option==RAW_BARE)+die(_("--format=%.*s cannot be used with"+"--python, --shell, --tcl, --perl"),(int)(ep-sp-2),sp+2);cp=ep+1;if(skip_prefix(used_atom[at].name,"color:",&color))
@@ -708,6 +737,15 @@ test_atom refs/tags/signed-long contents "subject line bodycontents$sig"+test_expect_successGPG'basic atom: refs/tags/signed-long raw''+gitcat-filetagrefs/tags/signed-long>expected&&+gitfor-each-ref--format="%(raw)"refs/tags/signed-long>actual&&+sanitize_pgp<expected>expected.clean&&+echo>>expected.clean&&+sanitize_pgp<actual>actual.clean&&+test_cmpexpected.cleanactual.clean+'+ test_expect_success'set up refs pointing to tree and blob''gitupdate-refrefs/mytrees/firstrefs/heads/main^{tree}&&gitupdate-refrefs/myblobs/firstrefs/heads/main:one
@@ -727,6 +775,174 @@ test_atom refs/myblobs/first contents:body "" test_atomrefs/myblobs/firstcontents:signature"" test_atomrefs/myblobs/firstcontents""+test_expect_success'basic atom: refs/myblobs/first raw''+gitcat-fileblobrefs/myblobs/first>expected&&+echo>>expected&&+gitfor-each-ref--format="%(raw)"refs/myblobs/first>actual&&+test_cmpexpectedactual&&+gitcat-file-srefs/myblobs/first>expected&&+gitfor-each-ref--format="%(raw:size)"refs/myblobs/first>actual&&+test_cmpexpectedactual+'++test_expect_success'set up refs pointing to binary blob''+printf"a\0b\0c">blob1&&+printf"a\0c\0b">blob2&&+printf"\0a\0b\0c">blob3&&+printf"abc">blob4&&+printf"\0 \0 \0 ">blob5&&+printf"\0 \0a\0 ">blob6&&+printf" ">blob7&&+>blob8&&+obj=$(githash-object-wblob1)&&+gitupdate-refrefs/myblobs/blob1"$obj"&&+obj=$(githash-object-wblob2)&&+gitupdate-refrefs/myblobs/blob2"$obj"&&+obj=$(githash-object-wblob3)&&+gitupdate-refrefs/myblobs/blob3"$obj"&&+obj=$(githash-object-wblob4)&&+gitupdate-refrefs/myblobs/blob4"$obj"&&+obj=$(githash-object-wblob5)&&+gitupdate-refrefs/myblobs/blob5"$obj"&&+obj=$(githash-object-wblob6)&&+gitupdate-refrefs/myblobs/blob6"$obj"&&+obj=$(githash-object-wblob7)&&+gitupdate-refrefs/myblobs/blob7"$obj"&&+obj=$(githash-object-wblob8)&&+gitupdate-refrefs/myblobs/blob8"$obj"+'++test_expect_success'Verify sorts with raw''+cat>expected<<-EOF&&+refs/myblobs/blob8+refs/myblobs/blob5+refs/myblobs/blob6+refs/myblobs/blob3+refs/myblobs/blob7+refs/mytrees/first+refs/myblobs/first+refs/myblobs/blob1+refs/myblobs/blob2+refs/myblobs/blob4+refs/heads/main+EOF+gitfor-each-ref--format="%(refname)"--sort=raw\+refs/heads/mainrefs/myblobs/refs/mytrees/first>actual&&+test_cmpexpectedactual+'++test_expect_success'Verify sorts with raw:size''+cat>expected<<-EOF&&+refs/myblobs/blob8+refs/myblobs/first+refs/myblobs/blob7+refs/heads/main+refs/myblobs/blob4+refs/myblobs/blob1+refs/myblobs/blob2+refs/myblobs/blob3+refs/myblobs/blob5+refs/myblobs/blob6+refs/mytrees/first+EOF+gitfor-each-ref--format="%(refname)"--sort=raw:size\+refs/heads/mainrefs/myblobs/refs/mytrees/first>actual&&+test_cmpexpectedactual+'++test_expect_success'validate raw atom with %(if:equals)''+cat>expected<<-EOF&&+notequals+notequals+notequals+notequals+notequals+notequals+refs/myblobs/blob4+notequals+notequals+notequals+notequals+notequals+EOF+gitfor-each-ref--format="%(if:equals=abc)%(raw)%(then)%(refname)%(else)not equals%(end)"\+refs/myblobs/refs/heads/>actual&&+test_cmpexpectedactual+'++test_expect_success'validate raw atom with %(if:notequals)''+cat>expected<<-EOF&&+refs/heads/ambiguous+refs/heads/main+refs/heads/newtag+refs/myblobs/blob1+refs/myblobs/blob2+refs/myblobs/blob3+equals+refs/myblobs/blob5+refs/myblobs/blob6+refs/myblobs/blob7+refs/myblobs/blob8+refs/myblobs/first+EOF+gitfor-each-ref--format="%(if:notequals=abc)%(raw)%(then)%(refname)%(else)equals%(end)"\+refs/myblobs/refs/heads/>actual&&+test_cmpexpectedactual+'++test_expect_success'empty raw refs with %(if)''+cat>expected<<-EOF&&+refs/myblobs/blob1notempty+refs/myblobs/blob2notempty+refs/myblobs/blob3notempty+refs/myblobs/blob4notempty+refs/myblobs/blob5notempty+refs/myblobs/blob6notempty+refs/myblobs/blob7empty+refs/myblobs/blob8empty+refs/myblobs/firstnotempty+EOF+gitfor-each-ref--format="%(refname) %(if)%(raw)%(then)not empty%(else)empty%(end)"\+refs/myblobs/>actual&&+test_cmpexpectedactual+'++test_expect_success'%(raw) with --python must fail''+test_must_failgitfor-each-ref--format="%(raw)"--python+'++test_expect_success'%(raw) with --tcl must fail''+test_must_failgitfor-each-ref--format="%(raw)"--tcl+'++test_expect_success'%(raw) with --perl must fail''+test_must_failgitfor-each-ref--format="%(raw)"--perl+'++test_expect_success'%(raw) with --shell must fail''+test_must_failgitfor-each-ref--format="%(raw)"--shell+'++test_expect_success'%(raw) with --shell and --sort=raw must fail''+test_must_failgitfor-each-ref--format="%(raw)"--sort=raw--shell+'++test_expect_success'%(raw:size) with --shell''+gitfor-each-ref--format="%(raw:size)"|whilereadline+do+echo"'\''$line'\''">>expect+done&&+gitfor-each-ref--format="%(raw:size)"--shell>actual&&+test_cmpexpectactual+'++test_expect_success'for-each-ref --format compare with cat-file --batch''+gitrev-parserefs/mytrees/first|gitcat-file--batch>expected&&+gitfor-each-ref--format="%(objectname) %(objecttype) %(objectsize)+%(raw)" refs/mytrees/first >actual &&+test_cmpexpectedactual+'+ test_expect_success'set up multiple-sort tags''forwhenin100000200000do
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-12 11:47:15
From: ZheNing Hu <redacted>
Because the perl language can handle binary data correctly,
add the function perl_quote_buf_with_len(), which can specify
the length of the data and prevent the data from being truncated
at '\0' to help `--format="%(raw)"` re-support `--perl`.
Helped-by: Ævar Arnfjörð Bjarmason [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
Documentation/git-for-each-ref.txt | 4 ++--
quote.c | 17 +++++++++++++++++
quote.h | 1 +
ref-filter.c | 15 +++++++++++----
t/t6300-for-each-ref.sh | 19 +++++++++++++++++--
5 files changed, 48 insertions(+), 8 deletions(-)
@@ -241,8 +241,8 @@ raw:size:: The raw data size of the object. Note that `--format=%(raw)` can not be used with `--python`, `--shell`, `--tcl`,-`--perl` because such language may not support arbitrary binary data in their-string variable type.+because such language may not support arbitrary binary data in their string+variable type. The message in a commit or a tag object is `contents`, from which `contents:<part>` can be used to extract various parts out of:
@@ -94,6 +94,7 @@ char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne/* quoting as a string literal for other languages */voidperl_quote_buf(structstrbuf*sb,constchar*src);+voidperl_quote_buf_with_len(structstrbuf*sb,constchar*src,size_tlen);voidpython_quote_buf(structstrbuf*sb,constchar*src);voidtcl_quote_buf(structstrbuf*sb,constchar*src);voidbasic_regex_quote_buf(structstrbuf*sb,constchar*src);
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-12 11:47:17
From: ZheNing Hu <redacted>
When we use ref_array_item which is not dynamically allocated and
want to free the space of its member "value" after the end of use,
free_array_item() does not meet our needs, because it tries to free
ref_array_item itself and its member "symref".
Introduce free_ref_array_item_value() for freeing ref_array_item value.
It will be called internally by free_array_item(), and it will help
`cat-file --batch` free ref_array_item's value memory later.
Helped-by: Junio C Hamano [off-list ref]
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
ref-filter.c | 11 ++++++++---
ref-filter.h | 2 ++
2 files changed, 10 insertions(+), 3 deletions(-)
@@ -111,6 +111,8 @@ struct ref_format {intfilter_refs(structref_array*array,structref_filter*filter,unsignedinttype);/* Clear all memory allocated to ref_array */voidref_array_clear(structref_array*array);+/* Free ref_array_item's value */+voidfree_ref_array_item_value(structref_array_item*item);/* Used to verify if the given format is correct and to parse out the used atoms */intverify_ref_format(structref_format*format);/* Sort the given ref_array as per the ref_sorting provided */
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-12 11:47:19
From: ZheNing Hu <redacted>
Let get_object() return 1 and print "<oid> missing" instead
of returning -1 and printing "missing object <oid> for <refname>"
if oid_object_info_extended() unable to find the data corresponding
to oid. When `cat-file --batch` use ref-filter logic later it can
help `format_ref_array_item()` just report that the object is missing
without letting Git exit.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
ref-filter.c | 4 ++--
t/t6301-for-each-ref-errors.sh | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
@@ -1762,8 +1762,8 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj}if(oid_object_info_extended(the_repository,&oi->oid,&oi->info,OBJECT_INFO_LOOKUP_REPLACE))-returnstrbuf_addf_ret(err,-1,_("missing object %s for %s"),-oid_to_hex(&oi->oid),ref->refname);+returnstrbuf_addf_ret(err,1,_("%s missing"),+oid_to_hex(&oi->oid));if(oi->info.disk_sizep&&oi->disk_size<0)BUG("Object size is less than zero.");
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-12 11:47:21
From: ZheNing Hu <redacted>
Add `cat_file_mode` member in struct `ref_format` and introduce
the function `reject_atom()`, when `cat-file --batch` use ref-filter
logic later, it can help us reject atoms in verify_ref_format()
which cat-file cannot use, e.g. `%(refname)`, `%(push)`,
`%(upstream)`... or the atom `%(rest)` which for-each-ref family
cannot use.
Helped-by: Eric Sunshine [off-list ref]
Helped-by: Ævar Arnfjörð Bjarmason [off-list ref]
Helped-by: Christian Couder [off-list ref]
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
ref-filter.c | 25 ++++++++++++++++++++++---
ref-filter.h | 1 +
2 files changed, 23 insertions(+), 3 deletions(-)
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-12 11:47:21
From: ZheNing Hu <redacted>
Use `has_object_file()` in `batch_one_object()` to check
whether the input object exists. This can help us reject
the missing oid when we let `cat-file --batch` use ref-filter
logic later.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
builtin/cat-file.c | 7 +++++++
1 file changed, 7 insertions(+)
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-12 11:47:23
From: ZheNing Hu <redacted>
Because later cat-file reuses ref-filter logic that will add
parameter "const struct option *options" to batch_objects(),
the two synonymous parameters of "opt" and "options" may
confuse readers, so change batch_options parameter of
batch_objects() from "opt" to "batch".
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
builtin/cat-file.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
@@ -513,20 +513,20 @@ 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,batch->format,expand_format,&data);data.mark_query=0;strbuf_release(&output);-if(opt->cmdmode)+if(batch->cmdmode)data.split_on_whitespace=1;/**Ifweareprintingouttheobject,thenalwaysfillinthetype,*sincewewillwanttodecidewhetherornottostream.*/-if(opt->print_contents)+if(batch->print_contents)data.info.typep=&data.type;-if(opt->all_objects){+if(batch->all_objects){structobject_cb_datacb;structobject_infoempty=OBJECT_INFO_INIT;
@@ -536,11 +536,11 @@ static int batch_objects(struct batch_options *opt)if(has_promisor_remote())warning("This repository uses promisor remotes. Some objects may not be loaded.");-cb.opt=opt;+cb.opt=batch;cb.expand=&data;cb.scratch=&output;-if(opt->unordered){+if(batch->unordered){structoidsetseen=OIDSET_INIT;cb.seen=&seen;
@@ -590,7 +590,7 @@ static int batch_objects(struct batch_options *opt)data.rest=p;}-batch_one_object(input.buf,&output,opt,&data);+batch_one_object(input.buf,&output,batch,&data);}strbuf_release(&input);
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-12 11:47:24
From: ZheNing Hu <redacted>
Reuse the `err` buffer in batch_object_write(), as the
buffer `scratch` does. This will reduce the overhead
of multiple allocations of memory of the err buffer.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
builtin/cat-file.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
@@ -212,35 +212,36 @@ static void batch_write(struct batch_options *opt, const void *data, int len)staticvoidbatch_object_write(constchar*obj_name,structstrbuf*scratch,+structstrbuf*err,structbatch_options*opt,structexpand_data*data){intret;-structstrbuferr=STRBUF_INIT;structref_array_itemitem={data->oid,data->rest};strbuf_reset(scratch);+strbuf_reset(err);-ret=format_ref_array_item(&item,&opt->format,scratch,&err);+ret=format_ref_array_item(&item,&opt->format,scratch,err);if(ret<0)-die("%s\n",err.buf);+die("%s\n",err->buf);if(ret){/* ret > 0 means when the object corresponding to oid*cannotbefoundinformat_ref_array_item(),weonlyprint*theerrormessage.*/-printf("%s\n",err.buf);+printf("%s\n",err->buf);fflush(stdout);}else{strbuf_addch(scratch,'\n');batch_write(opt,scratch->buf,scratch->len);}free_ref_array_item_value(&item);-strbuf_release(&err);}staticvoidbatch_one_object(constchar*obj_name,structstrbuf*scratch,+structstrbuf*err,structbatch_options*opt,structexpand_data*data){
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-12 11:47:26
From: ZheNing Hu <redacted>
After cat-file reuses the ref-filter logic, we re-implement the
functions of --textconv and --filters options.
Add members `cat_file_cmdmode` to struct `ref_array_item`,
so that struct `batch_option` member `cmdmode` will be passed
to ref-filter, and then ref-filter will take use of it to filter
the content of the object in get_object().
Use `actual_oi` to record the real expand_data: it may point to the
original `oi` or the `act_oi` processed by `textconv_object()` or
`convert_to_working_tree()`. `grab_values()` will grab the contents
of `actual_oi` and `grab_common_values()` to grab the contents of origin
`oi`, this ensures that `%(objectsize)` still uses the size of the
unfiltered data.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
builtin/cat-file.c | 2 +-
ref-filter.c | 35 +++++++++++++++++++++++++++++++++--
ref-filter.h | 1 +
3 files changed, 35 insertions(+), 3 deletions(-)
@@ -1755,6 +1756,9 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj{/* parse_object_buffer() will set eaten to 0 if free() will be needed */inteaten=1;+structexpand_data*actual_oi=oi;+structexpand_dataact_oi={0};+if(oi->info.contentp){/* We need to know that to use parse_object_buffer properly */oi->info.sizep=&oi->size;
@@ -1768,19 +1772,45 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **objBUG("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);+if((ref->cat_file_cmdmode=='c'||ref->cat_file_cmdmode=='w')&&!ref->rest)+returnstrbuf_addf_ret(err,-1,_("missing path for '%s'"),+oid_to_hex(&act_oi.oid));+if(oi->type==OBJ_BLOB){+if(ref->cat_file_cmdmode=='c'){+act_oi=*oi;+if(textconv_object(the_repository,+ref->rest,0100644,&act_oi.oid,+1,(char**)(&act_oi.content),&act_oi.size))+actual_oi=&act_oi;+}elseif(ref->cat_file_cmdmode=='w'){+structstrbufstrbuf=STRBUF_INIT;+structcheckout_metadatameta;+act_oi=*oi;++init_checkout_metadata(&meta,NULL,NULL,&act_oi.oid);+if(!convert_to_working_tree(&the_index,ref->rest,act_oi.content,act_oi.size,&strbuf,&meta))+die("could not convert '%s' %s",+oid_to_hex(&oi->oid),ref->rest);+act_oi.size=strbuf.len;+act_oi.content=strbuf_detach(&strbuf,NULL);+actual_oi=&act_oi;+}+}+*obj=parse_object_buffer(the_repository,&actual_oi->oid,actual_oi->type,actual_oi->size,actual_oi->content,&eaten);if(!*obj){if(!eaten)free(oi->content);returnstrbuf_addf_ret(err,-1,_("parse_object_buffer failed on %s for %s"),oid_to_hex(&oi->oid),ref->refname);}-grab_values(ref->value,deref,*obj,oi);+grab_values(ref->value,deref,*obj,actual_oi);}grab_common_values(ref->value,deref,oi);if(!eaten)free(oi->content);+if(actual_oi!=oi)+free(actual_oi->content);return0;}
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-12 11:47:27
From: ZheNing Hu <redacted>
In order to let cat-file use ref-filter logic, let's do the
following:
1. Change the type of member `format` in struct `batch_options`
to `ref_format`, we will pass it to ref-filter later.
2. Let `batch_objects()` add atoms to format, and use
`verify_ref_format()` to check atoms.
3. Use `format_ref_array_item()` in `batch_object_write()` to
get the formatted data corresponding to the object. If the
return value of `format_ref_array_item()` is equals to zero,
use `batch_write()` to print object data; else if the return
value is less than zero, use `die()` to print the error message
and exit; else if return value is greater than zero, only print
the error message, but don't exit.
4. Use free_ref_array_item_value() to free ref_array_item's
value.
Most of the atoms in `for-each-ref --format` are now supported,
such as `%(tree)`, `%(parent)`, `%(author)`, `%(tagger)`, `%(if)`,
`%(then)`, `%(else)`, `%(end)`. But these atoms will be rejected:
`%(refname)`, `%(symref)`, `%(upstream)`, `%(push)`, `%(worktreepath)`,
`%(flag)`, `%(HEAD)`, because these atoms are unique to those objects
that pointed to by a ref, "for-each-ref"'s family can naturally use
these atoms, but not all objects are pointed to be a ref, so "cat-file"
will not be able to use them.
The performance for `git cat-file --batch-all-objects
--batch-check` on the Git repository itself with performance
testing tool `hyperfine` changes from 669.4 ms ± 31.1 ms to
1.134 s ± 0.063 s.
The performance for `git cat-file --batch-all-objects --batch
/dev/null` on the Git repository itself with performance testing
tool `time` change from "27.37s user 0.29s system 98% cpu 28.089
total" to "33.69s user 1.54s system 87% cpu 40.258 total".
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
Documentation/git-cat-file.txt | 6 +
builtin/cat-file.c | 242 ++++++-------------------------
t/t1006-cat-file.sh | 251 +++++++++++++++++++++++++++++++++
3 files changed, 304 insertions(+), 195 deletions(-)
@@ -226,6 +226,12 @@ newline. The available atoms are: after that first run of whitespace (i.e., the "rest" of the line) are output in place of the `%(rest)` atom.+Note that most of the atoms in `for-each-ref --format` are now supported,+such as `%(tree)`, `%(parent)`, `%(author)`, `%(tagger)`, `%(if)`,+`%(then)`, `%(else)`, `%(end)`. But these atoms will be rejected:+`%(refname)`, `%(symref)`, `%(upstream)`, `%(push)`, `%(worktreepath)`,+`%(flag)`, `%(HEAD)`. See linkgit:git-for-each-ref[1].+ If no format is specified, the default format is `%(objectname) %(objecttype) %(objectsize)`.
@@ -25,7 +26,7 @@ struct batch_options {intall_objects;intunordered;intcmdmode;/* may be 'w' or 'c' for --filters or --textconv */-constchar*format;+structref_formatformat;};staticconstchar*force_path;
@@ -195,99 +196,10 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,structexpand_data{structobject_idoid;-enumobject_typetype;-unsignedlongsize;-off_tdisk_size;constchar*rest;-structobject_iddelta_base_oid;--/*-*Ifmark_queryistrue,wedonotexpandanything,butrather-*justmarktheobject_infowithitemswewishtoquery.-*/-intmark_query;--/*-*Whethertosplittheinputonwhitespacebeforefeedingitto-*get_sha1;thisisdecidedduringthemark_queryphasebasedon-*whetherwehavea%(rest)tokeninourformat.-*/intsplit_on_whitespace;--/*-*Afteramark_queryrun,thisobject_infoissetuptobe-*passedtooid_object_info_extended.Itwillpointtothedata-*elementsabove,soyoucanretrievetheresponsefromthere.-*/-structobject_infoinfo;--/*-*Thisflagwillbetrueiftherequestedbatchformatandoptions-*don'trequireustocalloid_object_info,whichcanthenbe-*optimizedout.-*/-unsignedskip_object_info:1;};-staticintis_atom(constchar*atom,constchar*s,intslen)-{-intalen=strlen(atom);-returnalen==slen&&!memcmp(atom,s,alen);-}--staticvoidexpand_atom(structstrbuf*sb,constchar*atom,intlen,-void*vdata)-{-structexpand_data*data=vdata;--if(is_atom("objectname",atom,len)){-if(!data->mark_query)-strbuf_addstr(sb,oid_to_hex(&data->oid));-}elseif(is_atom("objecttype",atom,len)){-if(data->mark_query)-data->info.typep=&data->type;-else-strbuf_addstr(sb,type_name(data->type));-}elseif(is_atom("objectsize",atom,len)){-if(data->mark_query)-data->info.sizep=&data->size;-else-strbuf_addf(sb,"%"PRIuMAX,(uintmax_t)data->size);-}elseif(is_atom("objectsize:disk",atom,len)){-if(data->mark_query)-data->info.disk_sizep=&data->disk_size;-else-strbuf_addf(sb,"%"PRIuMAX,(uintmax_t)data->disk_size);-}elseif(is_atom("rest",atom,len)){-if(data->mark_query)-data->split_on_whitespace=1;-elseif(data->rest)-strbuf_addstr(sb,data->rest);-}elseif(is_atom("deltabase",atom,len)){-if(data->mark_query)-data->info.delta_base_oid=&data->delta_base_oid;-else-strbuf_addstr(sb,-oid_to_hex(&data->delta_base_oid));-}else-die("unknown format element: %.*s",len,atom);-}--staticsize_texpand_format(structstrbuf*sb,constchar*start,void*data)-{-constchar*end;--if(*start!='(')-return0;-end=strchr(start+1,')');-if(!end)-die("format element '%s' does not end in ')'",start);--expand_atom(sb,start+1,end-start-1,data);--returnend-start+1;-}-staticvoidbatch_write(structbatch_options*opt,constvoid*data,intlen){if(opt->buffer_output){
@@ -297,87 +209,34 @@ static void batch_write(struct batch_options *opt, const void *data, int len)write_or_die(1,data,len);}-staticvoidprint_object_or_die(structbatch_options*opt,structexpand_data*data)-{-conststructobject_id*oid=&data->oid;--assert(data->info.typep);--if(data->type==OBJ_BLOB){-if(opt->buffer_output)-fflush(stdout);-if(opt->cmdmode){-char*contents;-unsignedlongsize;--if(!data->rest)-die("missing path for '%s'",oid_to_hex(oid));--if(opt->cmdmode=='w'){-if(filter_object(data->rest,0100644,oid,-&contents,&size))-die("could not convert '%s' %s",-oid_to_hex(oid),data->rest);-}elseif(opt->cmdmode=='c'){-enumobject_typetype;-if(!textconv_object(the_repository,-data->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);-}else-BUG("invalid cmdmode: %c",opt->cmdmode);-batch_write(opt,contents,size);-free(contents);-}else{-stream_blob(oid);-}-}-else{-enumobject_typetype;-unsignedlongsize;-void*contents;--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));--batch_write(opt,contents,size);-free(contents);-}-}staticvoidbatch_object_write(constchar*obj_name,structstrbuf*scratch,structbatch_options*opt,structexpand_data*data){-if(!data->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));-fflush(stdout);-return;-}+intret;+structstrbuferr=STRBUF_INIT;+structref_array_itemitem={data->oid,data->rest};strbuf_reset(scratch);-strbuf_expand(scratch,opt->format,expand_format,data);-strbuf_addch(scratch,'\n');-batch_write(opt,scratch->buf,scratch->len);-if(opt->print_contents){-print_object_or_die(opt,data);-batch_write(opt,"\n",1);+ret=format_ref_array_item(&item,&opt->format,scratch,&err);+if(ret<0)+die("%s\n",err.buf);+if(ret){+/* ret > 0 means when the object corresponding to oid+*cannotbefoundinformat_ref_array_item(),weonlyprint+*theerrormessage.+*/+printf("%s\n",err.buf);+fflush(stdout);+}else{+strbuf_addch(scratch,'\n');+batch_write(opt,scratch->buf,scratch->len);}+free_ref_array_item_value(&item);+strbuf_release(&err);}staticvoidbatch_one_object(constchar*obj_name,
@@ -495,43 +354,37 @@ static int batch_unordered_packed(const struct object_id *oid,returnbatch_unordered_object(oid,data);}-staticintbatch_objects(structbatch_options*batch)+staticconstchar*constcat_file_usage[]={+N_("git cat-file (-t [--allow-unknown-type] | -s [--allow-unknown-type] | -e | -p | <type> | --textconv | --filters) [--path=<path>] <object>"),+N_("git cat-file (--batch[=<format>] | --batch-check[=<format>]) [--follow-symlinks] [--textconv | --filters]"),+NULL+};++staticintbatch_objects(structbatch_options*batch,conststructoption*options){structstrbufinput=STRBUF_INIT;structstrbufoutput=STRBUF_INIT;+structstrbufformat=STRBUF_INIT;structexpand_datadata;intsave_warning;intretval=0;-if(!batch->format)-batch->format="%(objectname) %(objecttype) %(objectsize)";--/*-*Expandoncewithourspecialmark_queryflag,whichwillprimethe-*object_infotobehandedtooid_object_info_extendedforeach-*object.-*/memset(&data,0,sizeof(data));-data.mark_query=1;-strbuf_expand(&output,batch->format,expand_format,&data);-data.mark_query=0;-strbuf_release(&output);-if(batch->cmdmode)-data.split_on_whitespace=1;--/*-*Ifweareprintingouttheobject,thenalwaysfillinthetype,-*sincewewillwanttodecidewhetherornottostream.-*/+if(batch->format.format)+strbuf_addstr(&format,batch->format.format);+else+strbuf_addstr(&format,"%(objectname) %(objecttype) %(objectsize)");if(batch->print_contents)-data.info.typep=&data.type;+strbuf_addstr(&format,"\n%(raw)");+batch->format.format=format.buf;+if(verify_ref_format(&batch->format))+usage_with_options(cat_file_usage,options);++if(batch->cmdmode||batch->format.use_rest)+data.split_on_whitespace=1;if(batch->all_objects){structobject_cb_datacb;-structobject_infoempty=OBJECT_INFO_INIT;--if(!memcmp(&data.info,&empty,sizeof(empty)))-data.skip_object_info=1;if(has_promisor_remote())warning("This repository uses promisor remotes. Some objects may not be loaded.");
@@ -561,6 +414,7 @@ static int batch_objects(struct batch_options *batch)oid_array_clear(&sa);}+strbuf_release(&format);strbuf_release(&output);return0;}
@@ -718,7 +570,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)batch.buffer_output=batch.all_objects;if(batch.enabled)-returnbatch_objects(&batch);+returnbatch_objects(&batch,options);if(unknown_type&&opt!='t'&&opt!='s')die("git cat-file --allow-unknown-type: use with -s or -t");
@@ -607,5 +607,256 @@ test_expect_success 'cat-file --batch="batman" with --batch-all-objects will worgit-Call-twocat-file--batch-all-objects--batch="batman">actual&&cmpexpectactual'+."$TEST_DIRECTORY"/lib-gpg.sh+."$TEST_DIRECTORY"/lib-terminal.sh++test_expect_success'cat-file --batch|--batch-check setup''+echo1>blob1&&+printf"a\0b\0\c">blob2&&+gitaddblob1blob2&&+gitcommit-m"Commit Message"&&+gitbranch-Mmain&&+gittag-a-m"v0.0.0"testtag&&+gitupdate-refrefs/myblobs/blob1HEAD:blob1&&+gitupdate-refrefs/myblobs/blob2HEAD:blob2&&+gitupdate-refrefs/mytrees/tree1HEAD^{tree}+'++batch_test_atom(){+iftest"$3"="fail"+then+test_expect_${4:-success}$PREREQ"basic atom: $1$2 must fail""+test_must_failgitcat-file--batch-check='$2'>bad<<-EOF+$1+EOF+"+else+test_expect_${4:-success}$PREREQ"basic atom: $1$2""+gitfor-each-ref--format='$2'$1>expected&&+gitcat-file--batch-check='$2'>actual<<-EOF&&+$1+EOF+sanitize_pgp<actual>actual.clean&&+cmpexpectedactual.clean+"+fi+}++batch_test_atomrefs/heads/main'%(refname)'fail+batch_test_atomrefs/heads/main'%(refname:)'fail+batch_test_atomrefs/heads/main'%(refname:short)'fail+batch_test_atomrefs/heads/main'%(refname:lstrip=1)'fail+batch_test_atomrefs/heads/main'%(refname:lstrip=2)'fail+batch_test_atomrefs/heads/main'%(refname:lstrip=-1)'fail+batch_test_atomrefs/heads/main'%(refname:lstrip=-2)'fail+batch_test_atomrefs/heads/main'%(refname:rstrip=1)'fail+batch_test_atomrefs/heads/main'%(refname:rstrip=2)'fail+batch_test_atomrefs/heads/main'%(refname:rstrip=-1)'fail+batch_test_atomrefs/heads/main'%(refname:rstrip=-2)'fail+batch_test_atomrefs/heads/main'%(refname:strip=1)'fail+batch_test_atomrefs/heads/main'%(refname:strip=2)'fail+batch_test_atomrefs/heads/main'%(refname:strip=-1)'fail+batch_test_atomrefs/heads/main'%(refname:strip=-2)'fail+batch_test_atomrefs/heads/main'%(upstream)'fail+batch_test_atomrefs/heads/main'%(upstream:short)'fail+batch_test_atomrefs/heads/main'%(upstream:lstrip=2)'fail+batch_test_atomrefs/heads/main'%(upstream:lstrip=-2)'fail+batch_test_atomrefs/heads/main'%(upstream:rstrip=2)'fail+batch_test_atomrefs/heads/main'%(upstream:rstrip=-2)'fail+batch_test_atomrefs/heads/main'%(upstream:strip=2)'fail+batch_test_atomrefs/heads/main'%(upstream:strip=-2)'fail+batch_test_atomrefs/heads/main'%(push)'fail+batch_test_atomrefs/heads/main'%(push:short)'fail+batch_test_atomrefs/heads/main'%(push:lstrip=1)'fail+batch_test_atomrefs/heads/main'%(push:lstrip=-1)'fail+batch_test_atomrefs/heads/main'%(push:rstrip=1)'fail+batch_test_atomrefs/heads/main'%(push:rstrip=-1)'fail+batch_test_atomrefs/heads/main'%(push:strip=1)'fail+batch_test_atomrefs/heads/main'%(push:strip=-1)'fail+batch_test_atomrefs/heads/main'%(objecttype)'+batch_test_atomrefs/heads/main'%(objectsize)'+batch_test_atomrefs/heads/main'%(objectsize:disk)'+batch_test_atomrefs/heads/main'%(deltabase)'+batch_test_atomrefs/heads/main'%(objectname)'+batch_test_atomrefs/heads/main'%(objectname:short)'+batch_test_atomrefs/heads/main'%(objectname:short=1)'+batch_test_atomrefs/heads/main'%(objectname:short=10)'+batch_test_atomrefs/heads/main'%(tree)'+batch_test_atomrefs/heads/main'%(tree:short)'+batch_test_atomrefs/heads/main'%(tree:short=1)'+batch_test_atomrefs/heads/main'%(tree:short=10)'+batch_test_atomrefs/heads/main'%(parent)'+batch_test_atomrefs/heads/main'%(parent:short)'+batch_test_atomrefs/heads/main'%(parent:short=1)'+batch_test_atomrefs/heads/main'%(parent:short=10)'+batch_test_atomrefs/heads/main'%(numparent)'+batch_test_atomrefs/heads/main'%(object)'+batch_test_atomrefs/heads/main'%(type)'+batch_test_atomrefs/heads/main'%(raw)'+batch_test_atomrefs/heads/main'%(*objectname)'+batch_test_atomrefs/heads/main'%(*objecttype)'+batch_test_atomrefs/heads/main'%(author)'+batch_test_atomrefs/heads/main'%(authorname)'+batch_test_atomrefs/heads/main'%(authoremail)'+batch_test_atomrefs/heads/main'%(authoremail:trim)'+batch_test_atomrefs/heads/main'%(authoremail:localpart)'+batch_test_atomrefs/heads/main'%(authordate)'+batch_test_atomrefs/heads/main'%(committer)'+batch_test_atomrefs/heads/main'%(committername)'+batch_test_atomrefs/heads/main'%(committeremail)'+batch_test_atomrefs/heads/main'%(committeremail:trim)'+batch_test_atomrefs/heads/main'%(committeremail:localpart)'+batch_test_atomrefs/heads/main'%(committerdate)'+batch_test_atomrefs/heads/main'%(tag)'+batch_test_atomrefs/heads/main'%(tagger)'+batch_test_atomrefs/heads/main'%(taggername)'+batch_test_atomrefs/heads/main'%(taggeremail)'+batch_test_atomrefs/heads/main'%(taggeremail:trim)'+batch_test_atomrefs/heads/main'%(taggeremail:localpart)'+batch_test_atomrefs/heads/main'%(taggerdate)'+batch_test_atomrefs/heads/main'%(creator)'+batch_test_atomrefs/heads/main'%(creatordate)'+batch_test_atomrefs/heads/main'%(subject)'+batch_test_atomrefs/heads/main'%(subject:sanitize)'+batch_test_atomrefs/heads/main'%(contents:subject)'+batch_test_atomrefs/heads/main'%(body)'+batch_test_atomrefs/heads/main'%(contents:body)'+batch_test_atomrefs/heads/main'%(contents:signature)'+batch_test_atomrefs/heads/main'%(contents)'+batch_test_atomrefs/heads/main'%(HEAD)'fail+batch_test_atomrefs/heads/main'%(upstream:track)'fail+batch_test_atomrefs/heads/main'%(upstream:trackshort)'fail+batch_test_atomrefs/heads/main'%(upstream:track,nobracket)'fail+batch_test_atomrefs/heads/main'%(upstream:nobracket,track)'fail+batch_test_atomrefs/heads/main'%(push:track)'fail+batch_test_atomrefs/heads/main'%(push:trackshort)'fail+batch_test_atomrefs/heads/main'%(worktreepath)'fail+batch_test_atomrefs/heads/main'%(symref)'fail+batch_test_atomrefs/heads/main'%(flag)'fail++batch_test_atomrefs/tags/testtag'%(refname)'fail+batch_test_atomrefs/tags/testtag'%(refname:short)'fail+batch_test_atomrefs/tags/testtag'%(upstream)'fail+batch_test_atomrefs/tags/testtag'%(push)'fail+batch_test_atomrefs/tags/testtag'%(objecttype)'+batch_test_atomrefs/tags/testtag'%(objectsize)'+batch_test_atomrefs/tags/testtag'%(objectsize:disk)'+batch_test_atomrefs/tags/testtag'%(*objectsize:disk)'+batch_test_atomrefs/tags/testtag'%(deltabase)'+batch_test_atomrefs/tags/testtag'%(*deltabase)'+batch_test_atomrefs/tags/testtag'%(objectname)'+batch_test_atomrefs/tags/testtag'%(objectname:short)'+batch_test_atomrefs/tags/testtag'%(tree)'+batch_test_atomrefs/tags/testtag'%(tree:short)'+batch_test_atomrefs/tags/testtag'%(tree:short=1)'+batch_test_atomrefs/tags/testtag'%(tree:short=10)'+batch_test_atomrefs/tags/testtag'%(parent)'+batch_test_atomrefs/tags/testtag'%(parent:short)'+batch_test_atomrefs/tags/testtag'%(parent:short=1)'+batch_test_atomrefs/tags/testtag'%(parent:short=10)'+batch_test_atomrefs/tags/testtag'%(numparent)'+batch_test_atomrefs/tags/testtag'%(object)'+batch_test_atomrefs/tags/testtag'%(type)'+batch_test_atomrefs/tags/testtag'%(*objectname)'+batch_test_atomrefs/tags/testtag'%(*objecttype)'+batch_test_atomrefs/tags/testtag'%(author)'+batch_test_atomrefs/tags/testtag'%(authorname)'+batch_test_atomrefs/tags/testtag'%(authoremail)'+batch_test_atomrefs/tags/testtag'%(authoremail:trim)'+batch_test_atomrefs/tags/testtag'%(authoremail:localpart)'+batch_test_atomrefs/tags/testtag'%(authordate)'+batch_test_atomrefs/tags/testtag'%(committer)'+batch_test_atomrefs/tags/testtag'%(committername)'+batch_test_atomrefs/tags/testtag'%(committeremail)'+batch_test_atomrefs/tags/testtag'%(committeremail:trim)'+batch_test_atomrefs/tags/testtag'%(committeremail:localpart)'+batch_test_atomrefs/tags/testtag'%(committerdate)'+batch_test_atomrefs/tags/testtag'%(tag)'+batch_test_atomrefs/tags/testtag'%(tagger)'+batch_test_atomrefs/tags/testtag'%(taggername)'+batch_test_atomrefs/tags/testtag'%(taggeremail)'+batch_test_atomrefs/tags/testtag'%(taggeremail:trim)'+batch_test_atomrefs/tags/testtag'%(taggeremail:localpart)'+batch_test_atomrefs/tags/testtag'%(taggerdate)'+batch_test_atomrefs/tags/testtag'%(creator)'+batch_test_atomrefs/tags/testtag'%(creatordate)'+batch_test_atomrefs/tags/testtag'%(subject)'+batch_test_atomrefs/tags/testtag'%(subject:sanitize)'+batch_test_atomrefs/tags/testtag'%(contents:subject)'+batch_test_atomrefs/tags/testtag'%(body)'+batch_test_atomrefs/tags/testtag'%(contents:body)'+batch_test_atomrefs/tags/testtag'%(contents:signature)'+batch_test_atomrefs/tags/testtag'%(contents)'+batch_test_atomrefs/tags/testtag'%(HEAD)'fail++batch_test_atomrefs/myblobs/blob1'%(refname)'fail+batch_test_atomrefs/myblobs/blob1'%(upstream)'fail+batch_test_atomrefs/myblobs/blob1'%(push)'fail+batch_test_atomrefs/myblobs/blob1'%(HEAD)'fail++batch_test_atomrefs/myblobs/blob1'%(objectname)'+batch_test_atomrefs/myblobs/blob1'%(objecttype)'+batch_test_atomrefs/myblobs/blob1'%(objectsize)'+batch_test_atomrefs/myblobs/blob1'%(objectsize:disk)'+batch_test_atomrefs/myblobs/blob1'%(deltabase)'++batch_test_atomrefs/myblobs/blob1'%(contents)'+batch_test_atomrefs/myblobs/blob2'%(contents)'++batch_test_atomrefs/myblobs/blob1'%(raw)'+batch_test_atomrefs/myblobs/blob2'%(raw)'+batch_test_atomrefs/mytrees/tree1'%(raw)'++batch_test_atomrefs/myblobs/blob1'%(raw:size)'+batch_test_atomrefs/myblobs/blob2'%(raw:size)'+batch_test_atomrefs/mytrees/tree1'%(raw:size)'++batch_test_atomrefs/myblobs/blob1'%(if:equals=blob)%(objecttype)%(then)commit%(else)not commit%(end)'+batch_test_atomrefs/myblobs/blob2'%(if:equals=blob)%(objecttype)%(then)commit%(else)not commit%(end)'+batch_test_atomrefs/mytrees/tree1'%(if:equals=tree)%(objecttype)%(then)tree%(else)not tree%(end)'++batch_test_atomrefs/heads/main'%(align:60) objectname is %(objectname)%(end)|%(objectname)'+batch_test_atomrefs/heads/main'%(align:left,60) objectname is %(objectname)%(end)|%(objectname)'+batch_test_atomrefs/heads/main'%(align:middle,60) objectname is %(objectname)%(end)|%(objectname)'+batch_test_atomrefs/heads/main'%(align:60,right) objectname is %(objectname)%(end)|%(objectname)'++batch_test_atomrefs/heads/main'VALID'+batch_test_atomrefs/heads/main'%(INVALID)'fail+batch_test_atomrefs/heads/main'%(authordate:INVALID)'fail++test_expect_success'%(rest) works with both a branch and a tag''+cat>expected<<-EOF&&+123commit123+456tag456+EOF+gitcat-file--batch-check="%(rest) %(objecttype) %(rest)">actual<<-EOF&&+refs/heads/main123+refs/tags/testtag456+EOF+test_cmpexpectedactual+'++batch_test_atomrefs/heads/main'%(objectname)%(objecttype)%(objectsize)+%(raw)'+batch_test_atomrefs/tags/testtag'%(objectname)%(objecttype)%(objectsize)+%(raw)'+batch_test_atomrefs/myblobs/blob1'%(objectname)%(objecttype)%(objectsize)+%(raw)'+batch_test_atomrefs/myblobs/blob2'%(objectname)%(objecttype)%(objectsize)+%(raw)'+++test_expect_success'cat-file --batch equals to --batch-check with atoms''+gitcat-file--batch-check="%(objectname) %(objecttype) %(objectsize)+%(raw)" >expected <<-EOF &&+refs/heads/main+refs/tags/testtag+EOF+gitcat-file--batch>actual<<-EOF&&+refs/heads/main+refs/tags/testtag+EOF+cmpexpectedactual+' test_done
From: Christian Couder <hidden> Date: 2021-07-12 13:17:49
On Mon, Jul 12, 2021 at 1:47 PM ZheNing Hu via GitGitGadget
[off-list ref] wrote:
From: ZheNing Hu <redacted>
In order to let cat-file use ref-filter logic, let's do the
following:
1. Change the type of member `format` in struct `batch_options`
to `ref_format`, we will pass it to ref-filter later.
2. Let `batch_objects()` add atoms to format, and use
`verify_ref_format()` to check atoms.
3. Use `format_ref_array_item()` in `batch_object_write()` to
get the formatted data corresponding to the object. If the
return value of `format_ref_array_item()` is equals to zero,
use `batch_write()` to print object data; else if the return
value is less than zero, use `die()` to print the error message
and exit; else if return value is greater than zero, only print
the error message, but don't exit.
4. Use free_ref_array_item_value() to free ref_array_item's
value.
Most of the atoms in `for-each-ref --format` are now supported,
such as `%(tree)`, `%(parent)`, `%(author)`, `%(tagger)`, `%(if)`,
`%(then)`, `%(else)`, `%(end)`. But these atoms will be rejected:
`%(refname)`, `%(symref)`, `%(upstream)`, `%(push)`, `%(worktreepath)`,
`%(flag)`, `%(HEAD)`, because these atoms are unique to those objects
that pointed to by a ref, "for-each-ref"'s family can naturally use
these atoms, but not all objects are pointed to be a ref, so "cat-file"
will not be able to use them.
The performance for `git cat-file --batch-all-objects
--batch-check` on the Git repository itself with performance
testing tool `hyperfine` changes from 669.4 ms ± 31.1 ms to
1.134 s ± 0.063 s.
The performance for `git cat-file --batch-all-objects --batch
quoted
/dev/null` on the Git repository itself with performance testing
tool `time` change from "27.37s user 0.29s system 98% cpu 28.089
total" to "33.69s user 1.54s system 87% cpu 40.258 total".
Saying that a later patch will add a fast path which will mitigate the
performance regression introduced by this patch might help reassure
reviewers.
By the way it is not clear if adding the fast path fully mitigates
this performance regression or not. You might want to discuss that in
the cover letter, or maybe in the patch adding the fast path.
@@ -226,6 +226,12 @@ newline. The available atoms are: after that first run of whitespace (i.e., the "rest" of the line) are output in place of the `%(rest)` atom.+Note that most of the atoms in `for-each-ref --format` are now supported,+such as `%(tree)`, `%(parent)`, `%(author)`, `%(tagger)`, `%(if)`,+`%(then)`, `%(else)`, `%(end)`. But these atoms will be rejected:+`%(refname)`, `%(symref)`, `%(upstream)`, `%(push)`, `%(worktreepath)`,+`%(flag)`, `%(HEAD)`. See linkgit:git-for-each-ref[1].+ If no format is specified, the default format is `%(objectname) %(objecttype) %(objectsize)`.
@@ -25,7 +26,7 @@ struct batch_options {intall_objects;intunordered;intcmdmode;/* may be 'w' or 'c' for --filters or --textconv */-constchar*format;+structref_formatformat;};staticconstchar*force_path;
@@ -195,99 +196,10 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,structexpand_data{structobject_idoid;-enumobject_typetype;-unsignedlongsize;-off_tdisk_size;constchar*rest;-structobject_iddelta_base_oid;--/*-*Ifmark_queryistrue,wedonotexpandanything,butrather-*justmarktheobject_infowithitemswewishtoquery.-*/-intmark_query;--/*-*Whethertosplittheinputonwhitespacebeforefeedingitto-*get_sha1;thisisdecidedduringthemark_queryphasebasedon-*whetherwehavea%(rest)tokeninourformat.-*/intsplit_on_whitespace;--/*-*Afteramark_queryrun,thisobject_infoissetuptobe-*passedtooid_object_info_extended.Itwillpointtothedata-*elementsabove,soyoucanretrievetheresponsefromthere.-*/-structobject_infoinfo;--/*-*Thisflagwillbetrueiftherequestedbatchformatandoptions-*don'trequireustocalloid_object_info,whichcanthenbe-*optimizedout.-*/-unsignedskip_object_info:1;};-staticintis_atom(constchar*atom,constchar*s,intslen)-{-intalen=strlen(atom);-returnalen==slen&&!memcmp(atom,s,alen);-}--staticvoidexpand_atom(structstrbuf*sb,constchar*atom,intlen,-void*vdata)-{-structexpand_data*data=vdata;--if(is_atom("objectname",atom,len)){-if(!data->mark_query)-strbuf_addstr(sb,oid_to_hex(&data->oid));-}elseif(is_atom("objecttype",atom,len)){-if(data->mark_query)-data->info.typep=&data->type;-else-strbuf_addstr(sb,type_name(data->type));-}elseif(is_atom("objectsize",atom,len)){-if(data->mark_query)-data->info.sizep=&data->size;-else-strbuf_addf(sb,"%"PRIuMAX,(uintmax_t)data->size);-}elseif(is_atom("objectsize:disk",atom,len)){-if(data->mark_query)-data->info.disk_sizep=&data->disk_size;-else-strbuf_addf(sb,"%"PRIuMAX,(uintmax_t)data->disk_size);-}elseif(is_atom("rest",atom,len)){-if(data->mark_query)-data->split_on_whitespace=1;-elseif(data->rest)-strbuf_addstr(sb,data->rest);-}elseif(is_atom("deltabase",atom,len)){-if(data->mark_query)-data->info.delta_base_oid=&data->delta_base_oid;-else-strbuf_addstr(sb,-oid_to_hex(&data->delta_base_oid));-}else-die("unknown format element: %.*s",len,atom);-}--staticsize_texpand_format(structstrbuf*sb,constchar*start,void*data)-{-constchar*end;--if(*start!='(')-return0;-end=strchr(start+1,')');-if(!end)-die("format element '%s' does not end in ')'",start);--expand_atom(sb,start+1,end-start-1,data);--returnend-start+1;-}-staticvoidbatch_write(structbatch_options*opt,constvoid*data,intlen){if(opt->buffer_output){
@@ -297,87 +209,34 @@ static void batch_write(struct batch_options *opt, const void *data, int len)write_or_die(1,data,len);}-staticvoidprint_object_or_die(structbatch_options*opt,structexpand_data*data)-{-conststructobject_id*oid=&data->oid;--assert(data->info.typep);--if(data->type==OBJ_BLOB){-if(opt->buffer_output)-fflush(stdout);-if(opt->cmdmode){-char*contents;-unsignedlongsize;--if(!data->rest)-die("missing path for '%s'",oid_to_hex(oid));--if(opt->cmdmode=='w'){-if(filter_object(data->rest,0100644,oid,-&contents,&size))-die("could not convert '%s' %s",-oid_to_hex(oid),data->rest);-}elseif(opt->cmdmode=='c'){-enumobject_typetype;-if(!textconv_object(the_repository,-data->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);-}else-BUG("invalid cmdmode: %c",opt->cmdmode);-batch_write(opt,contents,size);-free(contents);-}else{-stream_blob(oid);-}-}-else{-enumobject_typetype;-unsignedlongsize;-void*contents;--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));--batch_write(opt,contents,size);-free(contents);-}-}staticvoidbatch_object_write(constchar*obj_name,structstrbuf*scratch,structbatch_options*opt,structexpand_data*data){-if(!data->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));-fflush(stdout);-return;-}+intret;+structstrbuferr=STRBUF_INIT;+structref_array_itemitem={data->oid,data->rest};strbuf_reset(scratch);-strbuf_expand(scratch,opt->format,expand_format,data);-strbuf_addch(scratch,'\n');-batch_write(opt,scratch->buf,scratch->len);-if(opt->print_contents){-print_object_or_die(opt,data);-batch_write(opt,"\n",1);+ret=format_ref_array_item(&item,&opt->format,scratch,&err);+if(ret<0)+die("%s\n",err.buf);+if(ret){+/* ret > 0 means when the object corresponding to oid+*cannotbefoundinformat_ref_array_item(),weonlyprint+*theerrormessage.+*/+printf("%s\n",err.buf);+fflush(stdout);+}else{+strbuf_addch(scratch,'\n');+batch_write(opt,scratch->buf,scratch->len);}+free_ref_array_item_value(&item);+strbuf_release(&err);}staticvoidbatch_one_object(constchar*obj_name,
@@ -495,43 +354,37 @@ static int batch_unordered_packed(const struct object_id *oid,returnbatch_unordered_object(oid,data);}-staticintbatch_objects(structbatch_options*batch)+staticconstchar*constcat_file_usage[]={+N_("git cat-file (-t [--allow-unknown-type] | -s [--allow-unknown-type] | -e | -p | <type> | --textconv | --filters) [--path=<path>] <object>"),+N_("git cat-file (--batch[=<format>] | --batch-check[=<format>]) [--follow-symlinks] [--textconv | --filters]"),+NULL+};++staticintbatch_objects(structbatch_options*batch,conststructoption*options){structstrbufinput=STRBUF_INIT;structstrbufoutput=STRBUF_INIT;+structstrbufformat=STRBUF_INIT;structexpand_datadata;intsave_warning;intretval=0;-if(!batch->format)-batch->format="%(objectname) %(objecttype) %(objectsize)";--/*-*Expandoncewithourspecialmark_queryflag,whichwillprimethe-*object_infotobehandedtooid_object_info_extendedforeach-*object.-*/memset(&data,0,sizeof(data));-data.mark_query=1;-strbuf_expand(&output,batch->format,expand_format,&data);-data.mark_query=0;-strbuf_release(&output);-if(batch->cmdmode)-data.split_on_whitespace=1;--/*-*Ifweareprintingouttheobject,thenalwaysfillinthetype,-*sincewewillwanttodecidewhetherornottostream.-*/+if(batch->format.format)+strbuf_addstr(&format,batch->format.format);+else+strbuf_addstr(&format,"%(objectname) %(objecttype) %(objectsize)");if(batch->print_contents)-data.info.typep=&data.type;+strbuf_addstr(&format,"\n%(raw)");+batch->format.format=format.buf;+if(verify_ref_format(&batch->format))+usage_with_options(cat_file_usage,options);++if(batch->cmdmode||batch->format.use_rest)+data.split_on_whitespace=1;if(batch->all_objects){structobject_cb_datacb;-structobject_infoempty=OBJECT_INFO_INIT;--if(!memcmp(&data.info,&empty,sizeof(empty)))-data.skip_object_info=1;if(has_promisor_remote())warning("This repository uses promisor remotes. Some objects may not be loaded.");
@@ -561,6 +414,7 @@ static int batch_objects(struct batch_options *batch)oid_array_clear(&sa);}+strbuf_release(&format);strbuf_release(&output);return0;}
@@ -718,7 +570,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)batch.buffer_output=batch.all_objects;if(batch.enabled)-returnbatch_objects(&batch);+returnbatch_objects(&batch,options);if(unknown_type&&opt!='t'&&opt!='s')die("git cat-file --allow-unknown-type: use with -s or -t");
@@ -607,5 +607,256 @@ test_expect_success 'cat-file --batch="batman" with --batch-all-objects will worgit-Call-twocat-file--batch-all-objects--batch="batman">actual&&cmpexpectactual'+."$TEST_DIRECTORY"/lib-gpg.sh+."$TEST_DIRECTORY"/lib-terminal.sh++test_expect_success'cat-file --batch|--batch-check setup''+echo1>blob1&&+printf"a\0b\0\c">blob2&&+gitaddblob1blob2&&+gitcommit-m"Commit Message"&&+gitbranch-Mmain&&+gittag-a-m"v0.0.0"testtag&&+gitupdate-refrefs/myblobs/blob1HEAD:blob1&&+gitupdate-refrefs/myblobs/blob2HEAD:blob2&&+gitupdate-refrefs/mytrees/tree1HEAD^{tree}+'++batch_test_atom(){+iftest"$3"="fail"+then+test_expect_${4:-success}$PREREQ"basic atom: $1$2 must fail""+test_must_failgitcat-file--batch-check='$2'>bad<<-EOF+$1+EOF+"+else+test_expect_${4:-success}$PREREQ"basic atom: $1$2""+gitfor-each-ref--format='$2'$1>expected&&+gitcat-file--batch-check='$2'>actual<<-EOF&&+$1+EOF+sanitize_pgp<actual>actual.clean&&+cmpexpectedactual.clean+"+fi+}
I wonder if the above function and some of the tests below could be
introduced in a preparatory patch before this one. It could help check
that reusing ref-filter doesn't change the behavior with some atoms
that were previously supported or rejected. Of course if some atoms
are now failing or are now supported, then it's ok to add new tests
for these atoms in this patch.
+batch_test_atom refs/heads/main 'VALID'
+batch_test_atom refs/heads/main '%(INVALID)' fail
+batch_test_atom refs/heads/main '%(authordate:INVALID)' fail
+
+test_expect_success '%(rest) works with both a branch and a tag' '
+ cat >expected <<-EOF &&
+ 123 commit 123
+ 456 tag 456
+ EOF
+ git cat-file --batch-check="%(rest) %(objecttype) %(rest)" >actual <<-EOF &&
+ refs/heads/main 123
+ refs/tags/testtag 456
+ EOF
+ test_cmp expected actual
+'
It's a bit strange that this test is added in this patch while the
commit message doesn't talk about %(rest). So I wonder if this new
test could move to another previous commit.
I wonder if the above function and some of the tests below could be
introduced in a preparatory patch before this one. It could help check
that reusing ref-filter doesn't change the behavior with some atoms
that were previously supported or rejected. Of course if some atoms
are now failing or are now supported, then it's ok to add new tests
for these atoms in this patch.
For example maybe some of the tests could be introduced earlier when
the reject_atom() function is introduced.
From: ZheNing Hu <hidden> Date: 2021-07-12 13:51:39
Christian Couder [off-list ref] 于2021年7月12日周一 下午9:26写道：
quoted
I wonder if the above function and some of the tests below could be
introduced in a preparatory patch before this one. It could help check
that reusing ref-filter doesn't change the behavior with some atoms
that were previously supported or rejected. Of course if some atoms
are now failing or are now supported, then it's ok to add new tests
for these atoms in this patch.
For example maybe some of the tests could be introduced earlier when
the reject_atom() function is introduced.
Yeah, move it to "[GSOC] ref-filter: add %(rest) atom" is better.
Thanks.
--
ZheNing Hu
From: ZheNing Hu <hidden> Date: 2021-07-12 13:49:36
Christian Couder [off-list ref] 于2021年7月12日周一 下午9:17写道：
On Mon, Jul 12, 2021 at 1:47 PM ZheNing Hu via GitGitGadget
[off-list ref] wrote:
quoted
From: ZheNing Hu <redacted>
In order to let cat-file use ref-filter logic, let's do the
following:
1. Change the type of member `format` in struct `batch_options`
to `ref_format`, we will pass it to ref-filter later.
2. Let `batch_objects()` add atoms to format, and use
`verify_ref_format()` to check atoms.
3. Use `format_ref_array_item()` in `batch_object_write()` to
get the formatted data corresponding to the object. If the
return value of `format_ref_array_item()` is equals to zero,
use `batch_write()` to print object data; else if the return
value is less than zero, use `die()` to print the error message
and exit; else if return value is greater than zero, only print
the error message, but don't exit.
4. Use free_ref_array_item_value() to free ref_array_item's
value.
Most of the atoms in `for-each-ref --format` are now supported,
such as `%(tree)`, `%(parent)`, `%(author)`, `%(tagger)`, `%(if)`,
`%(then)`, `%(else)`, `%(end)`. But these atoms will be rejected:
`%(refname)`, `%(symref)`, `%(upstream)`, `%(push)`, `%(worktreepath)`,
`%(flag)`, `%(HEAD)`, because these atoms are unique to those objects
that pointed to by a ref, "for-each-ref"'s family can naturally use
these atoms, but not all objects are pointed to be a ref, so "cat-file"
will not be able to use them.
The performance for `git cat-file --batch-all-objects
--batch-check` on the Git repository itself with performance
testing tool `hyperfine` changes from 669.4 ms ± 31.1 ms to
1.134 s ± 0.063 s.
The performance for `git cat-file --batch-all-objects --batch
quoted
/dev/null` on the Git repository itself with performance testing
tool `time` change from "27.37s user 0.29s system 98% cpu 28.089
total" to "33.69s user 1.54s system 87% cpu 40.258 total".
Saying that a later patch will add a fast path which will mitigate the
performance regression introduced by this patch might help reassure
reviewers.
OK.
By the way it is not clear if adding the fast path fully mitigates
this performance regression or not. You might want to discuss that in
the cover letter, or maybe in the patch adding the fast path.
I mentioned it: "By using this fast path, we can reduce some of the
extra overhead
when cat-file --batch using ref-filter. The running time of
git cat-file --batch-check will be similar to before, and the
running time of git cat-file --batch will be 9.1% less than before."
which is using the result of t/perf/p1006-cat-file.sh.
I wonder if the above function and some of the tests below could be
introduced in a preparatory patch before this one. It could help check
that reusing ref-filter doesn't change the behavior with some atoms
that were previously supported or rejected. Of course if some atoms
are now failing or are now supported, then it's ok to add new tests
for these atoms in this patch.
Yes, it might be worth splitting into two commits.
+batch_test_atom refs/heads/main 'VALID'
+batch_test_atom refs/heads/main '%(INVALID)' fail
+batch_test_atom refs/heads/main '%(authordate:INVALID)' fail
+
+test_expect_success '%(rest) works with both a branch and a tag' '
+ cat >expected <<-EOF &&
+ 123 commit 123
+ 456 tag 456
+ EOF
+ git cat-file --batch-check="%(rest) %(objecttype) %(rest)" >actual <<-EOF &&
+ refs/heads/main 123
+ refs/tags/testtag 456
+ EOF
+ test_cmp expected actual
+'
It's a bit strange that this test is added in this patch while the
commit message doesn't talk about %(rest). So I wonder if this new
test could move to another previous commit.
It's just used for checking the uncommonly atoms "%(rest)".
But as you said, we can move it to a split commit.
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-12 11:47:28
From: ZheNing Hu <redacted>
Because "atom_type == ATOM_OBJECTNAME" implies the condition
of `starts_with(name, "objectname")`, "atom_type == ATOM_TREE"
implies the condition of `starts_with(name, "tree")`, so the
check for `starts_with(name, field)` in grab_oid() is redundant.
So Remove the grab_oid() from ref-filter, to reduce repeated check.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
ref-filter.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-12 11:47:28
From: ZheNing Hu <redacted>
Create p1006-cat-file.sh to provide performance testing for
`git cat-file --batch` and `git cat-file --batch-check`. This
will help us compare the performance changes after we let
cat-file reuse the ref-filter logic.
Helped-by: Ævar Arnfjörð Bjarmason [off-list ref]
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
t/perf/p1006-cat-file.sh | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
create mode 100755 t/perf/p1006-cat-file.sh
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-12 11:47:29
From: ZheNing Hu <redacted>
Add the member `default_format` to struct `batch_options`,
when we are using the default format on `git cat-file --batch`,
or `git cat-file --batch-check`, `default_format` will be set,
if we don't use `--textconv` or `--filter`, then we will not call
verify_ref_format(), has_object_file() and format_ref_array_item().
Instead, we get the object data directly through
oid_object_info_extended() and then output the data directly.
By using this fast path, we can reduce some of the extra overhead
when `cat-file --batch` using ref-filter. The running time of
`git cat-file --batch-check` will be similar to before, and the
running time of `git cat-file --batch` will be 9.1% less than before.
Helped-by: Ævar Arnfjörð Bjarmason [off-list ref]
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
builtin/cat-file.c | 79 +++++++++++++++++++++++++++++++++-------------
1 file changed, 57 insertions(+), 22 deletions(-)
@@ -26,6 +26,7 @@ struct batch_options {intall_objects;intunordered;intcmdmode;/* may be 'w' or 'c' for --filters or --textconv */+intdefault_format;structref_formatformat;};
@@ -216,27 +218,58 @@ static void batch_object_write(const char *obj_name,structbatch_options*opt,structexpand_data*data){-intret;-structref_array_itemitem={data->oid,data->rest,opt->cmdmode};--strbuf_reset(scratch);-strbuf_reset(err);--ret=format_ref_array_item(&item,&opt->format,scratch,err);-if(ret<0)-die("%s\n",err->buf);-if(ret){-/* ret > 0 means when the object corresponding to oid-*cannotbefoundinformat_ref_array_item(),weonlyprint-*theerrormessage.-*/-printf("%s\n",err->buf);+if(opt->default_format&&!opt->cmdmode){+structstrbuftype_name=STRBUF_INIT;+unsignedlongsize;+void*content;++if(opt->print_contents)+data->info.contentp=&content;++data->info.type_name=&type_name;+data->info.sizep=&size;++if(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));+fflush(stdout);+return;+}++fprintf(stdout,"%s %s %"PRIuMAX"\n",oid_to_hex(&data->oid),+data->info.type_name->buf,+(uintmax_t)*data->info.sizep);fflush(stdout);+strbuf_release(&type_name);+if(opt->print_contents){+batch_write(opt,content,*data->info.sizep);+batch_write(opt,"\n",1);+free(content);+}}else{-strbuf_addch(scratch,'\n');-batch_write(opt,scratch->buf,scratch->len);+intret;+structref_array_itemitem={data->oid,data->rest,opt->cmdmode};++strbuf_reset(scratch);+strbuf_reset(err);++ret=format_ref_array_item(&item,&opt->format,scratch,err);+if(ret<0)+die("%s\n",err->buf);+if(ret){+/* ret > 0 means when the object corresponding to oid+*cannotbefoundinformat_ref_array_item(),weonlyprint+*theerrormessage.+*/+printf("%s\n",err->buf);+fflush(stdout);+}else{+strbuf_addch(scratch,'\n');+batch_write(opt,scratch->buf,scratch->len);+}+free_ref_array_item_value(&item);}-free_ref_array_item_value(&item);}staticvoidbatch_one_object(constchar*obj_name,
From: Christian Couder <hidden> Date: 2021-07-12 12:36:28
On Mon, Jul 12, 2021 at 1:47 PM ZheNing Hu via GitGitGadget
[off-list ref] wrote:
This patch series make cat-file reuse ref-filter logic.
s/make/makes/
By the way if you have already sent some of the patches in this series
(and if they haven't changed much since when you sent them), it's a
good idea to use V2 or V3, V4, etc, so we can easily refer to each of
the versions you sent. (See the `-v` option of `git format-patch`.)
Change from last version:
1. Declare buf_size in if (atom_type == ATOM_RAW) block.
2. Modify the code style of the test.
3. Delete "use_textconv" and "use_filter" flag. Instead, add member
s/flag/flags/
cat_file_cmdmode to struct ref_array_item.
4. Add function reject_atom() to enhance the readability of the code.
5. Create p1006-cat-file.sh for performance regression testing.
6. Use a "fast path" to output object data to reduce the performance
degradation of cat-file --batch with the suggest of Ævar Arnfjörð
Bjarmason.
Maybe:
s/with the suggest of Ævar Arnfjörð Bjarmason/as suggested by Ævar
Arnfjörð Bjarmason/
or:
s/with the suggest of Ævar Arnfjörð Bjarmason/according to Ævar
Arnfjörð Bjarmason's suggestion/
ZheNing Hu (19):
cat-file: handle trivial --batch format with --batch-all-objects
cat-file: merge two block into one
It's a bit strange that the above 2 don't have [GSOC] while the others
below have it.
[GSOC] ref-filter: add obj-type check in grab contents
[GSOC] ref-filter: add %(raw) atom
[GSOC] ref-filter: --format=%(raw) re-support --perl
[GSOC] ref-filter: use non-const ref_format in *_atom_parser()
[GSOC] ref-filter: add %(rest) atom
[GSOC] ref-filter: pass get_object() return value to their callers
[GSOC] ref-filter: introduce free_ref_array_item_value() function
[GSOC] ref-filter: introduce reject_atom()
[GSOC] ref-filter: modify the error message and value in get_object
[GSOC] cat-file: add has_object_file() check
[GSOC] cat-file: change batch_objects parameter name
[GSOC] cat-file: reuse ref-filter logic
[GSOC] cat-file: reuse err buf in batch_object_write()
[GSOC] cat-file: re-implement --textconv, --filters options
[GSOC] ref-filter: remove grab_oid() function
[GSOC] cat-file: create p1006-cat-file.sh
Maybe you could add the new perf test earlier in the series so that we
could see how performance changes when ref-filter logic is reused in
cat-file earlier in the series.
[GSOC] cat-file: use fast path when using default_format
From: ZheNing Hu <hidden> Date: 2021-07-12 13:01:12
Christian Couder [off-list ref] 于2021年7月12日周一 下午8:36写道：
On Mon, Jul 12, 2021 at 1:47 PM ZheNing Hu via GitGitGadget
[off-list ref] wrote:
quoted
This patch series make cat-file reuse ref-filter logic.
s/make/makes/
By the way if you have already sent some of the patches in this series
(and if they haven't changed much since when you sent them), it's a
good idea to use V2 or V3, V4, etc, so we can easily refer to each of
the versions you sent. (See the `-v` option of `git format-patch`.)
Yes, but my patch was generated by gitgitgadget, when I want to send
a new patch which is totally different with before, I will send a new PR.
GGG will not know the connection between these two PRs. So the
version number of the patch will be reset.
quoted
Change from last version:
1. Declare buf_size in if (atom_type == ATOM_RAW) block.
2. Modify the code style of the test.
3. Delete "use_textconv" and "use_filter" flag. Instead, add member
s/flag/flags/
quoted
cat_file_cmdmode to struct ref_array_item.
4. Add function reject_atom() to enhance the readability of the code.
5. Create p1006-cat-file.sh for performance regression testing.
6. Use a "fast path" to output object data to reduce the performance
degradation of cat-file --batch with the suggest of Ævar Arnfjörð
Bjarmason.
Maybe:
s/with the suggest of Ævar Arnfjörð Bjarmason/as suggested by Ævar
Arnfjörð Bjarmason/
or:
s/with the suggest of Ævar Arnfjörð Bjarmason/according to Ævar
Arnfjörð Bjarmason's suggestion/
quoted
ZheNing Hu (19):
cat-file: handle trivial --batch format with --batch-all-objects
cat-file: merge two block into one
It's a bit strange that the above 2 don't have [GSOC] while the others
below have it.
That's because it's belong to the branch zh/cat-file-batch-fix. I
should mention it
in the cover-letter.
quoted
[GSOC] ref-filter: add obj-type check in grab contents
[GSOC] ref-filter: add %(raw) atom
[GSOC] ref-filter: --format=%(raw) re-support --perl
[GSOC] ref-filter: use non-const ref_format in *_atom_parser()
[GSOC] ref-filter: add %(rest) atom
[GSOC] ref-filter: pass get_object() return value to their callers
[GSOC] ref-filter: introduce free_ref_array_item_value() function
[GSOC] ref-filter: introduce reject_atom()
[GSOC] ref-filter: modify the error message and value in get_object
[GSOC] cat-file: add has_object_file() check
[GSOC] cat-file: change batch_objects parameter name
[GSOC] cat-file: reuse ref-filter logic
[GSOC] cat-file: reuse err buf in batch_object_write()
[GSOC] cat-file: re-implement --textconv, --filters options
[GSOC] ref-filter: remove grab_oid() function
[GSOC] cat-file: create p1006-cat-file.sh
Maybe you could add the new perf test earlier in the series so that we
could see how performance changes when ref-filter logic is reused in
cat-file earlier in the series.
Make sence.
quoted
[GSOC] cat-file: use fast path when using default_format
From: Philip Oakley <hidden> Date: 2021-07-12 13:02:13
On 12/07/2021 12:46, ZheNing Hu via GitGitGadget wrote:
This patch series make cat-file reuse ref-filter logic.
Change from last version:
minor nit..
Not sure if this is a gitgitgadget feature, but would it be possible
that a version indication be included in future versions of the patch,
e.g. [PATCH vN 00/19] [GSOC] ?
--
Philip
From: ZheNing Hu <hidden> Date: 2021-07-12 13:27:14
Philip Oakley [off-list ref] 于2021年7月12日周一 下午9:02写道：
On 12/07/2021 12:46, ZheNing Hu via GitGitGadget wrote:
quoted
This patch series make cat-file reuse ref-filter logic.
Change from last version:
minor nit..
Not sure if this is a gitgitgadget feature, but would it be possible
that a version indication be included in future versions of the patch,
e.g. [PATCH vN 00/19] [GSOC] ?
--
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-15 15:40:31
From: ZheNing Hu <redacted>
Only tag and commit objects use `grab_sub_body_contents()` to grab
object contents in the current codebase. We want to teach the
function to also handle blobs and trees to get their raw data,
without parsing a blob (whose contents looks like a commit or a tag)
incorrectly as a commit or a tag.
Skip the block of code that is specific to handling commits and tags
early when the given object is of a wrong type to help later
addition to handle other types of objects in this function.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Helped-by: Junio C Hamano [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
ref-filter.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
@@ -1678,7 +1684,7 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **objreturnstrbuf_addf_ret(err,-1,_("parse_object_buffer failed on %s for %s"),oid_to_hex(&oi->oid),ref->refname);}-grab_values(ref->value,deref,*obj,oi->content);+grab_values(ref->value,deref,*obj,oi);}grab_common_values(ref->value,deref,oi);
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-15 15:40:33
From: ZheNing Hu <redacted>
Add new formatting option `%(raw)`, which will print the raw
object data without any changes. It will help further to migrate
all cat-file formatting logic from cat-file to ref-filter.
The raw data of blob, tree objects may contain '\0', but most of
the logic in `ref-filter` depends on the output of the atom being
text (specifically, no embedded NULs in it).
E.g. `quote_formatting()` use `strbuf_addstr()` or `*._quote_buf()`
add the data to the buffer. The raw data of a tree object is
`100644 one\0...`, only the `100644 one` will be added to the buffer,
which is incorrect.
Therefore, we need to find a way to record the length of the
atom_value's member `s`. Although strbuf can already record the
string and its length, if we want to replace the type of atom_value's
member `s` with strbuf, many places in ref-filter that are filled
with dynamically allocated mermory in `v->s` are not easy to replace.
At the same time, we need to check if `v->s == NULL` in
populate_value(), and strbuf cannot easily distinguish NULL and empty
strings, but c-style "const char *" can do it. So add a new member in
`struct atom_value`: `s_size`, which can record raw object size, it
can help us add raw object data to the buffer or compare two buffers
which contain raw object data.
Note that `--format=%(raw)` cannot be used with `--python`, `--shell`,
`--tcl`, and `--perl` because if the binary raw data is passed to a
variable in such languages, these may not support arbitrary binary data
in their string variable type.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Helped-by: Bagas Sanjaya [off-list ref]
Helped-by: Ævar Arnfjörð Bjarmason [off-list ref]
Helped-by: Felipe Contreras [off-list ref]
Helped-by: Phillip Wood [off-list ref]
Helped-by: Junio C Hamano [off-list ref]
Based-on-patch-by: Olga Telezhnaya [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
Documentation/git-for-each-ref.txt | 9 ++
ref-filter.c | 140 +++++++++++++++----
t/t6300-for-each-ref.sh | 216 +++++++++++++++++++++++++++++
3 files changed, 338 insertions(+), 27 deletions(-)
@@ -235,6 +235,15 @@ and `date` to extract the named component. For email fields (`authoremail`, without angle brackets, and `:localpart` to get the part before the `@` symbol out of the trimmed email.+The raw data in an object is `raw`.++raw:size::+ The raw data size of the object.++Note that `--format=%(raw)` can not be used with `--python`, `--shell`, `--tcl`,+`--perl` because such language may not support arbitrary binary data in their+string variable type.+ The message in a commit or a tag object is `contents`, from which `contents:<part>` can be used to extract various parts out of:
@@ -620,12 +637,15 @@ struct ref_formatting_state {structatom_value{constchar*s;+size_ts_size;int(*handler)(structatom_value*atomv,structref_formatting_state*state,structstrbuf*err);uintmax_tvalue;/* used for sorting when not FIELD_STR */structused_atom*atom;};+#define ATOM_VALUE_S_SIZE_INIT (-1)+/**Usedtoparseformatstringandsortspecifiers*/
@@ -644,13 +664,6 @@ static int parse_ref_filter_atom(const struct ref_format *format,returnstrbuf_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++){-intlen=strlen(used_atom[i].name);-if(len==ep-atom&&!memcmp(used_atom[i].name,atom,len))-returni;-}-/**Iftheatomnamehasacolon,stripitandeverythingafter*itoff-itspecifiestheformatforthisentry,and
@@ -660,6 +673,13 @@ static int parse_ref_filter_atom(const struct ref_format *format,arg=memchr(sp,':',ep-sp);atom_len=(arg?arg:ep)-sp;+/* Do we have the atom already used elsewhere? */+for(i=0;i<used_atom_cnt;i++){+intlen=strlen(used_atom[i].name);+if(len==ep-atom&&!memcmp(used_atom[i].name,atom,len))+returni;+}+/* Is the atom a valid one? */for(i=0;i<ARRAY_SIZE(valid_atom);i++){intlen=strlen(valid_atom[i].name);
@@ -709,11 +729,14 @@ static int parse_ref_filter_atom(const struct ref_format *format,returnat;}-staticvoidquote_formatting(structstrbuf*s,constchar*str,intquote_style)+staticvoidquote_formatting(structstrbuf*s,constchar*str,size_tlen,intquote_style){switch(quote_style){caseQUOTE_NONE:-strbuf_addstr(s,str);+if(len!=ATOM_VALUE_S_SIZE_INIT)+strbuf_add(s,str,len);+else+strbuf_addstr(s,str);break;caseQUOTE_SHELL:sq_quote_buf(s,str);
@@ -867,18 +895,22 @@ static int then_atom_handler(struct atom_value *atomv, struct ref_formatting_staif(if_then_else->else_atom_seen)returnstrbuf_addf_ret(err,-1,_("format: %%(then) atom used after %%(else)"));if_then_else->then_atom_seen=1;+if(if_then_else->str)+str_len=strlen(if_then_else->str);/**Ifthe'equals'or'notequals'attributeisusedthen*performtherequiredcomparison.Ifnot,onlynon-empty*stringssatisfythe'if'condition.*/if(if_then_else->cmp_status==COMPARE_EQUAL){-if(!strcmp(if_then_else->str,cur->output.buf))+if(str_len==cur->output.len&&+!memcmp(if_then_else->str,cur->output.buf,cur->output.len))if_then_else->condition_satisfied=1;}elseif(if_then_else->cmp_status==COMPARE_UNEQUAL){-if(strcmp(if_then_else->str,cur->output.buf))+if(str_len!=cur->output.len||+memcmp(if_then_else->str,cur->output.buf,cur->output.len))if_then_else->condition_satisfied=1;-}elseif(cur->output.len&&!is_empty(cur->output.buf))+}elseif(cur->output.len&&!is_empty(&cur->output))if_then_else->condition_satisfied=1;strbuf_reset(&cur->output);return0;
@@ -974,6 +1006,10 @@ int verify_ref_format(struct ref_format *format)at=parse_ref_filter_atom(format,sp+2,ep,&err);if(at<0)die("%s",err.buf);+if(format->quote_style&&used_atom[at].atom_type==ATOM_RAW&&+used_atom[at].u.raw_data.option==RAW_BARE)+die(_("--format=%.*s cannot be used with"+"--python, --shell, --tcl, --perl"),(int)(ep-sp-2),sp+2);cp=ep+1;if(skip_prefix(used_atom[at].name,"color:",&color))
@@ -708,6 +737,15 @@ test_atom refs/tags/signed-long contents "subject line bodycontents$sig"+test_expect_successGPG'basic atom: refs/tags/signed-long raw''+gitcat-filetagrefs/tags/signed-long>expected&&+gitfor-each-ref--format="%(raw)"refs/tags/signed-long>actual&&+sanitize_pgp<expected>expected.clean&&+echo>>expected.clean&&+sanitize_pgp<actual>actual.clean&&+test_cmpexpected.cleanactual.clean+'+ test_expect_success'set up refs pointing to tree and blob''gitupdate-refrefs/mytrees/firstrefs/heads/main^{tree}&&gitupdate-refrefs/myblobs/firstrefs/heads/main:one
@@ -727,6 +775,174 @@ test_atom refs/myblobs/first contents:body "" test_atomrefs/myblobs/firstcontents:signature"" test_atomrefs/myblobs/firstcontents""+test_expect_success'basic atom: refs/myblobs/first raw''+gitcat-fileblobrefs/myblobs/first>expected&&+echo>>expected&&+gitfor-each-ref--format="%(raw)"refs/myblobs/first>actual&&+test_cmpexpectedactual&&+gitcat-file-srefs/myblobs/first>expected&&+gitfor-each-ref--format="%(raw:size)"refs/myblobs/first>actual&&+test_cmpexpectedactual+'++test_expect_success'set up refs pointing to binary blob''+printf"a\0b\0c">blob1&&+printf"a\0c\0b">blob2&&+printf"\0a\0b\0c">blob3&&+printf"abc">blob4&&+printf"\0 \0 \0 ">blob5&&+printf"\0 \0a\0 ">blob6&&+printf" ">blob7&&+>blob8&&+obj=$(githash-object-wblob1)&&+gitupdate-refrefs/myblobs/blob1"$obj"&&+obj=$(githash-object-wblob2)&&+gitupdate-refrefs/myblobs/blob2"$obj"&&+obj=$(githash-object-wblob3)&&+gitupdate-refrefs/myblobs/blob3"$obj"&&+obj=$(githash-object-wblob4)&&+gitupdate-refrefs/myblobs/blob4"$obj"&&+obj=$(githash-object-wblob5)&&+gitupdate-refrefs/myblobs/blob5"$obj"&&+obj=$(githash-object-wblob6)&&+gitupdate-refrefs/myblobs/blob6"$obj"&&+obj=$(githash-object-wblob7)&&+gitupdate-refrefs/myblobs/blob7"$obj"&&+obj=$(githash-object-wblob8)&&+gitupdate-refrefs/myblobs/blob8"$obj"+'++test_expect_success'Verify sorts with raw''+cat>expected<<-EOF&&+refs/myblobs/blob8+refs/myblobs/blob5+refs/myblobs/blob6+refs/myblobs/blob3+refs/myblobs/blob7+refs/mytrees/first+refs/myblobs/first+refs/myblobs/blob1+refs/myblobs/blob2+refs/myblobs/blob4+refs/heads/main+EOF+gitfor-each-ref--format="%(refname)"--sort=raw\+refs/heads/mainrefs/myblobs/refs/mytrees/first>actual&&+test_cmpexpectedactual+'++test_expect_success'Verify sorts with raw:size''+cat>expected<<-EOF&&+refs/myblobs/blob8+refs/myblobs/first+refs/myblobs/blob7+refs/heads/main+refs/myblobs/blob4+refs/myblobs/blob1+refs/myblobs/blob2+refs/myblobs/blob3+refs/myblobs/blob5+refs/myblobs/blob6+refs/mytrees/first+EOF+gitfor-each-ref--format="%(refname)"--sort=raw:size\+refs/heads/mainrefs/myblobs/refs/mytrees/first>actual&&+test_cmpexpectedactual+'++test_expect_success'validate raw atom with %(if:equals)''+cat>expected<<-EOF&&+notequals+notequals+notequals+notequals+notequals+notequals+refs/myblobs/blob4+notequals+notequals+notequals+notequals+notequals+EOF+gitfor-each-ref--format="%(if:equals=abc)%(raw)%(then)%(refname)%(else)not equals%(end)"\+refs/myblobs/refs/heads/>actual&&+test_cmpexpectedactual+'++test_expect_success'validate raw atom with %(if:notequals)''+cat>expected<<-EOF&&+refs/heads/ambiguous+refs/heads/main+refs/heads/newtag+refs/myblobs/blob1+refs/myblobs/blob2+refs/myblobs/blob3+equals+refs/myblobs/blob5+refs/myblobs/blob6+refs/myblobs/blob7+refs/myblobs/blob8+refs/myblobs/first+EOF+gitfor-each-ref--format="%(if:notequals=abc)%(raw)%(then)%(refname)%(else)equals%(end)"\+refs/myblobs/refs/heads/>actual&&+test_cmpexpectedactual+'++test_expect_success'empty raw refs with %(if)''+cat>expected<<-EOF&&+refs/myblobs/blob1notempty+refs/myblobs/blob2notempty+refs/myblobs/blob3notempty+refs/myblobs/blob4notempty+refs/myblobs/blob5notempty+refs/myblobs/blob6notempty+refs/myblobs/blob7empty+refs/myblobs/blob8empty+refs/myblobs/firstnotempty+EOF+gitfor-each-ref--format="%(refname) %(if)%(raw)%(then)not empty%(else)empty%(end)"\+refs/myblobs/>actual&&+test_cmpexpectedactual+'++test_expect_success'%(raw) with --python must fail''+test_must_failgitfor-each-ref--format="%(raw)"--python+'++test_expect_success'%(raw) with --tcl must fail''+test_must_failgitfor-each-ref--format="%(raw)"--tcl+'++test_expect_success'%(raw) with --perl must fail''+test_must_failgitfor-each-ref--format="%(raw)"--perl+'++test_expect_success'%(raw) with --shell must fail''+test_must_failgitfor-each-ref--format="%(raw)"--shell+'++test_expect_success'%(raw) with --shell and --sort=raw must fail''+test_must_failgitfor-each-ref--format="%(raw)"--sort=raw--shell+'++test_expect_success'%(raw:size) with --shell''+gitfor-each-ref--format="%(raw:size)"|whilereadline+do+echo"'\''$line'\''">>expect+done&&+gitfor-each-ref--format="%(raw:size)"--shell>actual&&+test_cmpexpectactual+'++test_expect_success'for-each-ref --format compare with cat-file --batch''+gitrev-parserefs/mytrees/first|gitcat-file--batch>expected&&+gitfor-each-ref--format="%(objectname) %(objecttype) %(objectsize)+%(raw)" refs/mytrees/first >actual &&+test_cmpexpectedactual+'+ test_expect_success'set up multiple-sort tags''forwhenin100000200000do
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-15 15:40:35
From: ZheNing Hu <redacted>
Because the perl language can handle binary data correctly,
add the function perl_quote_buf_with_len(), which can specify
the length of the data and prevent the data from being truncated
at '\0' to help `--format="%(raw)"` re-support `--perl`.
Helped-by: Ævar Arnfjörð Bjarmason [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
Documentation/git-for-each-ref.txt | 4 ++--
quote.c | 17 +++++++++++++++++
quote.h | 1 +
ref-filter.c | 15 +++++++++++----
t/t6300-for-each-ref.sh | 19 +++++++++++++++++--
5 files changed, 48 insertions(+), 8 deletions(-)
@@ -241,8 +241,8 @@ raw:size:: The raw data size of the object. Note that `--format=%(raw)` can not be used with `--python`, `--shell`, `--tcl`,-`--perl` because such language may not support arbitrary binary data in their-string variable type.+because such language may not support arbitrary binary data in their string+variable type. The message in a commit or a tag object is `contents`, from which `contents:<part>` can be used to extract various parts out of:
@@ -94,6 +94,7 @@ char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne/* quoting as a string literal for other languages */voidperl_quote_buf(structstrbuf*sb,constchar*src);+voidperl_quote_buf_with_len(structstrbuf*sb,constchar*src,size_tlen);voidpython_quote_buf(structstrbuf*sb,constchar*src);voidtcl_quote_buf(structstrbuf*sb,constchar*src);voidbasic_regex_quote_buf(structstrbuf*sb,constchar*src);
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-15 15:40:37
From: ZheNing Hu <redacted>
Use non-const ref_format in *_atom_parser(), which can help us
modify the members of ref_format in *_atom_parser().
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
builtin/tag.c | 2 +-
ref-filter.c | 44 ++++++++++++++++++++++----------------------
ref-filter.h | 4 ++--
3 files changed, 25 insertions(+), 25 deletions(-)
@@ -116,7 +116,7 @@ void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);voidref_sorting_set_sort_flags_all(structref_sorting*sorting,unsignedintmask,inton);/* Based on the given format and quote_style, fill the strbuf */intformat_ref_array_item(structref_array_item*info,-conststructref_format*format,+structref_format*format,structstrbuf*final_buf,structstrbuf*error_buf);/* Parse a single sort specifier and add it to the list */
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-15 15:40:38
From: ZheNing Hu <redacted>
Because in the refactor of `git cat-file --batch` later,
oid_object_info_extended() in get_object() will be used to obtain
the info of an object with it's oid. When the object cannot be
obtained in the git repository, `cat-file --batch` expects to output
"<oid> missing" and continue the next oid query instead of letting
Git exit. In other error conditions, Git should exit normally. So we
can achieve this function by passing the return value of get_object().
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Helped-by: Ævar Arnfjörð Bjarmason [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
ref-filter.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-15 15:40:39
From: ZheNing Hu <redacted>
When we use ref_array_item which is not dynamically allocated and
want to free the space of its member "value" after the end of use,
free_array_item() does not meet our needs, because it tries to free
ref_array_item itself and its member "symref".
Introduce free_ref_array_item_value() for freeing ref_array_item value.
It will be called internally by free_array_item(), and it will help
`cat-file --batch` free ref_array_item's value memory later.
Helped-by: Junio C Hamano [off-list ref]
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
ref-filter.c | 11 ++++++++---
ref-filter.h | 2 ++
2 files changed, 10 insertions(+), 3 deletions(-)
@@ -111,6 +111,8 @@ struct ref_format {intfilter_refs(structref_array*array,structref_filter*filter,unsignedinttype);/* Clear all memory allocated to ref_array */voidref_array_clear(structref_array*array);+/* Free ref_array_item's value */+voidfree_ref_array_item_value(structref_array_item*item);/* Used to verify if the given format is correct and to parse out the used atoms */intverify_ref_format(structref_format*format);/* Sort the given ref_array as per the ref_sorting provided */
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-15 15:40:39
From: ZheNing Hu <redacted>
In order to let "cat-file --batch=%(rest)" use the ref-filter
interface, add %(rest) atom for ref-filter. Introduce the
reject_atom() to reject the atom %(rest) for "git for-each-ref",
"git branch", "git tag" and "git verify-tag".
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
ref-filter.c | 25 +++++++++++++++++++++++++
ref-filter.h | 5 ++++-
t/t3203-branch-output.sh | 4 ++++
t/t6300-for-each-ref.sh | 4 ++++
t/t7004-tag.sh | 4 ++++
t/t7030-verify-tag.sh | 4 ++++
6 files changed, 45 insertions(+), 1 deletion(-)
@@ -559,6 +560,15 @@ static int if_atom_parser(struct ref_format *format, struct used_atom *atom,return0;}+staticintrest_atom_parser(structref_format*format,structused_atom*atom,+constchar*arg,structstrbuf*err)+{+if(arg)+returnstrbuf_addf_ret(err,-1,_("%%(rest) does not take arguments"));+format->use_rest=1;+return0;+}+staticinthead_atom_parser(structref_format*format,structused_atom*atom,constchar*arg,structstrbuf*unused_err){
@@ -1998,6 +1998,10 @@ test_expect_success '--format should list tags as per format given' 'test_cmpexpectactual'+test_expect_success'git tag -l with --format="%(rest)" must fail''+test_must_failgittag-l--format="%(rest)""v1*"+'+ test_expect_success"set up color tests"'echo"<RED>v1.0<RESET>">expect.color&&echo"v1.0">expect.bare&&
@@ -194,6 +194,10 @@ test_expect_success GPG 'verifying tag with --format' 'test_cmpexpectactual'+test_expect_successGPG'verifying tag with --format="%(rest)" must fail''+test_must_failgitverify-tag--format="%(rest)""fourth-signed"+'+ test_expect_successGPG'verifying a forged tag with --format should fail silently''test_must_failgitverify-tag--format="tagname : %(tag)"$(catforged1.tag)>actual-forged&&test_must_be_emptyactual-forged
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-15 15:40:40
From: ZheNing Hu <redacted>
Let get_object() return 1 and print "<oid> missing" instead
of returning -1 and printing "missing object <oid> for <refname>"
if oid_object_info_extended() unable to find the data corresponding
to oid. When `cat-file --batch` use ref-filter logic later it can
help `format_ref_array_item()` just report that the object is missing
without letting Git exit.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
ref-filter.c | 4 ++--
t/t6301-for-each-ref-errors.sh | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
@@ -1762,8 +1762,8 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj}if(oid_object_info_extended(the_repository,&oi->oid,&oi->info,OBJECT_INFO_LOOKUP_REPLACE))-returnstrbuf_addf_ret(err,-1,_("missing object %s for %s"),-oid_to_hex(&oi->oid),ref->refname);+returnstrbuf_addf_ret(err,1,_("%s missing"),+oid_to_hex(&oi->oid));if(oi->info.disk_sizep&&oi->disk_size<0)BUG("Object size is less than zero.");
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-15 15:40:41
From: ZheNing Hu <redacted>
Use `has_object_file()` in `batch_one_object()` to check
whether the input object exists. This can help us reject
the missing oid when we let `cat-file --batch` use ref-filter
logic later.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
builtin/cat-file.c | 7 +++++++
1 file changed, 7 insertions(+)
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-15 15:40:42
From: ZheNing Hu <redacted>
Because later cat-file reuses ref-filter logic that will add
parameter "const struct option *options" to batch_objects(),
the two synonymous parameters of "opt" and "options" may
confuse readers, so change batch_options parameter of
batch_objects() from "opt" to "batch".
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
builtin/cat-file.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
@@ -513,20 +513,20 @@ 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,batch->format,expand_format,&data);data.mark_query=0;strbuf_release(&output);-if(opt->cmdmode)+if(batch->cmdmode)data.split_on_whitespace=1;/**Ifweareprintingouttheobject,thenalwaysfillinthetype,*sincewewillwanttodecidewhetherornottostream.*/-if(opt->print_contents)+if(batch->print_contents)data.info.typep=&data.type;-if(opt->all_objects){+if(batch->all_objects){structobject_cb_datacb;structobject_infoempty=OBJECT_INFO_INIT;
@@ -536,11 +536,11 @@ static int batch_objects(struct batch_options *opt)if(has_promisor_remote())warning("This repository uses promisor remotes. Some objects may not be loaded.");-cb.opt=opt;+cb.opt=batch;cb.expand=&data;cb.scratch=&output;-if(opt->unordered){+if(batch->unordered){structoidsetseen=OIDSET_INIT;cb.seen=&seen;
@@ -590,7 +590,7 @@ static int batch_objects(struct batch_options *opt)data.rest=p;}-batch_one_object(input.buf,&output,opt,&data);+batch_one_object(input.buf,&output,batch,&data);}strbuf_release(&input);
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-15 15:40:42
From: ZheNing Hu <redacted>
Create p1006-cat-file.sh to provide performance testing for
`git cat-file --batch` and `git cat-file --batch-check`. This
will help us compare the performance changes after we let
cat-file reuse the ref-filter logic.
Helped-by: Ævar Arnfjörð Bjarmason [off-list ref]
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
t/perf/p1006-cat-file.sh | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
create mode 100755 t/perf/p1006-cat-file.sh
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-15 15:40:44
From: ZheNing Hu <redacted>
Add `cat_file_mode` member to struct `ref_format`. When
`cat-file --batch` use ref-filter logic later, it can help us
reject atoms in verify_ref_format() which cat-file cannot use,
e.g. `%(refname)`, `%(push)`, `%(upstream)"...
Add batch_test_atom() to t/t1006-cat-file.sh and add check
for cat-file --batch, this can help us clearly show which
atoms cat-file accepts and which atoms it rejects.
Helped-by: Eric Sunshine [off-list ref]
Helped-by: Ævar Arnfjörð Bjarmason [off-list ref]
Helped-by: Christian Couder [off-list ref]
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
ref-filter.c | 23 ++++-
ref-filter.h | 1 +
t/t1006-cat-file.sh | 226 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 246 insertions(+), 4 deletions(-)
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-15 15:40:44
From: ZheNing Hu <redacted>
After cat-file reuses the ref-filter logic, we re-implement the
functions of --textconv and --filters options.
Add members `cat_file_cmdmode` to struct `ref_array_item`,
so that struct `batch_option` member `cmdmode` will be passed
to ref-filter, and then ref-filter will take use of it to filter
the content of the object in get_object().
Use `actual_oi` to record the real expand_data: it may point to the
original `oi` or the `act_oi` processed by `textconv_object()` or
`convert_to_working_tree()`. `grab_values()` will grab the contents
of `actual_oi` and `grab_common_values()` to grab the contents of origin
`oi`, this ensures that `%(objectsize)` still uses the size of the
unfiltered data.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
builtin/cat-file.c | 2 +-
ref-filter.c | 35 +++++++++++++++++++++++++++++++++--
ref-filter.h | 1 +
3 files changed, 35 insertions(+), 3 deletions(-)
@@ -1755,6 +1756,9 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj{/* parse_object_buffer() will set eaten to 0 if free() will be needed */inteaten=1;+structexpand_data*actual_oi=oi;+structexpand_dataact_oi={0};+if(oi->info.contentp){/* We need to know that to use parse_object_buffer properly */oi->info.sizep=&oi->size;
@@ -1768,19 +1772,45 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **objBUG("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);+if((ref->cat_file_cmdmode=='c'||ref->cat_file_cmdmode=='w')&&!ref->rest)+returnstrbuf_addf_ret(err,-1,_("missing path for '%s'"),+oid_to_hex(&act_oi.oid));+if(oi->type==OBJ_BLOB){+if(ref->cat_file_cmdmode=='c'){+act_oi=*oi;+if(textconv_object(the_repository,+ref->rest,0100644,&act_oi.oid,+1,(char**)(&act_oi.content),&act_oi.size))+actual_oi=&act_oi;+}elseif(ref->cat_file_cmdmode=='w'){+structstrbufstrbuf=STRBUF_INIT;+structcheckout_metadatameta;+act_oi=*oi;++init_checkout_metadata(&meta,NULL,NULL,&act_oi.oid);+if(!convert_to_working_tree(&the_index,ref->rest,act_oi.content,act_oi.size,&strbuf,&meta))+die("could not convert '%s' %s",+oid_to_hex(&oi->oid),ref->rest);+act_oi.size=strbuf.len;+act_oi.content=strbuf_detach(&strbuf,NULL);+actual_oi=&act_oi;+}+}+*obj=parse_object_buffer(the_repository,&actual_oi->oid,actual_oi->type,actual_oi->size,actual_oi->content,&eaten);if(!*obj){if(!eaten)free(oi->content);returnstrbuf_addf_ret(err,-1,_("parse_object_buffer failed on %s for %s"),oid_to_hex(&oi->oid),ref->refname);}-grab_values(ref->value,deref,*obj,oi);+grab_values(ref->value,deref,*obj,actual_oi);}grab_common_values(ref->value,deref,oi);if(!eaten)free(oi->content);+if(actual_oi!=oi)+free(actual_oi->content);return0;}
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-15 15:40:46
From: ZheNing Hu <redacted>
Reuse the `err` buffer in batch_object_write(), as the
buffer `scratch` does. This will reduce the overhead
of multiple allocations of memory of the err buffer.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
builtin/cat-file.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
@@ -212,35 +212,36 @@ static void batch_write(struct batch_options *opt, const void *data, int len)staticvoidbatch_object_write(constchar*obj_name,structstrbuf*scratch,+structstrbuf*err,structbatch_options*opt,structexpand_data*data){intret;-structstrbuferr=STRBUF_INIT;structref_array_itemitem={data->oid,data->rest};strbuf_reset(scratch);+strbuf_reset(err);-ret=format_ref_array_item(&item,&opt->format,scratch,&err);+ret=format_ref_array_item(&item,&opt->format,scratch,err);if(ret<0)-die("%s\n",err.buf);+die("%s\n",err->buf);if(ret){/* ret > 0 means when the object corresponding to oid*cannotbefoundinformat_ref_array_item(),weonlyprint*theerrormessage.*/-printf("%s\n",err.buf);+printf("%s\n",err->buf);fflush(stdout);}else{strbuf_addch(scratch,'\n');batch_write(opt,scratch->buf,scratch->len);}free_ref_array_item_value(&item);-strbuf_release(&err);}staticvoidbatch_one_object(constchar*obj_name,structstrbuf*scratch,+structstrbuf*err,structbatch_options*opt,structexpand_data*data){
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-15 15:40:46
From: ZheNing Hu <redacted>
In order to let cat-file use ref-filter logic, let's do the
following:
1. Change the type of member `format` in struct `batch_options`
to `ref_format`, we will pass it to ref-filter later.
2. Let `batch_objects()` add atoms to format, and use
`verify_ref_format()` to check atoms.
3. Use `format_ref_array_item()` in `batch_object_write()` to
get the formatted data corresponding to the object. If the
return value of `format_ref_array_item()` is equals to zero,
use `batch_write()` to print object data; else if the return
value is less than zero, use `die()` to print the error message
and exit; else if return value is greater than zero, only print
the error message, but don't exit.
4. Use free_ref_array_item_value() to free ref_array_item's
value.
Most of the atoms in `for-each-ref --format` are now supported,
such as `%(tree)`, `%(parent)`, `%(author)`, `%(tagger)`, `%(if)`,
`%(then)`, `%(else)`, `%(end)`. But these atoms will be rejected:
`%(refname)`, `%(symref)`, `%(upstream)`, `%(push)`, `%(worktreepath)`,
`%(flag)`, `%(HEAD)`, because these atoms are unique to those objects
that pointed to by a ref, "for-each-ref"'s family can naturally use
these atoms, but not all objects are pointed to be a ref, so "cat-file"
will not be able to use them.
The performance for `git cat-file --batch-all-objects
--batch-check` on the Git repository itself with performance
testing tool `hyperfine` changes from 616.7 ms ± 8.9 ms to
758.7 ms ± 16.4 ms.
The performance for `git cat-file --batch-all-objects --batch
/dev/null` on the Git repository itself with performance testing
tool `time` change from "25.26s user 0.30s system 98% cpu 25.840 total"
to "28.79s user 0.83s system 99% cpu 29.829 total".
The reasons for the performance degradation are as follows:
1. There are a lot of data copies in the logic of ref-filter.
2, In order to be able to support more useful formats, complex
data structure and parsing process are used in ref-filter.
A later patch will add a fast path which will mitigate the
performance regression introduced by this patch.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
Documentation/git-cat-file.txt | 6 +
builtin/cat-file.c | 242 +++++++--------------------------
t/t1006-cat-file.sh | 229 ++++++++++++++++---------------
3 files changed, 174 insertions(+), 303 deletions(-)
@@ -226,6 +226,12 @@ newline. The available atoms are: after that first run of whitespace (i.e., the "rest" of the line) are output in place of the `%(rest)` atom.+Note that most of the atoms in `for-each-ref --format` are now supported,+such as `%(tree)`, `%(parent)`, `%(author)`, `%(tagger)`, `%(if)`,+`%(then)`, `%(else)`, `%(end)`. But these atoms will be rejected:+`%(refname)`, `%(symref)`, `%(upstream)`, `%(push)`, `%(worktreepath)`,+`%(flag)`, `%(HEAD)`. See linkgit:git-for-each-ref[1].+ If no format is specified, the default format is `%(objectname) %(objecttype) %(objectsize)`.
@@ -25,7 +26,7 @@ struct batch_options {intall_objects;intunordered;intcmdmode;/* may be 'w' or 'c' for --filters or --textconv */-constchar*format;+structref_formatformat;};staticconstchar*force_path;
@@ -195,99 +196,10 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,structexpand_data{structobject_idoid;-enumobject_typetype;-unsignedlongsize;-off_tdisk_size;constchar*rest;-structobject_iddelta_base_oid;--/*-*Ifmark_queryistrue,wedonotexpandanything,butrather-*justmarktheobject_infowithitemswewishtoquery.-*/-intmark_query;--/*-*Whethertosplittheinputonwhitespacebeforefeedingitto-*get_sha1;thisisdecidedduringthemark_queryphasebasedon-*whetherwehavea%(rest)tokeninourformat.-*/intsplit_on_whitespace;--/*-*Afteramark_queryrun,thisobject_infoissetuptobe-*passedtooid_object_info_extended.Itwillpointtothedata-*elementsabove,soyoucanretrievetheresponsefromthere.-*/-structobject_infoinfo;--/*-*Thisflagwillbetrueiftherequestedbatchformatandoptions-*don'trequireustocalloid_object_info,whichcanthenbe-*optimizedout.-*/-unsignedskip_object_info:1;};-staticintis_atom(constchar*atom,constchar*s,intslen)-{-intalen=strlen(atom);-returnalen==slen&&!memcmp(atom,s,alen);-}--staticvoidexpand_atom(structstrbuf*sb,constchar*atom,intlen,-void*vdata)-{-structexpand_data*data=vdata;--if(is_atom("objectname",atom,len)){-if(!data->mark_query)-strbuf_addstr(sb,oid_to_hex(&data->oid));-}elseif(is_atom("objecttype",atom,len)){-if(data->mark_query)-data->info.typep=&data->type;-else-strbuf_addstr(sb,type_name(data->type));-}elseif(is_atom("objectsize",atom,len)){-if(data->mark_query)-data->info.sizep=&data->size;-else-strbuf_addf(sb,"%"PRIuMAX,(uintmax_t)data->size);-}elseif(is_atom("objectsize:disk",atom,len)){-if(data->mark_query)-data->info.disk_sizep=&data->disk_size;-else-strbuf_addf(sb,"%"PRIuMAX,(uintmax_t)data->disk_size);-}elseif(is_atom("rest",atom,len)){-if(data->mark_query)-data->split_on_whitespace=1;-elseif(data->rest)-strbuf_addstr(sb,data->rest);-}elseif(is_atom("deltabase",atom,len)){-if(data->mark_query)-data->info.delta_base_oid=&data->delta_base_oid;-else-strbuf_addstr(sb,-oid_to_hex(&data->delta_base_oid));-}else-die("unknown format element: %.*s",len,atom);-}--staticsize_texpand_format(structstrbuf*sb,constchar*start,void*data)-{-constchar*end;--if(*start!='(')-return0;-end=strchr(start+1,')');-if(!end)-die("format element '%s' does not end in ')'",start);--expand_atom(sb,start+1,end-start-1,data);--returnend-start+1;-}-staticvoidbatch_write(structbatch_options*opt,constvoid*data,intlen){if(opt->buffer_output){
@@ -297,87 +209,34 @@ static void batch_write(struct batch_options *opt, const void *data, int len)write_or_die(1,data,len);}-staticvoidprint_object_or_die(structbatch_options*opt,structexpand_data*data)-{-conststructobject_id*oid=&data->oid;--assert(data->info.typep);--if(data->type==OBJ_BLOB){-if(opt->buffer_output)-fflush(stdout);-if(opt->cmdmode){-char*contents;-unsignedlongsize;--if(!data->rest)-die("missing path for '%s'",oid_to_hex(oid));--if(opt->cmdmode=='w'){-if(filter_object(data->rest,0100644,oid,-&contents,&size))-die("could not convert '%s' %s",-oid_to_hex(oid),data->rest);-}elseif(opt->cmdmode=='c'){-enumobject_typetype;-if(!textconv_object(the_repository,-data->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);-}else-BUG("invalid cmdmode: %c",opt->cmdmode);-batch_write(opt,contents,size);-free(contents);-}else{-stream_blob(oid);-}-}-else{-enumobject_typetype;-unsignedlongsize;-void*contents;--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));--batch_write(opt,contents,size);-free(contents);-}-}staticvoidbatch_object_write(constchar*obj_name,structstrbuf*scratch,structbatch_options*opt,structexpand_data*data){-if(!data->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));-fflush(stdout);-return;-}+intret;+structstrbuferr=STRBUF_INIT;+structref_array_itemitem={data->oid,data->rest};strbuf_reset(scratch);-strbuf_expand(scratch,opt->format,expand_format,data);-strbuf_addch(scratch,'\n');-batch_write(opt,scratch->buf,scratch->len);-if(opt->print_contents){-print_object_or_die(opt,data);-batch_write(opt,"\n",1);+ret=format_ref_array_item(&item,&opt->format,scratch,&err);+if(ret<0)+die("%s\n",err.buf);+if(ret){+/* ret > 0 means when the object corresponding to oid+*cannotbefoundinformat_ref_array_item(),weonlyprint+*theerrormessage.+*/+printf("%s\n",err.buf);+fflush(stdout);+}else{+strbuf_addch(scratch,'\n');+batch_write(opt,scratch->buf,scratch->len);}+free_ref_array_item_value(&item);+strbuf_release(&err);}staticvoidbatch_one_object(constchar*obj_name,
@@ -495,43 +354,37 @@ static int batch_unordered_packed(const struct object_id *oid,returnbatch_unordered_object(oid,data);}-staticintbatch_objects(structbatch_options*batch)+staticconstchar*constcat_file_usage[]={+N_("git cat-file (-t [--allow-unknown-type] | -s [--allow-unknown-type] | -e | -p | <type> | --textconv | --filters) [--path=<path>] <object>"),+N_("git cat-file (--batch[=<format>] | --batch-check[=<format>]) [--follow-symlinks] [--textconv | --filters]"),+NULL+};++staticintbatch_objects(structbatch_options*batch,conststructoption*options){structstrbufinput=STRBUF_INIT;structstrbufoutput=STRBUF_INIT;+structstrbufformat=STRBUF_INIT;structexpand_datadata;intsave_warning;intretval=0;-if(!batch->format)-batch->format="%(objectname) %(objecttype) %(objectsize)";--/*-*Expandoncewithourspecialmark_queryflag,whichwillprimethe-*object_infotobehandedtooid_object_info_extendedforeach-*object.-*/memset(&data,0,sizeof(data));-data.mark_query=1;-strbuf_expand(&output,batch->format,expand_format,&data);-data.mark_query=0;-strbuf_release(&output);-if(batch->cmdmode)-data.split_on_whitespace=1;--/*-*Ifweareprintingouttheobject,thenalwaysfillinthetype,-*sincewewillwanttodecidewhetherornottostream.-*/+if(batch->format.format)+strbuf_addstr(&format,batch->format.format);+else+strbuf_addstr(&format,"%(objectname) %(objecttype) %(objectsize)");if(batch->print_contents)-data.info.typep=&data.type;+strbuf_addstr(&format,"\n%(raw)");+batch->format.format=format.buf;+if(verify_ref_format(&batch->format))+usage_with_options(cat_file_usage,options);++if(batch->cmdmode||batch->format.use_rest)+data.split_on_whitespace=1;if(batch->all_objects){structobject_cb_datacb;-structobject_infoempty=OBJECT_INFO_INIT;--if(!memcmp(&data.info,&empty,sizeof(empty)))-data.skip_object_info=1;if(has_promisor_remote())warning("This repository uses promisor remotes. Some objects may not be loaded.");
@@ -561,6 +414,7 @@ static int batch_objects(struct batch_options *batch)oid_array_clear(&sa);}+strbuf_release(&format);strbuf_release(&output);return0;}
@@ -718,7 +570,7 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)batch.buffer_output=batch.all_objects;if(batch.enabled)-returnbatch_objects(&batch);+returnbatch_objects(&batch,options);if(unknown_type&&opt!='t'&&opt!='s')die("git cat-file --allow-unknown-type: use with -s or -t");
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-15 15:40:48
From: ZheNing Hu <redacted>
Because "atom_type == ATOM_OBJECTNAME" implies the condition
of `starts_with(name, "objectname")`, "atom_type == ATOM_TREE"
implies the condition of `starts_with(name, "tree")`, so the
check for `starts_with(name, field)` in grab_oid() is redundant.
So Remove the grab_oid() from ref-filter, to reduce repeated check.
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
ref-filter.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)
From: ZheNing Hu via GitGitGadget <hidden> Date: 2021-07-15 15:40:50
From: ZheNing Hu <redacted>
Add the member `default_format` to struct `batch_options`,
when we are using the default format on `git cat-file --batch`,
or `git cat-file --batch-check`, `default_format` will be set,
if we don't use `--textconv` or `--filter`, then we will not call
verify_ref_format(), has_object_file() and format_ref_array_item().
Instead, we get the object data directly through
oid_object_info_extended() and then output the data directly.
By using this fast path, we can reduce some of the extra overhead
when `cat-file --batch` using ref-filter. The running time of
`git cat-file --batch-check` will be similar to before, and the
running time of `git cat-file --batch` will be 9.1% less than before.
Helped-by: Ævar Arnfjörð Bjarmason [off-list ref]
Mentored-by: Christian Couder [off-list ref]
Mentored-by: Hariom Verma [off-list ref]
Signed-off-by: ZheNing Hu <redacted>
---
builtin/cat-file.c | 79 +++++++++++++++++++++++++++++++++-------------
1 file changed, 57 insertions(+), 22 deletions(-)
@@ -26,6 +26,7 @@ struct batch_options {intall_objects;intunordered;intcmdmode;/* may be 'w' or 'c' for --filters or --textconv */+intdefault_format;structref_formatformat;};
@@ -216,27 +218,58 @@ static void batch_object_write(const char *obj_name,structbatch_options*opt,structexpand_data*data){-intret;-structref_array_itemitem={data->oid,data->rest,opt->cmdmode};--strbuf_reset(scratch);-strbuf_reset(err);--ret=format_ref_array_item(&item,&opt->format,scratch,err);-if(ret<0)-die("%s\n",err->buf);-if(ret){-/* ret > 0 means when the object corresponding to oid-*cannotbefoundinformat_ref_array_item(),weonlyprint-*theerrormessage.-*/-printf("%s\n",err->buf);+if(opt->default_format&&!opt->cmdmode){+structstrbuftype_name=STRBUF_INIT;+unsignedlongsize;+void*content;++if(opt->print_contents)+data->info.contentp=&content;++data->info.type_name=&type_name;+data->info.sizep=&size;++if(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));+fflush(stdout);+return;+}++fprintf(stdout,"%s %s %"PRIuMAX"\n",oid_to_hex(&data->oid),+data->info.type_name->buf,+(uintmax_t)*data->info.sizep);fflush(stdout);+strbuf_release(&type_name);+if(opt->print_contents){+batch_write(opt,content,*data->info.sizep);+batch_write(opt,"\n",1);+free(content);+}}else{-strbuf_addch(scratch,'\n');-batch_write(opt,scratch->buf,scratch->len);+intret;+structref_array_itemitem={data->oid,data->rest,opt->cmdmode};++strbuf_reset(scratch);+strbuf_reset(err);++ret=format_ref_array_item(&item,&opt->format,scratch,err);+if(ret<0)+die("%s\n",err->buf);+if(ret){+/* ret > 0 means when the object corresponding to oid+*cannotbefoundinformat_ref_array_item(),weonlyprint+*theerrormessage.+*/+printf("%s\n",err->buf);+fflush(stdout);+}else{+strbuf_addch(scratch,'\n');+batch_write(opt,scratch->buf,scratch->len);+}+free_ref_array_item_value(&item);}-free_ref_array_item_value(&item);}staticvoidbatch_one_object(constchar*obj_name,