ref-filter: add %(raw) and %(rest) atoms
cover ref-filter: add %(raw) and %(rest) atoms
This patch series is split from my main patch series zh/ref-filter-raw-data, to make things easier for reviewers to read.
This patch series provided %(raw) and %(rest) for ref-filter, which will be used by zh/cat-file-reuse-ref-filter-logic later.
ZheNing Hu (5):
[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
Documentation/git-for-each-ref.txt | 9 ++ builtin/tag.c | 2 +- quote.c | 17 ++ quote.h | 1 + ref-filter.c | 240 ++++++++++++++++++++++------- ref-filter.h | 9 +- t/t3203-branch-output.sh | 4 + t/t6300-for-each-ref.sh | 235 ++++++++++++++++++++++++++++ t/t7004-tag.sh | 4 + t/t7030-verify-tag.sh | 4 + 10 files changed, 463 insertions(+), 62 deletions(-)
base-commit: daab8a564f8bbac55f70f8bf86c070e001a9b006
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1000%2Fadlternative%2Fref-filter-raw-data-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1000/adlternative/ref-filter-raw-data-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1000
Signature · 1 line
gitgitgadget
PATCH 1/5 ref-filter: add obj-type check in grab contents
From: ZheNing Hu <[email protected]>
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.
--- ref-filter.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)
ref-filter.c +15 -9
@@ -1356,11 +1356,12 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size }/* See grab_values */ -static void grab_sub_body_contents (struct atom_value *val,int deref,void *buf) +static void grab_sub_body_contents (struct atom_value *val,int deref,struct expand_data *data) {int i;const char *subpos =NULL , *bodypos =NULL , *sigpos =NULL ;size_t sublen =0 , bodylen =0 , nonsiglen =0 , siglen =0 ; +void *buf = data->content;for (i =0 ; i < used_atom_cnt; i++) {struct used_atom *atom = &used_atom[i];
@@ -1371,10 +1372,13 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)continue ;if (deref) name++; -if (strcmp (name,"body" ) && - !starts_with (name,"subject" ) && - !starts_with (name,"trailers" ) && - !starts_with (name,"contents" )) + +if ((data->type !=OBJ_TAG && + data->type !=OBJ_COMMIT ) || + (strcmp (name,"body" ) && + !starts_with (name,"subject" ) && + !starts_with (name,"trailers" ) && + !starts_with (name,"contents" )))continue ;if (!subpos) find_subpos(buf,
@@ -1438,17 +1442,19 @@ static void fill_missing_values(struct atom_value *val)* pointed at by the ref itself; otherwise it is the object the * ref (which is a tag) refers to. */ -static void grab_values (struct atom_value *val,int deref,struct object *obj,void *buf) +static void grab_values (struct atom_value *val,int deref,struct object *obj,struct expand_data *data) { +void *buf = data->content; +switch (obj->type) {case OBJ_TAG :grab_tag_values (val, deref, obj); -grab_sub_body_contents (val, deref, buf); +grab_sub_body_contents (val, deref, data);grab_person ("tagger" , val, deref, buf);break ;case OBJ_COMMIT :grab_commit_values (val, deref, obj); -grab_sub_body_contents (val, deref, buf); +grab_sub_body_contents (val, deref, data);grab_person ("author" , val, deref, buf);grab_person ("committer" , val, deref, buf);break ;
@@ -1678,7 +1684,7 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **objreturn strbuf_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);
Signature · 1 line
gitgitgadget
PATCH 2/5 ref-filter: add %(raw) atom
In reply to ZheNing Hu via GitGitGadgetFrom: ZheNing Hu <[email protected]>
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.
--- 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(-)
Documentation/git-for-each-ref.txt +9 -0
@@ -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:
ref-filter.c +113 -27
289 lines not on this page · raw message
t/t6300-for-each-ref.sh +216 -0
258 lines not on this page · raw message
Signature · 1 line
gitgitgadget
PATCH 3/5 ref-filter: --format=%(raw) re-support --perl
In reply to ZheNing Hu via GitGitGadgetFrom: ZheNing Hu <[email protected]>
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.
--- 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(-)
Documentation/git-for-each-ref.txt +2 -2
@@ -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:
quote.c +17 -0
@@ -471,6 +471,23 @@ void perl_quote_buf(struct strbuf *sb, const char *src)strbuf_addch (sb, sq); } +void perl_quote_buf_with_len (struct strbuf *sb,const char *src,size_t len) +{ +const char sq ='\'' ; +const char bq ='\\' ; +const char *c = src; +const char *end = src + len; + +strbuf_addch (sb, sq); +while (c != end) { +if (*c == sq || *c == bq) +strbuf_addch (sb, bq); +strbuf_addch (sb, *c); + c++; + } +strbuf_addch (sb, sq); +} +void python_quote_buf (struct strbuf *sb,const char *src) {const char sq ='\'' ;
quote.h +1 -0
@@ -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 */ void perl_quote_buf (struct strbuf *sb,const char *src); +void perl_quote_buf_with_len (struct strbuf *sb,const char *src,size_t len);void python_quote_buf (struct strbuf *sb,const char *src);void tcl_quote_buf (struct strbuf *sb,const char *src);void basic_regex_quote_buf (struct strbuf *sb,const char *src);
ref-filter.c +11 -4
@@ -742,7 +742,10 @@ static void quote_formatting(struct strbuf *s, const char *str, size_t len, intsq_quote_buf (s, str);break ;case QUOTE_PERL : -perl_quote_buf (s, str); +if (len !=ATOM_VALUE_S_SIZE_INIT ) +perl_quote_buf_with_len (s, str, len); +else +perl_quote_buf (s, str);break ;case QUOTE_PYTHON :python_quote_buf (s, str);
@@ -1006,10 +1009,14 @@ 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 ) + +if ((format->quote_style ==QUOTE_PYTHON || + format->quote_style ==QUOTE_SHELL || + format->quote_style ==QUOTE_TCL ) && + 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 ); +"--python, --shell, --tcl" ), (int )(ep - sp -2 ), sp +2 ); cp = ep +1 ;if (skip_prefix (used_atom[at].name,"color:" , &color))
t/t6300-for-each-ref.sh +17 -2
@@ -915,8 +915,23 @@ test_expect_success '%(raw) with --tcl must fail' ' test_must_fail git for-each-ref --format="%(raw)" --tcl ' -test_expect_success '%(raw) with --perl must fail' ' - test_must_fail git for-each-ref --format="%(raw)" --perl +test_expect_success '%(raw) with --perl' ' + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/blob1 --perl | perl >actual && + cmp blob1 actual && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/blob3 --perl | perl >actual && + cmp blob3 actual && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/blob8 --perl | perl >actual && + cmp blob8 actual && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/first --perl | perl >actual && + cmp one actual && + git cat-file tree refs/mytrees/first > expected && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/mytrees/first --perl | perl >actual && + cmp expected actual ' test_expect_success '%(raw) with --shell must fail' '
Signature · 1 line
gitgitgadget
PATCH 4/5 ref-filter: use non-const ref_format in *_atom_parser()
In reply to ZheNing Hu via GitGitGadgetFrom: ZheNing Hu <[email protected]>
Use non-const ref_format in *_atom_parser(), which can help us modify the members of ref_format in *_atom_parser().
--- builtin/tag.c | 2 +- ref-filter.c | 44 ++++++++++++++++++++++---------------------- ref-filter.h | 4 ++-- 3 files changed, 25 insertions(+), 25 deletions(-)
builtin/tag.c +1 -1
@@ -146,7 +146,7 @@ static int verify_tag(const char *name, const char *ref, const struct object_id *oid, void *cb_data) { int flags; -const struct ref_format *format = cb_data; +struct ref_format *format = cb_data; flags =GPG_VERIFY_VERBOSE ;if (format->format)
ref-filter.c +22 -22
164 lines not on this page · raw message
ref-filter.h +2 -2
@@ -116,7 +116,7 @@ void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);void ref_sorting_set_sort_flags_all (struct ref_sorting *sorting,unsigned int mask,int on);/* Based on the given format and quote_style, fill the strbuf */ int format_ref_array_item (struct ref_array_item *info, -const struct ref_format *format, +struct ref_format *format,struct strbuf *final_buf,struct strbuf *error_buf);/* Parse a single sort specifier and add it to the list */
@@ -137,7 +137,7 @@ void setup_ref_filter_porcelain_msg(void);* name must be a fully qualified refname. */ void pretty_print_ref (const char *name,const struct object_id *oid, -const struct ref_format *format); +struct ref_format *format);/* * Push a single ref onto the array; this can be used to construct your own
Signature · 1 line
gitgitgadget
PATCH 5/5 ref-filter: add %(rest) atom
In reply to ZheNing Hu via GitGitGadgetFrom: ZheNing Hu <[email protected]>
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".
--- 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(-)
ref-filter.c +25 -0
@@ -157,6 +157,7 @@ enum atom_type {ATOM_IF ,ATOM_THEN ,ATOM_ELSE , + ATOM_REST, };/*
@@ -559,6 +560,15 @@ static int if_atom_parser(struct ref_format *format, struct used_atom *atom,return 0 ; } +static int rest_atom_parser (struct ref_format *format,struct used_atom *atom, +const char *arg,struct strbuf *err) +{ +if (arg) +return strbuf_addf_ret (err,-1 ,_ ("%%(rest) does not take arguments" )); + format->use_rest =1 ; +return 0 ; +} +static int head_atom_parser (struct ref_format *format,struct used_atom *atom,const char *arg,struct strbuf *unused_err) {
@@ -615,6 +625,7 @@ static struct { [ATOM_IF] = { "if", SOURCE_NONE, FIELD_STR, if_atom_parser }, [ATOM_THEN] = { "then", SOURCE_NONE }, [ATOM_ELSE] = { "else", SOURCE_NONE }, + [ATOM_REST] = { "rest", SOURCE_NONE, FIELD_STR, rest_atom_parser },/* * Please update $__git_ref_fieldlist in git-completion.bash * when you add new atoms
@@ -989,6 +1000,11 @@ static const char *find_next(const char *cp)return NULL ; } +static int reject_atom (enum atom_type atom_type) +{ +return atom_type ==ATOM_REST ; +} +/* * Make sure the format string is well formed, and parse out * the used atoms.
@@ -1009,6 +1025,8 @@ 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 (reject_atom (used_atom[at].atom_type)) +die (_ ("this command reject atom %%(%.*s)" ), (int )(ep - sp -2 ), sp +2 ); if ((format->quote_style == QUOTE_PYTHON || format->quote_style == QUOTE_SHELL ||
@@ -1928,6 +1946,12 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) v->handler = else_atom_handler; v->s =xstrdup ("" );continue ; + }else if (atom_type ==ATOM_REST ) { +if (ref->rest) + v->s =xstrdup (ref->rest); +else + v->s =xstrdup ("" ); +continue ; }else continue ;
@@ -2145,6 +2169,7 @@ static struct ref_array_item *new_ref_array_item(const char *refname,FLEX_ALLOC_STR (ref, refname, refname);oidcpy (&ref->objectname, oid); + ref->rest =NULL ;return ref; }
ref-filter.h +4 -1
@@ -38,6 +38,7 @@ struct ref_sorting {struct ref_array_item {struct object_id objectname; +const char *rest;int flag;unsigned int kind;const char *symref;
@@ -76,14 +77,16 @@ struct ref_format {* verify_ref_format() afterwards to finalize. */ const char *format; +const char *rest;int quote_style; +int use_rest;int use_color;/* Internal state to ref-filter */ int need_color_reset_at_eol; }; -#define REF_FORMAT_INIT { NULL, 0, -1 } +#define REF_FORMAT_INIT { .use_color = -1 }/* Macros for checking --merged and --no-merged options */ #define _OPT_MERGED_NO_MERGED (option, filter, h) \
t/t3203-branch-output.sh +4 -0
@@ -340,6 +340,10 @@ test_expect_success 'git branch --format option' ' test_cmp expect actual ' +test_expect_success 'git branch with --format=%(rest) must fail' ' + test_must_fail git branch --format="%(rest)" >actual +' + test_expect_success 'worktree colors correct' ' cat >expect <<-EOF && * <GREEN>(HEAD detached from fromtag)<RESET>
t/t6300-for-each-ref.sh +4 -0
@@ -1211,6 +1211,10 @@ test_expect_success 'basic atom: head contents:trailers' ' test_cmp expect actual.clean ' +test_expect_success 'basic atom: rest must fail' ' + test_must_fail git for-each-ref --format="%(rest)" refs/heads/main +' + test_expect_success 'trailer parsing not fooled by --- line' ' git commit --allow-empty -F - <<-\EOF && this is the subject
t/t7004-tag.sh +4 -0
@@ -1998,6 +1998,10 @@ test_expect_success '--format should list tags as per format given' ' test_cmp expect actual ' +test_expect_success 'git tag -l with --format="%(rest)" must fail' ' + test_must_fail git tag -l --format="%(rest)" "v1*" +' + test_expect_success "set up color tests" ' echo "<RED>v1.0<RESET>" >expect.color && echo "v1.0" >expect.bare &&
t/t7030-verify-tag.sh +4 -0
@@ -194,6 +194,10 @@ test_expect_success GPG 'verifying tag with --format' ' test_cmp expect actual ' +test_expect_success GPG 'verifying tag with --format="%(rest)" must fail' ' + test_must_fail git verify-tag --format="%(rest)" "fourth-signed" +' + test_expect_success GPG 'verifying a forged tag with --format should fail silently' ' test_must_fail git verify-tag --format="tagname : %(tag)" $(cat forged1.tag) >actual-forged && test_must_be_empty actual-forged
Signature · 1 line
gitgitgadget
Quoted 3 lines · ZheNing Hu via GitGitGadget
This patch series is split from my main patch series zh/ref-filter-raw-data, to make things easier for reviewers to read.
Thanks for doing that!
Quoted 2 lines
This patch series provided %(raw) and %(rest) for ref-filter, which will be used by zh/cat-file-reuse-ref-filter-logic later.
Great!
It would be nice if you could also point to the previous times when you sent this to the mailing list, so that previous reviews can more easily be found and maybe checked.
Another nice improvement would be to link to a branch on GitHub or GitLab, so that people could more easily fetch your work and try it.
Re: [PATCH 2/5] [GSOC] ref-filter: add %(raw) atom
In reply to ZheNing Hu via GitGitGadgetQuoted 4 lines · ZheNing Hu via GitGitGadget
[..] 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.
I didn't look in detail, just one nit/comment, no need to re-roll for this:
ref-filter.c struct ref_formatting_state { · line 648
Quoted 11 lines
struct
atom_value{ref-filter.c +3
-0const char *s; +size_t s_size;int (*handler)(struct atom_value *atomv,struct ref_formatting_state *state,struct strbuf *err);uintmax_t value;/* used for sorting when not FIELD_STR */ struct used_atom *atom; }; +#define ATOM_VALUE_S_SIZE_INIT (-1) +
This and then later doing (this appears to be the only initialization?):
ref-filter.c int format_ref_array_item(struct ref_array_item *info, · line 2583
Quoted 6 lines
ref-filter.c +1
-0if (format->need_color_reset_at_eol) {struct atom_value resetv; + resetv.s_size =ATOM_VALUE_S_SIZE_INIT ; resetv.s =GIT_COLOR_RESET ;if (append_atom (&resetv, &state, error_buf)) {pop_stack_element (&state.stack);
Is a rather unusual pattern, more typical would be:
struct atom_value { .... }
#define ATOM_VALUE_INIT { \
.s_size = -1, \
}Then:
struct atom_value resetv = ATOM_VALUE_INIT;
You could keep that "#define ATOM_VALUE_S_SIZE_INIT (-1)" to check the init value, personally I think it's just fine to hardcode the -1 literal and do "size < 0" checks, which we us as convention in a lot of other places for "not initialized".
Quoted 9 lines · Christian Couder <[email protected]> 于2021年7月22日周四 下午4:08写道:
Quoted 3 lines · ZheNing Hu via GitGitGadget
This patch series is split from my main patch series zh/ref-filter-raw-data, to make things easier for reviewers to read.
Thanks for doing that!
Quoted 9 lines
Quoted 2 lines
This patch series provided %(raw) and %(rest) for ref-filter, which will be used by zh/cat-file-reuse-ref-filter-logic later.
Great!
It would be nice if you could also point to the previous times when you sent this to the mailing list, so that previous reviews can more easily be found and maybe checked.
Okey, I will do it next time.
Quoted 2 lines
Another nice improvement would be to link to a branch on GitHub or GitLab, so that people could more easily fetch your work and try it.
Thanks.
--
ZheNing Hu
Re: [PATCH 5/5] [GSOC] ref-filter: add %(rest) atom
In reply to ZheNing Hu via GitGitGadgetReview
Quoted 8 lines · ZheNing Hu via GitGitGadget
(quoted diff) +6
-0+ } else if (atom_type == ATOM_REST) { + if (ref->rest) + v->s = xstrdup(ref->rest); + else + v->s = xstrdup(""); + continue; } else continue;
Another light reading nit, maybe more readable as:
} else if (atom_type == ATOM_REST && ref->rest) {
...
} else if (atom_type == ATOM_REST) {
...
}
continue;But we can't do that "continue" at the end because there's two cases where we don't continue, I wondered if elimanting that special case would make it easier, patch below. Probably not worth it & feel free to ignore:
ref-filter.c | 63 +++++++++++++++++++++++++----------------------------------- 1 file changed, 26 insertions(+), 37 deletions(-)
ref-filter.c +26 -37
@@ -1890,18 +1890,26 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) name++; } -if (atom_type ==ATOM_REFNAME ) +if (atom_type ==ATOM_REFNAME ) { refname =get_refname (atom, ref); -else if (atom_type ==ATOM_WORKTREEPATH ) { -if (ref->kind ==FILTER_REFS_BRANCHES ) - v->s =get_worktree_path (atom, ref); +if (!deref) + v->s =xstrdup (refname);else - v->s =xstrdup ("" ); -continue ; - } -else if (atom_type ==ATOM_SYMREF ) + v->s =xstrfmt ("%s^{}" , refname); +free ((char *)refname); + }else if (atom_type ==ATOM_WORKTREEPATH && + ref->kind ==FILTER_REFS_BRANCHES ) { + v->s =get_worktree_path (atom, ref); + }else if (atom_type ==ATOM_WORKTREEPATH ) { + v->s =xstrdup ("" ); + }else if (atom_type ==ATOM_SYMREF ) { refname =get_symref (atom, ref); - else if (atom_type == ATOM_UPSTREAM) { +if (!deref) + v->s =xstrdup (refname); +else + v->s =xstrfmt ("%s^{}" , refname); +free ((char *)refname); + } else if (atom_type == ATOM_UPSTREAM) { const char *branch_name; /* only local branches may have an upstream */ if (!skip_prefix(ref->refname, "refs/heads/",
@@ -1916,7 +1924,6 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)fill_remote_ref_details (atom, refname, branch, &v->s); else v->s = xstrdup(""); -continue ; }else if (atom_type ==ATOM_PUSH && atom->u.remote_ref.push) {const char *branch_name; v->s =xstrdup ("" );
@@ -1935,10 +1942,8 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)/* We will definitely re-init v->s on the next line. */ free ((char *)v->s);fill_remote_ref_details (atom, refname, branch, &v->s); -continue ; }else if (atom_type ==ATOM_COLOR ) { v->s =xstrdup (atom->u.color); -continue ; }else if (atom_type ==ATOM_FLAG ) {char buf[256 ], *cp = buf;if (ref->flag &REF_ISSYMREF )
@@ -1951,24 +1956,20 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) *cp ='\0' ; v->s =xstrdup (buf +1 ); } -continue ; }else if (!deref && atom_type ==ATOM_OBJECTNAME ) { v->s =xstrdup (do_grab_oid ("objectname" , &ref->objectname, atom)); -continue ; + }else if (atom_type ==ATOM_HEAD && + atom->u.head && + !strcmp (ref->refname, atom->u.head)) { + v->s =xstrdup ("*" ); }else if (atom_type ==ATOM_HEAD ) { -if (atom->u.head && !strcmp (ref->refname, atom->u.head)) - v->s =xstrdup ("*" ); -else - v->s =xstrdup (" " ); -continue ; + v->s =xstrdup (" " ); }else if (atom_type ==ATOM_ALIGN ) { v->handler = align_atom_handler; v->s =xstrdup ("" ); -continue ; }else if (atom_type ==ATOM_END ) { v->handler = end_atom_handler; v->s =xstrdup ("" ); -continue ; }else if (atom_type ==ATOM_IF ) {const char *s;if (skip_prefix (name,"if:" , &s))
@@ -1976,29 +1977,17 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)else v->s =xstrdup ("" ); v->handler = if_atom_handler; -continue ; }else if (atom_type ==ATOM_THEN ) { v->handler = then_atom_handler; v->s =xstrdup ("" ); -continue ; }else if (atom_type ==ATOM_ELSE ) { v->handler = else_atom_handler; v->s =xstrdup ("" ); -continue ; + }else if (atom_type ==ATOM_REST && ref->rest) { + v->s =xstrdup (ref->rest); }else if (atom_type ==ATOM_REST ) { -if (ref->rest) - v->s =xstrdup (ref->rest); -else - v->s =xstrdup ("" ); -continue ; - }else -continue ; - -if (!deref) - v->s =xstrdup (refname); -else - v->s =xstrfmt ("%s^{}" , refname); -free ((char *)refname); + v->s =xstrdup ("" ); + } }for (i =0 ; i < used_atom_cnt; i++) {
Re: [PATCH 2/5] [GSOC] ref-filter: add %(raw) atom
In reply to Ævar Arnfjörð BjarmasonQuoted 21 lines · Ævar Arnfjörð Bjarmason <[email protected]> 于2021年7月22日周四 下午4:10写道:
Quoted 4 lines · ZheNing Hu via GitGitGadget
[..] 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.I didn't look in detail, just one nit/comment, no need to re-roll for this:
Quoted 9 lines
struct atom_value { const char *s;+
size_ts_size;
int (*handler)(structatom_value*atomv, structref_formatting_state*state,
struct strbuf *err);
uintmax_tvalue; /* used for sorting when notFIELD_STR*/ structused_atom*atom;
};
Quoted 4 lines
ref-filter.c struct ref_formatting_state { · line 648
Quoted 2 lines
ref-filter.c +2
-0+#define ATOM_VALUE_S_SIZE_INIT (-1) +This and then later doing (this appears to be the only initialization?):
Yes, "s_size = -1” is the only initialization.
Quoted 15 lines
Quoted 7 lines
if (format->need_color_reset_at_eol) { struct atom_value resetv; + resetv.s_size = ATOM_VALUE_S_SIZE_INIT; resetv.s = GIT_COLOR_RESET; if (append_atom(&resetv, &state, error_buf)) { pop_stack_element(&state.stack);Is a rather unusual pattern, more typical would be:
struct atom_value { .... } #define ATOM_VALUE_INIT { \ .s_size = -1, \ }
Quoted 8 lines
Then:
struct atom_value resetv = ATOM_VALUE_INIT;You could keep that "#define
ATOM_VALUE_S_SIZE_INIT(-1)" to check the init value, personally I think it's just fine to hardcode the -1 literal and do "size < 0" checks, which we us as convention in a lot of other places for "not initialized".
Maybe you are right, hard-coding "-1" is ok.
Thanks.
--
ZheNing Hu
Re: [PATCH 1/5] [GSOC] ref-filter: add obj-type check in grab contents
In reply to ZheNing Hu via GitGitGadgetQuoted 9 lines · ZheNing Hu via GitGitGadget
From: ZheNing Hu <[email protected]>
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.
Quoted 3 lines
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.
Small nit in case the series is rerolled: it looks like this commit would change only grab_sub_body_contents(), but there are a few changes to grab_values() too. Maybe you could add that it's needed to pass a struct expand_data *data instead of only void *buf to both grab_sub_body_contents() and grab_values() to be able to check the object type.
Quoted 79 lines
Mentored-by Christian Couder <[email protected]>Mentored-by Hariom Verma <[email protected]>Helped-by Junio C Hamano <[email protected]>Signed-off-by ZheNing Hu <[email protected]>--- ref-filter.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)ref-filter.c +15
-10@@ -1356,11 +1356,12 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size }/* See grab_values */ -static void grab_sub_body_contents (struct atom_value *val,int deref,void *buf) +static void grab_sub_body_contents (struct atom_value *val,int deref,struct expand_data *data) {int i;const char *subpos =NULL , *bodypos =NULL , *sigpos =NULL ;size_t sublen =0 , bodylen =0 , nonsiglen =0 , siglen =0 ; +void *buf = data->content;for (i =0 ; i < used_atom_cnt; i++) {struct used_atom *atom = &used_atom[i];@@ -1371,10 +1372,13 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)continue ;if (deref) name++; -if (strcmp (name,"body" ) && - !starts_with (name,"subject" ) && - !starts_with (name,"trailers" ) && - !starts_with (name,"contents" )) + +if ((data->type !=OBJ_TAG && + data->type !=OBJ_COMMIT ) || + (strcmp (name,"body" ) && + !starts_with (name,"subject" ) && + !starts_with (name,"trailers" ) && + !starts_with (name,"contents" )))continue ;if (!subpos) find_subpos(buf,@@ -1438,17 +1442,19 @@ static void fill_missing_values(struct atom_value *val)* pointed at by the ref itself; otherwise it is the object the * ref (which is a tag) refers to. */ -static void grab_values (struct atom_value *val,int deref,struct object *obj,void *buf) +static void grab_values (struct atom_value *val,int deref,struct object *obj,struct expand_data *data) { +void *buf = data->content; +switch (obj->type) {case OBJ_TAG :grab_tag_values (val, deref, obj); -grab_sub_body_contents (val, deref, buf); +grab_sub_body_contents (val, deref, data);grab_person ("tagger" , val, deref, buf);break ;case OBJ_COMMIT :grab_commit_values (val, deref, obj); -grab_sub_body_contents (val, deref, buf); +grab_sub_body_contents (val, deref, data);grab_person ("author" , val, deref, buf);grab_person ("committer" , val, deref, buf);break ;@@ -1678,7 +1684,7 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **objreturn strbuf_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); --gitgitgadget
Re: [PATCH 5/5] [GSOC] ref-filter: add %(rest) atom
In reply to Ævar Arnfjörð BjarmasonQuoted 23 lines · Ævar Arnfjörð Bjarmason <[email protected]> 于2021年7月22日周四 下午4:24写道:
Review
Quoted 8 lines · ZheNing Hu via GitGitGadget
(quoted diff) +6
-0+ } else if (atom_type == ATOM_REST) { + if (ref->rest) + v->s = xstrdup(ref->rest); + else + v->s = xstrdup(""); + continue; } else continue;Another light reading nit, maybe more readable as:
} else if (atom_type == ATOM_REST && ref->rest) { ... } else if (atom_type == ATOM_REST) { ... } continue;
Quoted 4 lines
But we can't do that "continue" at the end because there's two cases where we don't continue, I wondered if elimanting that special case would make it easier, patch below. Probably not worth it & feel free to ignore:
Yeah, the readability here is really bad, so many "continue" just to not execute part of the operation on refname. In fact, I have a similar patch (which will not
be sent to the mailing list for the time being), it use switch and case to replace if
and else. [1]
Quoted 117 lines
ref-filter.c | 63 +++++++++++++++++++++++++----------------------------------- 1 file changed, 26 insertions(+), 37 deletions(-)ref-filter.c +24
-32@@ -1890,18 +1890,26 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) name++; } -if (atom_type ==ATOM_REFNAME ) +if (atom_type ==ATOM_REFNAME ) { refname =get_refname (atom, ref); -else if (atom_type ==ATOM_WORKTREEPATH ) { -if (ref->kind ==FILTER_REFS_BRANCHES ) - v->s =get_worktree_path (atom, ref); +if (!deref) + v->s =xstrdup (refname);else - v->s =xstrdup ("" ); -continue ; - } -else if (atom_type ==ATOM_SYMREF ) + v->s =xstrfmt ("%s^{}" , refname); +free ((char *)refname); + }else if (atom_type ==ATOM_WORKTREEPATH && + ref->kind ==FILTER_REFS_BRANCHES ) { + v->s =get_worktree_path (atom, ref); + }else if (atom_type ==ATOM_WORKTREEPATH ) { + v->s =xstrdup ("" ); + }else if (atom_type ==ATOM_SYMREF ) { refname =get_symref (atom, ref); - else if (atom_type == ATOM_UPSTREAM) { +if (!deref) + v->s =xstrdup (refname); +else + v->s =xstrfmt ("%s^{}" , refname); +free ((char *)refname); + } else if (atom_type == ATOM_UPSTREAM) { const char *branch_name; /* only local branches may have an upstream */ if (!skip_prefix(ref->refname, "refs/heads/",@@ -1916,7 +1924,6 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)fill_remote_ref_details (atom, refname, branch, &v->s); else v->s = xstrdup(""); -continue ; }else if (atom_type ==ATOM_PUSH && atom->u.remote_ref.push) {const char *branch_name; v->s =xstrdup ("" );@@ -1935,10 +1942,8 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)/* We will definitely re-init v->s on the next line. */ free ((char *)v->s);fill_remote_ref_details (atom, refname, branch, &v->s); -continue ; }else if (atom_type ==ATOM_COLOR ) { v->s =xstrdup (atom->u.color); -continue ; }else if (atom_type ==ATOM_FLAG ) {char buf[256 ], *cp = buf;if (ref->flag &REF_ISSYMREF )@@ -1951,24 +1956,20 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) *cp ='\0' ; v->s =xstrdup (buf +1 ); } -continue ; }else if (!deref && atom_type ==ATOM_OBJECTNAME ) { v->s =xstrdup (do_grab_oid ("objectname" , &ref->objectname, atom)); -continue ; + }else if (atom_type ==ATOM_HEAD && + atom->u.head && + !strcmp (ref->refname, atom->u.head)) { + v->s =xstrdup ("*" ); }else if (atom_type ==ATOM_HEAD ) { -if (atom->u.head && !strcmp (ref->refname, atom->u.head)) - v->s =xstrdup ("*" ); -else - v->s =xstrdup (" " ); -continue ; + v->s =xstrdup (" " ); }else if (atom_type ==ATOM_ALIGN ) { v->handler = align_atom_handler; v->s =xstrdup ("" ); -continue ; }else if (atom_type ==ATOM_END ) { v->handler = end_atom_handler; v->s =xstrdup ("" ); -continue ; }else if (atom_type ==ATOM_IF ) {const char *s;if (skip_prefix (name,"if:" , &s))@@ -1976,29 +1977,17 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)else v->s =xstrdup ("" ); v->handler = if_atom_handler; -continue ; }else if (atom_type ==ATOM_THEN ) { v->handler = then_atom_handler; v->s =xstrdup ("" ); -continue ; }else if (atom_type ==ATOM_ELSE ) { v->handler = else_atom_handler; v->s =xstrdup ("" ); -continue ; + }else if (atom_type ==ATOM_REST && ref->rest) { + v->s =xstrdup (ref->rest); }else if (atom_type ==ATOM_REST ) { -if (ref->rest) - v->s =xstrdup (ref->rest); -else - v->s =xstrdup ("" ); -continue ; - }else -continue ; -
ref-filter.c static int populate_value(struct ref_array_item *ref, struct strbuf *err) · line 1993
Quoted 10 lines
ref-filter.c +2
-5-if (!deref) - v->s =xstrdup (refname); -else - v->s =xstrfmt ("%s^{}" , refname); -free ((char *)refname); + v->s =xstrdup ("" ); + } }for (i =0 ; i < used_atom_cnt; i++) {
[1]: https://github.com/adlternative/git/commit/bbc6ecba4c0f65e7328e571b0d38ff99f800dc09
Re: [PATCH 1/5] [GSOC] ref-filter: add obj-type check in grab contents
In reply to Christian CouderQuoted 16 lines · Christian Couder <[email protected]> 于2021年7月22日周四 下午4:51写道:
Quoted 9 lines · ZheNing Hu via GitGitGadget
From: ZheNing Hu <[email protected]>
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.Quoted 3 lines
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.
Quoted 7 lines
Small nit in case the series is rerolled: it looks like this commit would change only
grab_sub_body_contents(), but there are a few changes tograb_values()too. Maybe you could add that it's needed to pass astruct expand_data *datainstead of onlyvoid *bufto bothgrab_sub_body_contents()andgrab_values()to be able to check the object type.
Indeed so. I will add them.
Thanks.
--
ZheNing Hu
v2 cover ref-filter: add %(raw) and %(rest) atoms
In reply to ZheNing Hu via GitGitGadgetThis patch series is split from my main patch series zh/ref-filter-raw-data, to make things easier for reviewers to read. It's last version is here:
https://lore.kernel.org/git/[email protected]/
This patch series provided %(raw) and %(rest) for ref-filter, which will be used by zh/cat-file-reuse-ref-filter-logic later.
Change from last version:
1. Define ATOM_VALUE_INIT used to initialize struct atom_value and delete ATOM_VALUE_S_SIZE_INIT macro.
2. Provide more complete commit message.
ZheNing Hu (5):
[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
Documentation/git-for-each-ref.txt | 9 ++ builtin/tag.c | 2 +- quote.c | 17 ++ quote.h | 1 + ref-filter.c | 242 ++++++++++++++++++++++------- ref-filter.h | 9 +- t/t3203-branch-output.sh | 4 + t/t6300-for-each-ref.sh | 235 ++++++++++++++++++++++++++++ t/t7004-tag.sh | 4 + t/t7030-verify-tag.sh | 4 + 10 files changed, 464 insertions(+), 63 deletions(-)
base-commit: daab8a564f8bbac55f70f8bf86c070e001a9b006
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1000%2Fadlternative%2Fref-filter-raw-data-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1000/adlternative/ref-filter-raw-data-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1000
Range-diff vs v1:
1: a7b4e45f14a ! 1: d77268bb188 [GSOC] ref-filter: add obj-type check in grab contents
@@ Commit message
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.
+ incorrectly as a commit or a tag. So it's needed to pass a
+ struct expand_data *data instread of only void *buf to both
+ grab_sub_body_contents() and grab_values() to be able to check
+ the object type.
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
2: ecd41b370e6 ! 2: eafb79bad62 [GSOC] ref-filter: add %(raw) atom
@@ ref-filter.c: struct ref_formatting_state {
struct used_atom *atom;
};
-+#define ATOM_VALUE_S_SIZE_INIT (-1)
++#define ATOM_VALUE_INIT { \
++ .s_size = -1 \
++}
+
/*
* Used to parse format string and sort specifiers
@@ ref-filter.c: static int parse_ref_filter_atom(const struct ref_format *format,
switch (quote_style) {
case QUOTE_NONE:
- strbuf_addstr(s, str);
-+ if (len != ATOM_VALUE_S_SIZE_INIT)
++ if (len != -1)
+ strbuf_add(s, str, len);
+ else
+ strbuf_addstr(s, str);
@@ ref-filter.c: static int append_atom(struct atom_value *v, struct ref_formatting
+ quote_formatting(&state->stack->output, v->s, v->s_size, state->quote_style);
else
- strbuf_addstr(&state->stack->output, v->s);
-+ if (v->s_size != ATOM_VALUE_S_SIZE_INIT)
++ if (v->s_size != -1)
+ strbuf_add(&state->stack->output, v->s, v->s_size);
+ else
+ strbuf_addstr(&state->stack->output, v->s);
@@ ref-filter.c: static int populate_value(struct ref_array_item *ref, struct strbu
const char *refname;
struct branch *branch = NULL;
-+ v->s_size = ATOM_VALUE_S_SIZE_INIT;
++ v->s_size = -1;
v->handler = append_atom;
v->atom = atom;
@@ ref-filter.c: static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array
- cmp_fn = s->sort_flags & REF_SORTING_ICASE
- ? strcasecmp : strcmp;
- cmp = cmp_fn(va->s, vb->s);
-+ if (va->s_size == ATOM_VALUE_S_SIZE_INIT &&
-+ vb->s_size == ATOM_VALUE_S_SIZE_INIT) {
++ if (va->s_size == -1 && vb->s_size == -1) {
+ int (*cmp_fn)(const char *, const char *);
+ cmp_fn = s->sort_flags & REF_SORTING_ICASE
+ ? strcasecmp : strcmp;
+ cmp = cmp_fn(va->s, vb->s);
+ } else {
-+ size_t a_size = va->s_size == ATOM_VALUE_S_SIZE_INIT ?
++ size_t a_size = va->s_size == -1 ?
+ strlen(va->s) : va->s_size;
-+ size_t b_size = vb->s_size == ATOM_VALUE_S_SIZE_INIT ?
++ size_t b_size = vb->s_size == -1 ?
+ strlen(vb->s) : vb->s_size;
+ int (*cmp_fn)(const void *, const void *, size_t);
+ cmp_fn = s->sort_flags & REF_SORTING_ICASE
@@ ref-filter.c: static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array
if (va->value < vb->value)
cmp = -1;
@@ ref-filter.c: int format_ref_array_item(struct ref_array_item *info,
+ append_literal(cp, sp, &state);
}
if (format->need_color_reset_at_eol) {
- struct atom_value resetv;
-+ resetv.s_size = ATOM_VALUE_S_SIZE_INIT;
+- struct atom_value resetv;
++ struct atom_value resetv = ATOM_VALUE_INIT;
resetv.s = GIT_COLOR_RESET;
if (append_atom(&resetv, &state, error_buf)) {
pop_stack_element(&state.stack);
3: b6438fd4308 ! 3: 00a29dd16d7 [GSOC] ref-filter: --format=%(raw) re-support --perl
@@ ref-filter.c: static void quote_formatting(struct strbuf *s, const char *str, si
break;
case QUOTE_PERL:
- perl_quote_buf(s, str);
-+ if (len != ATOM_VALUE_S_SIZE_INIT)
++ if (len != -1)
+ perl_quote_buf_with_len(s, str, len);
+ else
+ perl_quote_buf(s, str);
4: 33d332c664e = 4: 3a8173b42ed [GSOC] ref-filter: use non-const ref_format in *_atom_parser()
5: 75eb2f6740e = 5: 71249dfd4e0 [GSOC] ref-filter: add %(rest) atomSignature · 1 line
gitgitgadget
PATCH v2 1/5 ref-filter: add obj-type check in grab contents
From: ZheNing Hu <[email protected]>
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. So it's needed to pass a struct expand_data *data instread of only void *buf to both grab_sub_body_contents() and grab_values() to be able to check the object type.
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.
--- ref-filter.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)
ref-filter.c +15 -9
@@ -1356,11 +1356,12 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size }/* See grab_values */ -static void grab_sub_body_contents (struct atom_value *val,int deref,void *buf) +static void grab_sub_body_contents (struct atom_value *val,int deref,struct expand_data *data) {int i;const char *subpos =NULL , *bodypos =NULL , *sigpos =NULL ;size_t sublen =0 , bodylen =0 , nonsiglen =0 , siglen =0 ; +void *buf = data->content;for (i =0 ; i < used_atom_cnt; i++) {struct used_atom *atom = &used_atom[i];
@@ -1371,10 +1372,13 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)continue ;if (deref) name++; -if (strcmp (name,"body" ) && - !starts_with (name,"subject" ) && - !starts_with (name,"trailers" ) && - !starts_with (name,"contents" )) + +if ((data->type !=OBJ_TAG && + data->type !=OBJ_COMMIT ) || + (strcmp (name,"body" ) && + !starts_with (name,"subject" ) && + !starts_with (name,"trailers" ) && + !starts_with (name,"contents" )))continue ;if (!subpos) find_subpos(buf,
@@ -1438,17 +1442,19 @@ static void fill_missing_values(struct atom_value *val)* pointed at by the ref itself; otherwise it is the object the * ref (which is a tag) refers to. */ -static void grab_values (struct atom_value *val,int deref,struct object *obj,void *buf) +static void grab_values (struct atom_value *val,int deref,struct object *obj,struct expand_data *data) { +void *buf = data->content; +switch (obj->type) {case OBJ_TAG :grab_tag_values (val, deref, obj); -grab_sub_body_contents (val, deref, buf); +grab_sub_body_contents (val, deref, data);grab_person ("tagger" , val, deref, buf);break ;case OBJ_COMMIT :grab_commit_values (val, deref, obj); -grab_sub_body_contents (val, deref, buf); +grab_sub_body_contents (val, deref, data);grab_person ("author" , val, deref, buf);grab_person ("committer" , val, deref, buf);break ;
@@ -1678,7 +1684,7 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **objreturn strbuf_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);
Signature · 1 line
gitgitgadget
PATCH v2 3/5 ref-filter: --format=%(raw) re-support --perl
In reply to ZheNing Hu via GitGitGadgetFrom: ZheNing Hu <[email protected]>
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.
--- 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(-)
Documentation/git-for-each-ref.txt +2 -2
@@ -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:
quote.c +17 -0
@@ -471,6 +471,23 @@ void perl_quote_buf(struct strbuf *sb, const char *src)strbuf_addch (sb, sq); } +void perl_quote_buf_with_len (struct strbuf *sb,const char *src,size_t len) +{ +const char sq ='\'' ; +const char bq ='\\' ; +const char *c = src; +const char *end = src + len; + +strbuf_addch (sb, sq); +while (c != end) { +if (*c == sq || *c == bq) +strbuf_addch (sb, bq); +strbuf_addch (sb, *c); + c++; + } +strbuf_addch (sb, sq); +} +void python_quote_buf (struct strbuf *sb,const char *src) {const char sq ='\'' ;
quote.h +1 -0
@@ -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 */ void perl_quote_buf (struct strbuf *sb,const char *src); +void perl_quote_buf_with_len (struct strbuf *sb,const char *src,size_t len);void python_quote_buf (struct strbuf *sb,const char *src);void tcl_quote_buf (struct strbuf *sb,const char *src);void basic_regex_quote_buf (struct strbuf *sb,const char *src);
ref-filter.c +11 -4
@@ -744,7 +744,10 @@ static void quote_formatting(struct strbuf *s, const char *str, size_t len, intsq_quote_buf (s, str);break ;case QUOTE_PERL : -perl_quote_buf (s, str); +if (len !=-1 ) +perl_quote_buf_with_len (s, str, len); +else +perl_quote_buf (s, str);break ;case QUOTE_PYTHON :python_quote_buf (s, str);
@@ -1008,10 +1011,14 @@ 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 ) + +if ((format->quote_style ==QUOTE_PYTHON || + format->quote_style ==QUOTE_SHELL || + format->quote_style ==QUOTE_TCL ) && + 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 ); +"--python, --shell, --tcl" ), (int )(ep - sp -2 ), sp +2 ); cp = ep +1 ;if (skip_prefix (used_atom[at].name,"color:" , &color))
t/t6300-for-each-ref.sh +17 -2
@@ -915,8 +915,23 @@ test_expect_success '%(raw) with --tcl must fail' ' test_must_fail git for-each-ref --format="%(raw)" --tcl ' -test_expect_success '%(raw) with --perl must fail' ' - test_must_fail git for-each-ref --format="%(raw)" --perl +test_expect_success '%(raw) with --perl' ' + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/blob1 --perl | perl >actual && + cmp blob1 actual && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/blob3 --perl | perl >actual && + cmp blob3 actual && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/blob8 --perl | perl >actual && + cmp blob8 actual && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/first --perl | perl >actual && + cmp one actual && + git cat-file tree refs/mytrees/first > expected && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/mytrees/first --perl | perl >actual && + cmp expected actual ' test_expect_success '%(raw) with --shell must fail' '
Signature · 1 line
gitgitgadget
PATCH v2 2/5 ref-filter: add %(raw) atom
In reply to ZheNing Hu via GitGitGadgetFrom: ZheNing Hu <[email protected]>
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.
--- Documentation/git-for-each-ref.txt | 9 ++ ref-filter.c | 142 +++++++++++++++---- t/t6300-for-each-ref.sh | 216 +++++++++++++++++++++++++++++ 3 files changed, 339 insertions(+), 28 deletions(-)
Documentation/git-for-each-ref.txt +9 -0
@@ -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:
ref-filter.c +114 -28
291 lines not on this page · raw message
t/t6300-for-each-ref.sh +216 -0
258 lines not on this page · raw message
Signature · 1 line
gitgitgadget
PATCH v2 4/5 ref-filter: use non-const ref_format in *_atom_parser()
In reply to ZheNing Hu via GitGitGadgetFrom: ZheNing Hu <[email protected]>
Use non-const ref_format in *_atom_parser(), which can help us modify the members of ref_format in *_atom_parser().
--- builtin/tag.c | 2 +- ref-filter.c | 44 ++++++++++++++++++++++---------------------- ref-filter.h | 4 ++-- 3 files changed, 25 insertions(+), 25 deletions(-)
builtin/tag.c +1 -1
@@ -146,7 +146,7 @@ static int verify_tag(const char *name, const char *ref, const struct object_id *oid, void *cb_data) { int flags; -const struct ref_format *format = cb_data; +struct ref_format *format = cb_data; flags =GPG_VERIFY_VERBOSE ;if (format->format)
ref-filter.c +22 -22
164 lines not on this page · raw message
ref-filter.h +2 -2
@@ -116,7 +116,7 @@ void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);void ref_sorting_set_sort_flags_all (struct ref_sorting *sorting,unsigned int mask,int on);/* Based on the given format and quote_style, fill the strbuf */ int format_ref_array_item (struct ref_array_item *info, -const struct ref_format *format, +struct ref_format *format,struct strbuf *final_buf,struct strbuf *error_buf);/* Parse a single sort specifier and add it to the list */
@@ -137,7 +137,7 @@ void setup_ref_filter_porcelain_msg(void);* name must be a fully qualified refname. */ void pretty_print_ref (const char *name,const struct object_id *oid, -const struct ref_format *format); +struct ref_format *format);/* * Push a single ref onto the array; this can be used to construct your own
Signature · 1 line
gitgitgadget
PATCH v2 5/5 ref-filter: add %(rest) atom
In reply to ZheNing Hu via GitGitGadgetFrom: ZheNing Hu <[email protected]>
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".
--- 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(-)
ref-filter.c +25 -0
@@ -157,6 +157,7 @@ enum atom_type {ATOM_IF ,ATOM_THEN ,ATOM_ELSE , + ATOM_REST, };/*
@@ -559,6 +560,15 @@ static int if_atom_parser(struct ref_format *format, struct used_atom *atom,return 0 ; } +static int rest_atom_parser (struct ref_format *format,struct used_atom *atom, +const char *arg,struct strbuf *err) +{ +if (arg) +return strbuf_addf_ret (err,-1 ,_ ("%%(rest) does not take arguments" )); + format->use_rest =1 ; +return 0 ; +} +static int head_atom_parser (struct ref_format *format,struct used_atom *atom,const char *arg,struct strbuf *unused_err) {
@@ -615,6 +625,7 @@ static struct { [ATOM_IF] = { "if", SOURCE_NONE, FIELD_STR, if_atom_parser }, [ATOM_THEN] = { "then", SOURCE_NONE }, [ATOM_ELSE] = { "else", SOURCE_NONE }, + [ATOM_REST] = { "rest", SOURCE_NONE, FIELD_STR, rest_atom_parser },/* * Please update $__git_ref_fieldlist in git-completion.bash * when you add new atoms
@@ -991,6 +1002,11 @@ static const char *find_next(const char *cp)return NULL ; } +static int reject_atom (enum atom_type atom_type) +{ +return atom_type ==ATOM_REST ; +} +/* * Make sure the format string is well formed, and parse out * the used atoms.
@@ -1011,6 +1027,8 @@ 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 (reject_atom (used_atom[at].atom_type)) +die (_ ("this command reject atom %%(%.*s)" ), (int )(ep - sp -2 ), sp +2 ); if ((format->quote_style == QUOTE_PYTHON || format->quote_style == QUOTE_SHELL ||
@@ -1930,6 +1948,12 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) v->handler = else_atom_handler; v->s =xstrdup ("" );continue ; + }else if (atom_type ==ATOM_REST ) { +if (ref->rest) + v->s =xstrdup (ref->rest); +else + v->s =xstrdup ("" ); +continue ; }else continue ;
@@ -2147,6 +2171,7 @@ static struct ref_array_item *new_ref_array_item(const char *refname,FLEX_ALLOC_STR (ref, refname, refname);oidcpy (&ref->objectname, oid); + ref->rest =NULL ;return ref; }
ref-filter.h +4 -1
@@ -38,6 +38,7 @@ struct ref_sorting {struct ref_array_item {struct object_id objectname; +const char *rest;int flag;unsigned int kind;const char *symref;
@@ -76,14 +77,16 @@ struct ref_format {* verify_ref_format() afterwards to finalize. */ const char *format; +const char *rest;int quote_style; +int use_rest;int use_color;/* Internal state to ref-filter */ int need_color_reset_at_eol; }; -#define REF_FORMAT_INIT { NULL, 0, -1 } +#define REF_FORMAT_INIT { .use_color = -1 }/* Macros for checking --merged and --no-merged options */ #define _OPT_MERGED_NO_MERGED (option, filter, h) \
t/t3203-branch-output.sh +4 -0
@@ -340,6 +340,10 @@ test_expect_success 'git branch --format option' ' test_cmp expect actual ' +test_expect_success 'git branch with --format=%(rest) must fail' ' + test_must_fail git branch --format="%(rest)" >actual +' + test_expect_success 'worktree colors correct' ' cat >expect <<-EOF && * <GREEN>(HEAD detached from fromtag)<RESET>
t/t6300-for-each-ref.sh +4 -0
@@ -1211,6 +1211,10 @@ test_expect_success 'basic atom: head contents:trailers' ' test_cmp expect actual.clean ' +test_expect_success 'basic atom: rest must fail' ' + test_must_fail git for-each-ref --format="%(rest)" refs/heads/main +' + test_expect_success 'trailer parsing not fooled by --- line' ' git commit --allow-empty -F - <<-\EOF && this is the subject
t/t7004-tag.sh +4 -0
@@ -1998,6 +1998,10 @@ test_expect_success '--format should list tags as per format given' ' test_cmp expect actual ' +test_expect_success 'git tag -l with --format="%(rest)" must fail' ' + test_must_fail git tag -l --format="%(rest)" "v1*" +' + test_expect_success "set up color tests" ' echo "<RED>v1.0<RESET>" >expect.color && echo "v1.0" >expect.bare &&
t/t7030-verify-tag.sh +4 -0
@@ -194,6 +194,10 @@ test_expect_success GPG 'verifying tag with --format' ' test_cmp expect actual ' +test_expect_success GPG 'verifying tag with --format="%(rest)" must fail' ' + test_must_fail git verify-tag --format="%(rest)" "fourth-signed" +' + test_expect_success GPG 'verifying a forged tag with --format should fail silently' ' test_must_fail git verify-tag --format="tagname : %(tag)" $(cat forged1.tag) >actual-forged && test_must_be_empty actual-forged
Signature · 1 line
gitgitgadget
Re: [PATCH 5/5] [GSOC] ref-filter: add %(rest) atom
In reply to ZheNing Hu via GitGitGadgetQuoted 8 lines · ZheNing Hu via GitGitGadget
From: ZheNing Hu <[email protected]>
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".
To a reader intimately familiar with cat-file, %(rest) might be obvious.. but it would help the commit message read more easily if there was a short explanation of what %(rest) did. It is not obvious just from the name to me.
Thanks,
Jake
Re: [PATCH 5/5] [GSOC] ref-filter: add %(rest) atom
Quoted 12 lines · Jacob Keller <[email protected]> 于2021年7月23日周五 下午5:09写道:
Quoted 8 lines · ZheNing Hu via GitGitGadget
From: ZheNing Hu <[email protected]>
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".
Quoted 5 lines
To a reader intimately familiar with cat-file, %(rest) might be obvious.. but it would help the commit message read more easily if there was a short explanation of what %(rest) did. It is not obvious just from the name to me.
You are right, I may subconsciously think that everyone knows the %(rest) in cat-file.
I will change it.
Quoted 2 lines
Thanks,
Jake
Thanks.
--
ZheNing Hu
Re: [PATCH v2 2/5] [GSOC] ref-filter: add %(raw) atom
In reply to ZheNing Hu via GitGitGadgetref-filter.c struct ref_formatting_state { · line 649
Quoted 12 lines · ZheNing Hu via GitGitGadget
struct
atom_value{ref-filter.c +4
-0const char *s; +size_t s_size;int (*handler)(struct atom_value *atomv,struct ref_formatting_state *state,struct strbuf *err);uintmax_t value;/* used for sorting when not FIELD_STR */ struct used_atom *atom; }; +#define ATOM_VALUE_INIT { \ + .s_size = -1 \ +}
This will get turned into size_t that is unsigned, and comparisons like this one and ...
ref-filter.c static int parse_ref_filter_atom(const struct ref_format *format, · line 741
Quoted 6 lines
ref-filter.c +4
-1case QUOTE_NONE : -strbuf_addstr (s, str); +if (len !=-1 ) +strbuf_add (s, str, len); +else +strbuf_addstr (s, str);
this one ...
ref-filter.c static int append_atom(struct atom_value *v, struct ref_formatting_state *state, · line 775
Quoted 6 lines
ref-filter.c +4
-0+if (v->s_size !=-1 ) +strbuf_add (&state->stack->output, v->s, v->s_size); +else +strbuf_addstr (&state->stack->output, v->s);return 0 ; }
may work as expected, but it makes readers wonder if there is significance to negative values other than -1 (in other words, what does it mean if v->s_size == -2, for example?).
It may make sense to
* Turn atom_value.s_size field into ssize_t instead of size_t
* Rewrite (v->s_size != -1) comparison to (v->s_size < 0)
Optionally we could introduce #define ATOM_SIZE_UNSPECIFIED (-1) and use it to initialize .s_size in ATOM_VALUE_INIT, but if we are just going to test "is it negative, then it is not given", then it probably is not needed.
Other changes in the whole series relative to what has been queued looked all sensible to me.
THanks.
Re: [PATCH v2 2/5] [GSOC] ref-filter: add %(raw) atom
Quoted 35 lines · Junio C Hamano <[email protected]> 于2021年7月24日周六 上午12:38写道:
ref-filter.c struct ref_formatting_state { · line 649
Quoted 12 lines · ZheNing Hu via GitGitGadget
struct atom_value { const char *s;ref-filter.c +4
-0+size_t s_size;int (*handler)(struct atom_value *atomv,struct ref_formatting_state *state,struct strbuf *err);uintmax_t value;/* used for sorting when not FIELD_STR */ struct used_atom *atom; }; +#define ATOM_VALUE_INIT { \ + .s_size = -1 \ +}This will get turned into
size_tthat is unsigned, and comparisons like this one and ...ref-filter.c static int parse_ref_filter_atom(const struct ref_format *format, · line 741
Quoted 6 lines
case
QUOTE_NONE:ref-filter.c +4
-1-strbuf_addstr (s, str); +if (len !=-1 ) +strbuf_add (s, str, len); +else +strbuf_addstr (s, str);this one ...
ref-filter.c static int append_atom(struct atom_value *v, struct ref_formatting_state *state, · line 775
Quoted 6 lines
ref-filter.c +4
-0+if (v->s_size !=-1 ) +strbuf_add (&state->stack->output, v->s, v->s_size); +else +strbuf_addstr (&state->stack->output, v->s);return 0 ; }
Quoted 4 lines
may work as expected, but it makes readers wonder if there is significance to negative values other than -1 (in other words, what does it mean if
v->s_size== -2, for example?).
Yes, any value less than -1 is meaningless here.
Quoted 4 lines
It may make sense to
* Turn atom_value.s_size field into
ssize_tinstead ofsize_t
Will the conversion of size_t and ssize_t break -Wsign-conversion? Although there is a lot of code in Git that has broken it, but I am not sure if it is wise to use ssize_t here.
Quoted 2 lines
* Rewrite (
v->s_size!= -1) comparison to (v->s_size< 0)
For size_t, this scheme is wrong.
Quoted 6 lines
Optionally we could introduce #define
ATOM_SIZE_UNSPECIFIED(-1) and use it to initialize .s_size inATOM_VALUE_INIT, but if we are just going to test "is it negative, then it is not given", then it probably is not needed.
It seems that this is the only method left. Although I think ATOM_SIZE_UNSPECIFIED
is not very useful becasue we already have ATOM_VALUE_INIT.
Quoted 5 lines
Other changes in the whole series relative to what has been queued looked all sensible to me.
THanks.
Thanks.
--
ZheNing Hu
v3 cover ref-filter: add %(raw) and %(rest) atoms
In reply to ZheNing Hu via GitGitGadgetThis patch series is split from my main patch series zh/ref-filter-raw-data, to make things easier for reviewers to read. It's last version is here:
https://lore.kernel.org/git/[email protected]/
This patch series provided %(raw) and %(rest) for ref-filter, which will be used by zh/cat-file-reuse-ref-filter-logic later.
v2--->v3:
1. Add macro ATOM_SIZE_UNSPECIFIED to represent the initial value of
atom_value->s_size.
2. Add a description about %(rest) in the commit message. (copy from
https://git-scm.com/docs/git-cat-file)v2:
https://lore.kernel.org/git/CAP8UFD3ENBZdm8yQn6J-yb4PvUsrzTA3KGxes42dE3bTrYbr9Q@mail.gmail.com/
ZheNing Hu (5):
[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
Documentation/git-for-each-ref.txt | 9 ++ builtin/tag.c | 2 +- quote.c | 17 ++ quote.h | 1 + ref-filter.c | 244 ++++++++++++++++++++++------- ref-filter.h | 9 +- t/t3203-branch-output.sh | 4 + t/t6300-for-each-ref.sh | 235 +++++++++++++++++++++++++++ t/t7004-tag.sh | 4 + t/t7030-verify-tag.sh | 4 + 10 files changed, 466 insertions(+), 63 deletions(-)
base-commit: daab8a564f8bbac55f70f8bf86c070e001a9b006
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1000%2Fadlternative%2Fref-filter-raw-data-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1000/adlternative/ref-filter-raw-data-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1000
Range-diff vs v2:
1: d77268bb188 = 1: d77268bb188 [GSOC] ref-filter: add obj-type check in grab contents
2: eafb79bad62 ! 2: 0ffa549da6e [GSOC] ref-filter: add %(raw) atom
@@ ref-filter.c: struct ref_formatting_state {
struct used_atom *atom;
};
++#define ATOM_SIZE_UNSPECIFIED (-1)
++
+#define ATOM_VALUE_INIT { \
-+ .s_size = -1 \
++ .s_size = ATOM_SIZE_UNSPECIFIED \
+}
+
/*
@@ ref-filter.c: static int append_atom(struct atom_value *v, struct ref_formatting
+ quote_formatting(&state->stack->output, v->s, v->s_size, state->quote_style);
else
- strbuf_addstr(&state->stack->output, v->s);
-+ if (v->s_size != -1)
++ if (v->s_size != ATOM_SIZE_UNSPECIFIED)
+ strbuf_add(&state->stack->output, v->s, v->s_size);
+ else
+ strbuf_addstr(&state->stack->output, v->s);
@@ ref-filter.c: static int populate_value(struct ref_array_item *ref, struct strbu
const char *refname;
struct branch *branch = NULL;
-+ v->s_size = -1;
++ v->s_size = ATOM_SIZE_UNSPECIFIED;
v->handler = append_atom;
v->atom = atom;
@@ ref-filter.c: static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array
- cmp_fn = s->sort_flags & REF_SORTING_ICASE
- ? strcasecmp : strcmp;
- cmp = cmp_fn(va->s, vb->s);
-+ if (va->s_size == -1 && vb->s_size == -1) {
++ if (va->s_size == ATOM_SIZE_UNSPECIFIED && vb->s_size == ATOM_SIZE_UNSPECIFIED) {
+ int (*cmp_fn)(const char *, const char *);
+ cmp_fn = s->sort_flags & REF_SORTING_ICASE
+ ? strcasecmp : strcmp;
+ cmp = cmp_fn(va->s, vb->s);
+ } else {
-+ size_t a_size = va->s_size == -1 ?
++ size_t a_size = va->s_size == ATOM_SIZE_UNSPECIFIED ?
+ strlen(va->s) : va->s_size;
-+ size_t b_size = vb->s_size == -1 ?
++ size_t b_size = vb->s_size == ATOM_SIZE_UNSPECIFIED ?
+ strlen(vb->s) : vb->s_size;
+ int (*cmp_fn)(const void *, const void *, size_t);
+ cmp_fn = s->sort_flags & REF_SORTING_ICASE
3: 00a29dd16d7 = 3: ac5d98647da [GSOC] ref-filter: --format=%(raw) re-support --perl
4: 3a8173b42ed = 4: 0722f546df9 [GSOC] ref-filter: use non-const ref_format in *_atom_parser()
5: 71249dfd4e0 ! 5: 2300b0463f2 [GSOC] ref-filter: add %(rest) atom @@ Metadata
## Commit message ##
[GSOC] ref-filter: add %(rest) atom
+ %(rest) is a atom used for cat-file batch mode, which can split
+ the input lines at the first whitespace boundary, all characters
+ before that whitespace are considered to be the object name;
+ characters after that first run of whitespace (i.e., the "rest"
+ of the line) are output in place of the %(rest) atom.
+
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".
+ 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".
+ Suggected-by: Jacob Keller <[email protected]>
Signature · 1 line
gitgitgadget
PATCH v3 1/5 ref-filter: add obj-type check in grab contents
From: ZheNing Hu <[email protected]>
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. So it's needed to pass a struct expand_data *data instread of only void *buf to both grab_sub_body_contents() and grab_values() to be able to check the object type.
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.
--- ref-filter.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)
ref-filter.c +15 -9
@@ -1356,11 +1356,12 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size }/* See grab_values */ -static void grab_sub_body_contents (struct atom_value *val,int deref,void *buf) +static void grab_sub_body_contents (struct atom_value *val,int deref,struct expand_data *data) {int i;const char *subpos =NULL , *bodypos =NULL , *sigpos =NULL ;size_t sublen =0 , bodylen =0 , nonsiglen =0 , siglen =0 ; +void *buf = data->content;for (i =0 ; i < used_atom_cnt; i++) {struct used_atom *atom = &used_atom[i];
@@ -1371,10 +1372,13 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)continue ;if (deref) name++; -if (strcmp (name,"body" ) && - !starts_with (name,"subject" ) && - !starts_with (name,"trailers" ) && - !starts_with (name,"contents" )) + +if ((data->type !=OBJ_TAG && + data->type !=OBJ_COMMIT ) || + (strcmp (name,"body" ) && + !starts_with (name,"subject" ) && + !starts_with (name,"trailers" ) && + !starts_with (name,"contents" )))continue ;if (!subpos) find_subpos(buf,
@@ -1438,17 +1442,19 @@ static void fill_missing_values(struct atom_value *val)* pointed at by the ref itself; otherwise it is the object the * ref (which is a tag) refers to. */ -static void grab_values (struct atom_value *val,int deref,struct object *obj,void *buf) +static void grab_values (struct atom_value *val,int deref,struct object *obj,struct expand_data *data) { +void *buf = data->content; +switch (obj->type) {case OBJ_TAG :grab_tag_values (val, deref, obj); -grab_sub_body_contents (val, deref, buf); +grab_sub_body_contents (val, deref, data);grab_person ("tagger" , val, deref, buf);break ;case OBJ_COMMIT :grab_commit_values (val, deref, obj); -grab_sub_body_contents (val, deref, buf); +grab_sub_body_contents (val, deref, data);grab_person ("author" , val, deref, buf);grab_person ("committer" , val, deref, buf);break ;
@@ -1678,7 +1684,7 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **objreturn strbuf_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);
Signature · 1 line
gitgitgadget
PATCH v3 2/5 ref-filter: add %(raw) atom
In reply to ZheNing Hu via GitGitGadgetFrom: ZheNing Hu <[email protected]>
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.
--- Documentation/git-for-each-ref.txt | 9 ++ ref-filter.c | 144 +++++++++++++++---- t/t6300-for-each-ref.sh | 216 +++++++++++++++++++++++++++++ 3 files changed, 341 insertions(+), 28 deletions(-)
Documentation/git-for-each-ref.txt +9 -0
@@ -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:
ref-filter.c +116 -28
293 lines not on this page · raw message
t/t6300-for-each-ref.sh +216 -0
258 lines not on this page · raw message
Signature · 1 line
gitgitgadget
PATCH v3 3/5 ref-filter: --format=%(raw) re-support --perl
In reply to ZheNing Hu via GitGitGadgetFrom: ZheNing Hu <[email protected]>
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.
--- 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(-)
Documentation/git-for-each-ref.txt +2 -2
@@ -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:
quote.c +17 -0
@@ -471,6 +471,23 @@ void perl_quote_buf(struct strbuf *sb, const char *src)strbuf_addch (sb, sq); } +void perl_quote_buf_with_len (struct strbuf *sb,const char *src,size_t len) +{ +const char sq ='\'' ; +const char bq ='\\' ; +const char *c = src; +const char *end = src + len; + +strbuf_addch (sb, sq); +while (c != end) { +if (*c == sq || *c == bq) +strbuf_addch (sb, bq); +strbuf_addch (sb, *c); + c++; + } +strbuf_addch (sb, sq); +} +void python_quote_buf (struct strbuf *sb,const char *src) {const char sq ='\'' ;
quote.h +1 -0
@@ -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 */ void perl_quote_buf (struct strbuf *sb,const char *src); +void perl_quote_buf_with_len (struct strbuf *sb,const char *src,size_t len);void python_quote_buf (struct strbuf *sb,const char *src);void tcl_quote_buf (struct strbuf *sb,const char *src);void basic_regex_quote_buf (struct strbuf *sb,const char *src);
ref-filter.c +11 -4
@@ -746,7 +746,10 @@ static void quote_formatting(struct strbuf *s, const char *str, size_t len, intsq_quote_buf (s, str);break ;case QUOTE_PERL : -perl_quote_buf (s, str); +if (len !=-1 ) +perl_quote_buf_with_len (s, str, len); +else +perl_quote_buf (s, str);break ;case QUOTE_PYTHON :python_quote_buf (s, str);
@@ -1010,10 +1013,14 @@ 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 ) + +if ((format->quote_style ==QUOTE_PYTHON || + format->quote_style ==QUOTE_SHELL || + format->quote_style ==QUOTE_TCL ) && + 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 ); +"--python, --shell, --tcl" ), (int )(ep - sp -2 ), sp +2 ); cp = ep +1 ;if (skip_prefix (used_atom[at].name,"color:" , &color))
t/t6300-for-each-ref.sh +17 -2
@@ -915,8 +915,23 @@ test_expect_success '%(raw) with --tcl must fail' ' test_must_fail git for-each-ref --format="%(raw)" --tcl ' -test_expect_success '%(raw) with --perl must fail' ' - test_must_fail git for-each-ref --format="%(raw)" --perl +test_expect_success '%(raw) with --perl' ' + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/blob1 --perl | perl >actual && + cmp blob1 actual && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/blob3 --perl | perl >actual && + cmp blob3 actual && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/blob8 --perl | perl >actual && + cmp blob8 actual && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/first --perl | perl >actual && + cmp one actual && + git cat-file tree refs/mytrees/first > expected && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/mytrees/first --perl | perl >actual && + cmp expected actual ' test_expect_success '%(raw) with --shell must fail' '
Signature · 1 line
gitgitgadget
PATCH v3 4/5 ref-filter: use non-const ref_format in *_atom_parser()
In reply to ZheNing Hu via GitGitGadgetFrom: ZheNing Hu <[email protected]>
Use non-const ref_format in *_atom_parser(), which can help us modify the members of ref_format in *_atom_parser().
--- builtin/tag.c | 2 +- ref-filter.c | 44 ++++++++++++++++++++++---------------------- ref-filter.h | 4 ++-- 3 files changed, 25 insertions(+), 25 deletions(-)
builtin/tag.c +1 -1
@@ -146,7 +146,7 @@ static int verify_tag(const char *name, const char *ref, const struct object_id *oid, void *cb_data) { int flags; -const struct ref_format *format = cb_data; +struct ref_format *format = cb_data; flags =GPG_VERIFY_VERBOSE ;if (format->format)
ref-filter.c +22 -22
164 lines not on this page · raw message
ref-filter.h +2 -2
@@ -116,7 +116,7 @@ void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);void ref_sorting_set_sort_flags_all (struct ref_sorting *sorting,unsigned int mask,int on);/* Based on the given format and quote_style, fill the strbuf */ int format_ref_array_item (struct ref_array_item *info, -const struct ref_format *format, +struct ref_format *format,struct strbuf *final_buf,struct strbuf *error_buf);/* Parse a single sort specifier and add it to the list */
@@ -137,7 +137,7 @@ void setup_ref_filter_porcelain_msg(void);* name must be a fully qualified refname. */ void pretty_print_ref (const char *name,const struct object_id *oid, -const struct ref_format *format); +struct ref_format *format);/* * Push a single ref onto the array; this can be used to construct your own
Signature · 1 line
gitgitgadget
PATCH v3 5/5 ref-filter: add %(rest) atom
In reply to ZheNing Hu via GitGitGadgetFrom: ZheNing Hu <[email protected]>
%(rest) is a atom used for cat-file batch mode, which can split the input lines at the first whitespace boundary, all characters before that whitespace are considered to be the object name; characters after that first run of whitespace (i.e., the "rest" of the line) are output in place of the %(rest) atom.
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".
--- 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(-)
ref-filter.c +25 -0
@@ -157,6 +157,7 @@ enum atom_type {ATOM_IF ,ATOM_THEN ,ATOM_ELSE , + ATOM_REST, };/*
@@ -559,6 +560,15 @@ static int if_atom_parser(struct ref_format *format, struct used_atom *atom,return 0 ; } +static int rest_atom_parser (struct ref_format *format,struct used_atom *atom, +const char *arg,struct strbuf *err) +{ +if (arg) +return strbuf_addf_ret (err,-1 ,_ ("%%(rest) does not take arguments" )); + format->use_rest =1 ; +return 0 ; +} +static int head_atom_parser (struct ref_format *format,struct used_atom *atom,const char *arg,struct strbuf *unused_err) {
@@ -615,6 +625,7 @@ static struct { [ATOM_IF] = { "if", SOURCE_NONE, FIELD_STR, if_atom_parser }, [ATOM_THEN] = { "then", SOURCE_NONE }, [ATOM_ELSE] = { "else", SOURCE_NONE }, + [ATOM_REST] = { "rest", SOURCE_NONE, FIELD_STR, rest_atom_parser },/* * Please update $__git_ref_fieldlist in git-completion.bash * when you add new atoms
@@ -993,6 +1004,11 @@ static const char *find_next(const char *cp)return NULL ; } +static int reject_atom (enum atom_type atom_type) +{ +return atom_type ==ATOM_REST ; +} +/* * Make sure the format string is well formed, and parse out * the used atoms.
@@ -1013,6 +1029,8 @@ 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 (reject_atom (used_atom[at].atom_type)) +die (_ ("this command reject atom %%(%.*s)" ), (int )(ep - sp -2 ), sp +2 ); if ((format->quote_style == QUOTE_PYTHON || format->quote_style == QUOTE_SHELL ||
@@ -1932,6 +1950,12 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) v->handler = else_atom_handler; v->s =xstrdup ("" );continue ; + }else if (atom_type ==ATOM_REST ) { +if (ref->rest) + v->s =xstrdup (ref->rest); +else + v->s =xstrdup ("" ); +continue ; }else continue ;
@@ -2149,6 +2173,7 @@ static struct ref_array_item *new_ref_array_item(const char *refname,FLEX_ALLOC_STR (ref, refname, refname);oidcpy (&ref->objectname, oid); + ref->rest =NULL ;return ref; }
ref-filter.h +4 -1
@@ -38,6 +38,7 @@ struct ref_sorting {struct ref_array_item {struct object_id objectname; +const char *rest;int flag;unsigned int kind;const char *symref;
@@ -76,14 +77,16 @@ struct ref_format {* verify_ref_format() afterwards to finalize. */ const char *format; +const char *rest;int quote_style; +int use_rest;int use_color;/* Internal state to ref-filter */ int need_color_reset_at_eol; }; -#define REF_FORMAT_INIT { NULL, 0, -1 } +#define REF_FORMAT_INIT { .use_color = -1 }/* Macros for checking --merged and --no-merged options */ #define _OPT_MERGED_NO_MERGED (option, filter, h) \
t/t3203-branch-output.sh +4 -0
@@ -340,6 +340,10 @@ test_expect_success 'git branch --format option' ' test_cmp expect actual ' +test_expect_success 'git branch with --format=%(rest) must fail' ' + test_must_fail git branch --format="%(rest)" >actual +' + test_expect_success 'worktree colors correct' ' cat >expect <<-EOF && * <GREEN>(HEAD detached from fromtag)<RESET>
t/t6300-for-each-ref.sh +4 -0
@@ -1211,6 +1211,10 @@ test_expect_success 'basic atom: head contents:trailers' ' test_cmp expect actual.clean ' +test_expect_success 'basic atom: rest must fail' ' + test_must_fail git for-each-ref --format="%(rest)" refs/heads/main +' + test_expect_success 'trailer parsing not fooled by --- line' ' git commit --allow-empty -F - <<-\EOF && this is the subject
t/t7004-tag.sh +4 -0
@@ -1998,6 +1998,10 @@ test_expect_success '--format should list tags as per format given' ' test_cmp expect actual ' +test_expect_success 'git tag -l with --format="%(rest)" must fail' ' + test_must_fail git tag -l --format="%(rest)" "v1*" +' + test_expect_success "set up color tests" ' echo "<RED>v1.0<RESET>" >expect.color && echo "v1.0" >expect.bare &&
t/t7030-verify-tag.sh +4 -0
@@ -194,6 +194,10 @@ test_expect_success GPG 'verifying tag with --format' ' test_cmp expect actual ' +test_expect_success GPG 'verifying tag with --format="%(rest)" must fail' ' + test_must_fail git verify-tag --format="%(rest)" "fourth-signed" +' + test_expect_success GPG 'verifying a forged tag with --format should fail silently' ' test_must_fail git verify-tag --format="tagname : %(tag)" $(cat forged1.tag) >actual-forged && test_must_be_empty actual-forged
Signature · 1 line
gitgitgadget
Re: [PATCH v2 2/5] [GSOC] ref-filter: add %(raw) atom
In reply to ZheNing HuQuoted 21 lines · ZheNing Hu
Quoted 4 lines
It may make sense to
* Turn atom_value.s_size field into
ssize_tinstead ofsize_tWill the conversion of
size_tandssize_tbreak -Wsign-conversion? Although there is a lot of code in Git that has broken it, but I am not sure if it is wise to usessize_there.Quoted 2 lines
* Rewrite (
v->s_size!= -1) comparison to (v->s_size< 0)For
size_t, this scheme is wrong.Quoted 6 lines
Optionally we could introduce #define
ATOM_SIZE_UNSPECIFIED(-1) and use it to initialize .s_size inATOM_VALUE_INIT, but if we are just going to test "is it negative, then it is not given", then it probably is not needed.
Quoted 3 lines
It seems that this is the only method left. Although I think
ATOM_SIZE_UNSPECIFIED
is not very useful becasue we already haveATOM_VALUE_INIT.
Sorry, but I think you misread what I wrote.
These three were not offered as "you can do one of these three, pick one you like" choices. I meant to say "I think it makes sense to do all these three things, but the last one is optional, doing only the first two may be good enough". As the second one requires that the first is done, of course, doing only the second one would not make sense.
Also, you seem to have missed the distinction between _INIT and _UNSPECIFIED. You can initialize something to (1) a reasonable default value, or (2) unusable value that you can detect at runtime and notice that it was not set. If you called something to FOO_INIT, your readers cannot tell which one it is, but if you call it FOO_UNSPECIFIED, it is clear it is the latter case.
In many places, we do use ssize_t for "normally this is size, but we can express exception cases (like errors) by storing negative value" in our codebase (grep for it), and I think the member in question is prime candidate for such use.
Re: [PATCH v3 1/5] [GSOC] ref-filter: add obj-type check in grab contents
In reply to ZheNing Hu via GitGitGadgetQuoted 16 lines · ZheNing Hu via GitGitGadget
From: ZheNing Hu <[email protected]>
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. So it's needed to pass astruct expand_data *datainstread of onlyvoid *bufto bothgrab_sub_body_contents()andgrab_values()to be able to check the object type.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.
Quoted 5 lines
Mentored-by Christian Couder <[email protected]>Mentored-by Hariom Verma <[email protected]>Helped-by Junio C Hamano <[email protected]>Signed-off-by ZheNing Hu <[email protected]>---
Nice. The addition of changing the type made sense and is spelled out nicely.
Quoted 74 lines
ref-filter.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)ref-filter.c +15
-10@@ -1356,11 +1356,12 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size }/* See grab_values */ -static void grab_sub_body_contents (struct atom_value *val,int deref,void *buf) +static void grab_sub_body_contents (struct atom_value *val,int deref,struct expand_data *data) {int i;const char *subpos =NULL , *bodypos =NULL , *sigpos =NULL ;size_t sublen =0 , bodylen =0 , nonsiglen =0 , siglen =0 ; +void *buf = data->content;for (i =0 ; i < used_atom_cnt; i++) {struct used_atom *atom = &used_atom[i];@@ -1371,10 +1372,13 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)continue ;if (deref) name++; -if (strcmp (name,"body" ) && - !starts_with (name,"subject" ) && - !starts_with (name,"trailers" ) && - !starts_with (name,"contents" )) + +if ((data->type !=OBJ_TAG && + data->type !=OBJ_COMMIT ) || + (strcmp (name,"body" ) && + !starts_with (name,"subject" ) && + !starts_with (name,"trailers" ) && + !starts_with (name,"contents" )))continue ;if (!subpos) find_subpos(buf,@@ -1438,17 +1442,19 @@ static void fill_missing_values(struct atom_value *val)* pointed at by the ref itself; otherwise it is the object the * ref (which is a tag) refers to. */ -static void grab_values (struct atom_value *val,int deref,struct object *obj,void *buf) +static void grab_values (struct atom_value *val,int deref,struct object *obj,struct expand_data *data) { +void *buf = data->content; +switch (obj->type) {case OBJ_TAG :grab_tag_values (val, deref, obj); -grab_sub_body_contents (val, deref, buf); +grab_sub_body_contents (val, deref, data);grab_person ("tagger" , val, deref, buf);break ;case OBJ_COMMIT :grab_commit_values (val, deref, obj); -grab_sub_body_contents (val, deref, buf); +grab_sub_body_contents (val, deref, data);grab_person ("author" , val, deref, buf);grab_person ("committer" , val, deref, buf);break ;@@ -1678,7 +1684,7 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **objreturn strbuf_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); --gitgitgadget
Re: [PATCH v3 3/5] [GSOC] ref-filter: --format=%(raw) re-support --perl
In reply to ZheNing Hu via GitGitGadgetQuoted 11 lines · ZheNing Hu via GitGitGadget
From: ZheNing Hu <[email protected]>
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.Signed-off-by ZheNing Hu <[email protected]>---
I presume that we used to support raw with perl before in cat-file? It did read a bit weird to see "Re-support" in the title, because the previous patch which introduced the raw atom when in ref-filter code never supported perl. Still, I think it's fairly clear either way and that change wouldn't deserve a reroll on its own.
Makes sense. Reviewed-by: Jacob Keller <[email protected]>
Quoted 85 lines
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(-)Documentation/git-for-each-ref.txt +2
-2@@ -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:quote.c +17
-0@@ -471,6 +471,23 @@ void perl_quote_buf(struct strbuf *sb, const char *src)strbuf_addch (sb, sq); } +void perl_quote_buf_with_len (struct strbuf *sb,const char *src,size_t len) +{ +const char sq ='\'' ; +const char bq ='\\' ; +const char *c = src; +const char *end = src + len; + +strbuf_addch (sb, sq); +while (c != end) { +if (*c == sq || *c == bq) +strbuf_addch (sb, bq); +strbuf_addch (sb, *c); + c++; + } +strbuf_addch (sb, sq); +} +void python_quote_buf (struct strbuf *sb,const char *src) {const char sq ='\'' ;quote.h +1
-0@@ -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 */ void perl_quote_buf (struct strbuf *sb,const char *src); +void perl_quote_buf_with_len (struct strbuf *sb,const char *src,size_t len);void python_quote_buf (struct strbuf *sb,const char *src);void tcl_quote_buf (struct strbuf *sb,const char *src);void basic_regex_quote_buf (struct strbuf *sb,const char *src);ref-filter.c +5
-3@@ -746,7 +746,10 @@ static void quote_formatting(struct strbuf *s, const char *str, size_t len, intsq_quote_buf (s, str);break ;case QUOTE_PERL : -perl_quote_buf (s, str); +if (len !=-1 ) +perl_quote_buf_with_len (s, str, len); +else +perl_quote_buf (s, str);break ;case QUOTE_PYTHON :python_quote_buf (s, str);@@ -1010,10 +1013,14 @@ 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 ) +
ref-filter.c int verify_ref_format(struct ref_format *format) · line 1025
Quoted 10 lines
ref-filter.c +6
-1+if ((format->quote_style ==QUOTE_PYTHON || + format->quote_style ==QUOTE_SHELL || + format->quote_style ==QUOTE_TCL ) && + 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 ); +"--python, --shell, --tcl" ), (int )(ep - sp -2 ), sp +2 ); cp = ep +1 ;
Right, so now we have to check all of the format quote styles instead of just checking that its set to any of them, since we want to continue in the case of perl.
Ok.
Quoted 34 lines
ref-filter.c +0
-0if (skip_prefix (used_atom[at].name,"color:" , &color))t/t6300-for-each-ref.sh +17
-3@@ -915,8 +915,23 @@ test_expect_success '%(raw) with --tcl must fail' ' test_must_fail git for-each-ref --format="%(raw)" --tcl ' -test_expect_success '%(raw) with --perl must fail' ' - test_must_fail git for-each-ref --format="%(raw)" --perl +test_expect_success '%(raw) with --perl' ' + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/blob1 --perl | perl >actual && + cmp blob1 actual && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/blob3 --perl | perl >actual && + cmp blob3 actual && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/blob8 --perl | perl >actual && + cmp blob8 actual && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/first --perl | perl >actual && + cmp one actual && + git cat-file tree refs/mytrees/first > expected && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/mytrees/first --perl | perl >actual && + cmp expected actual ' test_expect_success '%(raw) with --shell must fail' ' --gitgitgadget
Re: [PATCH v3 5/5] [GSOC] ref-filter: add %(rest) atom
In reply to ZheNing Hu via GitGitGadgetQuoted 12 lines · ZheNing Hu via GitGitGadget
From: ZheNing Hu <[email protected]>
%(rest) is a atom used for cat-file batch mode, which can split the input lines at the first whitespace boundary, all characters before that whitespace are considered to be the object name; characters after that first run of whitespace (i.e., the "rest" of the line) are output in place of the %(rest) atom.
In order to let "cat-file --batch=%(rest)" use the ref-filter interface, add %(rest) atom for ref-filter.
Quoted 8 lines
Introduce the
reject_atom()to reject the atom %(rest) for "git for-each-ref", "git branch", "git tag" and "git verify-tag".Suggected-by Jacob Keller <[email protected]>Mentored-by Christian Couder <[email protected]>Mentored-by Hariom Verma <[email protected]>Signed-off-by ZheNing Hu <[email protected]>---
Thanks for the improved commit message here. That helps clarify what this atom does and aids in understanding the implementation.
Quoted 178 lines
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(-)ref-filter.c +25
-0@@ -157,6 +157,7 @@ enum atom_type {ATOM_IF ,ATOM_THEN ,ATOM_ELSE , + ATOM_REST, };/* @@ -559,6 +560,15 @@ static int if_atom_parser(struct ref_format *format, struct used_atom *atom,return 0 ; } +static int rest_atom_parser (struct ref_format *format,struct used_atom *atom, +const char *arg,struct strbuf *err) +{ +if (arg) +return strbuf_addf_ret (err,-1 ,_ ("%%(rest) does not take arguments" )); + format->use_rest =1 ; +return 0 ; +} +static int head_atom_parser (struct ref_format *format,struct used_atom *atom,const char *arg,struct strbuf *unused_err) {@@ -615,6 +625,7 @@ static struct { [ATOM_IF] = { "if", SOURCE_NONE, FIELD_STR, if_atom_parser }, [ATOM_THEN] = { "then", SOURCE_NONE }, [ATOM_ELSE] = { "else", SOURCE_NONE }, + [ATOM_REST] = { "rest", SOURCE_NONE, FIELD_STR, rest_atom_parser },/* * Please update $__git_ref_fieldlist in git-completion.bash * when you add new atoms @@ -993,6 +1004,11 @@ static const char *find_next(const char *cp)return NULL ; } +static int reject_atom (enum atom_type atom_type) +{ +return atom_type ==ATOM_REST ; +} +/* * Make sure the format string is well formed, and parse out * the used atoms. @@ -1013,6 +1029,8 @@ 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 (reject_atom (used_atom[at].atom_type)) +die (_ ("this command reject atom %%(%.*s)" ), (int )(ep - sp -2 ), sp +2 ); if ((format->quote_style == QUOTE_PYTHON || format->quote_style == QUOTE_SHELL ||@@ -1932,6 +1950,12 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) v->handler = else_atom_handler; v->s =xstrdup ("" );continue ; + }else if (atom_type ==ATOM_REST ) { +if (ref->rest) + v->s =xstrdup (ref->rest); +else + v->s =xstrdup ("" ); +continue ; }else continue ;@@ -2149,6 +2173,7 @@ static struct ref_array_item *new_ref_array_item(const char *refname,FLEX_ALLOC_STR (ref, refname, refname);oidcpy (&ref->objectname, oid); + ref->rest =NULL ;return ref; }ref-filter.h +4
-1@@ -38,6 +38,7 @@ struct ref_sorting {struct ref_array_item {struct object_id objectname; +const char *rest;int flag;unsigned int kind;const char *symref;@@ -76,14 +77,16 @@ struct ref_format {* verify_ref_format() afterwards to finalize. */ const char *format; +const char *rest;int quote_style; +int use_rest;int use_color;/* Internal state to ref-filter */ int need_color_reset_at_eol; }; -#define REF_FORMAT_INIT { NULL, 0, -1 } +#define REF_FORMAT_INIT { .use_color = -1 }/* Macros for checking --merged and --no-merged options */ #define _OPT_MERGED_NO_MERGED (option, filter, h) \t/t3203-branch-output.sh +4
-0@@ -340,6 +340,10 @@ test_expect_success 'git branch --format option' ' test_cmp expect actual ' +test_expect_success 'git branch with --format=%(rest) must fail' ' + test_must_fail git branch --format="%(rest)" >actual +' + test_expect_success 'worktree colors correct' ' cat >expect <<-EOF && * <GREEN>(HEAD detached from fromtag)<RESET>t/t6300-for-each-ref.sh +4
-0@@ -1211,6 +1211,10 @@ test_expect_success 'basic atom: head contents:trailers' ' test_cmp expect actual.clean ' +test_expect_success 'basic atom: rest must fail' ' + test_must_fail git for-each-ref --format="%(rest)" refs/heads/main +' + test_expect_success 'trailer parsing not fooled by --- line' ' git commit --allow-empty -F - <<-\EOF && this is the subjectt/t7004-tag.sh +4
-0@@ -1998,6 +1998,10 @@ test_expect_success '--format should list tags as per format given' ' test_cmp expect actual ' +test_expect_success 'git tag -l with --format="%(rest)" must fail' ' + test_must_fail git tag -l --format="%(rest)" "v1*" +' + test_expect_success "set up color tests" ' echo "<RED>v1.0<RESET>" >expect.color && echo "v1.0" >expect.bare &&t/t7030-verify-tag.sh +4
-1@@ -194,6 +194,10 @@ test_expect_success GPG 'verifying tag with --format' ' test_cmp expect actual ' +test_expect_success GPG 'verifying tag with --format="%(rest)" must fail' ' + test_must_fail git verify-tag --format="%(rest)" "fourth-signed" +' + test_expect_success GPG 'verifying a forged tag with --format should fail silently' ' test_must_fail git verify-tag --format="tagname : %(tag)" $(cat forged1.tag) >actual-forged && test_must_be_empty actual-forged --gitgitgadget
Re: [PATCH v2 2/5] [GSOC] ref-filter: add %(raw) atom
In reply to Junio C HamanoQuoted 28 lines · Junio C Hamano <[email protected]> 于2021年7月25日周日 上午1:41写道:
Quoted 21 lines · ZheNing Hu
Quoted 4 lines
It may make sense to
* Turn atom_value.s_size field into
ssize_tinstead ofsize_tWill the conversion of
size_tandssize_tbreak -Wsign-conversion? Although there is a lot of code in Git that has broken it, but I am not sure if it is wise to usessize_there.Quoted 2 lines
* Rewrite (
v->s_size!= -1) comparison to (v->s_size< 0)For
size_t, this scheme is wrong.Quoted 6 lines
Optionally we could introduce #define
ATOM_SIZE_UNSPECIFIED(-1) and use it to initialize .s_size inATOM_VALUE_INIT, but if we are just going to test "is it negative, then it is not given", then it probably is not needed.Quoted 3 lines
It seems that this is the only method left. Although I think
ATOM_SIZE_UNSPECIFIED
is not very useful becasue we already haveATOM_VALUE_INIT.
Quoted 9 lines
Sorry, but I think you misread what I wrote.
These three were not offered as "you can do one of these three, pick one you like" choices. I meant to say "I think it makes sense to do all these three things, but the last one is optional, doing only the first two may be good enough". As the second one requires that the first is done, of course, doing only the second one would not make sense.
OK. I know it now, I will do all of them.
Quoted 7 lines
Also, you seem to have missed the distinction between
_INITand_UNSPECIFIED. You can initialize something to (1) a reasonable default value, or (2) unusable value that you can detect at runtime and notice that it was not set. If you called something toFOO_INIT, your readers cannot tell which one it is, but if you call itFOO_UNSPECIFIED, it is clear it is the latter case.
Thanks for clarification, I understand the difference between them now.
Quoted 5 lines
In many places, we do use
ssize_tfor "normally this is size, but we can express exception cases (like errors) by storing negative value" in our codebase (grep for it), and I think the member in question is prime candidate for such use.
Yeah, as abspath.c:137, ssize_t len used to distinguish situations if an error is
returned from strbuf_readlink().
ssize_t len;
...
len = strbuf_readlink(&symlink, resolved->buf,
st.st_size);
if (len < 0) {
if (flags & REALPATH_DIE_ON_ERROR)
die_errno("Invalid symlink '%s'",
resolved->buf);
else
goto error_out;
}
...Or just read() and write(), they return is ssize_t too, which can return -1 and set errno when an exception occurs.
Thanks.
--
ZheNing Hu
Re: [PATCH v3 3/5] [GSOC] ref-filter: --format=%(raw) re-support --perl
In reply to Jacob KellerQuoted 15 lines · Jacob Keller <[email protected]> 于2021年7月25日周日 下午4:27写道:
Quoted 11 lines · ZheNing Hu via GitGitGadget
From: ZheNing Hu <[email protected]>
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.Signed-off-by ZheNing Hu <[email protected]>---
Quoted 6 lines
I presume that we used to support raw with perl before in cat-file? It did read a bit weird to see "Re-support" in the title, because the previous patch which introduced the raw atom when in ref-filter code never supported perl. Still, I think it's fairly clear either way and that change wouldn't deserve a reroll on its own.
No, we haven't support it. It can be said that this commit is only split from the previous commit. Therefore, "re-support" is indeed wrong. I will modify it.
Quoted 24 lines
Makes sense. Reviewed-by: Jacob Keller <[email protected]>
Quoted 7 lines
ref-filter.c +1
-2@@ -1010,10 +1013,14 @@ 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 ) +ref-filter.c int verify_ref_format(struct ref_format *format) · line 1025
Quoted 10 lines
ref-filter.c +6
-1+if ((format->quote_style ==QUOTE_PYTHON || + format->quote_style ==QUOTE_SHELL || + format->quote_style ==QUOTE_TCL ) && + 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 ); +"--python, --shell, --tcl" ), (int )(ep - sp -2 ), sp +2 ); cp = ep +1 ;Right, so now we have to check all of the format quote styles instead of just checking that its set to any of them, since we want to continue in the case of perl.
Quoted 2 lines
Ok.
Thanks.
--
ZheNing Hu
PATCH v4 1/5 ref-filter: add obj-type check in grab contents
In reply to ZheNing Hu via GitGitGadgetFrom: ZheNing Hu <[email protected]>
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. So it's needed to pass a struct expand_data *data instread of only void *buf to both grab_sub_body_contents() and grab_values() to be able to check the object type.
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.
--- ref-filter.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-)
ref-filter.c +15 -9
@@ -1356,11 +1356,12 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size }/* See grab_values */ -static void grab_sub_body_contents (struct atom_value *val,int deref,void *buf) +static void grab_sub_body_contents (struct atom_value *val,int deref,struct expand_data *data) {int i;const char *subpos =NULL , *bodypos =NULL , *sigpos =NULL ;size_t sublen =0 , bodylen =0 , nonsiglen =0 , siglen =0 ; +void *buf = data->content;for (i =0 ; i < used_atom_cnt; i++) {struct used_atom *atom = &used_atom[i];
@@ -1371,10 +1372,13 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)continue ;if (deref) name++; -if (strcmp (name,"body" ) && - !starts_with (name,"subject" ) && - !starts_with (name,"trailers" ) && - !starts_with (name,"contents" )) + +if ((data->type !=OBJ_TAG && + data->type !=OBJ_COMMIT ) || + (strcmp (name,"body" ) && + !starts_with (name,"subject" ) && + !starts_with (name,"trailers" ) && + !starts_with (name,"contents" )))continue ;if (!subpos) find_subpos(buf,
@@ -1438,17 +1442,19 @@ static void fill_missing_values(struct atom_value *val)* pointed at by the ref itself; otherwise it is the object the * ref (which is a tag) refers to. */ -static void grab_values (struct atom_value *val,int deref,struct object *obj,void *buf) +static void grab_values (struct atom_value *val,int deref,struct object *obj,struct expand_data *data) { +void *buf = data->content; +switch (obj->type) {case OBJ_TAG :grab_tag_values (val, deref, obj); -grab_sub_body_contents (val, deref, buf); +grab_sub_body_contents (val, deref, data);grab_person ("tagger" , val, deref, buf);break ;case OBJ_COMMIT :grab_commit_values (val, deref, obj); -grab_sub_body_contents (val, deref, buf); +grab_sub_body_contents (val, deref, data);grab_person ("author" , val, deref, buf);grab_person ("committer" , val, deref, buf);break ;
@@ -1678,7 +1684,7 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **objreturn strbuf_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);
Signature · 1 line
gitgitgadget
v4 cover ref-filter: add %(raw) and %(rest) atoms
In reply to ZheNing Hu via GitGitGadgetThis patch series is split from my main patch series zh/ref-filter-raw-data, to make things easier for reviewers to read. It's last version is here:
https://lore.kernel.org/git/[email protected]/
This patch series provided %(raw) and %(rest) for ref-filter, which will be used by zh/cat-file-reuse-ref-filter-logic later.
v3--->v4:
1. Change the type of atom_value->s_size from size_t to ssize_t, and change the type of quote_formatting() parameters len from size_t to ssize_t at the same time. All of if (v->s_size == -1) change to if (v->s_size < 0).
2. Correct the commit subject of "[GSOC] ref-filter: --format=%(raw) re-support --perl" to "[GSOC] ref-filter: --format=%(raw) support --perl" and its commit messages.
v3:
https://lore.kernel.org/git/[email protected]/
ZheNing Hu (5):
[GSOC] ref-filter: add obj-type check in grab contents [GSOC] ref-filter: add %(raw) atom
[GSOC] ref-filter: --format=%(raw) support --perl [GSOC] ref-filter: use non-const ref_format in *_atom_parser() [GSOC] ref-filter: add %(rest) atom
Documentation/git-for-each-ref.txt | 9 ++ builtin/tag.c | 2 +- quote.c | 17 ++ quote.h | 1 + ref-filter.c | 243 ++++++++++++++++++++++------- ref-filter.h | 9 +- t/t3203-branch-output.sh | 4 + t/t6300-for-each-ref.sh | 235 ++++++++++++++++++++++++++++ t/t7004-tag.sh | 4 + t/t7030-verify-tag.sh | 4 + 10 files changed, 465 insertions(+), 63 deletions(-)
base-commit: daab8a564f8bbac55f70f8bf86c070e001a9b006
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1000%2Fadlternative%2Fref-filter-raw-data-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1000/adlternative/ref-filter-raw-data-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1000
Range-diff vs v3:
1: d77268bb188 ! 1: 0e294ff1626 [GSOC] ref-filter: add obj-type check in grab contents
@@ Commit message
early when the given object is of a wrong type to help later addition to handle other types of objects in this function.
+ Reviewed-by: Jacob Keller <[email protected]>
2: 0ffa549da6e ! 2: 96d79a3d16b [GSOC] ref-filter: add %(raw) atom
@@ Commit message
variable in such languages, these may not support arbitrary binary data
in their string variable type.
+ Reviewed-by: Jacob Keller <[email protected]>
Mentored-by: Christian Couder <[email protected]>
Mentored-by: Hariom Verma <[email protected]>
Helped-by: Bagas Sanjaya <[email protected]>
@@ ref-filter.c: struct ref_formatting_state {
struct atom_value {
const char *s;
-+ size_t s_size;
++ ssize_t s_size;
int (*handler)(struct atom_value *atomv, struct ref_formatting_state *state,
struct strbuf *err);
uintmax_t value; /* used for sorting when not FIELD_STR */
@@ ref-filter.c: static int parse_ref_filter_atom(const struct ref_format *format,
}
-static void quote_formatting(struct strbuf *s, const char *str, int quote_style)
-+static void quote_formatting(struct strbuf *s, const char *str, size_t len, int quote_style)
++static void quote_formatting(struct strbuf *s, const char *str, ssize_t len, int quote_style)
{
switch (quote_style) {
case QUOTE_NONE:
- strbuf_addstr(s, str);
-+ if (len != -1)
-+ strbuf_add(s, str, len);
-+ else
++ if (len < 0)
+ strbuf_addstr(s, str);
++ else
++ strbuf_add(s, str, len);
break;
case QUOTE_SHELL:
sq_quote_buf(s, str);
@@ ref-filter.c: static int append_atom(struct atom_value *v, struct ref_formatting
*/
if (!state->stack->prev)
- quote_formatting(&state->stack->output, v->s, state->quote_style);
+- else
+ quote_formatting(&state->stack->output, v->s, v->s_size, state->quote_style);
- else
-- strbuf_addstr(&state->stack->output, v->s);
-+ if (v->s_size != ATOM_SIZE_UNSPECIFIED)
-+ strbuf_add(&state->stack->output, v->s, v->s_size);
-+ else
-+ strbuf_addstr(&state->stack->output, v->s);
++ else if (v->s_size < 0)
+ strbuf_addstr(&state->stack->output, v->s);
++ else
++ strbuf_add(&state->stack->output, v->s, v->s_size);
return 0;
}
@@ ref-filter.c: static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array
- cmp_fn = s->sort_flags & REF_SORTING_ICASE
- ? strcasecmp : strcmp;
- cmp = cmp_fn(va->s, vb->s);
-+ if (va->s_size == ATOM_SIZE_UNSPECIFIED && vb->s_size == ATOM_SIZE_UNSPECIFIED) {
++ if (va->s_size < 0 && vb->s_size < 0) {
+ int (*cmp_fn)(const char *, const char *);
+ cmp_fn = s->sort_flags & REF_SORTING_ICASE
+ ? strcasecmp : strcmp;
+ cmp = cmp_fn(va->s, vb->s);
+ } else {
-+ size_t a_size = va->s_size == ATOM_SIZE_UNSPECIFIED ?
++ size_t a_size = va->s_size < 0 ?
+ strlen(va->s) : va->s_size;
-+ size_t b_size = vb->s_size == ATOM_SIZE_UNSPECIFIED ?
++ size_t b_size = vb->s_size < 0 ?
+ strlen(vb->s) : vb->s_size;
+ int (*cmp_fn)(const void *, const void *, size_t);
+ cmp_fn = s->sort_flags & REF_SORTING_ICASE 3: ac5d98647da ! 3: d14fae52080 [GSOC] ref-filter: --format=%(raw) re-support --perl
@@ Metadata
Author: ZheNing Hu <[email protected]>
## Commit message ##
- [GSOC] ref-filter: --format=%(raw) re-support --perl
+ [GSOC] ref-filter: --format=%(raw) support --perl
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.
+ at '\0' to help --format="%(raw)" support --perl.
+ Reviewed-by: Jacob Keller <[email protected]>
@@ quote.h: char *quote_path(const char *in, const char *prefix, struct strbuf *out
void basic_regex_quote_buf(struct strbuf *sb, const char *src);
## ref-filter.c ##
-@@ ref-filter.c: static void quote_formatting(struct strbuf *s, const char *str, size_t len, int +@@ ref-filter.c: static void quote_formatting(struct strbuf *s, const char *str, ssize_t len, int
sq_quote_buf(s, str);
break;
case QUOTE_PERL:
- perl_quote_buf(s, str);
-+ if (len != -1)
-+ perl_quote_buf_with_len(s, str, len);
-+ else
++ if (len < 0)
+ perl_quote_buf(s, str);
++ else
++ perl_quote_buf_with_len(s, str, len);
break;
case QUOTE_PYTHON:
python_quote_buf(s, str);
4: 0722f546df9 = 4: 0aa398e8a8f [GSOC] ref-filter: use non-const ref_format in *_atom_parser() 5: 2300b0463f2 ! 5: 207cc512964 [GSOC] ref-filter: add %(rest) atom
@@ Commit message
Introduce the reject_atom() to reject the atom %(rest) for "git for-each-ref", "git branch", "git tag" and "git verify-tag".
+ Reviewed-by: Jacob Keller <[email protected]>
Signature · 1 line
gitgitgadget
PATCH v4 2/5 ref-filter: add %(raw) atom
From: ZheNing Hu <[email protected]>
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.
--- Documentation/git-for-each-ref.txt | 9 ++ ref-filter.c | 143 +++++++++++++++---- t/t6300-for-each-ref.sh | 216 +++++++++++++++++++++++++++++ 3 files changed, 340 insertions(+), 28 deletions(-)
Documentation/git-for-each-ref.txt +9 -0
@@ -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:
ref-filter.c +115 -28
292 lines not on this page · raw message
t/t6300-for-each-ref.sh +216 -0
258 lines not on this page · raw message
Signature · 1 line
gitgitgadget
PATCH v4 3/5 ref-filter: --format=%(raw) support --perl
In reply to ZheNing Hu via GitGitGadgetFrom: ZheNing Hu <[email protected]>
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)" support --perl.
--- 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(-)
Documentation/git-for-each-ref.txt +2 -2
@@ -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:
quote.c +17 -0
@@ -471,6 +471,23 @@ void perl_quote_buf(struct strbuf *sb, const char *src)strbuf_addch (sb, sq); } +void perl_quote_buf_with_len (struct strbuf *sb,const char *src,size_t len) +{ +const char sq ='\'' ; +const char bq ='\\' ; +const char *c = src; +const char *end = src + len; + +strbuf_addch (sb, sq); +while (c != end) { +if (*c == sq || *c == bq) +strbuf_addch (sb, bq); +strbuf_addch (sb, *c); + c++; + } +strbuf_addch (sb, sq); +} +void python_quote_buf (struct strbuf *sb,const char *src) {const char sq ='\'' ;
quote.h +1 -0
@@ -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 */ void perl_quote_buf (struct strbuf *sb,const char *src); +void perl_quote_buf_with_len (struct strbuf *sb,const char *src,size_t len);void python_quote_buf (struct strbuf *sb,const char *src);void tcl_quote_buf (struct strbuf *sb,const char *src);void basic_regex_quote_buf (struct strbuf *sb,const char *src);
ref-filter.c +11 -4
@@ -746,7 +746,10 @@ static void quote_formatting(struct strbuf *s, const char *str, ssize_t len, intsq_quote_buf (s, str);break ;case QUOTE_PERL : -perl_quote_buf (s, str); +if (len <0 ) +perl_quote_buf (s, str); +else +perl_quote_buf_with_len (s, str, len);break ;case QUOTE_PYTHON :python_quote_buf (s, str);
@@ -1009,10 +1012,14 @@ 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 ) + +if ((format->quote_style ==QUOTE_PYTHON || + format->quote_style ==QUOTE_SHELL || + format->quote_style ==QUOTE_TCL ) && + 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 ); +"--python, --shell, --tcl" ), (int )(ep - sp -2 ), sp +2 ); cp = ep +1 ;if (skip_prefix (used_atom[at].name,"color:" , &color))
t/t6300-for-each-ref.sh +17 -2
@@ -915,8 +915,23 @@ test_expect_success '%(raw) with --tcl must fail' ' test_must_fail git for-each-ref --format="%(raw)" --tcl ' -test_expect_success '%(raw) with --perl must fail' ' - test_must_fail git for-each-ref --format="%(raw)" --perl +test_expect_success '%(raw) with --perl' ' + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/blob1 --perl | perl >actual && + cmp blob1 actual && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/blob3 --perl | perl >actual && + cmp blob3 actual && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/blob8 --perl | perl >actual && + cmp blob8 actual && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/myblobs/first --perl | perl >actual && + cmp one actual && + git cat-file tree refs/mytrees/first > expected && + git for-each-ref --format="\$name= %(raw); +print \"\$name\"" refs/mytrees/first --perl | perl >actual && + cmp expected actual ' test_expect_success '%(raw) with --shell must fail' '
Signature · 1 line
gitgitgadget
PATCH v4 4/5 ref-filter: use non-const ref_format in *_atom_parser()
In reply to ZheNing Hu via GitGitGadgetFrom: ZheNing Hu <[email protected]>
Use non-const ref_format in *_atom_parser(), which can help us modify the members of ref_format in *_atom_parser().
--- builtin/tag.c | 2 +- ref-filter.c | 44 ++++++++++++++++++++++---------------------- ref-filter.h | 4 ++-- 3 files changed, 25 insertions(+), 25 deletions(-)
builtin/tag.c +1 -1
@@ -146,7 +146,7 @@ static int verify_tag(const char *name, const char *ref, const struct object_id *oid, void *cb_data) { int flags; -const struct ref_format *format = cb_data; +struct ref_format *format = cb_data; flags =GPG_VERIFY_VERBOSE ;if (format->format)
ref-filter.c +22 -22
164 lines not on this page · raw message
ref-filter.h +2 -2
@@ -116,7 +116,7 @@ void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);void ref_sorting_set_sort_flags_all (struct ref_sorting *sorting,unsigned int mask,int on);/* Based on the given format and quote_style, fill the strbuf */ int format_ref_array_item (struct ref_array_item *info, -const struct ref_format *format, +struct ref_format *format,struct strbuf *final_buf,struct strbuf *error_buf);/* Parse a single sort specifier and add it to the list */
@@ -137,7 +137,7 @@ void setup_ref_filter_porcelain_msg(void);* name must be a fully qualified refname. */ void pretty_print_ref (const char *name,const struct object_id *oid, -const struct ref_format *format); +struct ref_format *format);/* * Push a single ref onto the array; this can be used to construct your own
Signature · 1 line
gitgitgadget
PATCH v4 5/5 ref-filter: add %(rest) atom
In reply to ZheNing Hu via GitGitGadgetFrom: ZheNing Hu <[email protected]>
%(rest) is a atom used for cat-file batch mode, which can split the input lines at the first whitespace boundary, all characters before that whitespace are considered to be the object name; characters after that first run of whitespace (i.e., the "rest" of the line) are output in place of the %(rest) atom.
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".
--- 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(-)
ref-filter.c +25 -0
@@ -157,6 +157,7 @@ enum atom_type {ATOM_IF ,ATOM_THEN ,ATOM_ELSE , + ATOM_REST, };/*
@@ -559,6 +560,15 @@ static int if_atom_parser(struct ref_format *format, struct used_atom *atom,return 0 ; } +static int rest_atom_parser (struct ref_format *format,struct used_atom *atom, +const char *arg,struct strbuf *err) +{ +if (arg) +return strbuf_addf_ret (err,-1 ,_ ("%%(rest) does not take arguments" )); + format->use_rest =1 ; +return 0 ; +} +static int head_atom_parser (struct ref_format *format,struct used_atom *atom,const char *arg,struct strbuf *unused_err) {
@@ -615,6 +625,7 @@ static struct { [ATOM_IF] = { "if", SOURCE_NONE, FIELD_STR, if_atom_parser }, [ATOM_THEN] = { "then", SOURCE_NONE }, [ATOM_ELSE] = { "else", SOURCE_NONE }, + [ATOM_REST] = { "rest", SOURCE_NONE, FIELD_STR, rest_atom_parser },/* * Please update $__git_ref_fieldlist in git-completion.bash * when you add new atoms
@@ -992,6 +1003,11 @@ static const char *find_next(const char *cp)return NULL ; } +static int reject_atom (enum atom_type atom_type) +{ +return atom_type ==ATOM_REST ; +} +/* * Make sure the format string is well formed, and parse out * the used atoms.
@@ -1012,6 +1028,8 @@ 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 (reject_atom (used_atom[at].atom_type)) +die (_ ("this command reject atom %%(%.*s)" ), (int )(ep - sp -2 ), sp +2 ); if ((format->quote_style == QUOTE_PYTHON || format->quote_style == QUOTE_SHELL ||
@@ -1931,6 +1949,12 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) v->handler = else_atom_handler; v->s =xstrdup ("" );continue ; + }else if (atom_type ==ATOM_REST ) { +if (ref->rest) + v->s =xstrdup (ref->rest); +else + v->s =xstrdup ("" ); +continue ; }else continue ;
@@ -2148,6 +2172,7 @@ static struct ref_array_item *new_ref_array_item(const char *refname,FLEX_ALLOC_STR (ref, refname, refname);oidcpy (&ref->objectname, oid); + ref->rest =NULL ;return ref; }
ref-filter.h +4 -1
@@ -38,6 +38,7 @@ struct ref_sorting {struct ref_array_item {struct object_id objectname; +const char *rest;int flag;unsigned int kind;const char *symref;
@@ -76,14 +77,16 @@ struct ref_format {* verify_ref_format() afterwards to finalize. */ const char *format; +const char *rest;int quote_style; +int use_rest;int use_color;/* Internal state to ref-filter */ int need_color_reset_at_eol; }; -#define REF_FORMAT_INIT { NULL, 0, -1 } +#define REF_FORMAT_INIT { .use_color = -1 }/* Macros for checking --merged and --no-merged options */ #define _OPT_MERGED_NO_MERGED (option, filter, h) \
t/t3203-branch-output.sh +4 -0
@@ -340,6 +340,10 @@ test_expect_success 'git branch --format option' ' test_cmp expect actual ' +test_expect_success 'git branch with --format=%(rest) must fail' ' + test_must_fail git branch --format="%(rest)" >actual +' + test_expect_success 'worktree colors correct' ' cat >expect <<-EOF && * <GREEN>(HEAD detached from fromtag)<RESET>
t/t6300-for-each-ref.sh +4 -0
@@ -1211,6 +1211,10 @@ test_expect_success 'basic atom: head contents:trailers' ' test_cmp expect actual.clean ' +test_expect_success 'basic atom: rest must fail' ' + test_must_fail git for-each-ref --format="%(rest)" refs/heads/main +' + test_expect_success 'trailer parsing not fooled by --- line' ' git commit --allow-empty -F - <<-\EOF && this is the subject
t/t7004-tag.sh +4 -0
@@ -1998,6 +1998,10 @@ test_expect_success '--format should list tags as per format given' ' test_cmp expect actual ' +test_expect_success 'git tag -l with --format="%(rest)" must fail' ' + test_must_fail git tag -l --format="%(rest)" "v1*" +' + test_expect_success "set up color tests" ' echo "<RED>v1.0<RESET>" >expect.color && echo "v1.0" >expect.bare &&
t/t7030-verify-tag.sh +4 -0
@@ -194,6 +194,10 @@ test_expect_success GPG 'verifying tag with --format' ' test_cmp expect actual ' +test_expect_success GPG 'verifying tag with --format="%(rest)" must fail' ' + test_must_fail git verify-tag --format="%(rest)" "fourth-signed" +' + test_expect_success GPG 'verifying a forged tag with --format should fail silently' ' test_must_fail git verify-tag --format="tagname : %(tag)" $(cat forged1.tag) >actual-forged && test_must_be_empty actual-forged
Signature · 1 line
gitgitgadget
Re: [PATCH v3 5/5] [GSOC] ref-filter: add %(rest) atom
In reply to Jacob KellerQuoted 23 lines · Jacob Keller
Quoted 12 lines · ZheNing Hu via GitGitGadget
From: ZheNing Hu <[email protected]>
%(rest) is a atom used for cat-file batch mode, which can split the input lines at the first whitespace boundary, all characters before that whitespace are considered to be the object name; characters after that first run of whitespace (i.e., the "rest" of the line) are output in place of the %(rest) atom.
In order to let "cat-file --batch=%(rest)" use the ref-filter interface, add %(rest) atom for ref-filter.
Quoted 8 lines
Introduce the
reject_atom()to reject the atom %(rest) for "git for-each-ref", "git branch", "git tag" and "git verify-tag".Suggected-by Jacob Keller <[email protected]>Mentored-by Christian Couder <[email protected]>Mentored-by Hariom Verma <[email protected]>Signed-off-by ZheNing Hu <[email protected]>---
Quoted 4 lines
Thanks for the improved commit message here. That helps clarify what this atom does and aids in understanding the implementation.
Reviewed-by Jacob Keller <[email protected]>
Thanks, both.
Re: [PATCH v4 1/5] [GSOC] ref-filter: add obj-type check in grab contents
In reply to ZheNing Hu via GitGitGadgetQuoted 15 lines · ZheNing Hu via GitGitGadget
From: ZheNing Hu <[email protected]>
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. So it's needed to pass astruct expand_data *datainstread of onlyvoid *bufto bothgrab_sub_body_contents()andgrab_values()to be able to check the object type.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.
Quoted 5 lines
Reviewed-by Jacob Keller <[email protected]>Mentored-by Christian Couder <[email protected]>Mentored-by Hariom Verma <[email protected]>Helped-by Junio C Hamano <[email protected]>Signed-off-by ZheNing Hu <[email protected]>
It is not a huge deal, but are these trailers given in a sensible order? The order of event happened were that help and mentoring were given, your finished version of the patch came after that to be signed off by you, and then a reviewer gave you a reviewed-by, no?
Re: [PATCH v4 1/5] [GSOC] ref-filter: add obj-type check in grab contents
Quoted 24 lines · Junio C Hamano <[email protected]> 于2021年7月27日周二 上午3:15写道:
Quoted 15 lines · ZheNing Hu via GitGitGadget
From: ZheNing Hu <[email protected]>
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. So it's needed to pass astruct expand_data *datainstread of onlyvoid *bufto bothgrab_sub_body_contents()andgrab_values()to be able to check the object type.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.
Quoted 5 lines
Reviewed-by Jacob Keller <[email protected]>Mentored-by Christian Couder <[email protected]>Mentored-by Hariom Verma <[email protected]>Helped-by Junio C Hamano <[email protected]>Signed-off-by ZheNing Hu <[email protected]>
Quoted 4 lines
It is not a huge deal, but are these trailers given in a sensible order? The order of event happened were that help and mentoring were given, your finished version of the patch came after that to be signed off by you, and then a reviewer gave you a reviewed-by, no?
Indeed so.
Thanks.
--
ZheNing Hu