{"thread":{"id":"55782","subject":"[PATCH 0/2] [GSOC] ref-filter: add %(raw) atom","startedAt":"2021-05-27T14:43:29Z","lastAt":"2021-06-01T13:48:52Z","messageCount":41,"participants":["ZheNing Hu via GitGitGadget","Felipe Contreras","Junio C Hamano","ZheNing Hu","Phillip Wood","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"425682","messageId":"pull.963.git.1622126603.gitgitgadget@gmail.com","threadId":"55782","inReplyTo":null,"subject":"[PATCH 0/2] [GSOC] ref-filter: add %(raw) atom","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-27T14:43:20Z","receivedAt":"2021-05-27T14:43:29Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"In order to make git cat-file --batch use ref-filter logic, I add %(raw)\natom to ref-filter.\n\nChange from last version:\n\n 1. In my discussion with Junio, I came to the conclusion that\n --format=\"%(raw)\" should not be used with --python, --perl, --shell,\n --tcl. Therefore, die if both --format=\"%(raw)\" and\n\n--language are given in parse_ref_filter_atom(). The reason I don't move\nthis part to raw_atom_parser() is if I move it to raw_atom_parser(), when we\nuse:\n\ngit --format=%raw --sort=raw --python`\n\nGit will continue to run instead of die because parse_sorting_atom() will\nuse a dummy ref_format and don't remember --language details, next time\nformat_ref_array_item() will reuse the used_atom entry of sorting atom in\nparse_ref_filter_atom(), This will skip the check in raw_atom_parser(). 2.\nGive atom_value.s_size a init value ATOM_VALUE_S_SIZE_INIT (-1), which can\nhelp us distinguish an object whose length is 0 and an object whose s_size\nhas not been modified after initialization. 3. Add %(header) atom.\n\nZheNing Hu (2):\n [GSOC] ref-filter: add %(raw) atom\n [GSOC] ref-filter: add %(header) atom\n\n Documentation/git-for-each-ref.txt | 21 +++\n ref-filter.c | 182 ++++++++++++++++++----\n t/t6300-for-each-ref.sh | 236 +++++++++++++++++++++++++++++\n 3 files changed, 412 insertions(+), 27 deletions(-)\n\n\nbase-commit: 5d5b1473453400224ebb126bf3947e0a3276bdf5\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-963%2Fadlternative%2Fref-filter-raw-atom-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-963/adlternative/ref-filter-raw-atom-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/963\n-- \ngitgitgadget\n"},{"id":"425684","messageId":"b3848f24f2d3f91fc96f20b5a08cbfbe721acbd6.1622126603.git.gitgitgadget@gmail.com","threadId":"55782","inReplyTo":"pull.963.git.1622126603.gitgitgadget@gmail.com","subject":"[PATCH 1/2] [GSOC] ref-filter: add %(raw) atom","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-27T14:43:21Z","receivedAt":"2021-05-27T14:43:30Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nAdd new formatting option `%(raw)`, which will print the raw\nobject data without any changes. It will help further to migrate\nall cat-file formatting logic from cat-file to ref-filter.\n\nThe raw data of blob, tree objects may contain '\\0', but most of\nthe logic in `ref-filter` depands on the output of the atom being\na structured string (end with '\\0').\n\nE.g. `quote_formatting()` use `strbuf_addstr()` or `*._quote_buf()`\nadd the data to the buffer. The raw data of a tree object is\n`100644 one\\0...`, only the `100644 one` will be added to the buffer,\nwhich is incorrect.\n\nTherefore, add a new member in `struct atom_value`: `s_size`, which\ncan record raw object size, it can help us add raw object data to\nthe buffer or compare two buffers which contain raw object data.\n\nBeyond, `--format=%(raw)` should not combine with `--python`, `--shell`,\n`--tcl`, `--perl` because if our binary raw data is passed to a variable\nin the host language, the host languages may cause escape errors.\n\nBased-on-patch-by: Olga Telezhnaya \nSigned-off-by: ZheNing Hu \n---\n Documentation/git-for-each-ref.txt | 14 +++\n ref-filter.c | 156 +++++++++++++++++++----\n t/t6300-for-each-ref.sh | 191 +++++++++++++++++++++++++++++\n 3 files changed, 334 insertions(+), 27 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 2ae2478de706..f6ae751fd256 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -235,6 +235,20 @@ and `date` to extract the named component. For email fields (`authoremail`,\n without angle brackets, and `:localpart` to get the part before the `@` symbol\n out of the trimmed email.\n \n+The raw data in a object is `raw`, For commit and tag objects, `raw` contain\n+`header` and `contents` two parts, `header` is structured part of raw data, it\n+composed of \"tree XXX\", \"parent YYY\", etc lines in commits , or composed of\n+\"object OOO\", \"type TTT\", etc lines in tags; `contents` is unstructured \"free\n+text\" part of raw object data. For blob and tree objects, their raw data don't\n+have `header` and `contents` parts.\n+\n+raw:size::\n+\tThe raw data size of the object.\n+\n+Note that `--format=%(raw)` should not combine with `--python`, `--shell`, `--tcl`,\n+`--perl` because if our binary raw data is passed to a variable in the host language,\n+the host languages may cause escape errors.\n+\n The message in a commit or a tag object is `contents`, from which\n `contents:` can be used to extract various parts out of:\n \ndiff --git a/ref-filter.c b/ref-filter.c\nindex 97116e12d7c4..c2abf5da7006 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -138,6 +138,9 @@ static struct used_atom {\n \t\t\tstruct process_trailer_options trailer_opts;\n \t\t\tunsigned int nlines;\n \t\t} contents;\n+\t\tstruct {\n+\t\t\tenum { RAW_BARE, RAW_LENGTH } option;\n+\t\t} raw_data;\n \t\tstruct {\n \t\t\tcmp_status cmp_status;\n \t\t\tconst char *str;\n@@ -370,6 +373,18 @@ static int contents_atom_parser(const struct ref_format *format, struct used_ato\n \treturn 0;\n }\n \n+static int raw_atom_parser(const struct ref_format *format, struct used_atom *atom,\n+\t\t\t\tconst char *arg, struct strbuf *err)\n+{\n+\tif (!arg) {\n+\t\tatom->u.raw_data.option = RAW_BARE;\n+\t} else if (!strcmp(arg, \"size\"))\n+\t\tatom->u.raw_data.option = RAW_LENGTH;\n+\telse\n+\t\treturn strbuf_addf_ret(err, -1, _(\"unrecognized %%(raw) argument: %s\"), arg);\n+\treturn 0;\n+}\n+\n static int oid_atom_parser(const struct ref_format *format, struct used_atom *atom,\n \t\t\t const char *arg, struct strbuf *err)\n {\n@@ -530,6 +545,7 @@ static struct {\n \t{ \"body\", SOURCE_OBJ, FIELD_STR, body_atom_parser },\n \t{ \"trailers\", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },\n \t{ \"contents\", SOURCE_OBJ, FIELD_STR, contents_atom_parser },\n+\t{ \"raw\", SOURCE_OBJ, FIELD_STR, raw_atom_parser },\n \t{ \"upstream\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n \t{ \"push\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n \t{ \"symref\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n@@ -564,12 +580,15 @@ struct ref_formatting_state {\n \n struct atom_value {\n \tconst char *s;\n+\tsize_t s_size;\n \tint (*handler)(struct atom_value *atomv, struct ref_formatting_state *state,\n \t\t struct strbuf *err);\n \tuintmax_t value; /* used for sorting when not FIELD_STR */\n \tstruct used_atom *atom;\n };\n \n+#define ATOM_VALUE_S_SIZE_INIT (-1)\n+\n /*\n * Used to parse format string and sort specifiers\n */\n@@ -588,6 +607,10 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n \t\treturn strbuf_addf_ret(err, -1, _(\"malformed field name: %.*s\"),\n \t\t\t\t (int)(ep-atom), atom);\n \n+\tif (format->quote_style && starts_with(sp, \"raw\"))\n+\t\treturn strbuf_addf_ret(err, -1, _(\"--format=%.*s should not combine with\"\n+\t\t\t\t\"--python, --shell, --tcl, --perl\"), (int)(ep-atom), atom);\n+\n \t/* Do we have the atom already used elsewhere? */\n \tfor (i = 0; i < used_atom_cnt; i++) {\n \t\tint len = strlen(used_atom[i].name);\n@@ -652,11 +675,14 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n \treturn at;\n }\n \n-static void quote_formatting(struct strbuf *s, const char *str, int quote_style)\n+static void quote_formatting(struct strbuf *s, const char *str, size_t len, int quote_style)\n {\n \tswitch (quote_style) {\n \tcase QUOTE_NONE:\n-\t\tstrbuf_addstr(s, str);\n+\t\tif (len != ATOM_VALUE_S_SIZE_INIT)\n+\t\t\tstrbuf_add(s, str, len);\n+\t\telse\n+\t\t\tstrbuf_addstr(s, str);\n \t\tbreak;\n \tcase QUOTE_SHELL:\n \t\tsq_quote_buf(s, str);\n@@ -683,9 +709,12 @@ static int append_atom(struct atom_value *v, struct ref_formatting_state *state,\n \t * encountered.\n \t */\n \tif (!state->stack->prev)\n-\t\tquote_formatting(&state->stack->output, v->s, state->quote_style);\n+\t\tquote_formatting(&state->stack->output, v->s, v->s_size, state->quote_style);\n \telse\n-\t\tstrbuf_addstr(&state->stack->output, v->s);\n+\t\tif (v->s_size != ATOM_VALUE_S_SIZE_INIT)\n+\t\t\tstrbuf_add(&state->stack->output, v->s, v->s_size);\n+\t\telse\n+\t\t\tstrbuf_addstr(&state->stack->output, v->s);\n \treturn 0;\n }\n \n@@ -785,14 +814,16 @@ static int if_atom_handler(struct atom_value *atomv, struct ref_formatting_state\n \treturn 0;\n }\n \n-static int is_empty(const char *s)\n+static int is_empty(struct strbuf *buf)\n {\n-\twhile (*s != '\\0') {\n-\t\tif (!isspace(*s))\n-\t\t\treturn 0;\n+\tconst char *s = buf->buf;\n+\tsize_t cur_len = 0;\n+\n+\twhile ((cur_len != buf->len) && (isspace(*s) || *s == '\\0')) {\n \t\ts++;\n+\t\tcur_len++;\n \t}\n-\treturn 1;\n+\treturn cur_len == buf->len;\n }\n \n static int then_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state,\n@@ -800,6 +831,7 @@ static int then_atom_handler(struct atom_value *atomv, struct ref_formatting_sta\n {\n \tstruct ref_formatting_stack *cur = state->stack;\n \tstruct if_then_else *if_then_else = NULL;\n+\tsize_t str_len = 0;\n \n \tif (cur->at_end == if_then_else_handler)\n \t\tif_then_else = (struct if_then_else *)cur->at_end_data;\n@@ -810,18 +842,28 @@ static int then_atom_handler(struct atom_value *atomv, struct ref_formatting_sta\n \tif (if_then_else->else_atom_seen)\n \t\treturn strbuf_addf_ret(err, -1, _(\"format: %%(then) atom used after %%(else)\"));\n \tif_then_else->then_atom_seen = 1;\n+\tif (if_then_else->str)\n+\t\tstr_len = strlen(if_then_else->str);\n \t/*\n \t * If the 'equals' or 'notequals' attribute is used then\n \t * perform the required comparison. If not, only non-empty\n \t * strings satisfy the 'if' condition.\n \t */\n \tif (if_then_else->cmp_status == COMPARE_EQUAL) {\n-\t\tif (!strcmp(if_then_else->str, cur->output.buf))\n+\t\tif (!if_then_else->str)\n+\t\t\tBUG(\"when if_then_else->cmp_status == COMPARE_EQUAL,\"\n+\t\t\t \"if_then_else->str must not be null\");\n+\t\tif (str_len == cur->output.len &&\n+\t\t !memcmp(if_then_else->str, cur->output.buf, cur->output.len))\n \t\t\tif_then_else->condition_satisfied = 1;\n \t} else if (if_then_else->cmp_status == COMPARE_UNEQUAL) {\n-\t\tif (strcmp(if_then_else->str, cur->output.buf))\n+\t\tif (!if_then_else->str)\n+\t\t\tBUG(\"when if_then_else->cmp_status == COMPARE_UNEQUAL,\"\n+\t\t\t \"if_then_else->str must not be null\");\n+\t\tif (str_len != cur->output.len ||\n+\t\t memcmp(if_then_else->str, cur->output.buf, cur->output.len))\n \t\t\tif_then_else->condition_satisfied = 1;\n-\t} else if (cur->output.len && !is_empty(cur->output.buf))\n+\t} else if (cur->output.len && !is_empty(&cur->output))\n \t\tif_then_else->condition_satisfied = 1;\n \tstrbuf_reset(&cur->output);\n \treturn 0;\n@@ -867,7 +909,7 @@ static int end_atom_handler(struct atom_value *atomv, struct ref_formatting_stat\n \t * only on the topmost supporting atom.\n \t */\n \tif (!current->prev->prev) {\n-\t\tquote_formatting(&s, current->output.buf, state->quote_style);\n+\t\tquote_formatting(&s, current->output.buf, current->output.len, state->quote_style);\n \t\tstrbuf_swap(¤t->output, &s);\n \t}\n \tstrbuf_release(&s);\n@@ -1292,7 +1334,7 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size\n }\n \n /* See grab_values */\n-static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n+static void grab_raw_data(struct atom_value *val, int deref, void *buf, unsigned long buf_size, struct object *obj)\n {\n \tint i;\n \tconst char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;\n@@ -1307,10 +1349,22 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n \t\t\tcontinue;\n \t\tif (deref)\n \t\t\tname++;\n-\t\tif (strcmp(name, \"body\") &&\n-\t\t !starts_with(name, \"subject\") &&\n-\t\t !starts_with(name, \"trailers\") &&\n-\t\t !starts_with(name, \"contents\"))\n+\n+\t\tif (starts_with(name, \"raw\")) {\n+\t\t\tif (atom->u.raw_data.option == RAW_BARE) {\n+\t\t\t\tv->s = xmemdupz(buf, buf_size);\n+\t\t\t\tv->s_size = buf_size;\n+\t\t\t} else if (atom->u.raw_data.option == RAW_LENGTH)\n+\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)buf_size);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif ((obj->type != OBJ_TAG &&\n+\t\t obj->type != OBJ_COMMIT) ||\n+\t\t (strcmp(name, \"body\") &&\n+\t\t !starts_with(name, \"subject\") &&\n+\t\t !starts_with(name, \"trailers\") &&\n+\t\t !starts_with(name, \"contents\")))\n \t\t\tcontinue;\n \t\tif (!subpos)\n \t\t\tfind_subpos(buf,\n@@ -1374,25 +1428,30 @@ static void fill_missing_values(struct atom_value *val)\n * pointed at by the ref itself; otherwise it is the object the\n * ref (which is a tag) refers to.\n */\n-static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf)\n+static void grab_values(struct atom_value *val, int deref, struct object *obj, struct expand_data *data)\n {\n+\tvoid *buf = data->content;\n+\tunsigned long buf_size = data->size;\n+\n \tswitch (obj->type) {\n \tcase OBJ_TAG:\n \t\tgrab_tag_values(val, deref, obj);\n-\t\tgrab_sub_body_contents(val, deref, buf);\n+\t\tgrab_raw_data(val, deref, buf, buf_size, obj);\n \t\tgrab_person(\"tagger\", val, deref, buf);\n \t\tbreak;\n \tcase OBJ_COMMIT:\n \t\tgrab_commit_values(val, deref, obj);\n-\t\tgrab_sub_body_contents(val, deref, buf);\n+\t\tgrab_raw_data(val, deref, buf, buf_size, obj);\n \t\tgrab_person(\"author\", val, deref, buf);\n \t\tgrab_person(\"committer\", val, deref, buf);\n \t\tbreak;\n \tcase OBJ_TREE:\n \t\t/* grab_tree_values(val, deref, obj, buf, sz); */\n+\t\tgrab_raw_data(val, deref, buf, buf_size, obj);\n \t\tbreak;\n \tcase OBJ_BLOB:\n \t\t/* grab_blob_values(val, deref, obj, buf, sz); */\n+\t\tgrab_raw_data(val, deref, buf, buf_size, obj);\n \t\tbreak;\n \tdefault:\n \t\tdie(\"Eh? Object of type %d?\", obj->type);\n@@ -1614,7 +1673,7 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj\n \t\t\treturn strbuf_addf_ret(err, -1, _(\"parse_object_buffer failed on %s for %s\"),\n \t\t\t\t\t oid_to_hex(&oi->oid), ref->refname);\n \t\t}\n-\t\tgrab_values(ref->value, deref, *obj, oi->content);\n+\t\tgrab_values(ref->value, deref, *obj, oi);\n \t}\n \n \tgrab_common_values(ref->value, deref, oi);\n@@ -1694,7 +1753,7 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)\n \t\tint deref = 0;\n \t\tconst char *refname;\n \t\tstruct branch *branch = NULL;\n-\n+\t\tv->s_size = ATOM_VALUE_S_SIZE_INIT;\n \t\tv->handler = append_atom;\n \t\tv->atom = atom;\n \n@@ -2297,6 +2356,25 @@ static int compare_detached_head(struct ref_array_item *a, struct ref_array_item\n \treturn 0;\n }\n \n+static int memcasecmp(const void *vs1, const void *vs2, size_t n)\n+{\n+\tsize_t i;\n+\tconst char *s1 = (const char *)vs1;\n+\tconst char *s2 = (const char *)vs2;\n+\n+\tfor (i = 0; i < n; i++) {\n+\t\tunsigned char u1 = s1[i];\n+\t\tunsigned char u2 = s2[i];\n+\t\tint U1 = toupper (u1);\n+\t\tint U2 = toupper (u2);\n+\t\tint diff = (UCHAR_MAX <= INT_MAX ? U1 - U2\n+\t\t\t: U1 < U2 ? -1 : U2 < U1);\n+\t\tif (diff)\n+\t\t\treturn diff;\n+\t}\n+\treturn 0;\n+}\n+\n static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, struct ref_array_item *b)\n {\n \tstruct atom_value *va, *vb;\n@@ -2304,6 +2382,7 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru\n \tint cmp_detached_head = 0;\n \tcmp_type cmp_type = used_atom[s->atom].type;\n \tstruct strbuf err = STRBUF_INIT;\n+\tsize_t slen = 0;\n \n \tif (get_ref_atom_value(a, s->atom, &va, &err))\n \t\tdie(\"%s\", err.buf);\n@@ -2317,10 +2396,32 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru\n \t} else if (s->sort_flags & REF_SORTING_VERSION) {\n \t\tcmp = versioncmp(va->s, vb->s);\n \t} else if (cmp_type == FIELD_STR) {\n-\t\tint (*cmp_fn)(const char *, const char *);\n-\t\tcmp_fn = s->sort_flags & REF_SORTING_ICASE\n-\t\t\t? strcasecmp : strcmp;\n-\t\tcmp = cmp_fn(va->s, vb->s);\n+\t\tif (va->s_size == ATOM_VALUE_S_SIZE_INIT &&\n+\t\t vb->s_size == ATOM_VALUE_S_SIZE_INIT) {\n+\t\t\tint (*cmp_fn)(const char *, const char *);\n+\t\t\tcmp_fn = s->sort_flags & REF_SORTING_ICASE\n+\t\t\t\t? strcasecmp : strcmp;\n+\t\t\tcmp = cmp_fn(va->s, vb->s);\n+\t\t} else {\n+\t\t\tint (*cmp_fn)(const void *, const void *, size_t);\n+\t\t\tcmp_fn = s->sort_flags & REF_SORTING_ICASE\n+\t\t\t\t? memcasecmp : memcmp;\n+\n+\t\t\tif (va->s_size != ATOM_VALUE_S_SIZE_INIT &&\n+\t\t\t vb->s_size != ATOM_VALUE_S_SIZE_INIT) {\n+\t\t\t\tcmp = cmp_fn(va->s, vb->s, va->s_size > vb->s_size ?\n+\t\t\t\t vb->s_size : va->s_size);\n+\t\t\t} else if (va->s_size == ATOM_VALUE_S_SIZE_INIT) {\n+\t\t\t\tslen = strlen(va->s);\n+\t\t\t\tcmp = cmp_fn(va->s, vb->s, slen > vb->s_size ?\n+\t\t\t\t\t vb->s_size : slen);\n+\t\t\t} else {\n+\t\t\t\tslen = strlen(vb->s);\n+\t\t\t\tcmp = cmp_fn(va->s, vb->s, slen > va->s_size ?\n+\t\t\t\t\t slen : va->s_size);\n+\t\t\t}\n+\t\t\tcmp = cmp ? cmp : va->s_size - vb->s_size;\n+\t\t}\n \t} else {\n \t\tif (va->value < vb->value)\n \t\t\tcmp = -1;\n@@ -2420,6 +2521,7 @@ int format_ref_array_item(struct ref_array_item *info,\n \t}\n \tif (format->need_color_reset_at_eol) {\n \t\tstruct atom_value resetv;\n+\t\tresetv.s_size = ATOM_VALUE_S_SIZE_INIT;\n \t\tresetv.s = GIT_COLOR_RESET;\n \t\tif (append_atom(&resetv, &state, error_buf)) {\n \t\t\tpop_stack_element(&state.stack);\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 9e0214076b4d..07de4a84d70b 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -130,6 +130,8 @@ test_atom head parent:short=10 ''\n test_atom head numparent 0\n test_atom head object ''\n test_atom head type ''\n+test_atom head raw \"$(git cat-file commit refs/heads/main)\n+\"\n test_atom head '*objectname' ''\n test_atom head '*objecttype' ''\n test_atom head author 'A U Thor 1151968724 +0200'\n@@ -221,6 +223,15 @@ test_atom tag contents 'Tagging at 1151968727\n '\n test_atom tag HEAD ' '\n \n+test_expect_success 'basic atom: refs/tags/testtag *raw' '\n+\tgit cat-file commit refs/tags/testtag^{} >expected &&\n+\tgit for-each-ref --format=\"%(*raw)\" refs/tags/testtag >actual &&\n+\tsanitize_pgp expected.clean &&\n+\tsanitize_pgp actual.clean &&\n+\techo \"\" >>expected.clean &&\n+\ttest_cmp expected.clean actual.clean\n+'\n+\n test_expect_success 'Check invalid atoms names are errors' '\n \ttest_must_fail git for-each-ref --format=\"%(INVALID)\" refs/heads\n '\n@@ -686,6 +697,15 @@ test_atom refs/tags/signed-empty contents:body ''\n test_atom refs/tags/signed-empty contents:signature \"$sig\"\n test_atom refs/tags/signed-empty contents \"$sig\"\n \n+test_expect_success 'basic atom: refs/tags/signed-empty raw' '\n+\tgit cat-file tag refs/tags/signed-empty >expected &&\n+\tgit for-each-ref --format=\"%(raw)\" refs/tags/signed-empty >actual &&\n+\tsanitize_pgp expected.clean &&\n+\tsanitize_pgp actual.clean &&\n+\techo \"\" >>expected.clean &&\n+\ttest_cmp expected.clean actual.clean\n+'\n+\n test_atom refs/tags/signed-short subject 'subject line'\n test_atom refs/tags/signed-short subject:sanitize 'subject-line'\n test_atom refs/tags/signed-short contents:subject 'subject line'\n@@ -695,6 +715,15 @@ test_atom refs/tags/signed-short contents:signature \"$sig\"\n test_atom refs/tags/signed-short contents \"subject line\n $sig\"\n \n+test_expect_success 'basic atom: refs/tags/signed-short raw' '\n+\tgit cat-file tag refs/tags/signed-short >expected &&\n+\tgit for-each-ref --format=\"%(raw)\" refs/tags/signed-short >actual &&\n+\tsanitize_pgp expected.clean &&\n+\tsanitize_pgp actual.clean &&\n+\techo \"\" >>expected.clean &&\n+\ttest_cmp expected.clean actual.clean\n+'\n+\n test_atom refs/tags/signed-long subject 'subject line'\n test_atom refs/tags/signed-long subject:sanitize 'subject-line'\n test_atom refs/tags/signed-long contents:subject 'subject line'\n@@ -708,6 +737,15 @@ test_atom refs/tags/signed-long contents \"subject line\n body contents\n $sig\"\n \n+test_expect_success 'basic atom: refs/tags/signed-long raw' '\n+\tgit cat-file tag refs/tags/signed-long >expected &&\n+\tgit for-each-ref --format=\"%(raw)\" refs/tags/signed-long >actual &&\n+\tsanitize_pgp expected.clean &&\n+\tsanitize_pgp actual.clean &&\n+\techo \"\" >>expected.clean &&\n+\ttest_cmp expected.clean actual.clean\n+'\n+\n test_expect_success 'set up refs pointing to tree and blob' '\n \tgit update-ref refs/mytrees/first refs/heads/main^{tree} &&\n \tgit update-ref refs/myblobs/first refs/heads/main:one\n@@ -720,6 +758,16 @@ test_atom refs/mytrees/first contents:body \"\"\n test_atom refs/mytrees/first contents:signature \"\"\n test_atom refs/mytrees/first contents \"\"\n \n+test_expect_success 'basic atom: refs/mytrees/first raw' '\n+\tgit cat-file tree refs/mytrees/first >expected &&\n+\techo \"\" >>expected &&\n+\tgit for-each-ref --format=\"%(raw)\" refs/mytrees/first >actual &&\n+\ttest_cmp expected actual &&\n+\tgit cat-file -s refs/mytrees/first >expected &&\n+\tgit for-each-ref --format=\"%(raw:size)\" refs/mytrees/first >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_atom refs/myblobs/first subject \"\"\n test_atom refs/myblobs/first contents:subject \"\"\n test_atom refs/myblobs/first body \"\"\n@@ -727,6 +775,149 @@ test_atom refs/myblobs/first contents:body \"\"\n test_atom refs/myblobs/first contents:signature \"\"\n test_atom refs/myblobs/first contents \"\"\n \n+test_expect_success 'basic atom: refs/myblobs/first raw' '\n+\tgit cat-file blob refs/myblobs/first >expected &&\n+\techo \"\" >>expected &&\n+\tgit for-each-ref --format=\"%(raw)\" refs/myblobs/first >actual &&\n+\ttest_cmp expected actual &&\n+\tgit cat-file -s refs/myblobs/first >expected &&\n+\tgit for-each-ref --format=\"%(raw:size)\" refs/myblobs/first >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'set up refs pointing to binary blob' '\n+\tprintf \"%b\" \"a\\0b\\0c\" >blob1 &&\n+\tprintf \"%b\" \"a\\0c\\0b\" >blob2 &&\n+\tprintf \"%b\" \"\\0a\\0b\\0c\" >blob3 &&\n+\tprintf \"%b\" \"abc\" >blob4 &&\n+\tprintf \"%b\" \"\\0 \\0 \\0 \" >blob5 &&\n+\tprintf \"%b\" \"\\0 \\0a\\0 \" >blob6 &&\n+\t>blob7 &&\n+\tgit hash-object blob1 -w | xargs git update-ref refs/myblobs/blob1 &&\n+\tgit hash-object blob2 -w | xargs git update-ref refs/myblobs/blob2 &&\n+\tgit hash-object blob3 -w | xargs git update-ref refs/myblobs/blob3 &&\n+\tgit hash-object blob4 -w | xargs git update-ref refs/myblobs/blob4 &&\n+\tgit hash-object blob5 -w | xargs git update-ref refs/myblobs/blob5 &&\n+\tgit hash-object blob6 -w | xargs git update-ref refs/myblobs/blob6 &&\n+\tgit hash-object blob7 -w | xargs git update-ref refs/myblobs/blob7\n+'\n+\n+test_expect_success 'Verify sorts with raw' '\n+\tcat >expected <<-EOF &&\n+\trefs/myblobs/blob7\n+\trefs/myblobs/blob5\n+\trefs/myblobs/blob6\n+\trefs/myblobs/blob3\n+\trefs/mytrees/first\n+\trefs/myblobs/first\n+\trefs/myblobs/blob1\n+\trefs/myblobs/blob2\n+\trefs/myblobs/blob4\n+\trefs/heads/main\n+\tEOF\n+\tgit for-each-ref --format=\"%(refname)\" --sort=raw \\\n+\t\trefs/heads/main refs/myblobs/ refs/mytrees/first >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'Verify sorts with raw:size' '\n+\tcat >expected <<-EOF &&\n+\trefs/myblobs/blob7\n+\trefs/myblobs/first\n+\trefs/heads/main\n+\trefs/myblobs/blob4\n+\trefs/myblobs/blob1\n+\trefs/myblobs/blob2\n+\trefs/myblobs/blob3\n+\trefs/myblobs/blob5\n+\trefs/myblobs/blob6\n+\trefs/mytrees/first\n+\tEOF\n+\tgit for-each-ref --format=\"%(refname)\" --sort=raw:size \\\n+\t\trefs/heads/main refs/myblobs/ refs/mytrees/first >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'validate raw atom with %(if:equals)' '\n+\tcat >expected <<-EOF &&\n+\tnot equals\n+\tnot equals\n+\tnot equals\n+\tnot equals\n+\tnot equals\n+\tnot equals\n+\trefs/myblobs/blob4\n+\tnot equals\n+\tnot equals\n+\tnot equals\n+\tnot equals\n+\tEOF\n+\tgit for-each-ref --format=\"%(if:equals=abc)%(raw)%(then)%(refname)%(else)not equals%(end)\" \\\n+\t\trefs/myblobs/ refs/heads/ >actual &&\n+\ttest_cmp expected actual\n+'\n+test_expect_success 'validate raw atom with %(if:notequals)' '\n+\tcat >expected <<-EOF &&\n+\trefs/heads/ambiguous\n+\trefs/heads/main\n+\trefs/heads/newtag\n+\trefs/myblobs/blob1\n+\trefs/myblobs/blob2\n+\trefs/myblobs/blob3\n+\tequals\n+\trefs/myblobs/blob5\n+\trefs/myblobs/blob6\n+\trefs/myblobs/blob7\n+\trefs/myblobs/first\n+\tEOF\n+\tgit for-each-ref --format=\"%(if:notequals=abc)%(raw)%(then)%(refname)%(else)equals%(end)\" \\\n+\t\trefs/myblobs/ refs/heads/ >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'empty raw refs with %(if)' '\n+\tcat >expected <<-EOF &&\n+\trefs/myblobs/blob1 not empty\n+\trefs/myblobs/blob2 not empty\n+\trefs/myblobs/blob3 not empty\n+\trefs/myblobs/blob4 not empty\n+\trefs/myblobs/blob5 empty\n+\trefs/myblobs/blob6 not empty\n+\trefs/myblobs/blob7 empty\n+\trefs/myblobs/first not empty\n+\tEOF\n+\tgit for-each-ref --format=\"%(refname) %(if)%(raw)%(then)not empty%(else)empty%(end)\" \\\n+\trefs/myblobs/ >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success '%(raw) with --python must failed' '\n+\ttest_must_fail git for-each-ref --format=\"%(raw)\" --python\n+'\n+\n+test_expect_success '%(raw) with --tcl must failed' '\n+\ttest_must_fail git for-each-ref --format=\"%(raw)\" --tcl\n+'\n+\n+test_expect_success '%(raw) with --perl must failed' '\n+\ttest_must_fail git for-each-ref --format=\"%(raw)\" --perl\n+'\n+\n+test_expect_success '%(raw) with --shell must failed' '\n+\ttest_must_fail git for-each-ref --format=\"%(raw)\" --shell\n+'\n+\n+test_expect_success '%(raw) with --shell and --sort=raw must failed' '\n+\ttest_must_fail git for-each-ref --format=\"%(raw)\" --sort=raw --shell\n+'\n+\n+test_expect_success 'for-each-ref --format compare with cat-file --batch' '\n+\tgit rev-parse refs/mytrees/first | git cat-file --batch >expected &&\n+\tgit for-each-ref --format=\"%(objectname) %(objecttype) %(objectsize)\n+%(raw)\" refs/mytrees/first >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'set up multiple-sort tags' '\n \tfor when in 100000 200000\n \tdo\n-- \ngitgitgadget\n\n"},{"id":"425683","messageId":"aa6d73f3e526f416ee1e4e332e9ca3119efba0e8.1622126603.git.gitgitgadget@gmail.com","threadId":"55782","inReplyTo":"pull.963.git.1622126603.gitgitgadget@gmail.com","subject":"[PATCH 2/2] [GSOC] ref-filter: add %(header) atom","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-27T14:43:22Z","receivedAt":"2021-05-27T14:43:31Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu \n\nAdd new formatting option `%(header)`, which will print the\nthe structured header part of the raw object data.\n\nIn the storage layout of an object: blob and tree only\ncontains raw data; commit and tag raw data contains two part:\nheader and contents. The header of tag contains \"object OOO\",\n\"type TTT\", \"tag AAA\", \"tagger GGG\"; The header of commit\ncontains \"tree RRR\", \"parent PPP\", \"author UUU\", \"committer CCC\".\n\nSigned-off-by: ZheNing Hu \n---\n Documentation/git-for-each-ref.txt | 7 +++++\n ref-filter.c | 26 +++++++++++++++++\n t/t6300-for-each-ref.sh | 45 ++++++++++++++++++++++++++++++\n 3 files changed, 78 insertions(+)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex f6ae751fd256..7827e48cde75 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -249,6 +249,13 @@ Note that `--format=%(raw)` should not combine with `--python`, `--shell`, `--tc\n `--perl` because if our binary raw data is passed to a variable in the host language,\n the host languages may cause escape errors.\n \n+The structured header part of the raw data in a commit or a tag object is `header`,\n+it composed of \"tree XXX\", \"parent YYY\", etc lines in commits, or composed of\n+\"object OOO\", \"type TTT\", etc lines in tags.\n+\n+header:size::\n+\tThe header size of the object.\n+\n The message in a commit or a tag object is `contents`, from which\n `contents:` can be used to extract various parts out of:\n \ndiff --git a/ref-filter.c b/ref-filter.c\nindex c2abf5da7006..2f426830f562 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -141,6 +141,9 @@ static struct used_atom {\n \t\tstruct {\n \t\t\tenum { RAW_BARE, RAW_LENGTH } option;\n \t\t} raw_data;\n+\t\tstruct {\n+\t\t\tenum { H_BARE, H_LENGTH } option;\n+\t\t} header;\n \t\tstruct {\n \t\t\tcmp_status cmp_status;\n \t\t\tconst char *str;\n@@ -385,6 +388,18 @@ static int raw_atom_parser(const struct ref_format *format, struct used_atom *at\n \treturn 0;\n }\n \n+static int header_atom_parser(const struct ref_format *format, struct used_atom *atom,\n+\t\t\t const char *arg, struct strbuf *err)\n+{\n+\tif (!arg)\n+\t\tatom->u.header.option = H_BARE;\n+\telse if (!strcmp(arg, \"size\"))\n+\t\tatom->u.header.option = H_LENGTH;\n+\telse\n+\t\treturn strbuf_addf_ret(err, -1, _(\"unrecognized %%(header) argument: %s\"), arg);\n+\treturn 0;\n+}\n+\n static int oid_atom_parser(const struct ref_format *format, struct used_atom *atom,\n \t\t\t const char *arg, struct strbuf *err)\n {\n@@ -546,6 +561,7 @@ static struct {\n \t{ \"trailers\", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },\n \t{ \"contents\", SOURCE_OBJ, FIELD_STR, contents_atom_parser },\n \t{ \"raw\", SOURCE_OBJ, FIELD_STR, raw_atom_parser },\n+\t{ \"header\", SOURCE_OBJ, FIELD_STR, header_atom_parser },\n \t{ \"upstream\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n \t{ \"push\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n \t{ \"symref\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n@@ -1362,6 +1378,7 @@ static void grab_raw_data(struct atom_value *val, int deref, void *buf, unsigned\n \t\tif ((obj->type != OBJ_TAG &&\n \t\t obj->type != OBJ_COMMIT) ||\n \t\t (strcmp(name, \"body\") &&\n+\t\t !starts_with(name, \"header\") &&\n \t\t !starts_with(name, \"subject\") &&\n \t\t !starts_with(name, \"trailers\") &&\n \t\t !starts_with(name, \"contents\")))\n@@ -1372,6 +1389,15 @@ static void grab_raw_data(struct atom_value *val, int deref, void *buf, unsigned\n \t\t\t\t &bodypos, &bodylen, &nonsiglen,\n \t\t\t\t &sigpos, &siglen);\n \n+\t\tif (starts_with(name, \"header\")) {\n+\t\t\tsize_t header_len = subpos - (const char *)buf - 1;\n+\t\t\tif (atom->u.header.option == H_BARE) {\n+\t\t\t\tv->s = xmemdupz(buf, header_len);\n+\t\t\t} else if (atom->u.header.option == H_LENGTH)\n+\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)header_len);\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\tif (atom->u.contents.option == C_SUB)\n \t\t\tv->s = copy_subject(subpos, sublen);\n \t\telse if (atom->u.contents.option == C_SUB_SANITIZE) {\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 07de4a84d70b..11fc8fc53649 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -232,6 +232,35 @@ test_expect_success 'basic atom: refs/tags/testtag *raw' '\n \ttest_cmp expected.clean actual.clean\n '\n \n+test_expect_success 'basic atom: refs/tags/testtag header' '\n+\tcat >expected <<-EOF &&\n+\tobject ea122842f48be4afb2d1fc6a4b96c05885ab7463\n+\ttype commit\n+\ttag testtag\n+\ttagger C O Mitter 1151968725 +0200\n+\n+\tEOF\n+\tgit for-each-ref --format=\"%(header)\" refs/tags/testtag >actual &&\n+\ttest_cmp expected actual &&\n+\techo \"131\" >expected &&\n+\tgit for-each-ref --format=\"%(header:size)\" refs/tags/testtag >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'basic atom: refs/heads/main header' '\n+\tcat >expected <<-EOF &&\n+\ttree 8039ce043250c402d62ca312e9596e42ce1c7bb0\n+\tauthor A U Thor 1151968724 +0200\n+\tcommitter C O Mitter 1151968723 +0200\n+\n+\tEOF\n+\tgit for-each-ref --format=\"%(header)\" refs/heads/main >actual &&\n+\ttest_cmp expected actual &&\n+\techo \"162\" >expected &&\n+\tgit for-each-ref --format=\"%(header:size)\" refs/heads/main >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'Check invalid atoms names are errors' '\n \ttest_must_fail git for-each-ref --format=\"%(INVALID)\" refs/heads\n '\n@@ -768,6 +797,14 @@ test_expect_success 'basic atom: refs/mytrees/first raw' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'basic atom: refs/mytrees/first header' '\n+\techo \"\" >expected &&\n+\tgit for-each-ref --format=\"%(header)\" refs/mytrees/first >actual &&\n+\ttest_cmp expected actual &&\n+\tgit for-each-ref --format=\"%(header:size)\" refs/mytrees/first >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_atom refs/myblobs/first subject \"\"\n test_atom refs/myblobs/first contents:subject \"\"\n test_atom refs/myblobs/first body \"\"\n@@ -785,6 +822,14 @@ test_expect_success 'basic atom: refs/myblobs/first raw' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'basic atom: refs/myblobs/first header' '\n+\techo \"\" >expected &&\n+\tgit for-each-ref --format=\"%(header)\" refs/myblobs/first >actual &&\n+\ttest_cmp expected actual &&\n+\tgit for-each-ref --format=\"%(header:size)\" refs/myblobs/first >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'set up refs pointing to binary blob' '\n \tprintf \"%b\" \"a\\0b\\0c\" >blob1 &&\n \tprintf \"%b\" \"a\\0c\\0b\" >blob2 &&\n-- \ngitgitgadget\n"},{"id":"425686","messageId":"60afbd4b81e44_265302086d@natae.notmuch","threadId":"55782","inReplyTo":"pull.963.git.1622126603.gitgitgadget@gmail.com","subject":"RE: [PATCH 0/2] [GSOC] ref-filter: add %(raw) atom","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-27T15:39:55Z","receivedAt":"2021-05-27T15:40:00Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"ZheNing Hu via GitGitGadget wrote:\n> In order to make git cat-file --batch use ref-filter logic, I add %(raw)\n> atom to ref-filter.\n> \n> Change from last version:\n> \n> 1. In my discussion with Junio, I came to the conclusion that\n> --format=\"%(raw)\" should not be used with --python, --perl, --shell,\n> --tcl. Therefore, die if both --format=\"%(raw)\" and\n> \n> --language are given in parse_ref_filter_atom(). The reason I don't move\n> this part to raw_atom_parser() is if I move it to raw_atom_parser(), when we\n> use:\n> \n> git --format=%raw --sort=raw --python`\n\nMissing the command I presume (and the other backtick).\n\n-- \nFelipe Contreras\n"},{"id":"425689","messageId":"60afca827a28f_265302085b@natae.notmuch","threadId":"55782","inReplyTo":"b3848f24f2d3f91fc96f20b5a08cbfbe721acbd6.1622126603.git.gitgitgadget@gmail.com","subject":"RE: [PATCH 1/2] [GSOC] ref-filter: add %(raw) atom","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-27T16:36:18Z","receivedAt":"2021-05-27T16:36:24Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"ZheNing Hu via GitGitGadget wrote:\n\n> +static int raw_atom_parser(const struct ref_format *format, struct used_atom *atom,\n> +\t\t\t\tconst char *arg, struct strbuf *err)\n> +{\n> +\tif (!arg) {\n> +\t\tatom->u.raw_data.option = RAW_BARE;\n> +\t} else if (!strcmp(arg, \"size\"))\n\nNo need for braces.\n\n if (!arg)\n ...\n else\n\n> @@ -1307,10 +1349,22 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n> \t\t\tcontinue;\n> \t\tif (deref)\n> \t\t\tname++;\n> -\t\tif (strcmp(name, \"body\") &&\n> -\t\t !starts_with(name, \"subject\") &&\n> -\t\t !starts_with(name, \"trailers\") &&\n> -\t\t !starts_with(name, \"contents\"))\n> +\n> +\t\tif (starts_with(name, \"raw\")) {\n> +\t\t\tif (atom->u.raw_data.option == RAW_BARE) {\n> +\t\t\t\tv->s = xmemdupz(buf, buf_size);\n> +\t\t\t\tv->s_size = buf_size;\n> +\t\t\t} else if (atom->u.raw_data.option == RAW_LENGTH)\n> +\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)buf_size);\n\nI think it's better to be consistent: if you used braces in the if, uses\nbraces in else.\n\n> +\t\t\tcontinue;\n> +\t\t}\n\n> +static int memcasecmp(const void *vs1, const void *vs2, size_t n)\n\nWhy void *? We can delcare as char *.\n\n> +{\n> +\tsize_t i;\n> +\tconst char *s1 = (const char *)vs1;\n> +\tconst char *s2 = (const char *)vs2;\n\nThen we avoid this extra step.\n\n> +\tfor (i = 0; i < n; i++) {\n> +\t\tunsigned char u1 = s1[i];\n> +\t\tunsigned char u2 = s2[i];\n\nThere's no need for two entirely new variables...\n\n> +\t\tint U1 = toupper (u1);\n> +\t\tint U2 = toupper (u2);\n\nYou can do toupper(s1[i]) directly (BTW, there's an extra space: `foo(x)`,\nnot `foo (x)`).\n\nWhile we are at it, why keep an extra index from s1, when s1 is never\nused again?\n\nWe can simply advance both s1 and s2:\n\n s1++, s2++\n\n> +\t\tint diff = (UCHAR_MAX <= INT_MAX ? U1 - U2\n> +\t\t\t: U1 < U2 ? -1 : U2 < U1);\n\nI don't understand what this is supposed to achieve. Both U1 and U2 are\nintegers, pretty low integers actually.\n\nIf we get rid if that complexity we don't even need U1 or U2, just do:\n\n diff = toupper(u1) - toupper(u2);\n\n> +\t\tif (diff)\n> +\t\t\treturn diff;\n> +\t}\n> +\treturn 0;\n> +}\n\nAll we have to do is define the end point, and then we don't need i:\n\n\tstatic int memcasecmp(const char *s1, const char *s2, size_t n)\n\t{\n\t\tconst char *end = s1 + n;\n\t\tfor (; s1 < end; s1++, s2++) {\n\t\t\tint diff = tolower(*s1) - tolower(*s2);\n\t\t\tif (diff)\n\t\t\t\treturn diff;\n\t\t}\n\t\treturn 0;\n\t}\n\n(and I personally prefer lower to upper)\n\nCheck the following resource for a detailed explanation of why my\nmodified version is considered good taste:\n\nhttps://github.com/felipec/linked-list-good-taste\n\n> static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, struct ref_array_item *b)\n> {\n> \tstruct atom_value *va, *vb;\n> @@ -2304,6 +2382,7 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru\n> \tint cmp_detached_head = 0;\n> \tcmp_type cmp_type = used_atom[s->atom].type;\n> \tstruct strbuf err = STRBUF_INIT;\n> +\tsize_t slen = 0;\n> \n> \tif (get_ref_atom_value(a, s->atom, &va, &err))\n> \t\tdie(\"%s\", err.buf);\n> @@ -2317,10 +2396,32 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru\n> \t} else if (s->sort_flags & REF_SORTING_VERSION) {\n> \t\tcmp = versioncmp(va->s, vb->s);\n> \t} else if (cmp_type == FIELD_STR) {\n> -\t\tint (*cmp_fn)(const char *, const char *);\n> -\t\tcmp_fn = s->sort_flags & REF_SORTING_ICASE\n> -\t\t\t? strcasecmp : strcmp;\n> -\t\tcmp = cmp_fn(va->s, vb->s);\n> +\t\tif (va->s_size == ATOM_VALUE_S_SIZE_INIT &&\n> +\t\t vb->s_size == ATOM_VALUE_S_SIZE_INIT) {\n> +\t\t\tint (*cmp_fn)(const char *, const char *);\n> +\t\t\tcmp_fn = s->sort_flags & REF_SORTING_ICASE\n> +\t\t\t\t? strcasecmp : strcmp;\n> +\t\t\tcmp = cmp_fn(va->s, vb->s);\n> +\t\t} else {\n> +\t\t\tint (*cmp_fn)(const void *, const void *, size_t);\n> +\t\t\tcmp_fn = s->sort_flags & REF_SORTING_ICASE\n> +\t\t\t\t? memcasecmp : memcmp;\n> +\n> +\t\t\tif (va->s_size != ATOM_VALUE_S_SIZE_INIT &&\n> +\t\t\t vb->s_size != ATOM_VALUE_S_SIZE_INIT) {\n> +\t\t\t\tcmp = cmp_fn(va->s, vb->s, va->s_size > vb->s_size ?\n> +\t\t\t\t vb->s_size : va->s_size);\n> +\t\t\t} else if (va->s_size == ATOM_VALUE_S_SIZE_INIT) {\n> +\t\t\t\tslen = strlen(va->s);\n> +\t\t\t\tcmp = cmp_fn(va->s, vb->s, slen > vb->s_size ?\n> +\t\t\t\t\t vb->s_size : slen);\n> +\t\t\t} else {\n> +\t\t\t\tslen = strlen(vb->s);\n> +\t\t\t\tcmp = cmp_fn(va->s, vb->s, slen > va->s_size ?\n> +\t\t\t\t\t slen : va->s_size);\n> +\t\t\t}\n> +\t\t\tcmp = cmp ? cmp : va->s_size - vb->s_size;\n> +\t\t}\n\nThis hurts my eyes. I think the complexity of this chunk warrants a\nseparate function. Then the logic would be easer to see.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"425690","messageId":"60afcada976dc_26530208a1@natae.notmuch","threadId":"55782","inReplyTo":"aa6d73f3e526f416ee1e4e332e9ca3119efba0e8.1622126603.git.gitgitgadget@gmail.com","subject":"RE: [PATCH 2/2] [GSOC] ref-filter: add %(header) atom","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-27T16:37:46Z","receivedAt":"2021-05-27T16:37:53Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"ZheNing Hu via GitGitGadget wrote:\n\n> @@ -1372,6 +1389,15 @@ static void grab_raw_data(struct atom_value *val, int deref, void *buf, unsigned\n> \t\t\t\t &bodypos, &bodylen, &nonsiglen,\n> \t\t\t\t &sigpos, &siglen);\n> \n> +\t\tif (starts_with(name, \"header\")) {\n> +\t\t\tsize_t header_len = subpos - (const char *)buf - 1;\n> +\t\t\tif (atom->u.header.option == H_BARE) {\n> +\t\t\t\tv->s = xmemdupz(buf, header_len);\n> +\t\t\t} else if (atom->u.header.option == H_LENGTH)\n\nNo need for braces in the if.\n\n> +\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)header_len);\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> \t\tif (atom->u.contents.option == C_SUB)\n> \t\t\tv->s = copy_subject(subpos, sublen);\n> \t\telse if (atom->u.contents.option == C_SUB_SANITIZE) {\n\n-- \nFelipe Contreras\n"},{"id":"425727","messageId":"xmqq1r9r8spu.fsf@gitster.g","threadId":"55782","inReplyTo":"b3848f24f2d3f91fc96f20b5a08cbfbe721acbd6.1622126603.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] [GSOC] ref-filter: add %(raw) atom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-28T03:03:57Z","receivedAt":"2021-05-28T03:04:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZheNing Hu via GitGitGadget\" writes:\n\n> The raw data of blob, tree objects may contain '\\0', but most of\n> the logic in `ref-filter` depands on the output of the atom being\n> a structured string (end with '\\0').\n\nText, yes, string is also OK. But structured? Probably not.\n\n ... being text (specifically, no embedded NULs in it).\n\n> E.g. `quote_formatting()` use `strbuf_addstr()` or `*._quote_buf()`\n> add the data to the buffer. The raw data of a tree object is\n> `100644 one\\0...`, only the `100644 one` will be added to the buffer,\n> which is incorrect.\n>\n> Therefore, add a new member in `struct atom_value`: `s_size`, which\n> can record raw object size, it can help us add raw object data to\n> the buffer or compare two buffers which contain raw object data.\n\nOther than the phrasing issue around \"structured\", all of the above\nis a good description.\n\n> Beyond, `--format=%(raw)` should not combine with `--python`, `--shell`,\n> `--tcl`, `--perl` because if our binary raw data is passed to a variable\n> in the host language, the host languages may cause escape errors.\n\nOK. I think at least --perl and possibly --python should be able to\nexpress NULs in the \"string\" type we use from the host language, but\nI am perfectly fine with the decision to leave it to later updates.\n\nAfter all, the -- feature to write scriptlets via --format and\nexecute them in the named language interpreter probably is not as\noften used as it was originally designed for, so it might be that\nnobody will ask for such \"later updates\".\n\n> @@ -235,6 +235,20 @@ and `date` to extract the named component. For email fields (`authoremail`,\n> without angle brackets, and `:localpart` to get the part before the `@` symbol\n> out of the trimmed email.\n> \n> +The raw data in a object is `raw`, For commit and tag objects, `raw` contain\n> +`header` and `contents` two parts, `header` is structured part of raw data, it\n> +composed of \"tree XXX\", \"parent YYY\", etc lines in commits , or composed of\n> +\"object OOO\", \"type TTT\", etc lines in tags; `contents` is unstructured \"free\n> +text\" part of raw object data. For blob and tree objects, their raw data don't\n> +have `header` and `contents` parts.\n> +\n> +raw:size::\n> +\tThe raw data size of the object.\n> +\n> +Note that `--format=%(raw)` should not combine with `--python`, `--shell`, `--tcl`,\n\n\"should not combine\" -> \"cannot be used\" would make it read more\nnaturally (ditto for the phrase used in the proposed log message).\n\n> +`--perl` because if our binary raw data is passed to a variable in the host language,\n> +the host languages may cause escape errors.\n> +\n> The message in a commit or a tag object is `contents`, from which\n> `contents:` can be used to extract various parts out of:\n> \n> diff --git a/ref-filter.c b/ref-filter.c\n> index 97116e12d7c4..c2abf5da7006 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -138,6 +138,9 @@ static struct used_atom {\n> \t\t\tstruct process_trailer_options trailer_opts;\n> \t\t\tunsigned int nlines;\n> \t\t} contents;\n> +\t\tstruct {\n> +\t\t\tenum { RAW_BARE, RAW_LENGTH } option;\n> +\t\t} raw_data;\n> \t\tstruct {\n> \t\t\tcmp_status cmp_status;\n> \t\t\tconst char *str;\n> @@ -370,6 +373,18 @@ static int contents_atom_parser(const struct ref_format *format, struct used_ato\n> \treturn 0;\n> }\n> \n> +static int raw_atom_parser(const struct ref_format *format, struct used_atom *atom,\n> +\t\t\t\tconst char *arg, struct strbuf *err)\n> +{\n> +\tif (!arg) {\n> +\t\tatom->u.raw_data.option = RAW_BARE;\n> +\t} else if (!strcmp(arg, \"size\"))\n> +\t\tatom->u.raw_data.option = RAW_LENGTH;\n> +\telse\n> +\t\treturn strbuf_addf_ret(err, -1, _(\"unrecognized %%(raw) argument: %s\"), arg);\n> +\treturn 0;\n> +}\n> +\n> static int oid_atom_parser(const struct ref_format *format, struct used_atom *atom,\n> \t\t\t const char *arg, struct strbuf *err)\n> {\n> @@ -530,6 +545,7 @@ static struct {\n> \t{ \"body\", SOURCE_OBJ, FIELD_STR, body_atom_parser },\n> \t{ \"trailers\", SOURCE_OBJ, FIELD_STR, trailers_atom_parser },\n> \t{ \"contents\", SOURCE_OBJ, FIELD_STR, contents_atom_parser },\n> +\t{ \"raw\", SOURCE_OBJ, FIELD_STR, raw_atom_parser },\n> \t{ \"upstream\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n> \t{ \"push\", SOURCE_NONE, FIELD_STR, remote_ref_atom_parser },\n> \t{ \"symref\", SOURCE_NONE, FIELD_STR, refname_atom_parser },\n> @@ -564,12 +580,15 @@ struct ref_formatting_state {\n> \n> struct atom_value {\n> \tconst char *s;\n> +\tsize_t s_size;\n\nOK, so everything used to be a C-string that cannot hold NULs in it,\nbut now it is a counted string. Good.\n\n> \tint (*handler)(struct atom_value *atomv, struct ref_formatting_state *state,\n> \t\t struct strbuf *err);\n> \tuintmax_t value; /* used for sorting when not FIELD_STR */\n> \tstruct used_atom *atom;\n> };\n> \n> +#define ATOM_VALUE_S_SIZE_INIT (-1)\n> +\n> /*\n> * Used to parse format string and sort specifiers\n> */\n> @@ -588,6 +607,10 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n> \t\treturn strbuf_addf_ret(err, -1, _(\"malformed field name: %.*s\"),\n> \t\t\t\t (int)(ep-atom), atom);\n> \n> +\tif (format->quote_style && starts_with(sp, \"raw\"))\n> +\t\treturn strbuf_addf_ret(err, -1, _(\"--format=%.*s should not combine with\"\n> +\t\t\t\t\"--python, --shell, --tcl, --perl\"), (int)(ep-atom), atom);\n\nDon't we want to allow \"raw:size\" that would be a plain text?\nI am not sure if this check belongs here in the first place.\nShouldn't it be done in raw_atom_parser() instead?\n\nAnother idea is to teach a more generic rule to quote_formatting()\nto detect NULs in v->s[0..v->s_size] at runtime and barf, i.e. a\nplain-text blob object can be used with \"--shell --format=%(raw)\"\njust fine.\n\n> @@ -652,11 +675,14 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n> \treturn at;\n> }\n> \n> -static void quote_formatting(struct strbuf *s, const char *str, int quote_style)\n> +static void quote_formatting(struct strbuf *s, const char *str, size_t len, int quote_style)\n> {\n> \tswitch (quote_style) {\n> \tcase QUOTE_NONE:\n> -\t\tstrbuf_addstr(s, str);\n> +\t\tif (len != ATOM_VALUE_S_SIZE_INIT)\n> +\t\t\tstrbuf_add(s, str, len);\n> +\t\telse\n> +\t\t\tstrbuf_addstr(s, str);\n\nIt probably is a good idea to invent a C preprocessor macro for a\nnamed constant to be used when a structure is initialized, but it\nwould be easier to read if the rule is \"len field is negative when\nthe value is a C-string\", e.g.\n\n\t\tif (len < 0)\n\n> @@ -683,9 +709,12 @@ static int append_atom(struct atom_value *v, struct ref_formatting_state *state,\n> \t * encountered.\n> \t */\n> \tif (!state->stack->prev)\n> -\t\tquote_formatting(&state->stack->output, v->s, state->quote_style);\n> +\t\tquote_formatting(&state->stack->output, v->s, v->s_size, state->quote_style);\n> \telse\n> -\t\tstrbuf_addstr(&state->stack->output, v->s);\n> +\t\tif (v->s_size != ATOM_VALUE_S_SIZE_INIT)\n> +\t\t\tstrbuf_add(&state->stack->output, v->s, v->s_size);\n> +\t\telse\n> +\t\t\tstrbuf_addstr(&state->stack->output, v->s);\n> \treturn 0;\n> }\n> \n> @@ -785,14 +814,16 @@ static int if_atom_handler(struct atom_value *atomv, struct ref_formatting_state\n> \treturn 0;\n> }\n> \n> -static int is_empty(const char *s)\n> +static int is_empty(struct strbuf *buf)\n> {\n> -\twhile (*s != '\\0') {\n> -\t\tif (!isspace(*s))\n> -\t\t\treturn 0;\n> +\tconst char *s = buf->buf;\n> +\tsize_t cur_len = 0;\n> +\n> +\twhile ((cur_len != buf->len) && (isspace(*s) || *s == '\\0')) {\n> \t\ts++;\n> +\t\tcur_len++;\n> \t}\n> -\treturn 1;\n> +\treturn cur_len == buf->len;\n> }\n\nAssuming that we do want to treat NULs the same way as whitespaces,\nthe updated code works as intended, which is good. But I have no\nreason to support that design decision. I do not have a strong\nreason to support a design decision that goes the opposite way to\ntreat a NUL just like we treat an 'X', but at least I can understand\nit (i.e. \"because we have no reason to special case NUL any more\nthan 'X' when trying to see if a buffer is 'empty', we don't\").\nThis code on the other hand must be supported with \"because we need\nto special case NUL for such and such reasons for the purpose of\ndetermining if a buffer is 'empty', we treat them the same way as\nwhitespaces\".\n\n> @@ -800,6 +831,7 @@ static int then_atom_handler(struct atom_value *atomv, struct ref_formatting_sta\n> {\n> \tstruct ref_formatting_stack *cur = state->stack;\n> \tstruct if_then_else *if_then_else = NULL;\n> +\tsize_t str_len = 0;\n> \n> \tif (cur->at_end == if_then_else_handler)\n> \t\tif_then_else = (struct if_then_else *)cur->at_end_data;\n> @@ -810,18 +842,28 @@ static int then_atom_handler(struct atom_value *atomv, struct ref_formatting_sta\n> \tif (if_then_else->else_atom_seen)\n> \t\treturn strbuf_addf_ret(err, -1, _(\"format: %%(then) atom used after %%(else)\"));\n> \tif_then_else->then_atom_seen = 1;\n> +\tif (if_then_else->str)\n> +\t\tstr_len = strlen(if_then_else->str);\n> \t/*\n> \t * If the 'equals' or 'notequals' attribute is used then\n> \t * perform the required comparison. If not, only non-empty\n> \t * strings satisfy the 'if' condition.\n> \t */\n> \tif (if_then_else->cmp_status == COMPARE_EQUAL) {\n> -\t\tif (!strcmp(if_then_else->str, cur->output.buf))\n> +\t\tif (!if_then_else->str)\n> +\t\t\tBUG(\"when if_then_else->cmp_status == COMPARE_EQUAL,\"\n> +\t\t\t \"if_then_else->str must not be null\");\n\nCan the change in this commit violate the invariant that\nif_then_else->str cannot be NULL, which seems to have been the case\nforever as we see an unchecked strcmp() done in the original?\n\nIf so, perhaps you can check the condition upfront, where you\ncompute str_len above, e.g.\n\n\tif (!if_then_else->str) {\n\t\tif (if_then_else->cmp_status == COMPARE_EQUAL ||\n if_then_else->cmp_status == COMPARE_UNEQUAL)\n\t\t\tBUG(...);\n\t} else\n\t\tstr_len = strlen(...);\n\nIf not, then I do not see the point of adding this (and later) check\nwith BUG to this code.\n\nOr is the invariant that .str must not be NULL could have been\nviolated without this patch (i.e. the original was buggy in running\nstrcmp() on .str without checking)? If so, please make it a separate\npreliminary change to add such an assert.\n\n> +\t\tif (str_len == cur->output.len &&\n> +\t\t !memcmp(if_then_else->str, cur->output.buf, cur->output.len))\n> \t\t\tif_then_else->condition_satisfied = 1;\n> \t} else if (if_then_else->cmp_status == COMPARE_UNEQUAL) {\n> -\t\tif (strcmp(if_then_else->str, cur->output.buf))\n> +\t\tif (!if_then_else->str)\n> +\t\t\tBUG(\"when if_then_else->cmp_status == COMPARE_UNEQUAL,\"\n> +\t\t\t \"if_then_else->str must not be null\");\n\n\n\n> /* See grab_values */\n> -static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n> +static void grab_raw_data(struct atom_value *val, int deref, void *buf, unsigned long buf_size, struct object *obj)\n> {\n> \tint i;\n> \tconst char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;\n> @@ -1307,10 +1349,22 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\n> \t\t\tcontinue;\n> \t\tif (deref)\n> \t\t\tname++;\n> -\t\tif (strcmp(name, \"body\") &&\n> -\t\t !starts_with(name, \"subject\") &&\n> -\t\t !starts_with(name, \"trailers\") &&\n> -\t\t !starts_with(name, \"contents\"))\n> +\n> +\t\tif (starts_with(name, \"raw\")) {\n> +\t\t\tif (atom->u.raw_data.option == RAW_BARE) {\n> +\t\t\t\tv->s = xmemdupz(buf, buf_size);\n> +\t\t\t\tv->s_size = buf_size;\n> +\t\t\t} else if (atom->u.raw_data.option == RAW_LENGTH)\n> +\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)buf_size);\n> +\t\t\tcontinue;\n> +\t\t}\n\nI can understand that \"raw[:]\" handling has been inserted\nabove the existing \"from here on, we only deal with log message\ncomponents\" check. But\n\n> +\t\tif ((obj->type != OBJ_TAG &&\n> +\t\t obj->type != OBJ_COMMIT) ||\n\nI do not see why these new conditions are added. The change is not\njustified in the proposed log message, the original did not need\nthese conditions, and this does not concern the primary point of the\nchange, which is to start supporting %(raw[: