ref-filter: add %(raw) atom
cover ref-filter: add %(raw) atom
In order to make git cat-file --batch use ref-filter logic, I add %(raw) atom to ref-filter.
Change from last version:
1. Change is_empty() logic.
2. Simplify memcasecmp().
3. rebase on zh/ref-filter-atom-type.
ZheNing Hu (2):
[GSOC] ref-filter: add obj-type check in grab contents [GSOC] ref-filter: add %(raw) atom
Documentation/git-for-each-ref.txt | 9 ++ ref-filter.c | 164 +++++++++++++++++------ t/t6300-for-each-ref.sh | 207 +++++++++++++++++++++++++++++ 3 files changed, 343 insertions(+), 37 deletions(-)
base-commit: 1197f1a46360d3ae96bd9c15908a3a6f8e562207
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-966%2Fadlternative%2Fref-filter-raw-atom-v4-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-966/adlternative/ref-filter-raw-atom-v4-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/966
Signature · 1 line
gitgitgadget
PATCH 1/2 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 | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
ref-filter.c +11 -7
@@ -1356,7 +1356,8 @@ 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,void *buf, +struct object *obj) {int i;const char *subpos =NULL , *bodypos =NULL , *sigpos =NULL ;
@@ -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 ((obj->type !=OBJ_TAG && + obj->type !=OBJ_COMMIT ) || + (strcmp (name,"body" ) && + !starts_with (name,"subject" ) && + !starts_with (name,"trailers" ) && + !starts_with (name,"contents" )))continue ;if (!subpos) find_subpos(buf,
@@ -1443,12 +1447,12 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, vswitch (obj->type) {case OBJ_TAG :grab_tag_values (val, deref, obj); -grab_sub_body_contents (val, deref, buf); +grab_sub_body_contents (val, deref, buf, obj);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, buf, obj);grab_person ("author" , val, deref, buf);grab_person ("committer" , val, deref, buf);break ;
Signature · 1 line
gitgitgadget
PATCH 2/2 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 depands 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, 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.
Beyond, --format=%(raw) cannot be used with --python, --shell, --tcl, --perl because if our binary raw data is passed to a variable in the host language, the host language may not support arbitrary binary data in the variables of its string type.
--- Documentation/git-for-each-ref.txt | 9 ++ ref-filter.c | 152 ++++++++++++++++----- t/t6300-for-each-ref.sh | 207 +++++++++++++++++++++++++++++ 3 files changed, 335 insertions(+), 33 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 a 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 the host language may not support arbitrary binary data in the +variables of its string 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 +119 -33
320 lines not on this page · raw message
t/t6300-for-each-ref.sh +207 -0
249 lines not on this page · raw message
Signature · 1 line
gitgitgadget
Re: [PATCH 1/2] [GSOC] ref-filter: add obj-type check in grab contents
In reply to ZheNing Hu via GitGitGadgetref-filter.c static void append_lines(struct strbuf *out, const char *buf, unsigned long size · line 1360
Quoted 4 lines · ZheNing Hu via GitGitGadget
/* See
grab_values*/ref-filter.c +2
-1-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,void *buf, +struct object *obj)
Neither this step or the next change needs anything but type member of the 'obj' (and 'buf' is coming from oi.content of the result of asking about that same 'obj').
I wonder if we should do one of the following:
(1) stop passing "void *buf" and instead "struct expand_data
*data", and use "data->content" to access "buf", which would
allow you to access "data->type" to perform the added check. (2) instead of adding "struct obj *obj" to the parameters, just add "enum object_type type", as that is the only thing you need.
Obviously (2) is with lessor impact, but if it can be done safely without breaking the code [*], (1) would probably be a much more preferrable direction to go in the longer term.
Side note [*]. A caller is allowed to choose to feed "buf" that is different from "oi.content" (perhaps buf may sometimes want to be a utf-8 recoded version of oi.content for certain types of objects) with the current system, but if we pass expand_data throughout the callchain, such a caller is broken, for example.
Quoted 19 lines · ZheNing Hu via GitGitGadget
ref-filter.c +1
-7@@ -644,13 +664,6 @@ static int parse_ref_filter_atom(const struct ref_format *format,return strbuf_addf_ret (err,-1 ,_ ("malformed field name: %.*s" ), (int )(ep-atom), atom); -/* Do we have the atom already used elsewhere? */ -for (i =0 ; i < used_atom_cnt; i++) { -int len =strlen (used_atom[i].name); -if (len == ep - atom && !memcmp (used_atom[i].name, atom, len)) -return i; - } -/* * If the atom name has a colon, strip it and everything after * it off - it specifies the format for this entry, and @@ -660,6 +673,17 @@ static int parse_ref_filter_atom(const struct ref_format *format, arg =memchr (sp,':' , ep - sp); atom_len = (arg ? arg : ep) - sp; +if (format->quote_style && !strncmp (sp,"raw" ,3 ) && !arg)
ref-filter.c static int parse_ref_filter_atom(const struct ref_format *format, · line 686
Quoted 10 lines
ref-filter.c +10
-0+return strbuf_addf_ret (err,-1 ,_ ("--format=%.*s cannot be used with" +"--python, --shell, --tcl, --perl" ), (int )(ep-atom), atom); + +/* Do we have the atom already used elsewhere? */ +for (i =0 ; i < used_atom_cnt; i++) { +int len =strlen (used_atom[i].name); +if (len == ep - atom && !memcmp (used_atom[i].name, atom, len)) +return i; + } +
These two hunks
- hoists up the code that sets 'arg' to optional string after "<atom>:" and counts how long the "<atom>" is in 'atom_len'; the change causes the counting done even when the same placeholder is already used elsewhere (in which case we do not have to do such counting);
- inserts the early return to reject use of "raw" atom when language specific quoting is used.
It probably makes it easier to understand if the former is split into a separate commit, but at the same time a series with too many small steps is harder to manage, so let's keep them in a single change.
But I do not think we want to add the new change at this location, at least for two reasons:
* The posted patch checks '!arg' to avoid rejecting "raw:size", which would not scale at all. What if you wanted to later add "raw:upcase", which you must reject?
* We do have enumerated constants for each atom types, but this early check and return does string comparison.
Where it belongs is either after "Is the atom a valid one?" loop where 'atom_len' is used to locate the placeholder's atom in the table of valid atoms [*], or inside raw_atom_parser().
Side note: If you read the original code, you would notice that there already is a similar "this is a valid atom that appear in the valid_atom[] table, but unallowed in this situation" check done with .source != SOURCE_NONE conditional. One downside is that until calling raw_atom_parser(), you do not know if RAW_BARE or RAW_LENGTH is requested.
If we do inside raw_atom_parser(), it would probably look like this:
+static int raw_atom_parser(const struct ref_format *format, struct used_atom *atom,
+ const char *arg, struct strbuf *err)
+{
+ if (!arg)
+ atom->u.raw_data.option = RAW_BARE;
+ else if (!strcmp(arg, "size"))
+ atom->u.raw_data.option = RAW_LENGTH;
+ else
+ return strbuf_addf_ret(err, -1, _("unrecognized %%(raw) argument: %s"), arg);
+
+ if (atom->u.raw_data.option == RAW_BARE && format->quote_style)
+ return strbuf_addf_ret(err, -1,
+ _("--format=%%(raw) cannot be used with ...")...);
+
+ return 0;
+}
Re: [PATCH 1/2] [GSOC] ref-filter: add obj-type check in grab contents
In reply to Junio C HamanoQuoted 18 lines · Junio C Hamano <[email protected]> 于2021年6月3日周四 上午10:10写道：
ref-filter.c static void append_lines(struct strbuf *out, const char *buf, unsigned long size · line 1360
Quoted 4 lines · ZheNing Hu via GitGitGadget
/* See
grab_values*/ref-filter.c +2
-1-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,void *buf, +struct object *obj)Neither this step or the next change needs anything but type member of the 'obj' (and 'buf' is coming from oi.content of the result of asking about that same 'obj').
I wonder if we should do one of the following:
(1) stop passing "void *buf" and instead "struct
expand_data*data", and use "data->content" to access "buf", which would allow you to access "data->type" to perform the added check.
Quoted 7 lines
(2) instead of adding "struct obj *obj" to the parameters, just add "enum
object_typetype", as that is the only thing you need.Obviously (2) is with lessor impact, but if it can be done safely without breaking the code [*], (1) would probably be a much more preferrable direction to go in the longer term.
I agree with (1). In future versions of grab_sub_body_contents(), we will use the content of "data" more frequently instead of using the crude "obj". The type provided by "obj" can also be provided by "data". So yes, I would be very willing to let grab_sub_body_contents() only use "data". (delete "obj")
E.g.
static void grab_sub_body_contents(struct atom_value *val, int deref, struct expand_data *data)
Using (2), we will need more parameters to pass other object info.
Quoted 6 lines
Side note [*]. A caller is allowed to choose to feed "buf" that is different from "oi.content" (perhaps buf may sometimes want to be a utf-8 recoded version of oi.content for certain types of objects) with the current system, but if we pass
expand_datathroughout the callchain, such a caller is broken, for example.
Just see the situation in front of us: grab_sub_body_contents() have only one caller: grab_values(). If someone need a function like grab_sub_body_contents() to grab another buf, they can use rewrite a more universal function interface:
static void grab_sub_body_contents(struct atom_value *val, int deref, struct expand_data *data)
{
grab_sub_body_contents_internal(val, deref, data->content,data->size, data->type);
}
static void grab_sub_body_contents_internal(struct atom_value *val, int deref, void *buf,
unsigned long buf_size,
enum object_type type)
{
...
}
But for the time being, the above one is sufficient.
Thanks.
--
ZheNing Hu
Hi,
Quoted 9 lines · ZheNing Hu via GitGitGadget
In order to make git cat-file --batch use ref-filter logic, I add %(raw) atom to ref-filter.
Change from last version:
1. Change
is_empty()logic.
2. Simplifymemcasecmp().
3. rebase on zh/ref-filter-atom-type.
I prefer no first-person pronouns (I and we) in patch cover letter and commit message, so better say:
"Add %(raw) atom to ref-filter to make git cat-file --batch use ref-filter logic."
Signature · 1 line
An old man doll... just what I always wanted! - Clara
Quoted 10 lines · Junio C Hamano <[email protected]> 于2021年6月3日周四 上午10:38写道：
These two hunks
- hoists up the code that sets 'arg' to optional string after "<atom>:" and counts how long the "<atom>" is in '
atom_len'; the change causes the counting done even when the same placeholder is already used elsewhere (in which case we do not have to do such counting);
I admit that I am doing repetitive work here.
Quoted 8 lines
- inserts the early return to reject use of "raw" atom when language specific quoting is used.
It probably makes it easier to understand if the former is split into a separate commit, but at the same time a series with too many small steps is harder to manage, so let's keep them in a single change.
Quoted 7 lines
But I do not think we want to add the new change at this location, at least for two reasons:
* The posted patch checks '!arg' to avoid rejecting "raw:size", which would not scale at all. What if you wanted to later add "raw:upcase", which you must reject?
Yeah, the code here makes "raw" lack of scalability. Especially we want to add "%(raw:textconv)" and "%(raw:filter)" later.
Quoted 3 lines
* We do have enumerated constants for each atom types, but this early check and return does string comparison.
Note that at this time we must compare strings... parse_ref_filter_atom() passes string form of the atom. Code block A also requires comparing strings.
-------------------
Code block A:
for (i = 0; i < used_atom_cnt; i++) {
int len = strlen(used_atom[i].name);
if (len == ep - atom && !memcmp(used_atom[i].name, atom, len))
return i;
}-------------------
All the following replies are based on such a fact:
We will reuse used atoms as much as possible.
Think about this situation:
$ git for-each-ref --format="%(raw)" --sort="raw" --python
Since we specified --sort="raw",parse_sorting_atom()
will be called in parse_opt_ref_sorting(), but at this time we haven't parsed --<lang> yet. So format->quote_style == 0, we cannot refuse --<lang> at this time, and a "%(raw)" atom item will be added to used_atom, when we use verify_ref_format() to call parse_sorting_atom() for the second time, we already have raw atom item in used_atom, in Code Block A we directly returned, We can't refuse --<lang> too after Code Block A. So as a last solution, we refuse --<lang> with "%(raw)" before Code Block A.
Quoted 11 lines
Where it belongs is either after "Is the atom a valid one?" loop where '
atom_len' is used to locate the placeholder's atom in the table of valid atoms [*], or insideraw_atom_parser().Side note: If you read the original code, you would notice that there already is a similar "this is a valid atom that appear in the valid_atom[] table, but unallowed in this situation" check done with .source !=
SOURCE_NONEconditional. One downside is that until callingraw_atom_parser(), you do not know ifRAW_BAREorRAW_LENGTHis requested.
Yes, but we need to pay attention to it is below Code Block A.
Quoted 8 lines
If we do inside
raw_atom_parser(), it would probably look like this:ref-filter.c +6
-0+static int raw_atom_parser (const struct ref_format *format,struct used_atom *atom, +const char *arg,struct strbuf *err) +{ +if (!arg) + atom->u.raw_data.option =RAW_BARE ; +else if (!strcmp (arg,"size" ))
ref-filter.c static int contents_atom_parser(const struct ref_format *format, struct used_ato
Quoted 10 lines
ref-filter.c +10
-0+ atom->u.raw_data.option =RAW_LENGTH ; + else + return strbuf_addf_ret(err, -1, _("unrecognized %%(raw) argument: %s"), arg); + +if (atom->u.raw_data.option ==RAW_BARE && format->quote_style) +return strbuf_addf_ret (err,-1 , + _("--format=%%(raw) cannot be used with ...")...); + +return 0 ; +}
It's same, "*.parser()" is below Code Block A.
After all, the reason why this must be done here is the ref-filter original logic
has not considered rejecting a format atom and an option.
Thanks.
--
ZheNing Hu
Quoted 14 lines · Bagas Sanjaya <[email protected]> 于2021年6月3日周四 下午1:11写道：
Hi,
Quoted 9 lines · ZheNing Hu via GitGitGadget
In order to make git cat-file --batch use ref-filter logic, I add %(raw) atom to ref-filter.
Change from last version:
1. Change
is_empty()logic.
2. Simplifymemcasecmp().
3. rebase on zh/ref-filter-atom-type.
Quoted 6 lines
I prefer no first-person pronouns (I and we) in patch cover letter and commit message, so better say:
"Add %(raw) atom to ref-filter to make git cat-file --batch use ref-filter logic."
Ok. I will change my way of narrating.
Quoted 2 lines
--
An old man doll... just what I always wanted! - Clara
Quoted 5 lines · ZheNing Hu <[email protected]> 于2021年6月3日周四 下午1:36写道：
Quoted 3 lines · Junio C Hamano <[email protected]> 于2021年6月3日周四 上午10:38写道：
But I do not think we want to add the new change at this location, at least for two reasons:
Quoted 8 lines
Quoted 4 lines
* The posted patch checks '!arg' to avoid rejecting "raw:size", which would not scale at all. What if you wanted to later add "raw:upcase", which you must reject?
Yeah, the code here makes "raw" lack of scalability. Especially we want to add "%(raw:textconv)" and "%(raw:filter)" later.
Now I am building %(raw:textconv) and %(raw:filter), the code will be very difficult to write:
if (format->quote_style && !strncmp(sp, "raw", 3)
&& ((!arg) || (!strncmp(arg,
":textconv", 9)) || (!strncmp(arg, ":filter", 7))))
return strbuf_addf_ret(err, -1, _("--format=%.*s
cannot be used with"
"--python, --shell, --tcl, --perl"),(int)(ep-atom), atom);
Is there any good way?
Thanks.
--
ZheNing Hu
Quoted 10 lines · ZheNing Hu
Quoted 3 lines
* We do have enumerated constants for each atom types, but this early check and return does string comparison.
Note that at this time we must compare strings...
parse_ref_filter_atom()passes string form of the atom. Code block A also requires comparing strings.-------------------
Code block A:
Quoted 6 lines
for (i = 0; i < used_atom_cnt; i++) { int len = strlen(used_atom[i].name); if (len == ep - atom && !memcmp(used_atom[i].name, atom, len)) return i; }-------------------
The point is that you can piggyback existing string comparison (which is called "parsing") and use the parsed result (i.e. if you can compare with ATOM_RAW instead of adding another strcmp(), that can be a better solution).
Quoted 10 lines
All the following replies are based on such a fact:
We will reuse used atoms as much as possible.Think about this situation:
$ git for-each-ref --format="%(raw)" --sort="raw" --pythonSince we specified --sort="raw",
parse_sorting_atom()will be called inparse_opt_ref_sorting(), but at this time we haven't parsed --<lang> yet.
That only says using parse_sorting_atom() and relying on the check in the function is still too early, and does not necessarily support the posted patch that redundantly runs strcmp().
After parsing all the command line options, we have used_atom[] fully populated and we know what host language we are quoting the result for---and that makes a good place to check for comflicting requests.
Quoted 19 lines
Quoted 6 lines
ref-filter.c +6
-0+static int raw_atom_parser (const struct ref_format *format,struct used_atom *atom, +const char *arg,struct strbuf *err) +{ +if (!arg) + atom->u.raw_data.option =RAW_BARE ; +else if (!strcmp (arg,"size" ))ref-filter.c static int contents_atom_parser(const struct ref_format *format, struct used_ato
Quoted 10 lines
ref-filter.c +10
-0+ atom->u.raw_data.option =RAW_LENGTH ; + else + return strbuf_addf_ret(err, -1, _("unrecognized %%(raw) argument: %s"), arg); + +if (atom->u.raw_data.option ==RAW_BARE && format->quote_style) +return strbuf_addf_ret (err,-1 , + _("--format=%%(raw) cannot be used with ...")...); + +return 0 ; +}It's same, "*.parser()" is below Code Block A.
Quoted 3 lines
After all, the reason why this must be done here is the ref-filter original logic
has not considered rejecting a format atom and an option.
That is something you can fix to make the code easier to follow, no?
Quoted 9 lines · ZheNing Hu
Now I am building %(raw:textconv) and %(raw:filter), the code will be very difficult to write:
if (format->quote_style && !strncmp(sp, "raw", 3) && ((!arg) || (!strncmp(arg, ":textconv", 9)) || (!strncmp(arg, ":filter", 7)))) return strbuf_addf_ret(err, -1, _("--format=%.*s cannot be used with" "--python, --shell, --tcl, --perl"),
Quoted 3 lines
(int)(ep-atom), atom);
Is there any good way?
The problem you are having sounds like a natural consequence of doing the check at the wrong place in the code, at least to me.
Quoted 6 lines · Junio C Hamano <[email protected]> 于2021年6月4日周五 上午5:35写道：
The point is that you can piggyback existing string comparison (which is called "parsing") and use the parsed result (i.e. if you can compare with
ATOM_RAWinstead of adding anotherstrcmp(), that can be a better solution).
Quoted 9 lines
That only says using
parse_sorting_atom()and relying on the check in the function is still too early, and does not necessarily support the posted patch that redundantly runsstrcmp().After parsing all the command line options, we have used_atom[] fully populated and we know what host language we are quoting the result for---and that makes a good place to check for comflicting requests.
Alright, I got it: we can perform related check in verify_ref_format(), after parse_ref_filter_atom(), It is a good checkpoint.
Quoted 5 lines
Quoted 3 lines
After all, the reason why this must be done here is the ref-filter original logic
has not considered rejecting a format atom and an option.That is something you can fix to make the code easier to follow, no?
You are right. ;-)
Thanks.
--
ZheNing Hu
v2 cover ref-filter: add %(raw) atom
In reply to ZheNing Hu via GitGitGadgetIn order to make git cat-file --batch use ref-filter logic, %(raw) atom is adding to ref-filter.
Change from last version:
1. Change --<lang> and --format=%(raw) checkpoint to verify_ref_format(), which make it more scalable.
2. Change grab_sub_body_contents() use struct expand_data *data instread of using obj,buf,buf_size to pass object info which can reduce the delivery of function parameters.
ZheNing Hu (2):
[GSOC] ref-filter: add obj-type check in grab contents [GSOC] ref-filter: add %(raw) atom
Documentation/git-for-each-ref.txt | 9 ++ ref-filter.c | 164 +++++++++++++++++------ t/t6300-for-each-ref.sh | 207 +++++++++++++++++++++++++++++ 3 files changed, 343 insertions(+), 37 deletions(-)
base-commit: 1197f1a46360d3ae96bd9c15908a3a6f8e562207
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-966%2Fadlternative%2Fref-filter-raw-atom-v4-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-966/adlternative/ref-filter-raw-atom-v4-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/966
Range-diff vs v1:
1: 97955705c22e ! 1: 48d256db5c34 [GSOC] ref-filter: add obj-type check in grab contents
@@ ref-filter.c: static void append_lines(struct strbuf *out, const char *buf, unsi
/* 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, void *buf,
-+ struct object *obj)
++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];
@@ ref-filter.c: static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)
continue;
if (deref)
@@ ref-filter.c: static void grab_sub_body_contents(struct atom_value *val, int der
- !starts_with(name, "trailers") &&
- !starts_with(name, "contents"))
+
-+ if ((obj->type != OBJ_TAG &&
-+ obj->type != OBJ_COMMIT) ||
++ if ((data->type != OBJ_TAG &&
++ data->type != OBJ_COMMIT) ||
+ (strcmp(name, "body") &&
+ !starts_with(name, "subject") &&
+ !starts_with(name, "trailers") &&
@@ ref-filter.c: static void grab_sub_body_contents(struct atom_value *val, int der
continue;
if (!subpos)
find_subpos(buf,
-@@ ref-filter.c: static void grab_values(struct atom_value *val, int deref, struct object *obj, v
+@@ ref-filter.c: 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, buf, obj);
++ 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, buf, obj);
++ grab_sub_body_contents(val, deref, data);
grab_person("author", val, deref, buf);
grab_person("committer", val, deref, buf);
break;
+@@ ref-filter.c: static int get_object(struct ref_array_item *ref, int deref, struct object **obj
+ return 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); 2: 5a94705cdbc1 ! 2: 0efed9435b59 [GSOC] ref-filter: add %(raw) atom
@@ ref-filter.c: static int parse_ref_filter_atom(const struct ref_format *format,
arg = memchr(sp, ':', ep - sp);
atom_len = (arg ? arg : ep) - sp;
-+ if (format->quote_style && !strncmp(sp, "raw", 3) && !arg)
-+ return strbuf_addf_ret(err, -1, _("--format=%.*s cannot be used with"
-+ "--python, --shell, --tcl, --perl"), (int)(ep-atom), atom);
-+
+ /* Do we have the atom already used elsewhere? */
+ for (i = 0; i < used_atom_cnt; i++) {
+ int len = strlen(used_atom[i].name);
@@ ref-filter.c: static int end_atom_handler(struct atom_value *atomv, struct ref_f
strbuf_swap(¤t->output, &s);
}
strbuf_release(&s);
-@@ ref-filter.c: static void append_lines(struct strbuf *out, const char *buf, unsigned long size +@@ ref-filter.c: int verify_ref_format(struct ref_format *format)
+ at = parse_ref_filter_atom(format, sp + 2, ep, &err);
+ if (at < 0)
+ die("%s", err.buf);
++ if (format->quote_style && used_atom[at].atom_type == ATOM_RAW &&
++ used_atom[at].u.raw_data.option == RAW_BARE)
++ die(_("--format=%.*s cannot be used with"
++ "--python, --shell, --tcl, --perl"), (int)(ep - sp - 2), sp + 2);
+ cp = ep + 1;
- /* See grab_values */
- static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf,
-- struct object *obj)
-+ unsigned long buf_size, struct object *obj)
- {
- int i;
+ if (skip_prefix(used_atom[at].name, "color:", &color))
+@@ ref-filter.c: static void grab_sub_body_contents(struct atom_value *val, int deref, struct exp
const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;
-@@ ref-filter.c: static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf,
+ size_t sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;
+ void *buf = data->content;
++ unsigned long buf_size = data->size;
+
+ for (i = 0; i < used_atom_cnt; i++) {
struct used_atom *atom = &used_atom[i];
const char *name = atom->name;
struct atom_value *v = &val[i];
@@ ref-filter.c: static void grab_sub_body_contents(struct atom_value *val, int der
+ continue;
+ }
+
- if ((obj->type != OBJ_TAG &&
- obj->type != OBJ_COMMIT) ||
+ if ((data->type != OBJ_TAG &&
+ data->type != OBJ_COMMIT) ||
(strcmp(name, "body") &&
-@@ ref-filter.c: 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;
-+ unsigned long buf_size = data->size;
-+
- switch (obj->type) {
- case OBJ_TAG:
- grab_tag_values(val, deref, obj);
-- grab_sub_body_contents(val, deref, buf, obj);
-+ grab_sub_body_contents(val, deref, buf, buf_size, obj);
- grab_person("tagger", val, deref, buf);
- break;
- case OBJ_COMMIT:
- grab_commit_values(val, deref, obj);
-- grab_sub_body_contents(val, deref, buf, obj);
-+ grab_sub_body_contents(val, deref, buf, buf_size, obj);
- grab_person("author", val, deref, buf);
- grab_person("committer", val, deref, buf);
+@@ ref-filter.c: static void grab_values(struct atom_value *val, int deref, struct object *obj, s
break;
case OBJ_TREE:
/* grab_tree_values(val, deref, obj, buf, sz); */
-+ grab_sub_body_contents(val, deref, buf, buf_size, obj);
++ grab_sub_body_contents(val, deref, data);
break;
case OBJ_BLOB:
/* grab_blob_values(val, deref, obj, buf, sz); */
-+ grab_sub_body_contents(val, deref, buf, buf_size, obj);
++ grab_sub_body_contents(val, deref, data);
break;
default:
die("Eh? Object of type %d?", obj->type);
-@@ ref-filter.c: static int get_object(struct ref_array_item *ref, int deref, struct object **obj
- return 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);
@@ ref-filter.c: static int populate_value(struct ref_array_item *ref, struct strbuf *err)
int deref = 0;
const char *refname;
Signature · 1 line
gitgitgadget
PATCH v2 1/2 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 v2 2/2 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 depands 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, 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.
Beyond, --format=%(raw) cannot be used with --python, --shell, --tcl, --perl because if our binary raw data is passed to a variable in the host language, the host language may not support arbitrary binary data in the variables of its string type.
--- Documentation/git-for-each-ref.txt | 9 ++ ref-filter.c | 140 +++++++++++++++---- t/t6300-for-each-ref.sh | 207 +++++++++++++++++++++++++++++ 3 files changed, 328 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 a 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 the host language may not support arbitrary binary data in the +variables of its string 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 +112 -28
294 lines not on this page · raw message
t/t6300-for-each-ref.sh +207 -0
249 lines not on this page · raw message
Signature · 1 line
gitgitgadget
No need to resend as it's a cover letter, but just in case there is another round and you copy things from this cover letter:
Quoted 3 lines · ZheNing Hu via GitGitGadget
In order to make git cat-file --batch use ref-filter logic, %(raw) atom is adding to ref-filter.
s/adding/added/
Quoted 4 lines
Change from last version:
1. Change --<lang> and --format=%(raw) checkpoint to
verify_ref_format(), which make it more scalable.
s/make/makes/
Quoted 1 line
2. Change
grab_sub_body_contents()use structexpand_data*data instread of
s/use/to use/
s/instread/instead/
Quoted 2 lines
using obj,buf,buf_size to pass object info which can reduce the delivery of function parameters.
Quoted 9 lines · ZheNing Hu via GitGitGadget
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-filterdepands on the output of the atom being
s/depands/depends/
Documentation/git-for-each-ref.txt and `date` to extract the named component. For email fields (`authoremail`, · line 238
Quoted 11 lines
text (specifically, no embedded NULs in it).
Documentation/git-for-each-ref.txt +1
-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 a object is `raw`.
s/a object/an object/
Quoted 13 lines
Documentation/git-for-each-ref.txt +7
-0+ +raw:size:: + The raw data size of the object. + +Note that `--format=%(raw)` can not be used with `--python`, `--shell`, `--tcl`, +`--perl` because the host language may not support arbitrary binary data in the +variables of its string type.
Documentation/git-for-each-ref.txt static int populate_value(struct ref_array_item *ref, struct strbuf *err) · line 1818
Quoted 5 lines
Documentation/git-for-each-ref.txt +1
-1@@ -1765,7 +1815,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err) int deref = 0; const char *refname; struct branch *branch = NULL; - + v->s_size = ATOM_VALUE_S_SIZE_INIT;
It looks like a blank line was removed as you added the above new line.
Quoted 3 lines
Documentation/git-for-each-ref.txt +0
-0v->handler = append_atom; v->atom = atom;
Hi, Christian,
Quoted 4 lines · Christian Couder <[email protected]> 于2021年6月4日周五 下午8:53写道：
No need to resend as it's a cover letter, but just in case there is another round and you copy things from this cover letter:
Sorry, what is the bad place in this cover letter I write? This cover letter is also different from the last time ...
Quoted 13 lines
Quoted 3 lines · ZheNing Hu via GitGitGadget
In order to make git cat-file --batch use ref-filter logic, %(raw) atom is adding to ref-filter.
s/adding/added/
Quoted 4 lines
Change from last version:
1. Change --<lang> and --format=%(raw) checkpoint to
verify_ref_format(), which make it more scalable.
Quoted 9 lines
s/make/makes/
Quoted 1 line
2. Change
grab_sub_body_contents()use structexpand_data*data instread ofs/use/to use/
s/instread/instead/Quoted 2 lines
using obj,buf,buf_size to pass object info which can reduce the delivery of function parameters.
Thanks for these grammatical corrections.
--
ZheNing Hu
Quoted 11 lines · ZheNing Hu
Hi, Christian,
Quoted 4 lines · Christian Couder <[email protected]> 于2021年6月4日周五 下午8:53写道：
No need to resend as it's a cover letter, but just in case there is another round and you copy things from this cover letter:
Sorry, what is the bad place in this cover letter I write? This cover letter is also different from the last time ...
I was talking about the grammatical issues below in the cover letter. Sometimes people copy things, for example a text explaining what the patch series is about, from the cover letter of version N to the cover letter of version N + 1, so I thought that telling you about grammatical issues in this cover letter could perhaps help you if you have to write another cover letter for another version of this patch series.
Quoted 14 lines · Christian Couder <[email protected]> 于2021年6月5日周六 下午12:49写道：
Quoted 11 lines · ZheNing Hu
Hi, Christian,
Quoted 4 lines · Christian Couder <[email protected]> 于2021年6月4日周五 下午8:53写道：
No need to resend as it's a cover letter, but just in case there is another round and you copy things from this cover letter:
Sorry, what is the bad place in this cover letter I write? This cover letter is also different from the last time ...
Quoted 7 lines
I was talking about the grammatical issues below in the cover letter. Sometimes people copy things, for example a text explaining what the patch series is about, from the cover letter of version N to the cover letter of version N + 1, so I thought that telling you about grammatical issues in this cover letter could perhaps help you if you have to write another cover letter for another version of this patch series.
Ok, I get it.
I want to mention another question:
If I have a new patch series about %(rest) is based on the current %(raw) patch series, should I submit it immediately?
Thanks.
--
ZheNing Hu
Quoted 3 lines · ZheNing Hu
I want to mention another question:
If I have a new patch series about %(rest) is based on the current %(raw) patch series, should I submit it immediately?
Yeah, I think it's ok to send it as long as you explicitly specify (using a link for example) the patch series on the mailing list it depends on.
Quoted 10 lines · Christian Couder <[email protected]> 于2021年6月5日周六 下午2:45写道：
Quoted 3 lines · ZheNing Hu
I want to mention another question:
If I have a new patch series about %(rest) is based on the current %(raw) patch series, should I submit it immediately?Yeah, I think it's ok to send it as long as you explicitly specify (using a link for example) the patch series on the mailing list it depends on.
Ok. Because %(raw:textconv) %(raw:filter) dependent on %(raw) and %(rest), %(raw) seems to have experienced a long cycle. These codes about new atoms seem to stay in my local git repo for a long time. It may be a good thing to send them earlier. :)
Thanks.