{"thread":{"id":"55762","subject":"[PATCH 1/3] [GSOC] quote: add *.quote_buf_with_size functions","startedAt":"2021-05-23T09:53:37Z","lastAt":"2021-05-26T09:18:12Z","messageCount":18,"participants":["ZheNing Hu via GitGitGadget","Junio C Hamano","Felipe Contreras","Bagas Sanjaya","ZheNing Hu"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"425372","messageId":"f7121bdc4d8d8ac0071e6410960fd8787b0c36ae.1621763612.git.gitgitgadget@gmail.com","threadId":"55762","inReplyTo":"pull.959.git.1621763612.gitgitgadget@gmail.com","subject":"[PATCH 1/3] [GSOC] quote: add *.quote_buf_with_size functions","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-23T09:53:30Z","receivedAt":"2021-05-23T09:53:37Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\nWith the original functions `*._quote_buf()`, there may be\n'\\0' in our source string, only the content before the first '\\0'\nwill be processed and added to the buffer.\n\nAdd `perl_quote_buf_with_size()`, `python_quote_buf_with_size()`,\n`tcl_quote_buf_with_size`,`sq_quote_buf_with_size()` to\n`quote.c`, they will process the source string with specified\nlength characters. With them, the content after '\\0' will not\nbe truncated.\n\nThis will help us add binary data containing '\\0' to\n`quote_formatting()` in `ref-filter.c`.\n\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\n---\n quote.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n quote.h |   4 ++\n 2 files changed, 120 insertions(+)\n\ndiff --git a/quote.c b/quote.c\nindex 8a3a5e39eb12..9a1d9dde1fdb 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -43,6 +43,39 @@ void sq_quote_buf(struct strbuf *dst, const char *src)\n \tfree(to_free);\n }\n \n+void sq_quote_buf_with_size(struct strbuf *dst, const char *src, size_t size)\n+{\n+\tchar *to_free = NULL;\n+\tsize_t cur_size = 0;\n+\n+\tif (dst->buf == src)\n+\t\tto_free = strbuf_detach(dst, NULL);\n+\n+\tstrbuf_addch(dst, '\\'');\n+\twhile (cur_size < size) {\n+\t\tsize_t len = strcspn(src, \"'!\");\n+\t\tif (!len) {\n+\t\t\tstrbuf_add(dst, src, 1);\n+\t\t\tsrc++;\n+\t\t\tcur_size++;\n+\t\t} else {\n+\t\t\tstrbuf_add(dst, src, len);\n+\t\t\tsrc += len;\n+\t\t\tcur_size += len;\n+\t\t}\n+\t\tif (cur_size >= size)\n+\t\t\tbreak;\n+\t\twhile (need_bs_quote(*src)) {\n+\t\t\tstrbuf_addstr(dst, \"'\\\\\");\n+\t\t\tstrbuf_addch(dst, *src++);\n+\t\t\tcur_size++;\n+\t\t\tstrbuf_addch(dst, '\\'');\n+\t\t}\n+\t}\n+\tstrbuf_addch(dst, '\\'');\n+\tfree(to_free);\n+}\n+\n void sq_quote_buf_pretty(struct strbuf *dst, const char *src)\n {\n \tstatic const char ok_punct[] = \"+,-./:=@_^\";\n@@ -471,6 +504,25 @@ void perl_quote_buf(struct strbuf *sb, const char *src)\n \tstrbuf_addch(sb, sq);\n }\n \n+void perl_quote_buf_with_size(struct strbuf *sb, const char *src, size_t size)\n+{\n+\tconst char sq = '\\'';\n+\tconst char bq = '\\\\';\n+\tchar c;\n+\tsize_t cur_size = 0;\n+\n+\tstrbuf_addch(sb, sq);\n+\twhile (cur_size < size) {\n+\t\tc = *src++;\n+\t\tcur_size++;\n+\n+\t\tif (c == sq || c == bq)\n+\t\t\tstrbuf_addch(sb, bq);\n+\t\tstrbuf_addch(sb, c);\n+\t}\n+\tstrbuf_addch(sb, sq);\n+}\n+\n void python_quote_buf(struct strbuf *sb, const char *src)\n {\n \tconst char sq = '\\'';\n@@ -492,6 +544,31 @@ void python_quote_buf(struct strbuf *sb, const char *src)\n \tstrbuf_addch(sb, sq);\n }\n \n+void python_quote_buf_with_size(struct strbuf *sb, const char *src, size_t size)\n+{\n+\tconst char sq = '\\'';\n+\tconst char bq = '\\\\';\n+\tconst char nl = '\\n';\n+\tchar c;\n+\tsize_t cur_size = 0;\n+\n+\tstrbuf_addch(sb, sq);\n+\twhile (cur_size < size) {\n+\t\tc = *src++;\n+\t\tcur_size++;\n+\n+\t\tif (c == nl) {\n+\t\t\tstrbuf_addch(sb, bq);\n+\t\t\tstrbuf_addch(sb, 'n');\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (c == sq || c == bq)\n+\t\t\tstrbuf_addch(sb, bq);\n+\t\tstrbuf_addch(sb, c);\n+\t}\n+\tstrbuf_addch(sb, sq);\n+}\n+\n void tcl_quote_buf(struct strbuf *sb, const char *src)\n {\n \tchar c;\n@@ -527,6 +604,45 @@ void tcl_quote_buf(struct strbuf *sb, const char *src)\n \tstrbuf_addch(sb, '\"');\n }\n \n+void tcl_quote_buf_with_size(struct strbuf *sb, const char *src, size_t size)\n+{\n+\tchar c;\n+\tsize_t cur_size = 0;\n+\n+\tstrbuf_addch(sb, '\"');\n+\twhile (cur_size < size) {\n+\t\tc = *src++;\n+\t\tcur_size++;\n+\n+\t\tswitch (c) {\n+\t\tcase '[': case ']':\n+\t\tcase '{': case '}':\n+\t\tcase '$': case '\\\\': case '\"':\n+\t\t\tstrbuf_addch(sb, '\\\\');\n+\t\t\t/* fallthrough */\n+\t\tdefault:\n+\t\t\tstrbuf_addch(sb, c);\n+\t\t\tbreak;\n+\t\tcase '\\f':\n+\t\t\tstrbuf_addstr(sb, \"\\\\f\");\n+\t\t\tbreak;\n+\t\tcase '\\r':\n+\t\t\tstrbuf_addstr(sb, \"\\\\r\");\n+\t\t\tbreak;\n+\t\tcase '\\n':\n+\t\t\tstrbuf_addstr(sb, \"\\\\n\");\n+\t\t\tbreak;\n+\t\tcase '\\t':\n+\t\t\tstrbuf_addstr(sb, \"\\\\t\");\n+\t\t\tbreak;\n+\t\tcase '\\v':\n+\t\t\tstrbuf_addstr(sb, \"\\\\v\");\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\tstrbuf_addch(sb, '\"');\n+}\n+\n void basic_regex_quote_buf(struct strbuf *sb, const char *src)\n {\n \tchar c;\ndiff --git a/quote.h b/quote.h\nindex 768cc6338e27..e894507329cc 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -30,6 +30,7 @@ struct strbuf;\n  */\n \n void sq_quote_buf(struct strbuf *, const char *src);\n+void sq_quote_buf_with_size(struct strbuf *, const char *src, size_t size);\n void sq_quote_argv(struct strbuf *, const char **argv);\n void sq_quotef(struct strbuf *, const char *fmt, ...);\n \n@@ -94,8 +95,11 @@ char *quote_path(const char *in, const char *prefix, struct strbuf *out, unsigne\n \n /* quoting as a string literal for other languages */\n void perl_quote_buf(struct strbuf *sb, const char *src);\n+void perl_quote_buf_with_size(struct strbuf *sb, const char *src, size_t size);\n void python_quote_buf(struct strbuf *sb, const char *src);\n+void python_quote_buf_with_size(struct strbuf *sb, const char *src, size_t size);\n void tcl_quote_buf(struct strbuf *sb, const char *src);\n+void tcl_quote_buf_with_size(struct strbuf *sb, const char *src, size_t size);\n void basic_regex_quote_buf(struct strbuf *sb, const char *src);\n \n #endif\n-- \ngitgitgadget\n\n"},{"id":"425373","messageId":"pull.959.git.1621763612.gitgitgadget@gmail.com","threadId":"55762","inReplyTo":null,"subject":"[PATCH 0/3] [GSOC][RFC] ref-filter: add contents:raw atom","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-23T09:53:29Z","receivedAt":"2021-05-23T09:53:38Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"In (a2f3241: [GSOC] ref-filter: add contents:raw atom) I did not notice the\nbreakage that occurred during the test, In the later remediation, I found a\nvery serious problem: The output object data of \"git cat-file tree foo\" or\n\"git cat-file blob foo\" may contain '\\0'. However, most of the logic in\nref-filter depends on the atomic output not containing'\\0'.\n\nTherefore, we must carry out a series of repairs to ref-filter so that it\ncan support output of data containing '\\0'.\n\nIn first patch, I add *.quote_buf_with_size() functions, this can deal with\ndata with containing'\\0'.\n\nIn second patch, I add the member s_size in struct atom_value, and protects\nthe output of the atom from being truncated at '\\0', and successfully\nsupported the %(contents) of blob and tree.\n\nIn third patch, I added the%(contents:raw) atom, It can print the original\ncontent of an object.\n\nWhat needs to be reconsidered:\n\nFor a binary object blob, tree,\n\ngit for-each-ref --format=\"%(contents)\" --python refs/mytrees/first\n\nwill output a string processed by python_quote_buf_with_size(), which\ncontains'\\0'. But the binary files seem to be useless after quoting. Should\nwe allow these binary files to be output in the default way with\nstrbuf_add()? If so, we can remove the first patch.\n\nZheNing Hu (3):\n  [GSOC] quote: add *.quote_buf_with_size functions\n  [GSOC] ref-filter: support %(contents) for blob, tree\n  [GSOC] ref-filter: add contents:raw atom\n\n Documentation/git-for-each-ref.txt |  19 ++-\n quote.c                            | 116 +++++++++++++++\n quote.h                            |   4 +\n ref-filter.c                       | 229 +++++++++++++++++++++--------\n t/t6300-for-each-ref.sh            | 214 ++++++++++++++++++++++++++-\n 5 files changed, 511 insertions(+), 71 deletions(-)\n\n\nbase-commit: 97eea85a0a1ec66d356567808a1e4ca2367e0ce7\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-959%2Fadlternative%2Fref-filter-contents-raw-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-959/adlternative/ref-filter-contents-raw-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/959\n-- \ngitgitgadget\n"},{"id":"425374","messageId":"19413cfdb1ea50de401ab958b954a550b6514688.1621763612.git.gitgitgadget@gmail.com","threadId":"55762","inReplyTo":"pull.959.git.1621763612.gitgitgadget@gmail.com","subject":"[PATCH 2/3] [GSOC] ref-filter: support %(contents) for blob, tree","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-23T09:53:31Z","receivedAt":"2021-05-23T09:53:40Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\nIn order to let `cat-file --batch` use ref-filter logic,\nwe need to print the original content of an object. We\ncan reuse the existing atom `%(contents)` in `ref-filter`,\nThe original `%(contents)` only supports tag and commit\nobjects. If we want to support both blob and tree objects,\nwe must consider the following issues:\n\nThe original contents of blob, tree objects may contain '\\0',\nmost of the logic in `ref-filter` depends on the output of\nthe atom being a string (end with '\\0'):\n\n`quote_formatting()` use `strbuf_addstr()` or `*._quote_buf()`\nadd content to the buffer. E.g. The original content of a tree\nobject is `100644 one\\0...`, only the `100644 one` will be\nadded to the buffer, which is incorrect.\n\nTherefore, add a new member in `struct atom_value`: `s_size`,\nwhen we record the original content of the blob and tree\nobjects in `grab_contents()`, use `v->s_size` to record\nthe size of the objects contents, then in `quote_formatting()`,\nif the length of the contents passed in `quote_formatting()` is\nnot equal to 0, we can use `strbuf_add()` instead of\n`strbuf_addstr()`  or `*._quote_buf_with_size()` instead of\n`*._quote_buf()` to add contents with a specified length if the\nlength of the contents is not equal to 0. It will not cause\ntruncation problems.\n\nSimilarly, in `append_atom()`, we use `strbuf_add()` instead\nof `strbuf_addstr()`; In `then_atom_handler()`, we use `memcmp()`\ninstread of `strcmp()`; In `cmp_ref_sorting()`, we use `memcmp()`\nand `memcasecmp()` instead of `strcmp()` and `strcasecmp()` when\nthe `v->s_size` of one of the two atoms is not equals to 0.\n\nBased-on-patch-by: Olga Telezhnaya <olyatelezhnaya@gmail.com>\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\n---\n Documentation/git-for-each-ref.txt |  11 +-\n ref-filter.c                       | 220 +++++++++++++++++++++--------\n t/t6300-for-each-ref.sh            | 166 +++++++++++++++++++++-\n 3 files changed, 327 insertions(+), 70 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 2ae2478de706..30b93d2e5178 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -235,11 +235,12 @@ 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 message in a commit or a tag object is `contents`, from which\n-`contents:<part>` can be used to extract various parts out of:\n+The data in a object is `contents`, from which `contents:<part>` can be used\n+to extract various parts out of:\n \n contents:size::\n-\tThe size in bytes of the commit or tag message.\n+\tThe size in bytes of the commit or tag message, and the raw object size\n+\tof the blob or tree.\n \n contents:subject::\n \tThe first paragraph of the message, which typically is a\n@@ -257,7 +258,9 @@ contents:signature::\n \tThe optional GPG signature of the tag.\n \n contents:lines=N::\n-\tThe first `N` lines of the message.\n+\tThe first `N` lines of the message of the commit or tag message.\n+\n+Note: blob and tree objects only support `%(contents)` and `%(contents:size)`.\n \n Additionally, the trailers as interpreted by linkgit:git-interpret-trailers[1]\n are obtained as `trailers[:options]` (or by using the historical alias\ndiff --git a/ref-filter.c b/ref-filter.c\nindex e2eac50d9508..e59907188e79 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -564,6 +564,7 @@ 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@@ -652,23 +653,38 @@ 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)\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+\t\tif (len)\n+\t\t\tsq_quote_buf_with_size(s, str, len);\n+\t\telse\n+\t\t\tsq_quote_buf(s, str);\n \t\tbreak;\n \tcase QUOTE_PERL:\n-\t\tperl_quote_buf(s, str);\n+\t\tif (len)\n+\t\t\tperl_quote_buf_with_size(s, str, len);\n+\t\telse\n+\t\t\tperl_quote_buf(s, str);\n \t\tbreak;\n \tcase QUOTE_PYTHON:\n-\t\tpython_quote_buf(s, str);\n+\t\tif (len)\n+\t\t\tpython_quote_buf_with_size(s, str, len);\n+\t\telse\n+\t\t\tpython_quote_buf(s, str);\n \t\tbreak;\n \tcase QUOTE_TCL:\n-\t\ttcl_quote_buf(s, str);\n+\t\tif (len)\n+\t\t\ttcl_quote_buf_with_size(s, str, len);\n+\t\telse\n+\t\t\ttcl_quote_buf(s, str);\n \t\tbreak;\n \t}\n }\n@@ -683,9 +699,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)\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 +804,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 +821,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 +832,30 @@ 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 (cur->output.len > str_len)\n+\t\t\tstr_len = cur->output.len;\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 (!memcmp(if_then_else->str, cur->output.buf, str_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 (cur->output.len > str_len)\n+\t\t\tstr_len = cur->output.len;\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 (memcmp(if_then_else->str, cur->output.buf, str_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 +901,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(&current->output, &s);\n \t}\n \tstrbuf_release(&s);\n@@ -1292,7 +1326,8 @@ 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_contents(struct atom_value *val, int deref, void *buf,\n+\t\t\t  unsigned long buf_size, enum object_type object_type)\n {\n \tint i;\n \tconst char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;\n@@ -1312,43 +1347,60 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)\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-\t\t\t\t    &subpos, &sublen,\n-\t\t\t\t    &bodypos, &bodylen, &nonsiglen,\n-\t\t\t\t    &sigpos, &siglen);\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-\t\t\tstruct strbuf sb = STRBUF_INIT;\n-\t\t\tformat_sanitized_subject(&sb, subpos, sublen);\n-\t\t\tv->s = strbuf_detach(&sb, NULL);\n-\t\t} else if (atom->u.contents.option == C_BODY_DEP)\n-\t\t\tv->s = xmemdupz(bodypos, bodylen);\n-\t\telse if (atom->u.contents.option == C_LENGTH)\n-\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)strlen(subpos));\n-\t\telse if (atom->u.contents.option == C_BODY)\n-\t\t\tv->s = xmemdupz(bodypos, nonsiglen);\n-\t\telse if (atom->u.contents.option == C_SIG)\n-\t\t\tv->s = xmemdupz(sigpos, siglen);\n-\t\telse if (atom->u.contents.option == C_LINES) {\n-\t\t\tstruct strbuf s = STRBUF_INIT;\n-\t\t\tconst char *contents_end = bodypos + nonsiglen;\n-\n-\t\t\t/*  Size is the length of the message after removing the signature */\n-\t\t\tappend_lines(&s, subpos, contents_end - subpos, atom->u.contents.nlines);\n-\t\t\tv->s = strbuf_detach(&s, NULL);\n-\t\t} else if (atom->u.contents.option == C_TRAILERS) {\n-\t\t\tstruct strbuf s = STRBUF_INIT;\n-\n-\t\t\t/* Format the trailer info according to the trailer_opts given */\n-\t\t\tformat_trailers_from_commit(&s, subpos, &atom->u.contents.trailer_opts);\n-\n-\t\t\tv->s = strbuf_detach(&s, NULL);\n-\t\t} else if (atom->u.contents.option == C_BARE)\n-\t\t\tv->s = xstrdup(subpos);\n \n+\t\tswitch (object_type) {\n+\t\tcase OBJ_TAG:\n+\t\tcase OBJ_COMMIT: {\n+\t\t\tif (!subpos)\n+\t\t\t\tfind_subpos(buf,\n+\t\t\t\t\t&subpos, &sublen,\n+\t\t\t\t\t&bodypos, &bodylen, &nonsiglen,\n+\t\t\t\t\t&sigpos, &siglen);\n+\n+\t\t\tif (atom->u.contents.option == C_SUB)\n+\t\t\t\tv->s = copy_subject(subpos, sublen);\n+\t\t\telse if (atom->u.contents.option == C_SUB_SANITIZE) {\n+\t\t\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\t\t\tformat_sanitized_subject(&sb, subpos, sublen);\n+\t\t\t\tv->s = strbuf_detach(&sb, NULL);\n+\t\t\t} else if (atom->u.contents.option == C_BODY_DEP)\n+\t\t\t\tv->s = xmemdupz(bodypos, bodylen);\n+\t\t\telse if (atom->u.contents.option == C_LENGTH)\n+\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)strlen(subpos));\n+\t\t\telse if (atom->u.contents.option == C_BODY)\n+\t\t\t\tv->s = xmemdupz(bodypos, nonsiglen);\n+\t\t\telse if (atom->u.contents.option == C_SIG)\n+\t\t\t\tv->s = xmemdupz(sigpos, siglen);\n+\t\t\telse if (atom->u.contents.option == C_LINES) {\n+\t\t\t\tstruct strbuf s = STRBUF_INIT;\n+\t\t\t\tconst char *contents_end = bodypos + nonsiglen;\n+\n+\t\t\t\t/*  Size is the length of the message after removing the signature */\n+\t\t\t\tappend_lines(&s, subpos, contents_end - subpos, atom->u.contents.nlines);\n+\t\t\t\tv->s = strbuf_detach(&s, NULL);\n+\t\t\t} else if (atom->u.contents.option == C_TRAILERS) {\n+\t\t\t\tstruct strbuf s = STRBUF_INIT;\n+\n+\t\t\t\t/* Format the trailer info according to the trailer_opts given */\n+\t\t\t\tformat_trailers_from_commit(&s, subpos, &atom->u.contents.trailer_opts);\n+\n+\t\t\t\tv->s = strbuf_detach(&s, NULL);\n+\t\t\t} else if (atom->u.contents.option == C_BARE)\n+\t\t\t\tv->s = xstrdup(subpos);\n+\t\t\tbreak;\n+\t\t}\n+\t\tcase OBJ_BLOB:\n+\t\tcase OBJ_TREE: {\n+\t\t\tif (atom->u.contents.option == C_BARE) {\n+\t\t\t\tv->s_size = buf_size;\n+\t\t\t\tv->s = xmemdupz(buf, buf_size);\n+\t\t\t} else if (atom->u.contents.option == C_LENGTH)\n+\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, buf_size);\n+\t\t\tbreak;\n+\t\t}\n+\t\tdefault:\n+\t\t\tBUG(\"unknown object type\");\n+\t\t}\n \t}\n \tfree((void *)sigpos);\n }\n@@ -1374,25 +1426,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_contents(val, deref, buf, buf_size, obj->type);\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_contents(val, deref, buf, buf_size, obj->type);\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_contents(val, deref, buf, buf_size, obj->type);\n \t\tbreak;\n \tcase OBJ_BLOB:\n \t\t/* grab_blob_values(val, deref, obj, buf, sz); */\n+\t\tgrab_contents(val, deref, buf, buf_size, obj->type);\n \t\tbreak;\n \tdefault:\n \t\tdie(\"Eh?  Object of type %d?\", obj->type);\n@@ -1614,7 +1671,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@@ -2297,6 +2354,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+  size_t i;\n+  const char *s1 = (const char *)vs1;\n+  const char *s2 = (const char *)vs2;\n+\n+  for (i = 0; i < n; i++) {\n+\tunsigned char u1 = s1[i];\n+\tunsigned char u2 = s2[i];\n+\tint U1 = toupper (u1);\n+\tint U2 = toupper (u2);\n+\tint diff = (UCHAR_MAX <= INT_MAX ? U1 - U2\n+\t\t    : U1 < U2 ? -1 : U2 < U1);\n+\tif (diff)\n+\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 +2380,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 +2394,28 @@ 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 && !vb->s_size) {\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+\t\t\tif (va->s_size && vb->s_size) {\n+\t\t\t\tcmp = cmp_fn(va->s, vb->s, va->s_size > vb->s_size ?\n+\t\t\t\t\t     va->s_size : vb->s_size);\n+\t\t\t} else if (!va->s_size) {\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     slen : vb->s_size);\n+\t\t\t} else {\n+\t\t\t\tslen = strlen(vb->s);\n+\t\t\t\tcmp = cmp_fn(va->s, vb->s, va->s_size > slen ?\n+\t\t\t\t\t     va->s_size : slen);\n+\t\t\t}\n+\t\t}\n \t} else {\n \t\tif (va->value < vb->value)\n \t\t\tcmp = -1;\n@@ -2420,6 +2515,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 = 0;\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..4754ec639797 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -63,8 +63,10 @@ test_atom() {\n \t\ttag)\n \t\t\t# We cannot use $3 as it expects sanitize_pgp to run\n \t\t\texpect=$(git cat-file tag $ref | tail -n +6 | wc -c) ;;\n-\t\ttree | blob)\n-\t\t\texpect='' ;;\n+\t\ttree)\n+\t\t\texpect=$(git cat-file tree $ref | wc -c) ;;\n+\t\tblob)\n+\t\t\texpect=$(git cat-file blob $ref | wc -c) ;;\n \t\tcommit)\n \t\t\texpect=$(printf '%s' \"$3\" | wc -c) ;;\n \t\tesac\n@@ -718,14 +720,170 @@ test_atom refs/mytrees/first contents:subject \"\"\n test_atom refs/mytrees/first body \"\"\n 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 contents' '\n+\tgit cat-file tree refs/mytrees/first >expected &&\n+\tcat expected | wc -c >size_expected &&\n+\techo \"\" >>expected &&\n+\tgit for-each-ref --format=\"%(contents)\" refs/mytrees/first >actual &&\n+\ttest_cmp expected actual &&\n+\tgit for-each-ref --format=\"%(contents:size)\" refs/mytrees/first >actual &&\n+\ttest_cmp size_expected actual\n+'\n+\n+test_expect_success 'basic atom: refs/mytrees/first contents with --python' '\n+\tcat >expected <<-\\EOF &&\n+\t0000000 030447 030060 032066 020064 067157 000145 157155 153143\n+\t0000020 106210 070754 101352 115504 123726 045150 042451 077455\n+\t0000040 030061 033060 032064 072040 067567 072056 173400 167431\n+\t0000060 030324 025725 144317 065126 131103 062753 104126 104323\n+\t0000100 023561 000012\n+\t0000103\n+\tEOF\n+\tgit for-each-ref --python --format=\"%(contents)\" refs/mytrees/first >actual &&\n+\tod actual >od_actual &&\n+\ttest_cmp expected od_actual\n+'\n+\n+test_expect_success 'basic atom: refs/mytrees/first contents with --tcl' '\n+\tcat >expected <<-\\EOF &&\n+\t0000000 030442 030060 032066 020064 067157 000145 157155 153143\n+\t0000020 106210 070754 101352 115504 123726 045150 042451 077455\n+\t0000040 030061 033060 032064 072040 067567 072056 173400 167431\n+\t0000060 030324 025725 144317 065126 131103 062753 104126 104323\n+\t0000100 021161 000012\n+\t0000103\n+\tEOF\n+\tgit for-each-ref --tcl --format=\"%(contents)\" refs/mytrees/first >actual &&\n+\tod actual >od_actual &&\n+\ttest_cmp expected od_actual\n+'\n+\n+test_expect_success 'basic atom: refs/mytrees/first contents with --shell' '\n+\tcat >expected <<-\\EOF &&\n+\t0000000 030447 030060 032066 020064 067157 000145 157155 153143\n+\t0000020 106210 070754 101352 115504 123726 045150 042451 077455\n+\t0000040 030061 033060 032064 072040 067567 072056 173400 167431\n+\t0000060 030324 025725 144317 065126 131103 062753 104126 104323\n+\t0000100 023561 000012\n+\t0000103\n+\tEOF\n+\tgit for-each-ref --shell --format=\"%(contents)\" refs/mytrees/first >actual &&\n+\tod actual >od_actual &&\n+\ttest_cmp expected od_actual\n+'\n+\n+test_expect_success 'basic atom: refs/mytrees/first contents with --perl' '\n+\tcat >expected <<-\\EOF &&\n+\t0000000 030447 030060 032066 020064 067157 000145 157155 153143\n+\t0000020 106210 070754 101352 115504 123726 045150 042451 077455\n+\t0000040 030061 033060 032064 072040 067567 072056 173400 167431\n+\t0000060 030324 025725 144317 065126 131103 062753 104126 104323\n+\t0000100 023561 000012\n+\t0000103\n+\tEOF\n+\tgit for-each-ref --perl --format=\"%(contents)\" refs/mytrees/first >actual &&\n+\tod actual >od_actual &&\n+\ttest_cmp expected od_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 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 contents' '\n+\tgit cat-file blob refs/myblobs/first >expected &&\n+\tcat expected | wc -c >size_expected &&\n+\techo \"\" >>expected &&\n+\tgit for-each-ref --format=\"%(contents)\" refs/myblobs/first >actual &&\n+\ttest_cmp expected actual &&\n+\tgit for-each-ref --format=\"%(contents:size)\" refs/myblobs/first >actual &&\n+\ttest_cmp size_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+\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+'\n+\n+test_expect_success 'Verify sorts with contents' '\n+\tcat >expected <<-EOF &&\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=contents \\\n+\t\trefs/heads/main refs/myblobs/ refs/mytrees/first >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'validate contents atom with %(if:equals)' '\n+\tcat >expect <<-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+\tEOF\n+\tgit for-each-ref --format=\"%(if:equals=abc)%(contents)%(then)%(refname)%(else)not equals%(end)\" \\\n+\t\trefs/myblobs/ refs/heads/ >actual &&\n+\ttest_cmp expect actual\n+'\n+test_expect_success 'validate contents atom with %(if:notequals)' '\n+\tcat >expect <<-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/first\n+\tEOF\n+\tgit for-each-ref --format=\"%(if:notequals=abc)%(contents)%(then)%(refname)%(else)equals%(end)\" \\\n+\t\trefs/myblobs/ refs/heads/ >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'empty contents refs with %(if)' '\n+\tcat >expect <<-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/first not empty\n+\tEOF\n+\tgit for-each-ref --format=\"%(refname) %(if)%(contents)%(then)not empty%(else)empty%(end)\" \\\n+\trefs/myblobs/ >actual &&\n+\ttest_cmp expect actual\n+'\n \n test_expect_success 'set up multiple-sort tags' '\n \tfor when in 100000 200000\n-- \ngitgitgadget\n\n"},{"id":"425375","messageId":"320037a8b98dc8bf21a8636a58e04c8f8a23f80a.1621763612.git.gitgitgadget@gmail.com","threadId":"55762","inReplyTo":"pull.959.git.1621763612.gitgitgadget@gmail.com","subject":"[PATCH 3/3] [GSOC] ref-filter: add contents:raw atom","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-05-23T09:53:32Z","receivedAt":"2021-05-23T09:53:40Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\nAdd new formatting option `%(contents:raw)`, which will\nprint all the object contents without any changes.\nFor blob and tree, it priont all object data, its output\nis equivalent to `%(contents)`; For commit and tag,\nit not only print objects’ messages (as `%(contents)` does),\nbut also print objects headers, like the \"tree XXX\",\n\"parent YYY\", etc lines in commits, and the \"object OOO\",\n\"type TTT\", etc lines in tags.\n\nIt will help further to migrate all cat-file formatting\nlogic from cat-file to ref-filter.\n\nE.g.\n\ngit for-each-ref --format=%(contents:raw) refs/heads/foo\n\nIt will have similar output to:\n\ngit rev-parse refs/heads/foo | git cat-file --batch\n\nBased-on-patch-by: Olga Telezhnaya <olyatelezhnaya@gmail.com>\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\n---\n Documentation/git-for-each-ref.txt | 10 ++++++-\n ref-filter.c                       | 11 +++++--\n t/t6300-for-each-ref.sh            | 48 ++++++++++++++++++++++++++++++\n 3 files changed, 66 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt\nindex 30b93d2e5178..de77a7454434 100644\n--- a/Documentation/git-for-each-ref.txt\n+++ b/Documentation/git-for-each-ref.txt\n@@ -260,7 +260,15 @@ contents:signature::\n contents:lines=N::\n \tThe first `N` lines of the message of the commit or tag message.\n \n-Note: blob and tree objects only support `%(contents)` and `%(contents:size)`.\n+contents:raw::\n+\tThe raw data of the object. For blob and tree, it priont all object data,\n+\tits output is equivalent to `%(contents)`; For commit and tag, it not only\n+\tprint objects’ messages (as `%(contents)` does), but also print objects\n+\theaders, like the \"tree XXX\", \"parent YYY\", etc lines in commits, and the\n+\t\"object OOO\", \"type TTT\", etc lines in tags.\n+\n+Note: blob and tree objects only support `%(contents)`, `%(contents:raw)` and\n+`%(contents:size)`.\n \n Additionally, the trailers as interpreted by linkgit:git-interpret-trailers[1]\n are obtained as `trailers[:options]` (or by using the historical alias\ndiff --git a/ref-filter.c b/ref-filter.c\nindex e59907188e79..17ef39fe7454 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -134,7 +134,7 @@ static struct used_atom {\n \t\t} remote_ref;\n \t\tstruct {\n \t\t\tenum { C_BARE, C_BODY, C_BODY_DEP, C_LENGTH, C_LINES,\n-\t\t\t       C_SIG, C_SUB, C_SUB_SANITIZE, C_TRAILERS } option;\n+\t\t\t       C_RAW, C_SIG, C_SUB, C_SUB_SANITIZE, C_TRAILERS } option;\n \t\t\tstruct process_trailer_options trailer_opts;\n \t\t\tunsigned int nlines;\n \t\t} contents;\n@@ -347,6 +347,8 @@ static int contents_atom_parser(const struct ref_format *format, struct used_ato\n {\n \tif (!arg)\n \t\tatom->u.contents.option = C_BARE;\n+\telse if (!strcmp(arg,\"raw\"))\n+\t\tatom->u.contents.option = C_RAW;\n \telse if (!strcmp(arg, \"body\"))\n \t\tatom->u.contents.option = C_BODY;\n \telse if (!strcmp(arg, \"size\"))\n@@ -1387,11 +1389,16 @@ static void grab_contents(struct atom_value *val, int deref, void *buf,\n \t\t\t\tv->s = strbuf_detach(&s, NULL);\n \t\t\t} else if (atom->u.contents.option == C_BARE)\n \t\t\t\tv->s = xstrdup(subpos);\n+\t\t\telse if (atom->u.contents.option == C_RAW) {\n+\t\t\t\tv->s_size = buf_size;\n+\t\t\t\tv->s = xmemdupz(buf, buf_size);\n+\t\t\t}\n \t\t\tbreak;\n \t\t}\n \t\tcase OBJ_BLOB:\n \t\tcase OBJ_TREE: {\n-\t\t\tif (atom->u.contents.option == C_BARE) {\n+\t\t\tif (atom->u.contents.option == C_BARE ||\n+\t\t\t    atom->u.contents.option == C_RAW) {\n \t\t\t\tv->s_size = buf_size;\n \t\t\t\tv->s = xmemdupz(buf, buf_size);\n \t\t\t} else if (atom->u.contents.option == C_LENGTH)\ndiff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh\nindex 4754ec639797..d2a7e1f4384e 100755\n--- a/t/t6300-for-each-ref.sh\n+++ b/t/t6300-for-each-ref.sh\n@@ -159,6 +159,8 @@ test_atom head subject 'Initial'\n test_atom head subject:sanitize 'Initial'\n test_atom head contents:subject 'Initial'\n test_atom head body ''\n+test_atom head contents:raw \"$(git cat-file commit refs/heads/main)\n+\"\n test_atom head contents:body ''\n test_atom head contents:signature ''\n test_atom head contents 'Initial\n@@ -688,6 +690,18 @@ 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 contents:raw' '\n+\tgit cat-file tag refs/tags/signed-empty >expected &&\n+\tgit for-each-ref --format=\"%(contents:raw)\" refs/tags/signed-empty >actual &&\n+\tsanitize_pgp <expected >expected.clean &&\n+\tsanitize_pgp <actual >actual.clean &&\n+\techo \"\" >>expected.clean &&\n+\ttest_cmp expected.clean actual.clean\n+'\n+\n+test_atom refs/tags/signed-empty '*contents:raw' \"$(git cat-file commit HEAD)\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@@ -697,6 +711,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 contents:raw' '\n+\tgit cat-file tag refs/tags/signed-short >expected &&\n+\tgit for-each-ref --format=\"%(contents:raw)\" refs/tags/signed-short >actual &&\n+\tsanitize_pgp <expected >expected.clean &&\n+\tsanitize_pgp <actual >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@@ -710,6 +733,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 contents:raw' '\n+\tgit cat-file tag refs/tags/signed-long >expected &&\n+\tgit for-each-ref --format=\"%(contents:raw)\" refs/tags/signed-long >actual &&\n+\tsanitize_pgp <expected >expected.clean &&\n+\tsanitize_pgp <actual >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@@ -731,6 +763,14 @@ test_expect_success 'basic atom: refs/mytrees/first contents' '\n \ttest_cmp size_expected actual\n '\n \n+test_expect_success 'basic atom: refs/mytrees/first contents:raw' '\n+\tgit cat-file tree refs/mytrees/first >expected &&\n+\tcat expected | wc -c >size_expected &&\n+\techo \"\" >>expected &&\n+\tgit for-each-ref --format=\"%(contents:raw)\" refs/mytrees/first >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'basic atom: refs/mytrees/first contents with --python' '\n \tcat >expected <<-\\EOF &&\n \t0000000 030447 030060 032066 020064 067157 000145 157155 153143\n@@ -803,6 +843,14 @@ test_expect_success 'basic atom: refs/myblobs/first contents' '\n \ttest_cmp size_expected actual\n '\n \n+test_expect_success 'basic atom: refs/myblobs/first contents:raw' '\n+\tgit cat-file blob refs/myblobs/first >expected &&\n+\tcat expected | wc -c >size_expected &&\n+\techo \"\" >>expected &&\n+\tgit for-each-ref --format=\"%(contents:raw)\" 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":"425391","messageId":"xmqq1r9xndjf.fsf@gitster.g","threadId":"55762","inReplyTo":"pull.959.git.1621763612.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] [GSOC][RFC] ref-filter: add contents:raw atom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-24T01:09:08Z","receivedAt":"2021-05-24T01:09:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZheNing Hu via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> In (a2f3241: [GSOC] ref-filter: add contents:raw atom) I did not notice the\n> ...\n\nBefore going into any of these details, remember that the cover\nletter is where you first sell the series to the readers.  Why is it\nworth their time to read it?  What problem does it solve in the\nbigger picture?  Mention that we want to let \"cat-file --batch\" use\nthe ref-filter --format logic, if that is the primary reason why we\nhave these three patches, for example.\n\nI actually do not know if a modified form of %(contents) is a good\nmatch for this feature.  It was invented as a way to extract the\nunstructured \"free text\" part of structured textual objects (namely,\ncommits and tags) so it is natural that it would not apply to trees\n(they are structured and there is no unstractured \"free text\" part)\nnor blobs (they are totally unstructured and does not even have to\nbe text).\n\nIs there another word to refer to the entire payload unadulterated?\n\n> git for-each-ref --format=\"%(contents)\" --python refs/mytrees/first\n>\n> will output a string processed by python_quote_buf_with_size(), which\n> contains'\\0'. But the binary files seem to be useless after quoting. Should\n> we allow these binary files to be output in the default way with\n> strbuf_add()? If so, we can remove the first patch.\n\nThe --language option is designed to be used to write a small script\nin the language and used like this:\n\n    git for-each-ref --format='\n\t\tname=%(refname)\n\t\tvar=%(placeholder)\n                mkdir -p \"$(dirname \"$name\")\"\n\t\tprintf \"%%s\" \"$var\" >\"$name\"\n    ' --shell | /bin/sh\n\nNote that %(refname) and %(placeholder) in the --format string is\nnot quoted at all; the \"--shell\" option knows how values are quoted\nin the host language (shell) and writes single-quotes around\n%(refname).  If %(placeholder) produces something with a single-quote\nin it, that will (eh, at least \"should\") be quoted appropriately.\n\nSo It does not make any sense not to quote a value that comes from\n%(placeholder), whether it is binary or not, to match the syntax of\nthe host language you are making the \"for-each-ref --format=\" to\nwrite such a script in.\n\nSo, \"binary files seem to be useless after quoting\" is a\nmisunderstanding.  They are useless if you do not quote them.\n\nThanks.\n\nP.S. I am mostly offline today, and response will be slow.\n\n"},{"id":"425394","messageId":"60ab1246d2d14_1691c2083b@natae.notmuch","threadId":"55762","inReplyTo":"xmqq1r9xndjf.fsf@gitster.g","subject":"Re: [PATCH 0/3] [GSOC][RFC] ref-filter: add contents:raw atom","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-05-24T02:41:10Z","receivedAt":"2021-05-24T02:41:16Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n\n> Is there another word to refer to the entire payload unadulterated?\n\nAnother word better than \"raw\"? I don't think so: basic, crude, green,\nrough.\n\n-- \nFelipe Contreras\n"},{"id":"425396","messageId":"72ce81a2-68eb-a750-8ca1-425af3cac370@gmail.com","threadId":"55762","inReplyTo":"60ab1246d2d14_1691c2083b@natae.notmuch","subject":"Re: [PATCH 0/3] [GSOC][RFC] ref-filter: add contents:raw atom","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-05-24T05:22:39Z","receivedAt":"2021-05-24T05:22:46Z","isPatch":true,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"On 24/05/21 09.41, Felipe Contreras wrote:\n> Junio C Hamano wrote:\n> \n>> Is there another word to refer to the entire payload unadulterated?\n> \n> Another word better than \"raw\"? I don't think so: basic, crude, green,\n> rough.\n> \n\nI think we should go with \"raw\", because the payloads we discussed here\nare unmodified. It is akin to \"raw data\" that is processed further to\nproduce porcelain output, such as templating engine that process raw\nHTML into HTML pages that would be served to end user.\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"425430","messageId":"CAOLTT8RgVO44Lf9jtz=8Jj5CxiGqbeAcFQOwQTazZzBwtKTr6A@mail.gmail.com","threadId":"55762","inReplyTo":"xmqq1r9xndjf.fsf@gitster.g","subject":"Re: [PATCH 0/3] [GSOC][RFC] ref-filter: add contents:raw atom","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-05-24T13:09:09Z","receivedAt":"2021-05-24T13:09:27Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Junio C Hamano <gitster@pobox.com> 于2021年5月24日周一 上午9:09写道：\n>\n> \"ZheNing Hu via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>\n> > In (a2f3241: [GSOC] ref-filter: add contents:raw atom) I did not notice the\n> > ...\n>\n> Before going into any of these details, remember that the cover\n> letter is where you first sell the series to the readers.  Why is it\n> worth their time to read it?  What problem does it solve in the\n> bigger picture?  Mention that we want to let \"cat-file --batch\" use\n> the ref-filter --format logic, if that is the primary reason why we\n> have these three patches, for example.\n>\n\nThanks for reminding, Indeed, as you said, this cover-letter needs to\nexplain the patches' main purpose: let \"cat-file --batch\" use the ref-filter\n--format logic.\n\n> I actually do not know if a modified form of %(contents) is a good\n> match for this feature.  It was invented as a way to extract the\n> unstructured \"free text\" part of structured textual objects (namely,\n> commits and tags) so it is natural that it would not apply to trees\n> (they are structured and there is no unstractured \"free text\" part)\n> nor blobs (they are totally unstructured and does not even have to\n> be text).\n>\n\nThis is exactly what I worry about: Does %(contents) have good\nsemantics to explain things like blob, tree objects which have no\nunstractured \"free text\" part or totally unstructured?\n\n> Is there another word to refer to the entire payload unadulterated?\n>\n\nIf you specifically mean \"raw\" in %(contents:raw), I agree with Felipe\nContreras or Bagas Sanjaya, \"raw\" seems to be enough. But if you\nmean we need another atom name, then \"%(binary)\", \"%(text)\" may\nbe possible choices.\n\nThere is a good word in `ref-filter.c`: C_BARE, but this enum vale\ncorresponds to %(contents). Therefore, I cannot use the name\n\"%(contents:bare)\".\n\n> > git for-each-ref --format=\"%(contents)\" --python refs/mytrees/first\n> >\n> > will output a string processed by python_quote_buf_with_size(), which\n> > contains'\\0'. But the binary files seem to be useless after quoting. Should\n> > we allow these binary files to be output in the default way with\n> > strbuf_add()? If so, we can remove the first patch.\n>\n> The --language option is designed to be used to write a small script\n> in the language and used like this:\n>\n>     git for-each-ref --format='\n>                 name=%(refname)\n>                 var=%(placeholder)\n>                 mkdir -p \"$(dirname \"$name\")\"\n>                 printf \"%%s\" \"$var\" >\"$name\"\n>     ' --shell | /bin/sh\n>\n> Note that %(refname) and %(placeholder) in the --format string is\n> not quoted at all; the \"--shell\" option knows how values are quoted\n> in the host language (shell) and writes single-quotes around\n> %(refname).  If %(placeholder) produces something with a single-quote\n> in it, that will (eh, at least \"should\") be quoted appropriately.\n>\n\nWell, now I have also noticed that --language can make these atoms be\nsurrounded by single quotes, special characters in the output will be\nescaped e.g. \"a'b\" will turn to \"'a\\'b'\", In this way, shell, python... can\ndirectly quote the content.\n\n> So It does not make any sense not to quote a value that comes from\n> %(placeholder), whether it is binary or not, to match the syntax of\n> the host language you are making the \"for-each-ref --format=\" to\n> write such a script in.\n>\n> So, \"binary files seem to be useless after quoting\" is a\n> misunderstanding.  They are useless if you do not quote them.\n>\n\nNow I agree that It seems that the content of the blob or tree is\nmeaningful after being quoted.\n\n> Thanks.\n>\n> P.S. I am mostly offline today, and response will be slow.\n>\n\nThanks!\n--\nZheNing Hu\n"},{"id":"425434","messageId":"xmqqo8d0jgxa.fsf@gitster.g","threadId":"55762","inReplyTo":"72ce81a2-68eb-a750-8ca1-425af3cac370@gmail.com","subject":"Re: [PATCH 0/3] [GSOC][RFC] ref-filter: add contents:raw atom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-24T15:21:53Z","receivedAt":"2021-05-24T15:23:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bagas Sanjaya <bagasdotme@gmail.com> writes:\n\n> I think we should go with \"raw\", because the payloads we discussed here\n> are unmodified. It is akin to \"raw data\" that is processed further to\n> produce porcelain output, such as templating engine that process raw\n> HTML into HTML pages that would be served to end user.\n\nOh, I can certainly live with --format='%(raw)'; I just view that\n%(contents:raw) is problematic, especially if we meant to apply it\nto trees and blobs, because they are not what %(contents) is about.\n\nThanks.\n"},{"id":"425484","messageId":"xmqqfsybh0bn.fsf@gitster.g","threadId":"55762","inReplyTo":"19413cfdb1ea50de401ab958b954a550b6514688.1621763612.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/3] [GSOC] ref-filter: support %(contents) for blob, tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-25T05:03:24Z","receivedAt":"2021-05-25T05:03:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZheNing Hu via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> @@ -1292,7 +1326,8 @@ 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\n\nAs I already said, I do not think %(contents) mixes well with this\nuse for trees and blobs where you give the raw bytes, since\n%(contents) for commit and tag objects was never about the full byte\nsequence of the object.  It was to give the unstructured part meant\nfor human consumption, stripping the structured \"header\" part of the\nobject.\n\nNevertheless, since queuing this topic breaks the build and gets in\nthe way to find issues in other proposed regression fixes of higher\nimportance,...\n\n> +static void grab_contents(struct atom_value *val, int deref, void *buf,\n> +\t\t\t  unsigned long buf_size, enum object_type object_type)\n>  {\n> ...\n> +\t\tswitch (object_type) {\n> +\t\tcase OBJ_TAG:\n> +\t\tcase OBJ_COMMIT: {\n> ...\n> +\t\t\t\tv->s = xmemdupz(bodypos, bodylen);\n> +\t\t\telse if (atom->u.contents.option == C_LENGTH)\n> +\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)strlen(subpos));\n> ...\n\nNote that this part inherits from the original and knows how to feed\na value to PRIuMAX with correct cast ...\n\n> +\t\t\t\tv->s = xmemdupz(bodypos, nonsiglen);\n> +\t\t\telse if (atom->u.contents.option == C_SIG)\n> ...\n> +\t\t}\n> +\t\tcase OBJ_BLOB:\n> +\t\tcase OBJ_TREE: {\n> +\t\t\tif (atom->u.contents.option == C_BARE) {\n> +\t\t\t\tv->s_size = buf_size;\n> +\t\t\t\tv->s = xmemdupz(buf, buf_size);\n> +\t\t\t} else if (atom->u.contents.option == C_LENGTH)\n> +\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, buf_size);\n\n... but this one gets sloppy, and breaks the windows-build of\n'seen'.  Fix is simple:\n\n+\t\t\t\tv->s = xstrfmt(\"%\"PRIuMAX, (uintmax_t)buf_size);\n\nI'll squash it in before rebuilding 'seen'.\n\n"},{"id":"425485","messageId":"xmqqa6ojgy9h.fsf@gitster.g","threadId":"55762","inReplyTo":"xmqqfsybh0bn.fsf@gitster.g","subject":"Re: [PATCH 2/3] [GSOC] ref-filter: support %(contents) for blob, tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-25T05:47:54Z","receivedAt":"2021-05-25T05:48:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> As I already said, I do not think %(contents) mixes well with this\n> use for trees and blobs where you give the raw bytes, since\n> %(contents) for commit and tag objects was never about the full byte\n> sequence of the object.  It was to give the unstructured part meant\n> for human consumption, stripping the structured \"header\" part of the\n> object.\n\nTo extend on this point a bit (even though this is not all that\nurgent during the prerelease freeze), conceptually, the %(content)\nfield is understood in the larger picture like this:\n\n+--- (the whole thing) ----------------------------------------------+\n|                                                                    |\n| +--- \"header\" ---------------------------------------------------+ |\n| | tree 678c03dca0a26afd746e8c8bb9e4aadc8bf479b1                  | |\n| | parent 378c7c6ad48c4ccddf9b534616a0e86f28440bd3                | |\n| | author Junio C Hamano <gitster@pobox.com> 1621675665 +0900     | |\n| | committer Junio C Hamano <gitster@pobox.com> 1621675741 +0900  | |\n| +----------------------------------------------------------------+ |\n|                                                                    |\n| +--- \"contents\" -------------------------------------------------+ |\n| |                                                                | |\n| | +--- \"subject\" ----------------------------------------------+ | |\n| | | Git 2.32-rc1                                               | | |\n| | +------------------------------------------------------------+ | |\n| |                                                                | |\n| | +--- \"body\" -------------------------------------------------+ | |\n| | | Signed-off-by: Junio C Hamano <gitster@pobox.com>          | | |\n| | +------------------------------------------------------------+ | |\n| |                                                                | |\n| +----------------------------------------------------------------+ |\n|                                                                    |\n+--------------------------------------------------------------------+\n\nEven though %(header), when it is invented, would make perfect sense\nfor commits and tags, it will never make sense for trees and blobs.\nWhich means \"contents\", which is \"the whole thing except for the\nheader part\", would not, either.\n\nThere is no %(placeholder) to ask for \"the whole thing\", and that is\nwhat you want to use for cat-file --batch if I am not mistaken, and\nadding one would be a good idea.  There is no %(header) yet, either,\nbut if somebody needs it for their scripts, it is clear where it fits\nin the picture.\n\n"},{"id":"425504","messageId":"CAOLTT8TaJm=9mQNXMGxt-bME-ynOpv4CKZvZw+yo9zNueH_VPw@mail.gmail.com","threadId":"55762","inReplyTo":"xmqqa6ojgy9h.fsf@gitster.g","subject":"Re: [PATCH 2/3] [GSOC] ref-filter: support %(contents) for blob, tree","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-05-25T09:28:03Z","receivedAt":"2021-05-25T09:28:17Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":">\n> To extend on this point a bit (even though this is not all that\n> urgent during the prerelease freeze), conceptually, the %(content)\n> field is understood in the larger picture like this:\n>\n> +--- (the whole thing) ----------------------------------------------+\n> |                                                                    |\n> | +--- \"header\" ---------------------------------------------------+ |\n> | | tree 678c03dca0a26afd746e8c8bb9e4aadc8bf479b1                  | |\n> | | parent 378c7c6ad48c4ccddf9b534616a0e86f28440bd3                | |\n> | | author Junio C Hamano <gitster@pobox.com> 1621675665 +0900     | |\n> | | committer Junio C Hamano <gitster@pobox.com> 1621675741 +0900  | |\n> | +----------------------------------------------------------------+ |\n> |                                                                    |\n> | +--- \"contents\" -------------------------------------------------+ |\n> | |                                                                | |\n> | | +--- \"subject\" ----------------------------------------------+ | |\n> | | | Git 2.32-rc1                                               | | |\n> | | +------------------------------------------------------------+ | |\n> | |                                                                | |\n> | | +--- \"body\" -------------------------------------------------+ | |\n> | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>          | | |\n> | | +------------------------------------------------------------+ | |\n> | |                                                                | |\n> | +----------------------------------------------------------------+ |\n> |                                                                    |\n> +--------------------------------------------------------------------+\n>\n\nThank you for providing such a complete view. This also means\nthat the \"raw\" of contents and tag will contain two parts: \"header\"\nand \"contents\". But for blobs and trees, they don’t have these things.\n\n> Even though %(header), when it is invented, would make perfect sense\n> for commits and tags, it will never make sense for trees and blobs.\n> Which means \"contents\", which is \"the whole thing except for the\n> header part\", would not, either.\n>\n\nAlthough we don’t have a %(header), but in fact we already have fragments\nof \"%(numparent)\", \"%(parent)\" %(tree)\" (see grab_commit_values()) and\n\"%(tag)\",\" %(type)\",\"%(object)\" (see grab_tag_values()), but they are not\nobtained through the \"header\" part of the raw object buffer.\n\n> There is no %(placeholder) to ask for \"the whole thing\", and that is\n> what you want to use for cat-file --batch if I am not mistaken, and\n> adding one would be a good idea.  There is no %(header) yet, either,\n> but if somebody needs it for their scripts, it is clear where it fits\n> in the picture.\n>\n\nSo I don't know if adding %(header) will cause duplication of functions.\n\nThanks!\n--\nZheNing Hu\n"},{"id":"425530","messageId":"xmqqczteeo11.fsf@gitster.g","threadId":"55762","inReplyTo":"CAOLTT8TaJm=9mQNXMGxt-bME-ynOpv4CKZvZw+yo9zNueH_VPw@mail.gmail.com","subject":"Re: [PATCH 2/3] [GSOC] ref-filter: support %(contents) for blob, tree","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-25T17:11:54Z","receivedAt":"2021-05-25T17:11:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"ZheNing Hu <adlternative@gmail.com> writes:\n\n> So I don't know if adding %(header) will cause duplication of functions.\n\nI do not think you can duplicate the feature of %(header) with\nother existing placeholders.  You may want to dump the headers of\nseries of commits so that you can find if there is any commit with\nunusal header elements, but %(tree), %(parents), etc. would by\ndefinition be the known ones, so any combination of them will not be\na substitute.\n\nBut nobody is asking for it right now, and your cat-file --batch\ndoes not need it right away.\n\nWhat I wanted to say in the message you are responding to was *not*\nthat you would want to add %(header) for completeness right now.\nBut thinking beyond your immediate need (i.e. the \"whole thing\") and\nimagining how various pieces, including the ones that do not exist\nyet, would fit together, would help you to avoid designing the parts\nyou immediately need in a way you would regret in the future.\n"},{"id":"425543","messageId":"xmqq8s42cnyb.fsf@gitster.g","threadId":"55762","inReplyTo":"xmqq1r9xndjf.fsf@gitster.g","subject":"Re: [PATCH 0/3] [GSOC][RFC] ref-filter: add contents:raw atom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-26T00:56:28Z","receivedAt":"2021-05-26T00:56:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>> git for-each-ref --format=\"%(contents)\" --python refs/mytrees/first\n>>\n>> will output a string processed by python_quote_buf_with_size(), which\n>> contains'\\0'. But the binary files seem to be useless after quoting. Should\n>> we allow these binary files to be output in the default way with\n>> strbuf_add()? If so, we can remove the first patch.\n>\n> The --language option is designed to be used to write a small script\n> in the language and used like this:\n>\n>     git for-each-ref --format='\n> \t\tname=%(refname)\n> \t\tvar=%(placeholder)\n>                 mkdir -p \"$(dirname \"$name\")\"\n> \t\tprintf \"%%s\" \"$var\" >\"$name\"\n>     ' --shell | /bin/sh\n>\n> Note that %(refname) and %(placeholder) in the --format string is\n> not quoted at all; the \"--shell\" option knows how values are quoted\n> in the host language (shell) and writes single-quotes around\n> %(refname).  If %(placeholder) produces something with a single-quote\n> in it, that will (eh, at least \"should\") be quoted appropriately.\n>\n> So It does not make any sense not to quote a value that comes from\n> %(placeholder), whether it is binary or not, to match the syntax of\n> the host language you are making the \"for-each-ref --format=\" to\n> write such a script in.\n>\n> So, \"binary files seem to be useless after quoting\" is a\n> misunderstanding.  They are useless if you do not quote them.\n\nAnother thing to keep in mind is that not all host languages may be\ncapable of expressing a string with NUL in it.  Most notably, shell.\nThe --shell quoting rule used by for-each-ref would produce an\nequivalent of the \"script\" produced like this:\n\n\t$ tr Q '\\000' >script <<\\EOF\n\t#!/bin/sh\n\tvarname='varQname'\n\techo \"$varname\"\n\tEOF\n\nbut I do not think it would say 'var' followed by a NUL followed by\n'name'.  The NUL is likely lost when assigned to the variable.\n\nSo for some host languages, binaries may be useless with or without\nquoting.  But for ones that can use strings to hold arbitrary byte\nsequence, it should be OK to let for-each-ref to quote the byte\nsequence as a string literal for the language (so that the exact\nbyte sequence will end up being in the variable after assignment).\n\nThat reminds me of another thing.  The --python thing was written\nback when Python3 was still a distant dream and strings were the\nappropriate type for a random sequence of bytes (as opposed to\nunicode, which cannot have a random sequence of bytes).  Somebody\nneeds to check if it needs any update to work with Python3.\n"},{"id":"425549","messageId":"CAOLTT8ReZffY5gznSDD=Fgbt7YTtA5aJWX+f8Q8npcj0OwcuFQ@mail.gmail.com","threadId":"55762","inReplyTo":"xmqq8s42cnyb.fsf@gitster.g","subject":"Re: [PATCH 0/3] [GSOC][RFC] ref-filter: add contents:raw atom","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-05-26T06:45:58Z","receivedAt":"2021-05-26T06:46:18Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":">\n> Another thing to keep in mind is that not all host languages may be\n> capable of expressing a string with NUL in it.  Most notably, shell.\n> The --shell quoting rule used by for-each-ref would produce an\n> equivalent of the \"script\" produced like this:\n>\n>         $ tr Q '\\000' >script <<\\EOF\n>         #!/bin/sh\n>         varname='varQname'\n>         echo \"$varname\"\n>         EOF\n>\n> but I do not think it would say 'var' followed by a NUL followed by\n> 'name'.  The NUL is likely lost when assigned to the variable.\n>\n\nYes, in the following example you mentioned earlier, I have also\nnoticed the loss of '\\0'.\n\n> >     git for-each-ref --format='\n> >               name=%(refname)\n> >               var=%(placeholder)\n> >                 mkdir -p \"$(dirname \"$name\")\"\n> >               printf \"%%s\" \"$var\" >\"$name\"\n> >     ' --shell | /bin/sh\n> >\n\n> So for some host languages, binaries may be useless with or without\n> quoting.  But for ones that can use strings to hold arbitrary byte\n> sequence, it should be OK to let for-each-ref to quote the byte\n> sequence as a string literal for the language (so that the exact\n> byte sequence will end up being in the variable after assignment).\n>\n\nI agree, and maybe some'\\0' can be escaped appropriately to let host\nlanguages recognize it....\n\n> That reminds me of another thing.  The --python thing was written\n> back when Python3 was still a distant dream and strings were the\n> appropriate type for a random sequence of bytes (as opposed to\n> unicode, which cannot have a random sequence of bytes).  Somebody\n> needs to check if it needs any update to work with Python3.\n\n$ printf '%b' \"name='a\\\\\\0b\\\\\\0c'\\nprint(name)\" | python2.7 | od -c\n0000000   a  \\0   b  \\0   c  \\n\n0000006\n\n$ printf '%b' \"name='a\\\\\\0b\\\\\\0c'\\necho -e \\\"\\$name\\\"\" | sh | od -c\n0000000   a  \\0   b  \\0   c  \\n\n0000006\n\nIn shell or python2/3, we can replace'\\0' with \"\\\\0\".\n\nIn Tcl and perl, they are ok with '\\0'.\n\n$ printf '%b' \"set name \\\"a\\0b\\0c\\\"\\nputs \\$name\" | tclsh | od -c\n0000000   '   a  \\0   b  \\0   c   '  \\n\n0000010\n\n$ printf '%b' \"\\$name = 'a\\0b\\0c';\\n print \\\"\\$name\\\"\"  | perl | od -c\n0000000   a  \\0   b  \\0   c\n0000005\n\nSo I currently think that a possible strategy is to modify\n`python_quote_buf_with_size()` and `sq_quote_buf_with_size()` from\n'\\0' to \"\\\\0\".\n\nThanks!\n\n--\nZheNing Hu\n"},{"id":"425550","messageId":"xmqqpmxeas8p.fsf@gitster.g","threadId":"55762","inReplyTo":"CAOLTT8ReZffY5gznSDD=Fgbt7YTtA5aJWX+f8Q8npcj0OwcuFQ@mail.gmail.com","subject":"Re: [PATCH 0/3] [GSOC][RFC] ref-filter: add contents:raw atom","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-26T07:06:46Z","receivedAt":"2021-05-26T07:07:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"ZheNing Hu <adlternative@gmail.com> writes:\n\n> $ printf '%b' \"name='a\\\\\\0b\\\\\\0c'\\necho -e \\\"\\$name\\\"\" | sh | od -c\n> 0000000   a  \\0   b  \\0   c  \\n\n> 0000006\n\nThis is wrong.  In the above, the variable name does not have a NUL\nin it.  It insead has 2-byte sequence \"\\\" and \"0\" in it, and you are\nletting \"echo -e\" to convert it into binary, which is not portable\nat all.\n\nI'd suggest instead to declare that some host languages, like shell,\nare not binary-clean and either refuse to process atoms whose values\nhave NUL in them.  Silently truncating strings at NUL or striping\nNULs in strings are also acceptable options if clearly documented.\nClaiming that we stuff binaries into variables of the host language,\nwhile not doing so and instead assigning a quoted form, is not good.\n\nI have not thought about Python3 very much.  For the purpose of most\n%(placeholders), it is vastly more preferrable to use str (i.e. text\nsequence type) as opposed to bytes, as you do not have to .decode()\nto use the resulting \"string\", but even for things like %(refname),\nit is not technically kosher to assume that the contents are UTF-8\nencoded text, as filenames used to represent refnames are merely a\nsequence of bytes except NUL, but for consistency with binary gunk,\nwe might have to emit everything as bytes.  I dunno.\n\n> In shell or python2/3, we can replace'\\0' with \"\\\\0\".\n\nNot for shell.  We should declare that it is not supported to feed\nbinary to shell.\n"},{"id":"425551","messageId":"CAOLTT8RdEive+fCtO-c0fZVjpmaYi0WYMZofCE-8wd94SCrdMg@mail.gmail.com","threadId":"55762","inReplyTo":"xmqqczteeo11.fsf@gitster.g","subject":"Re: [PATCH 2/3] [GSOC] ref-filter: support %(contents) for blob, tree","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-05-26T07:48:59Z","receivedAt":"2021-05-26T07:49:18Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Junio C Hamano <gitster@pobox.com> 于2021年5月26日周三 上午1:11写道：\n>\n> ZheNing Hu <adlternative@gmail.com> writes:\n>\n> > So I don't know if adding %(header) will cause duplication of functions.\n>\n> I do not think you can duplicate the feature of %(header) with\n> other existing placeholders.  You may want to dump the headers of\n> series of commits so that you can find if there is any commit with\n> unusal header elements, but %(tree), %(parents), etc. would by\n> definition be the known ones, so any combination of them will not be\n> a substitute.\n>\n\nWell, \"find if there is any commit with unusal header elements\"\nis indeed one thing that may need to be done (not now).\n\nI tried to build \"%(header)\" yesterday and found some problems:\n\n1. These atoms are too \"flat\":\n\"tree\", \"parent\", \"numparent\", \"object\"... they are all part of the\nheader, why not use \"%(header:tree)\" like \"%(contents:subject)\"\ndoes? Similarly, why not use \"%(author:name)\" instead of \"%(authorname)\"...\n\n2. Why is there no state machine to save the parsing state of an object?\n`parse_tag_buffer()`, `parse_commit_buffer()`, they only parse part of the\ncontent of the object. E.g. tag parsed \"object\", \"type\", \"tag\" and \"taggerdate\"\nin `parse_tag_buffer()`, but it did not save the starting position and length of\nthe parsed fields, which caused `grab_person()`, `grab_date()` to parse the\nobject again and again. I think this may be an optimization point.\n\n> But nobody is asking for it right now, and your cat-file --batch\n> does not need it right away.\n>\n\nOr we only provide the simplest \"%(header)\" and \"%(header:size)\"\n(it can be easily supported with \"%(raw)\"), leaving the other feature\nto the future.\n\n> What I wanted to say in the message you are responding to was *not*\n> that you would want to add %(header) for completeness right now.\n> But thinking beyond your immediate need (i.e. the \"whole thing\") and\n> imagining how various pieces, including the ones that do not exist\n> yet, would fit together, would help you to avoid designing the parts\n> you immediately need in a way you would regret in the future.\n\nAs I said before, many atomic designs are not very regular.\nOur ultimate goal may be we can simultaneously support:\n\n\"%(raw)\"\n\"%(raw:header)\"\n\"%(raw:header:tagger)\"\n\"%(raw:header:tagger:eamil)\"\n\"%(raw:header:taggereamil)\"\n\"%(header:taggereamil)\"\n\"%(header:tagger:eamil)\"\n\"%(taggereamil)\"\n\"%(tagger:eamil)\"\n...\n\nEach layer is a namespace. Of course, it may take a long\ndistance to support this.\n\nThank.\n--\nZheNing Hu\n"},{"id":"425554","messageId":"CAOLTT8QR_GRm4TYk0E_eazQ+unVQODc-3L+b4V5JUN5jtZR8uA@mail.gmail.com","threadId":"55762","inReplyTo":"xmqqpmxeas8p.fsf@gitster.g","subject":"Re: [PATCH 0/3] [GSOC][RFC] ref-filter: add contents:raw atom","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-05-26T09:17:55Z","receivedAt":"2021-05-26T09:18:12Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Junio C Hamano <gitster@pobox.com> 于2021年5月26日周三 下午3:06写道：\n>\n> ZheNing Hu <adlternative@gmail.com> writes:\n>\n> > $ printf '%b' \"name='a\\\\\\0b\\\\\\0c'\\necho -e \\\"\\$name\\\"\" | sh | od -c\n> > 0000000   a  \\0   b  \\0   c  \\n\n> > 0000006\n>\n> This is wrong.  In the above, the variable name does not have a NUL\n> in it.  It insead has 2-byte sequence \"\\\" and \"0\" in it, and you are\n> letting \"echo -e\" to convert it into binary, which is not portable\n> at all.\n>\n\nIndeed I was wrong, the var name does not contain '\\0'.\n\n> I'd suggest instead to declare that some host languages, like shell,\n> are not binary-clean and either refuse to process atoms whose values\n> have NUL in them.  Silently truncating strings at NUL or striping\n> NULs in strings are also acceptable options if clearly documented.\n> Claiming that we stuff binaries into variables of the host language,\n> while not doing so and instead assigning a quoted form, is not good.\n>\n\nMakes sense. Either choose to truncate, or choose to reject.\n\n> I have not thought about Python3 very much.  For the purpose of most\n> %(placeholders), it is vastly more preferrable to use str (i.e. text\n> sequence type) as opposed to bytes, as you do not have to .decode()\n> to use the resulting \"string\", but even for things like %(refname),\n> it is not technically kosher to assume that the contents are UTF-8\n> encoded text, as filenames used to represent refnames are merely a\n> sequence of bytes except NUL, but for consistency with binary gunk,\n> we might have to emit everything as bytes.  I dunno.\n>\n> > In shell or python2/3, we can replace'\\0' with \"\\\\0\".\n>\n> Not for shell.  We should declare that it is not supported to feed\n> binary to shell.\n\nEh, it seems that we adopt a \"reject\" strategy.\n\n$ git hash-object a.out -w | xargs git update-ref refs/myblobs/aoutblob\n$ git for-each-ref --format=\"name=%(raw)\" refs/myblobs/aoutblob\n--python | python2\n  File \"<stdin>\", line 1\nSyntaxError: Non-ASCII character '\\x8b' in file <stdin> on line 2, but\nno encoding declared;\n see http://python.org/dev/peps/pep-0263/ for details\n\n$ git for-each-ref --format=\"name=%(raw)\" refs/myblobs/aoutblob\n--python |python3\nSyntaxError: Non-UTF-8 code starting with '\\x8b' in file <stdin> on\nline 2, but no encoding declared;\n see http://python.org/dev/peps/pep-0263/ for details\n\nIt seems that --python also needs to \"reject\", no matter python2 or python3.\nWhat about tcl and perl?\n\n$ cat a.out | od >1.od\n$ git for-each-ref --format=\"set name %(raw)\nputs -nonewline \\$name\" refs/myblobs/aoutblob --tcl | tclsh | od > 2.od\n$ diff 1.od 2.od | head\n7,12c7,12\n< 0000140 114303 000002 000000 000000 141400 001230 000000 000000\n< 0000160 000000 000010 000000 000000 000000 000003 000000 000004\n< 0000200 000000 001430 000000 000000 000000 001430 000000 000000\n< 0000220 000000 001430 000000 000000 000000 000034 000000 000000\n< 0000240 000000 000034 000000 000000 000000 000001 000000 000000\n< 0000260 000000 000001 000000 000004 000000 000000 000000 000000\n---\n> 0000140 001330 000000 000000 000000 001330 000000 000000 000000\n> 0000160 000010 000000 000000 000000 000003 000000 000004 000000\n> 0000200 001430 000000 000000 000000 001430 000000 000000 000000\n> 0000220 001430 000000 000000 000000 000034 000000 000000 000000\n> 0000240 000034 000000 000000 000000 000001 000000 000000 000000\n> 0000260 000001 000000 000004 000000 000000 000000 000000 000000\n\nIt seems that a.out contents passed into tcl and then the output is\nvery different...\n\nBut,\n\n$ cat a.out | od >1.od\n$ git for-each-ref --format=\"\\$name= %(raw);\nprint \\\"\\$name\\\"\" refs/myblobs/aoutblob --perl | perl | od >6.od\n$ diff 1.od 2.od\n\nThere was no error this time, so for perl, it's ok...\nThe \"binary security\" we care about is currently only complied with\nby the Perl language.\n\nSo I think we better reject them all languages together for normative.\nThe clear definition of this rejection strategy is that %(raw) and --language\ncannot be used at the same time. If our binary data is passed to a variable\nin the host language, there may be escape errors for the host language.\n\nThanks.\n--\nZheNing Hu\n"}]}