cat-file: reuse ref-filter logic

From: ZheNing Hu <[email protected]>

Use non-const ref_format in *_atom_parser(), which can help us modify the members of ref_format in *_atom_parser().

---
 builtin/tag.c |  2 +-
 ref-filter.c  | 44 ++++++++++++++++++++++----------------------
 ref-filter.h  |  4 ++--
 3 files changed, 25 insertions(+), 25 deletions(-)

index 82fcfc098242..452558ec9575 100644
@@ -146,7 +146,7 @@ static int verify_tag(const char *name, const char *ref,
 		      const struct object_id *oid, void *cb_data)
 {
 	int flags;
-	const struct ref_format *format = cb_data;
+	struct ref_format *format = cb_data;
 	flags = GPG_VERIFY_VERBOSE;
 
 	if (format->format)

index baf72a718965..74fb423fc89f 100644
@@ -116,7 +116,7 @@ void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);
 void ref_sorting_set_sort_flags_all(struct ref_sorting *sorting, unsigned int mask, int on);
 /*  Based on the given format and quote_style, fill the strbuf */
 int format_ref_array_item(struct ref_array_item *info,
-			  const struct ref_format *format,
+			  struct ref_format *format,
 			  struct strbuf *final_buf,
 			  struct strbuf *error_buf);
 /*  Parse a single sort specifier and add it to the list */

@@ -137,7 +137,7 @@ void setup_ref_filter_porcelain_msg(void);
  * name must be a fully qualified refname.
  */
 void pretty_print_ref(const char *name, const struct object_id *oid,
-		      const struct ref_format *format);
+		      struct ref_format *format);
 
 /*
  * Push a single ref onto the array; this can be used to construct your own

gitgitgadget

From: ZheNing Hu <[email protected]>

In order to let cat-file use ref-filter logic, the following methods are used:

1. Add cat_file_mode member in struct ref_format, this can help us reject atoms in verify_ref_format() which cat-file cannot use, e.g. %(refname), %(push), %(upstream)...
2. Change the type of member format in struct batch_options to ref_format, We can add format data in it.
3. Let batch_objects() add atoms to format, and use verify_ref_format() to check atoms.
4. Use has_object_file() in batch_one_object() to check whether the input object exists.
5. Use format_ref_array_item() in batch_object_write() to get the formatted data corresponding to the object. If the return value of format_ref_array_item() is equals to zero, use batch_write() to print object data; else if the return value less than zero, use die() to print the error message and exit; else return value greater than zero, only print the error message, but not exit.
6. Let get_object() return 1 and print "<oid> missing" instead of returning -1 and printing "missing object <oid> for <refname>", this can help format_ref_array_item() just report that the object is missing without letting Git exit.

Most of the atoms in for-each-ref --format are now supported, such as %(tree), %(parent), %(author), %(tagger), %(if), %(then), %(else), %(end). But these atoms will be rejected:
%(refname), %(symref), %(upstream), %(push), %(worktreepath), %(flag), %(HEAD), because our objects don't have refname.

---
 Documentation/git-cat-file.txt |   6 +
 builtin/cat-file.c             | 250 +++++++-------------------------
 ref-filter.c                   |  15 +-
 ref-filter.h                   |   3 +-
 t/t1006-cat-file.sh            | 252 +++++++++++++++++++++++++++++++++
 t/t6301-for-each-ref-errors.sh |   2 +-
 6 files changed, 324 insertions(+), 204 deletions(-)

index 4eb0421b3fd9..ef8ab952b2fa 100644
@@ -226,6 +226,12 @@ newline. The available atoms are:
 	after that first run of whitespace (i.e., the "rest" of the
 	line) are output in place of the `%(rest)` atom.
 
+Note that most of the atoms in `for-each-ref --format` are now supported,
+such as `%(tree)`, `%(parent)`, `%(author)`, `%(tagger)`, `%(if)`,
+`%(then)`, `%(else)`, `%(end)`. But these atoms will be rejected:
+`%(refname)`, `%(symref)`, `%(upstream)`, `%(push)`, `%(worktreepath)`,
+`%(flag)`, `%(HEAD)`. See linkgit:git-for-each-ref[1].
+
 If no format is specified, the default format is `%(objectname)
 %(objecttype) %(objectsize)`.
 

index 420c0bf9384f..d4c88d496698 100644
@@ -1017,8 +1017,15 @@ int verify_ref_format(struct ref_format *format)
 		at = parse_ref_filter_atom(format, sp + 2, ep, &err);
 		if (at < 0)
 			die("%s", err.buf);
-		if (used_atom[at].atom_type == ATOM_REST)
-			die("this command reject atom %%(%.*s)", (int)(ep - sp - 2), sp + 2);
+		if ((!format->cat_file_mode && used_atom[at].atom_type == ATOM_REST) ||
+		    (format->cat_file_mode && (used_atom[at].atom_type == ATOM_FLAG ||
+					       used_atom[at].atom_type == ATOM_HEAD ||
+					       used_atom[at].atom_type == ATOM_PUSH ||
+					       used_atom[at].atom_type == ATOM_REFNAME ||
+					       used_atom[at].atom_type == ATOM_SYMREF ||
+					       used_atom[at].atom_type == ATOM_UPSTREAM ||
+					       used_atom[at].atom_type == ATOM_WORKTREEPATH)))
+			die(_("this command reject atom %%(%.*s)"), (int)(ep - sp - 2), sp + 2);
 
 		if (format->quote_style && used_atom[at].atom_type == ATOM_RAW &&
 		    used_atom[at].u.raw_data.option == RAW_BARE)

@@ -1735,8 +1742,8 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj
 	}
 	if (oid_object_info_extended(the_repository, &oi->oid, &oi->info,
 				     OBJECT_INFO_LOOKUP_REPLACE))
-		return strbuf_addf_ret(err, -1, _("missing object %s for %s"),
-				       oid_to_hex(&oi->oid), ref->refname);
+		return strbuf_addf_ret(err, 1, _("%s missing"),
+				       oid_to_hex(&oi->oid));
 	if (oi->info.disk_sizep && oi->disk_size < 0)
 		BUG("Object size is less than zero.");
 

index 9dc07476a584..bece9583cf18 100644
@@ -78,6 +78,7 @@ struct ref_format {
 	 */
 	const char *format;
 	const char *rest;
+	int cat_file_mode;
 	int quote_style;
 	int use_rest;
 	int use_color;

@@ -86,7 +87,7 @@ struct ref_format {
 	int need_color_reset_at_eol;
 };
 
-#define REF_FORMAT_INIT { NULL, NULL, 0, 0, -1 }
+#define REF_FORMAT_INIT { NULL, NULL, 0, 0, 0, -1 }
 
 /*  Macros for checking --merged and --no-merged options */
 #define _OPT_MERGED_NO_MERGED(option, filter, h) \

index 40edf9dab534..3553f84a00c1 100755
@@ -41,7 +41,7 @@ test_expect_success 'Missing objects are reported correctly' '
 	r=refs/heads/missing &&
 	echo $MISSING >.git/$r &&
 	test_when_finished "rm -f .git/$r" &&
-	echo "fatal: missing object $MISSING for $r" >missing-err &&
+	echo "fatal: $MISSING missing" >missing-err &&
 	test_must_fail git for-each-ref 2>err &&
 	test_cmp missing-err err &&
 	(

gitgitgadget

From: ZheNing Hu <[email protected]>

After cat-file reuses the ref-filter logic, we re-implement the functions of --textconv and --filters options.

Add members use_textconv and use_filters in struct ref_format, and use global variables use_filters and use_textconv in ref-filter.c, so that we can filter the content of the object in get_object(). Use actual_oi to record the real expand_data:
it may point to the original oi or the act_oi processed by textconv_object() or convert_to_working_tree(). grab_values() will grab the contents of actual_oi and grab_common_values() to grab the contents of origin oi, this ensures that %(objectsize) still uses the size of the unfiltered data.

In get_object(), we made an optimization: Firstly, get the size and type of the object instead of directly getting the object data. If using --textconv, after successfully obtaining the filtered object data, an extra oid_object_info_extended() will be skipped, which can reduce the cost of object data copy; If using --filter, the data of the object first will be getted first, and then convert_to_working_tree() will be used to get the filtered object data.

---
 builtin/cat-file.c |  5 ++++
 ref-filter.c       | 66 ++++++++++++++++++++++++++++++++++++++++++++--
 ref-filter.h       |  5 ++--
 3 files changed, 72 insertions(+), 4 deletions(-)

index 1a73c3d23dde..3fde2587201b 100644
@@ -376,6 +376,11 @@ static int batch_objects(struct batch_options *opt, const struct option *options
 	if (opt->print_contents)
 		strbuf_addstr(&format, "\n%(raw)");
 	opt->format.format = format.buf;
+	if (opt->cmdmode == 'c')
+		opt->format.use_textconv = 1;
+	if (opt->cmdmode == 'w')
+		opt->format.use_filters = 1;
+
 	if (verify_ref_format(&opt->format))
 		usage_with_options(cat_file_usage, options);
 

index bece9583cf18..cf7bad4e8b49 100644
@@ -80,14 +80,15 @@ struct ref_format {
 	const char *rest;
 	int cat_file_mode;
 	int quote_style;
+	int use_textconv;
+	int use_filters;
 	int use_rest;
 	int use_color;
-
 	/* Internal state to ref-filter */
 	int need_color_reset_at_eol;
 };
 
-#define REF_FORMAT_INIT { NULL, NULL, 0, 0, 0, -1 }
+#define REF_FORMAT_INIT { NULL, NULL, 0, 0, 0, 0, 0, -1 }
 
 /*  Macros for checking --merged and --no-merged options */
 #define _OPT_MERGED_NO_MERGED(option, filter, h) \

gitgitgadget

From: ZheNing Hu <[email protected]>

Add new formatting option %(raw), which will print the raw object data without any changes. It will help further to migrate all cat-file formatting logic from cat-file to ref-filter.

The raw data of blob, tree objects may contain '\0', but most of the logic in ref-filter depends on the output of the atom being text (specifically, no embedded NULs in it).

E.g. quote_formatting() use strbuf_addstr() or *._quote_buf() add the data to the buffer. The raw data of a tree object is 100644 one\0..., only the 100644 one will be added to the buffer, which is incorrect.

Therefore, add a new member in struct atom_value: s_size, which can record raw object size, it can help us add raw object data to the buffer or compare two buffers which contain raw object data.

Beyond, --format=%(raw) cannot be used with --python, --shell, --tcl, --perl because if our binary raw data is passed to a variable in the host language, the host language may not support arbitrary binary data in the variables of its string type.

---
 Documentation/git-for-each-ref.txt |   9 ++
 ref-filter.c                       | 139 +++++++++++++++----
 t/t6300-for-each-ref.sh            | 207 +++++++++++++++++++++++++++++
 3 files changed, 328 insertions(+), 27 deletions(-)

index 2ae2478de706..7f1f0a1ca3b6 100644
@@ -235,6 +235,15 @@ and `date` to extract the named component.  For email fields (`authoremail`,
 without angle brackets, and `:localpart` to get the part before the `@` symbol
 out of the trimmed email.
 
+The raw data in an object is `raw`.
+
+raw:size::
+	The raw data size of the object.
+
+Note that `--format=%(raw)` can not be used with `--python`, `--shell`, `--tcl`,
+`--perl` because the host language may not support arbitrary binary data in the
+variables of its string type.
+
 The message in a commit or a tag object is `contents`, from which
 `contents:<part>` can be used to extract various parts out of:
 

gitgitgadget

From: ZheNing Hu <[email protected]>

In order to let "cat-file --batch=%(rest)" use the ref-filter interface, add %(rest) atom for ref-filter. "git for-each-ref", "git branch", "git tag" and "git verify-tag" will reject %(rest) by default.

---
 ref-filter.c             | 21 +++++++++++++++++++++
 ref-filter.h             |  5 ++++-
 t/t3203-branch-output.sh |  4 ++++
 t/t6300-for-each-ref.sh  |  4 ++++
 t/t7004-tag.sh           |  4 ++++
 t/t7030-verify-tag.sh    |  4 ++++
 6 files changed, 41 insertions(+), 1 deletion(-)

index af8c15aef44d..8868cf98f090 100644
@@ -157,6 +157,7 @@ enum atom_type {
 	ATOM_IF,
 	ATOM_THEN,
 	ATOM_ELSE,
+	ATOM_REST,
 };
 
 /*

@@ -559,6 +560,15 @@ static int if_atom_parser(struct ref_format *format, struct used_atom *atom,
 	return 0;
 }
 
+static int rest_atom_parser(struct ref_format *format, struct used_atom *atom,
+			    const char *arg, struct strbuf *err)
+{
+	if (arg)
+		return strbuf_addf_ret(err, -1, _("%%(rest) does not take arguments"));
+	format->use_rest = 1;
+	return 0;
+}
+
 static int head_atom_parser(struct ref_format *format, struct used_atom *atom,
 			    const char *arg, struct strbuf *unused_err)
 {

@@ -615,6 +625,7 @@ static struct {
 	[ATOM_IF] = { "if", SOURCE_NONE, FIELD_STR, if_atom_parser },
 	[ATOM_THEN] = { "then", SOURCE_NONE },
 	[ATOM_ELSE] = { "else", SOURCE_NONE },
+	[ATOM_REST] = { "rest", SOURCE_NONE, FIELD_STR, rest_atom_parser },
 	/*
 	 * Please update $__git_ref_fieldlist in git-completion.bash
 	 * when you add new atoms

@@ -1006,6 +1017,9 @@ int verify_ref_format(struct ref_format *format)
 		at = parse_ref_filter_atom(format, sp + 2, ep, &err);
 		if (at < 0)
 			die("%s", err.buf);
+		if (used_atom[at].atom_type == ATOM_REST)
+			die("this command reject atom %%(%.*s)", (int)(ep - sp - 2), sp + 2);
+
 		if (format->quote_style && used_atom[at].atom_type == ATOM_RAW &&
 		    used_atom[at].u.raw_data.option == RAW_BARE)
 			die(_("--format=%.*s cannot be used with"

@@ -1920,6 +1934,12 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 			v->handler = else_atom_handler;
 			v->s = xstrdup("");
 			continue;
+		} else if (atom_type == ATOM_REST) {
+			if (ref->rest)
+				v->s = xstrdup(ref->rest);
+			else
+				v->s = xstrdup("");
+			continue;
 		} else
 			continue;
 

@@ -2137,6 +2157,7 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
 
 	FLEX_ALLOC_STR(ref, refname, refname);
 	oidcpy(&ref->objectname, oid);
+	ref->rest = NULL;
 
 	return ref;
 }

index 74fb423fc89f..9dc07476a584 100644
@@ -38,6 +38,7 @@ struct ref_sorting {
 
 struct ref_array_item {
 	struct object_id objectname;
+	const char *rest;
 	int flag;
 	unsigned int kind;
 	const char *symref;

@@ -76,14 +77,16 @@ struct ref_format {
 	 * verify_ref_format() afterwards to finalize.
 	 */
 	const char *format;
+	const char *rest;
 	int quote_style;
+	int use_rest;
 	int use_color;
 
 	/* Internal state to ref-filter */
 	int need_color_reset_at_eol;
 };
 
-#define REF_FORMAT_INIT { NULL, 0, -1 }
+#define REF_FORMAT_INIT { NULL, NULL, 0, 0, -1 }
 
 /*  Macros for checking --merged and --no-merged options */
 #define _OPT_MERGED_NO_MERGED(option, filter, h) \

index 5325b9f67a00..2780ec8803fd 100755
@@ -340,6 +340,10 @@ test_expect_success 'git branch --format option' '
 	test_cmp expect actual
 '
 
+test_expect_success 'git branch with --format=%(rest) must failed' '
+	test_must_fail git branch --format="%(rest)" >actual
+'
+
 test_expect_success 'worktree colors correct' '
 	cat >expect <<-EOF &&
 	* <GREEN>(HEAD detached from fromtag)<RESET>

index e2867de791e7..8c97c3b877c6 100755
@@ -1187,6 +1187,10 @@ test_expect_success 'basic atom: head contents:trailers' '
 	test_cmp expect actual.clean
 '
 
+test_expect_success 'basic atom: rest must failed' '
+	test_must_fail git for-each-ref --format="%(rest)" refs/heads/main
+'
+
 test_expect_success 'trailer parsing not fooled by --- line' '
 	git commit --allow-empty -F - <<-\EOF &&
 	this is the subject

index 2f72c5c6883e..9fc4c4323949 100755
@@ -1998,6 +1998,10 @@ test_expect_success '--format should list tags as per format given' '
 	test_cmp expect actual
 '
 
+test_expect_success 'git tag -l with --format="%(rest)" must failed' '
+	test_must_fail git tag -l --format="%(rest)" "v1*"
+'
+
 test_expect_success "set up color tests" '
 	echo "<RED>v1.0<RESET>" >expect.color &&
 	echo "v1.0" >expect.bare &&

index 3cefde9602bf..785b32eb88f9 100755
@@ -194,6 +194,10 @@ test_expect_success GPG 'verifying tag with --format' '
 	test_cmp expect actual
 '
 
+test_expect_success GPG 'verifying tag with --format="%(rest)" must failed' '
+	test_must_fail git verify-tag --format="%(rest)" "fourth-signed"
+'
+
 test_expect_success GPG 'verifying a forged tag with --format should fail silently' '
 	test_must_fail git verify-tag --format="tagname : %(tag)" $(cat forged1.tag) >actual-forged &&
 	test_must_be_empty actual-forged

gitgitgadget

This patch series make cat-file reuse ref-filter logic, which based on 5a5b5f78 ([GSOC] ref-filter: add %(rest) atom)

1. Modified the logic of cat-file --batch, use verify_ref_format() and format_ref_array_item() to get object data.
2. Re-implement --textconv, --filters.

Now cat-file can support most ref-filter atoms, like %(tree), %(parent), %(if)...

There is still an unresolved issue: performance overhead is very large, so that when we use:

git cat-file --batch --batch-all-objects >/dev/null

on git.git, it may fail.

ZheNing Hu (8):
[GSOC] ref-filter: add obj-type check in grab contents [GSOC] ref-filter: add %(raw) atom
[GSOC] ref-filter: use non-const ref_format in *_atom_parser() [GSOC] ref-filter: add %(rest) atom
[GSOC] ref-filter: teach get_object() return useful value [GSOC] cat-file: reuse ref-filter logic
[GSOC] cat-file: reuse err buf in batch_objet_write() [GSOC] cat-file: re-implement --textconv, --filters options

 Documentation/git-cat-file.txt     |   6 +
 Documentation/git-for-each-ref.txt |   9 +
 builtin/cat-file.c                 | 267 ++++++------------------
 builtin/tag.c                      |   2 +-
 ref-filter.c                       | 320 +++++++++++++++++++++++------
 ref-filter.h                       |  13 +-
 t/t1006-cat-file.sh                | 252 +++++++++++++++++++++++
 t/t3203-branch-output.sh           |   4 +
 t/t6300-for-each-ref.sh            | 211 +++++++++++++++++++
 t/t6301-for-each-ref-errors.sh     |   2 +-
 t/t7004-tag.sh                     |   4 +
 t/t7030-verify-tag.sh              |   4 +
 12 files changed, 818 insertions(+), 276 deletions(-)

base-commit: 1197f1a46360d3ae96bd9c15908a3a6f8e562207
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-980%2Fadlternative%2Fcat-file-batch-refactor-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-980/adlternative/cat-file-batch-refactor-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/980

gitgitgadget

From: ZheNing Hu <[email protected]>

Only tag and commit objects use grab_sub_body_contents() to grab object contents in the current codebase. We want to teach the function to also handle blobs and trees to get their raw data, without parsing a blob (whose contents looks like a commit or a tag) incorrectly as a commit or a tag.

Skip the block of code that is specific to handling commits and tags early when the given object is of a wrong type to help later addition to handle other types of objects in this function.

---
 ref-filter.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

index 4db0e40ff4c6..5cee6512fbaf 100644
@@ -1356,11 +1356,12 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size
 }
 
 /* See grab_values */
-static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)
+static void grab_sub_body_contents(struct atom_value *val, int deref, struct expand_data *data)
 {
 	int i;
 	const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;
 	size_t sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;
+	void *buf = data->content;
 
 	for (i = 0; i < used_atom_cnt; i++) {
 		struct used_atom *atom = &used_atom[i];

@@ -1371,10 +1372,13 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)
 			continue;
 		if (deref)
 			name++;
-		if (strcmp(name, "body") &&
-		    !starts_with(name, "subject") &&
-		    !starts_with(name, "trailers") &&
-		    !starts_with(name, "contents"))
+
+		if ((data->type != OBJ_TAG &&
+		     data->type != OBJ_COMMIT) ||
+		    (strcmp(name, "body") &&
+		     !starts_with(name, "subject") &&
+		     !starts_with(name, "trailers") &&
+		     !starts_with(name, "contents")))
 			continue;
 		if (!subpos)
 			find_subpos(buf,

@@ -1438,17 +1442,19 @@ static void fill_missing_values(struct atom_value *val)
  * pointed at by the ref itself; otherwise it is the object the
  * ref (which is a tag) refers to.
  */
-static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf)
+static void grab_values(struct atom_value *val, int deref, struct object *obj, struct expand_data *data)
 {
+	void *buf = data->content;
+
 	switch (obj->type) {
 	case OBJ_TAG:
 		grab_tag_values(val, deref, obj);
-		grab_sub_body_contents(val, deref, buf);
+		grab_sub_body_contents(val, deref, data);
 		grab_person("tagger", val, deref, buf);
 		break;
 	case OBJ_COMMIT:
 		grab_commit_values(val, deref, obj);
-		grab_sub_body_contents(val, deref, buf);
+		grab_sub_body_contents(val, deref, data);
 		grab_person("author", val, deref, buf);
 		grab_person("committer", val, deref, buf);
 		break;

@@ -1678,7 +1684,7 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj
 			return strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"),
 					       oid_to_hex(&oi->oid), ref->refname);
 		}
-		grab_values(ref->value, deref, *obj, oi->content);
+		grab_values(ref->value, deref, *obj, oi);
 	}
 
 	grab_common_values(ref->value, deref, oi);

gitgitgadget

From: ZheNing Hu <[email protected]>

Let populate_value(), get_ref_atom_value() and format_ref_array_item() get the return value of get_value() correctly. This can help us later let cat-file --batch get the correct error message and return value of get_value().

---
 ref-filter.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

index 8868cf98f090..420c0bf9384f 100644
@@ -1808,7 +1808,7 @@ static char *get_worktree_path(const struct used_atom *atom, const struct ref_ar
 static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 {
 	struct object *obj;
-	int i;
+	int i, ret = 0;
 	struct object_info empty = OBJECT_INFO_INIT;
 
 	CALLOC_ARRAY(ref->value, used_atom_cnt);

@@ -1965,8 +1965,8 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 
 
 	oi.oid = ref->objectname;
-	if (get_object(ref, 0, &obj, &oi, err))
-		return -1;
+	if ((ret = get_object(ref, 0, &obj, &oi, err)))
+		return ret;
 
 	/*
 	 * If there is no atom that wants to know about tagged

@@ -1997,9 +1997,11 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 static int get_ref_atom_value(struct ref_array_item *ref, int atom,
 			      struct atom_value **v, struct strbuf *err)
 {
+	int ret = 0;
+
 	if (!ref->value) {
-		if (populate_value(ref, err))
-			return -1;
+		if ((ret = populate_value(ref, err)))
+			return ret;
 		fill_missing_values(ref->value);
 	}
 	*v = &ref->value[atom];

@@ -2573,6 +2575,7 @@ int format_ref_array_item(struct ref_array_item *info,
 {
 	const char *cp, *sp, *ep;
 	struct ref_formatting_state state = REF_FORMATTING_STATE_INIT;
+	int ret = 0;
 
 	state.quote_style = format->quote_style;
 	push_stack_element(&state.stack);

@@ -2585,10 +2588,10 @@ int format_ref_array_item(struct ref_array_item *info,
 		if (cp < sp)
 			append_literal(cp, sp, &state);
 		pos = parse_ref_filter_atom(format, sp + 2, ep, error_buf);
-		if (pos < 0 || get_ref_atom_value(info, pos, &atomv, error_buf) ||
+		if (pos < 0 || (ret = get_ref_atom_value(info, pos, &atomv, error_buf)) ||
 		    atomv->handler(atomv, &state, error_buf)) {
 			pop_stack_element(&state.stack);
-			return -1;
+			return ret ? ret : -1;
 		}
 	}
 	if (*cp) {

@@ -2610,7 +2613,7 @@ int format_ref_array_item(struct ref_array_item *info,
 	}
 	strbuf_addbuf(final_buf, &state.stack->output);
 	pop_stack_element(&state.stack);
-	return 0;
+	return ret;
 }
 
 void pretty_print_ref(const char *name, const struct object_id *oid,

gitgitgadget

From: ZheNing Hu <[email protected]>

Reuse the err buffer in batch_object_write(), as the buffer scratch does. This will reduce the overhead of multiple allocations of memory of the err buffer.

---
 builtin/cat-file.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

index 0bc524e656e1..1a73c3d23dde 100644
@@ -212,33 +212,32 @@ static void batch_write(struct batch_options *opt, const void *data, int len)
 
 static void batch_object_write(const char *obj_name,
 			       struct strbuf *scratch,
+			       struct strbuf *err,
 			       struct batch_options *opt,
 			       struct expand_data *data)
 {
 	int ret = 0;
-	struct strbuf err = STRBUF_INIT;
 	struct ref_array_item item = { data->oid, data->rest };
 
 	strbuf_reset(scratch);
+	strbuf_reset(err);
 
-	ret = format_ref_array_item(&item, &opt->format, scratch, &err);
+	ret = format_ref_array_item(&item, &opt->format, scratch, err);
 	if (!ret) {
 		strbuf_addch(scratch, '\n');
 		batch_write(opt, scratch->buf, scratch->len);
-		strbuf_release(&err);
 	} else if (ret < 0) {
-		die("%s\n", err.buf);
-		strbuf_release(&err);
+		die("%s\n", err->buf);
 	} else {
 		/* when ret > 0 , don't call die and print the err to stdout*/
-		printf("%s\n", err.buf);
+		printf("%s\n", err->buf);
 		fflush(stdout);
-		strbuf_release(&err);
 	}
 }
 
 static void batch_one_object(const char *obj_name,
 			     struct strbuf *scratch,
+			     struct strbuf *err,
 			     struct batch_options *opt,
 			     struct expand_data *data)
 {

@@ -292,7 +291,7 @@ static void batch_one_object(const char *obj_name,
 		return;
 	}
 
-	batch_object_write(obj_name, scratch, opt, data);
+	batch_object_write(obj_name, scratch, err, opt, data);
 }
 
 struct object_cb_data {

@@ -300,13 +299,14 @@ struct object_cb_data {
 	struct expand_data *expand;
 	struct oidset *seen;
 	struct strbuf *scratch;
+	struct strbuf *err;
 };
 
 static int batch_object_cb(const struct object_id *oid, void *vdata)
 {
 	struct object_cb_data *data = vdata;
 	oidcpy(&data->expand->oid, oid);
-	batch_object_write(NULL, data->scratch, data->opt, data->expand);
+	batch_object_write(NULL, data->scratch, data->err, data->opt, data->expand);
 	return 0;
 }
 

@@ -362,6 +362,7 @@ static int batch_objects(struct batch_options *opt, const struct option *options
 {
 	struct strbuf input = STRBUF_INIT;
 	struct strbuf output = STRBUF_INIT;
+	struct strbuf err = STRBUF_INIT;
 	struct strbuf format = STRBUF_INIT;
 	struct expand_data data;
 	int save_warning;

@@ -390,6 +391,7 @@ static int batch_objects(struct batch_options *opt, const struct option *options
 		cb.opt = opt;
 		cb.expand = &data;
 		cb.scratch = &output;
+		cb.err = &err;
 
 		if (opt->unordered) {
 			struct oidset seen = OIDSET_INIT;

@@ -414,6 +416,7 @@ static int batch_objects(struct batch_options *opt, const struct option *options
 
 		strbuf_release(&format);
 		strbuf_release(&output);
+		strbuf_release(&err);
 		return 0;
 	}
 

@@ -442,11 +445,12 @@ static int batch_objects(struct batch_options *opt, const struct option *options
 			data.rest = p;
 		}
 
-		batch_one_object(input.buf, &output, opt, &data);
+		batch_one_object(input.buf, &output, &err, opt, &data);
 	}
 	strbuf_release(&format);
 	strbuf_release(&input);
 	strbuf_release(&output);
+	strbuf_release(&err);
 	warn_on_object_refname_ambiguity = save_warning;
 	return retval;
 }

gitgitgadget

Is it get_object() or get_value()?

Oh, it's get_object().

Thanks for pointing out:)
--
ZheNing Hu

This patch series make cat-file reuse ref-filter logic, which based on 5a5b5f78 ([GSOC] ref-filter: add %(rest) atom)

Change from last version:

1. Use free_array_item_internal() to solve the memory leak problem.
2. Change commit message of ([GSOC] ref-filter: teach get_object() return useful value).

ZheNing Hu (9):
[GSOC] ref-filter: add obj-type check in grab contents [GSOC] ref-filter: add %(raw) atom
[GSOC] ref-filter: use non-const ref_format in *_atom_parser() [GSOC] ref-filter: add %(rest) atom
[GSOC] ref-filter: teach get_object() return useful value [GSOC] ref-filter: introduce free_array_item_internal() function [GSOC] cat-file: reuse ref-filter logic
[GSOC] cat-file: reuse err buf in batch_objet_write() [GSOC] cat-file: re-implement --textconv, --filters options

 Documentation/git-cat-file.txt     |   6 +
 Documentation/git-for-each-ref.txt |   9 +
 builtin/cat-file.c                 | 267 ++++++-----------------
 builtin/tag.c                      |   2 +-
 ref-filter.c                       | 331 ++++++++++++++++++++++-------
 ref-filter.h                       |  14 +-
 t/t1006-cat-file.sh                | 252 ++++++++++++++++++++++
 t/t3203-branch-output.sh           |   4 +
 t/t6300-for-each-ref.sh            | 211 ++++++++++++++++++
 t/t6301-for-each-ref-errors.sh     |   2 +-
 t/t7004-tag.sh                     |   4 +
 t/t7030-verify-tag.sh              |   4 +
 12 files changed, 829 insertions(+), 277 deletions(-)

base-commit: 1197f1a46360d3ae96bd9c15908a3a6f8e562207
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-980%2Fadlternative%2Fcat-file-batch-refactor-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-980/adlternative/cat-file-batch-refactor-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/980

Range-diff vs v1:

  1:  48d256db5c34 =  1:  48d256db5c34 [GSOC] ref-filter: add obj-type check in grab contents
  2:  abee6a03becb =  2:  abee6a03becb [GSOC] ref-filter: add %(raw) atom
  3:  c99d1d070a18 =  3:  c99d1d070a18 [GSOC] ref-filter: use non-const ref_format in *_atom_parser()
  4:  5a5b5f78aeea =  4:  5a5b5f78aeea [GSOC] ref-filter: add %(rest) atom
  5:  c208b8a45d66 !  5:  49063372e003 [GSOC] ref-filter: teach get_object() return useful value

@@ Commit message
[GSOC] ref-filter: teach get_object() return useful value

Let populate_value(), get_ref_atom_value() and
- format_ref_array_item() get the return value of get_value()
+ format_ref_array_item() get the return value of get_object()
correctly. This can help us later let cat-file --batch get the
- correct error message and return value of get_value().
+ correct error message and return value of get_object().

  -:  ------------ >  6:  d2f2563eb76a [GSOC] ref-filter: introduce free_array_item_internal() function
  6:  44ebf75e2e93 !  7:  765337a46ab0 [GSOC] cat-file: reuse ref-filter logic
     @@ builtin/cat-file.c: static void batch_write(struct batch_options *opt, const voi
      +	if (!ret) {
      +		strbuf_addch(scratch, '\n');
      +		batch_write(opt, scratch->buf, scratch->len);
     -+		strbuf_release(&err);
      +	} else if (ret < 0) {
      +		die("%s\n", err.buf);
     -+		strbuf_release(&err);
      +	} else {
      +		/* when ret > 0 , don't call die and print the err to stdout*/
      +		printf("%s\n", err.buf);
      +		fflush(stdout);
     -+		strbuf_release(&err);
       	}
     ++	free_array_item_internal(&item);
     ++	strbuf_release(&err);
       }
       
     + static void batch_one_object(const char *obj_name,
      @@ builtin/cat-file.c: static void batch_one_object(const char *obj_name,
       		return;
       	}
     @@ builtin/cat-file.c: static int batch_objects(struct batch_options *opt)
       		return 0;
       	}
      @@ builtin/cat-file.c: static int batch_objects(struct batch_options *opt)
     - 
       		batch_one_object(input.buf, &output, opt, &data);
       	}
     --
     + 
      +	strbuf_release(&format);
       	strbuf_release(&input);
       	strbuf_release(&output);
  7:  d31059c391d0 !  8:  058b304686fd [GSOC] cat-file: reuse err buf in batch_objet_write()
     @@ builtin/cat-file.c: static void batch_write(struct batch_options *opt, const voi
       	if (!ret) {
       		strbuf_addch(scratch, '\n');
       		batch_write(opt, scratch->buf, scratch->len);
     --		strbuf_release(&err);
       	} else if (ret < 0) {
      -		die("%s\n", err.buf);
     --		strbuf_release(&err);
      +		die("%s\n", err->buf);
       	} else {
       		/* when ret > 0 , don't call die and print the err to stdout*/
      -		printf("%s\n", err.buf);
      +		printf("%s\n", err->buf);
       		fflush(stdout);
     --		strbuf_release(&err);
       	}
     + 	free_array_item_internal(&item);
     +-	strbuf_release(&err);
       }
       
       static void batch_one_object(const char *obj_name,
     @@ builtin/cat-file.c: static int batch_objects(struct batch_options *opt, const st
      -		batch_one_object(input.buf, &output, opt, &data);
      +		batch_one_object(input.buf, &output, &err, opt, &data);
       	}
     + 
       	strbuf_release(&format);
       	strbuf_release(&input);
       	strbuf_release(&output);
  8:  0004d5b24a0f !  9:  cbf7d51933ea [GSOC] cat-file: re-implement --textconv, --filters options
     @@ ref-filter.h: struct ref_format {
      +	int use_filters;
       	int use_rest;
       	int use_color;
     --
     - 	/* Internal state to ref-filter */
     + 
     +@@ ref-filter.h: struct ref_format {
       	int need_color_reset_at_eol;
       };

gitgitgadget

From: ZheNing Hu <[email protected]>

Only tag and commit objects use grab_sub_body_contents() to grab object contents in the current codebase. We want to teach the function to also handle blobs and trees to get their raw data, without parsing a blob (whose contents looks like a commit or a tag) incorrectly as a commit or a tag.

Skip the block of code that is specific to handling commits and tags early when the given object is of a wrong type to help later addition to handle other types of objects in this function.

---
 ref-filter.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

index 4db0e40ff4c6..5cee6512fbaf 100644
@@ -1356,11 +1356,12 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size
 }
 
 /* See grab_values */
-static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)
+static void grab_sub_body_contents(struct atom_value *val, int deref, struct expand_data *data)
 {
 	int i;
 	const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;
 	size_t sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;
+	void *buf = data->content;
 
 	for (i = 0; i < used_atom_cnt; i++) {
 		struct used_atom *atom = &used_atom[i];

@@ -1371,10 +1372,13 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)
 			continue;
 		if (deref)
 			name++;
-		if (strcmp(name, "body") &&
-		    !starts_with(name, "subject") &&
-		    !starts_with(name, "trailers") &&
-		    !starts_with(name, "contents"))
+
+		if ((data->type != OBJ_TAG &&
+		     data->type != OBJ_COMMIT) ||
+		    (strcmp(name, "body") &&
+		     !starts_with(name, "subject") &&
+		     !starts_with(name, "trailers") &&
+		     !starts_with(name, "contents")))
 			continue;
 		if (!subpos)
 			find_subpos(buf,

@@ -1438,17 +1442,19 @@ static void fill_missing_values(struct atom_value *val)
  * pointed at by the ref itself; otherwise it is the object the
  * ref (which is a tag) refers to.
  */
-static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf)
+static void grab_values(struct atom_value *val, int deref, struct object *obj, struct expand_data *data)
 {
+	void *buf = data->content;
+
 	switch (obj->type) {
 	case OBJ_TAG:
 		grab_tag_values(val, deref, obj);
-		grab_sub_body_contents(val, deref, buf);
+		grab_sub_body_contents(val, deref, data);
 		grab_person("tagger", val, deref, buf);
 		break;
 	case OBJ_COMMIT:
 		grab_commit_values(val, deref, obj);
-		grab_sub_body_contents(val, deref, buf);
+		grab_sub_body_contents(val, deref, data);
 		grab_person("author", val, deref, buf);
 		grab_person("committer", val, deref, buf);
 		break;

@@ -1678,7 +1684,7 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj
 			return strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"),
 					       oid_to_hex(&oi->oid), ref->refname);
 		}
-		grab_values(ref->value, deref, *obj, oi->content);
+		grab_values(ref->value, deref, *obj, oi);
 	}
 
 	grab_common_values(ref->value, deref, oi);

gitgitgadget

From: ZheNing Hu <[email protected]>

Add new formatting option %(raw), which will print the raw object data without any changes. It will help further to migrate all cat-file formatting logic from cat-file to ref-filter.

The raw data of blob, tree objects may contain '\0', but most of the logic in ref-filter depends on the output of the atom being text (specifically, no embedded NULs in it).

E.g. quote_formatting() use strbuf_addstr() or *._quote_buf() add the data to the buffer. The raw data of a tree object is 100644 one\0..., only the 100644 one will be added to the buffer, which is incorrect.

Therefore, add a new member in struct atom_value: s_size, which can record raw object size, it can help us add raw object data to the buffer or compare two buffers which contain raw object data.

Beyond, --format=%(raw) cannot be used with --python, --shell, --tcl, --perl because if our binary raw data is passed to a variable in the host language, the host language may not support arbitrary binary data in the variables of its string type.

---
 Documentation/git-for-each-ref.txt |   9 ++
 ref-filter.c                       | 139 +++++++++++++++----
 t/t6300-for-each-ref.sh            | 207 +++++++++++++++++++++++++++++
 3 files changed, 328 insertions(+), 27 deletions(-)

index 2ae2478de706..7f1f0a1ca3b6 100644
@@ -235,6 +235,15 @@ and `date` to extract the named component.  For email fields (`authoremail`,
 without angle brackets, and `:localpart` to get the part before the `@` symbol
 out of the trimmed email.
 
+The raw data in an object is `raw`.
+
+raw:size::
+	The raw data size of the object.
+
+Note that `--format=%(raw)` can not be used with `--python`, `--shell`, `--tcl`,
+`--perl` because the host language may not support arbitrary binary data in the
+variables of its string type.
+
 The message in a commit or a tag object is `contents`, from which
 `contents:<part>` can be used to extract various parts out of:
 

gitgitgadget

From: ZheNing Hu <[email protected]>

In order to let "cat-file --batch=%(rest)" use the ref-filter interface, add %(rest) atom for ref-filter. "git for-each-ref", "git branch", "git tag" and "git verify-tag" will reject %(rest) by default.

---
 ref-filter.c             | 21 +++++++++++++++++++++
 ref-filter.h             |  5 ++++-
 t/t3203-branch-output.sh |  4 ++++
 t/t6300-for-each-ref.sh  |  4 ++++
 t/t7004-tag.sh           |  4 ++++
 t/t7030-verify-tag.sh    |  4 ++++
 6 files changed, 41 insertions(+), 1 deletion(-)

index af8c15aef44d..8868cf98f090 100644
@@ -157,6 +157,7 @@ enum atom_type {
 	ATOM_IF,
 	ATOM_THEN,
 	ATOM_ELSE,
+	ATOM_REST,
 };
 
 /*

@@ -559,6 +560,15 @@ static int if_atom_parser(struct ref_format *format, struct used_atom *atom,
 	return 0;
 }
 
+static int rest_atom_parser(struct ref_format *format, struct used_atom *atom,
+			    const char *arg, struct strbuf *err)
+{
+	if (arg)
+		return strbuf_addf_ret(err, -1, _("%%(rest) does not take arguments"));
+	format->use_rest = 1;
+	return 0;
+}
+
 static int head_atom_parser(struct ref_format *format, struct used_atom *atom,
 			    const char *arg, struct strbuf *unused_err)
 {

@@ -615,6 +625,7 @@ static struct {
 	[ATOM_IF] = { "if", SOURCE_NONE, FIELD_STR, if_atom_parser },
 	[ATOM_THEN] = { "then", SOURCE_NONE },
 	[ATOM_ELSE] = { "else", SOURCE_NONE },
+	[ATOM_REST] = { "rest", SOURCE_NONE, FIELD_STR, rest_atom_parser },
 	/*
 	 * Please update $__git_ref_fieldlist in git-completion.bash
 	 * when you add new atoms

@@ -1006,6 +1017,9 @@ int verify_ref_format(struct ref_format *format)
 		at = parse_ref_filter_atom(format, sp + 2, ep, &err);
 		if (at < 0)
 			die("%s", err.buf);
+		if (used_atom[at].atom_type == ATOM_REST)
+			die("this command reject atom %%(%.*s)", (int)(ep - sp - 2), sp + 2);
+
 		if (format->quote_style && used_atom[at].atom_type == ATOM_RAW &&
 		    used_atom[at].u.raw_data.option == RAW_BARE)
 			die(_("--format=%.*s cannot be used with"

@@ -1920,6 +1934,12 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 			v->handler = else_atom_handler;
 			v->s = xstrdup("");
 			continue;
+		} else if (atom_type == ATOM_REST) {
+			if (ref->rest)
+				v->s = xstrdup(ref->rest);
+			else
+				v->s = xstrdup("");
+			continue;
 		} else
 			continue;
 

@@ -2137,6 +2157,7 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
 
 	FLEX_ALLOC_STR(ref, refname, refname);
 	oidcpy(&ref->objectname, oid);
+	ref->rest = NULL;
 
 	return ref;
 }

index 74fb423fc89f..9dc07476a584 100644
@@ -38,6 +38,7 @@ struct ref_sorting {
 
 struct ref_array_item {
 	struct object_id objectname;
+	const char *rest;
 	int flag;
 	unsigned int kind;
 	const char *symref;

@@ -76,14 +77,16 @@ struct ref_format {
 	 * verify_ref_format() afterwards to finalize.
 	 */
 	const char *format;
+	const char *rest;
 	int quote_style;
+	int use_rest;
 	int use_color;
 
 	/* Internal state to ref-filter */
 	int need_color_reset_at_eol;
 };
 
-#define REF_FORMAT_INIT { NULL, 0, -1 }
+#define REF_FORMAT_INIT { NULL, NULL, 0, 0, -1 }
 
 /*  Macros for checking --merged and --no-merged options */
 #define _OPT_MERGED_NO_MERGED(option, filter, h) \

index 5325b9f67a00..2780ec8803fd 100755
@@ -340,6 +340,10 @@ test_expect_success 'git branch --format option' '
 	test_cmp expect actual
 '
 
+test_expect_success 'git branch with --format=%(rest) must failed' '
+	test_must_fail git branch --format="%(rest)" >actual
+'
+
 test_expect_success 'worktree colors correct' '
 	cat >expect <<-EOF &&
 	* <GREEN>(HEAD detached from fromtag)<RESET>

index e2867de791e7..8c97c3b877c6 100755
@@ -1187,6 +1187,10 @@ test_expect_success 'basic atom: head contents:trailers' '
 	test_cmp expect actual.clean
 '
 
+test_expect_success 'basic atom: rest must failed' '
+	test_must_fail git for-each-ref --format="%(rest)" refs/heads/main
+'
+
 test_expect_success 'trailer parsing not fooled by --- line' '
 	git commit --allow-empty -F - <<-\EOF &&
 	this is the subject

index 2f72c5c6883e..9fc4c4323949 100755
@@ -1998,6 +1998,10 @@ test_expect_success '--format should list tags as per format given' '
 	test_cmp expect actual
 '
 
+test_expect_success 'git tag -l with --format="%(rest)" must failed' '
+	test_must_fail git tag -l --format="%(rest)" "v1*"
+'
+
 test_expect_success "set up color tests" '
 	echo "<RED>v1.0<RESET>" >expect.color &&
 	echo "v1.0" >expect.bare &&

index 3cefde9602bf..785b32eb88f9 100755
@@ -194,6 +194,10 @@ test_expect_success GPG 'verifying tag with --format' '
 	test_cmp expect actual
 '
 
+test_expect_success GPG 'verifying tag with --format="%(rest)" must failed' '
+	test_must_fail git verify-tag --format="%(rest)" "fourth-signed"
+'
+
 test_expect_success GPG 'verifying a forged tag with --format should fail silently' '
 	test_must_fail git verify-tag --format="tagname : %(tag)" $(cat forged1.tag) >actual-forged &&
 	test_must_be_empty actual-forged

gitgitgadget

From: ZheNing Hu <[email protected]>

Use non-const ref_format in *_atom_parser(), which can help us modify the members of ref_format in *_atom_parser().

---
 builtin/tag.c |  2 +-
 ref-filter.c  | 44 ++++++++++++++++++++++----------------------
 ref-filter.h  |  4 ++--
 3 files changed, 25 insertions(+), 25 deletions(-)

index 82fcfc098242..452558ec9575 100644
@@ -146,7 +146,7 @@ static int verify_tag(const char *name, const char *ref,
 		      const struct object_id *oid, void *cb_data)
 {
 	int flags;
-	const struct ref_format *format = cb_data;
+	struct ref_format *format = cb_data;
 	flags = GPG_VERIFY_VERBOSE;
 
 	if (format->format)

index baf72a718965..74fb423fc89f 100644
@@ -116,7 +116,7 @@ void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);
 void ref_sorting_set_sort_flags_all(struct ref_sorting *sorting, unsigned int mask, int on);
 /*  Based on the given format and quote_style, fill the strbuf */
 int format_ref_array_item(struct ref_array_item *info,
-			  const struct ref_format *format,
+			  struct ref_format *format,
 			  struct strbuf *final_buf,
 			  struct strbuf *error_buf);
 /*  Parse a single sort specifier and add it to the list */

@@ -137,7 +137,7 @@ void setup_ref_filter_porcelain_msg(void);
  * name must be a fully qualified refname.
  */
 void pretty_print_ref(const char *name, const struct object_id *oid,
-		      const struct ref_format *format);
+		      struct ref_format *format);
 
 /*
  * Push a single ref onto the array; this can be used to construct your own

gitgitgadget

From: ZheNing Hu <[email protected]>

Introduce free_array_item_internal() for freeing ref_array_item value. It will be called internally by free_array_item(), and it will help cat-file --batch free ref_array_item's memory later.

---
 ref-filter.c | 11 ++++++++---
 ref-filter.h |  2 ++
 2 files changed, 10 insertions(+), 3 deletions(-)

index 420c0bf9384f..aa2ce634106b 100644
@@ -2282,16 +2282,21 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
 	return 0;
 }
 
-/*  Free memory allocated for a ref_array_item */
-static void free_array_item(struct ref_array_item *item)
+void free_array_item_internal(struct ref_array_item *item)
 {
-	free((char *)item->symref);
 	if (item->value) {
 		int i;
 		for (i = 0; i < used_atom_cnt; i++)
 			free((char *)item->value[i].s);
 		free(item->value);
 	}
+}
+
+/*  Free memory allocated for a ref_array_item */
+static void free_array_item(struct ref_array_item *item)
+{
+	free((char *)item->symref);
+	free_array_item_internal(item);
 	free(item);
 }
 

index 9dc07476a584..d4531fef5f91 100644
@@ -111,6 +111,8 @@ struct ref_format {
 int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int type);
 /*  Clear all memory allocated to ref_array */
 void ref_array_clear(struct ref_array *array);
+/* Free array item's value */
+void free_array_item_internal(struct ref_array_item *item);
 /*  Used to verify if the given format is correct and to parse out the used atoms */
 int verify_ref_format(struct ref_format *format);
 /*  Sort the given ref_array as per the ref_sorting provided */

gitgitgadget

From: ZheNing Hu <[email protected]>

Let populate_value(), get_ref_atom_value() and format_ref_array_item() get the return value of get_object() correctly. This can help us later let cat-file --batch get the correct error message and return value of get_object().

---
 ref-filter.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

index 8868cf98f090..420c0bf9384f 100644
@@ -1808,7 +1808,7 @@ static char *get_worktree_path(const struct used_atom *atom, const struct ref_ar
 static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 {
 	struct object *obj;
-	int i;
+	int i, ret = 0;
 	struct object_info empty = OBJECT_INFO_INIT;
 
 	CALLOC_ARRAY(ref->value, used_atom_cnt);

@@ -1965,8 +1965,8 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 
 
 	oi.oid = ref->objectname;
-	if (get_object(ref, 0, &obj, &oi, err))
-		return -1;
+	if ((ret = get_object(ref, 0, &obj, &oi, err)))
+		return ret;
 
 	/*
 	 * If there is no atom that wants to know about tagged

@@ -1997,9 +1997,11 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
 static int get_ref_atom_value(struct ref_array_item *ref, int atom,
 			      struct atom_value **v, struct strbuf *err)
 {
+	int ret = 0;
+
 	if (!ref->value) {
-		if (populate_value(ref, err))
-			return -1;
+		if ((ret = populate_value(ref, err)))
+			return ret;
 		fill_missing_values(ref->value);
 	}
 	*v = &ref->value[atom];

@@ -2573,6 +2575,7 @@ int format_ref_array_item(struct ref_array_item *info,
 {
 	const char *cp, *sp, *ep;
 	struct ref_formatting_state state = REF_FORMATTING_STATE_INIT;
+	int ret = 0;
 
 	state.quote_style = format->quote_style;
 	push_stack_element(&state.stack);

@@ -2585,10 +2588,10 @@ int format_ref_array_item(struct ref_array_item *info,
 		if (cp < sp)
 			append_literal(cp, sp, &state);
 		pos = parse_ref_filter_atom(format, sp + 2, ep, error_buf);
-		if (pos < 0 || get_ref_atom_value(info, pos, &atomv, error_buf) ||
+		if (pos < 0 || (ret = get_ref_atom_value(info, pos, &atomv, error_buf)) ||
 		    atomv->handler(atomv, &state, error_buf)) {
 			pop_stack_element(&state.stack);
-			return -1;
+			return ret ? ret : -1;
 		}
 	}
 	if (*cp) {

@@ -2610,7 +2613,7 @@ int format_ref_array_item(struct ref_array_item *info,
 	}
 	strbuf_addbuf(final_buf, &state.stack->output);
 	pop_stack_element(&state.stack);
-	return 0;
+	return ret;
 }
 
 void pretty_print_ref(const char *name, const struct object_id *oid,

gitgitgadget

From: ZheNing Hu <[email protected]>

In order to let cat-file use ref-filter logic, the following methods are used:

1. Add cat_file_mode member in struct ref_format, this can help us reject atoms in verify_ref_format() which cat-file cannot use, e.g. %(refname), %(push), %(upstream)...
2. Change the type of member format in struct batch_options to ref_format, We can add format data in it.
3. Let batch_objects() add atoms to format, and use verify_ref_format() to check atoms.
4. Use has_object_file() in batch_one_object() to check whether the input object exists.
5. Use format_ref_array_item() in batch_object_write() to get the formatted data corresponding to the object. If the return value of format_ref_array_item() is equals to zero, use batch_write() to print object data; else if the return value less than zero, use die() to print the error message and exit; else return value greater than zero, only print the error message, but not exit.
6. Let get_object() return 1 and print "<oid> missing" instead of returning -1 and printing "missing object <oid> for <refname>", this can help format_ref_array_item() just report that the object is missing without letting Git exit.

Most of the atoms in for-each-ref --format are now supported, such as %(tree), %(parent), %(author), %(tagger), %(if), %(then), %(else), %(end). But these atoms will be rejected:
%(refname), %(symref), %(upstream), %(push), %(worktreepath), %(flag), %(HEAD), because our objects don't have refname.

---
 Documentation/git-cat-file.txt |   6 +
 builtin/cat-file.c             | 248 +++++++-------------------------
 ref-filter.c                   |  15 +-
 ref-filter.h                   |   3 +-
 t/t1006-cat-file.sh            | 252 +++++++++++++++++++++++++++++++++
 t/t6301-for-each-ref-errors.sh |   2 +-
 6 files changed, 323 insertions(+), 203 deletions(-)

index 4eb0421b3fd9..ef8ab952b2fa 100644
@@ -226,6 +226,12 @@ newline. The available atoms are:
 	after that first run of whitespace (i.e., the "rest" of the
 	line) are output in place of the `%(rest)` atom.
 
+Note that most of the atoms in `for-each-ref --format` are now supported,
+such as `%(tree)`, `%(parent)`, `%(author)`, `%(tagger)`, `%(if)`,
+`%(then)`, `%(else)`, `%(end)`. But these atoms will be rejected:
+`%(refname)`, `%(symref)`, `%(upstream)`, `%(push)`, `%(worktreepath)`,
+`%(flag)`, `%(HEAD)`. See linkgit:git-for-each-ref[1].
+
 If no format is specified, the default format is `%(objectname)
 %(objecttype) %(objectsize)`.
 

index aa2ce634106b..660e52f97f79 100644
@@ -1017,8 +1017,15 @@ int verify_ref_format(struct ref_format *format)
 		at = parse_ref_filter_atom(format, sp + 2, ep, &err);
 		if (at < 0)
 			die("%s", err.buf);
-		if (used_atom[at].atom_type == ATOM_REST)
-			die("this command reject atom %%(%.*s)", (int)(ep - sp - 2), sp + 2);
+		if ((!format->cat_file_mode && used_atom[at].atom_type == ATOM_REST) ||
+		    (format->cat_file_mode && (used_atom[at].atom_type == ATOM_FLAG ||
+					       used_atom[at].atom_type == ATOM_HEAD ||
+					       used_atom[at].atom_type == ATOM_PUSH ||
+					       used_atom[at].atom_type == ATOM_REFNAME ||
+					       used_atom[at].atom_type == ATOM_SYMREF ||
+					       used_atom[at].atom_type == ATOM_UPSTREAM ||
+					       used_atom[at].atom_type == ATOM_WORKTREEPATH)))
+			die(_("this command reject atom %%(%.*s)"), (int)(ep - sp - 2), sp + 2);
 
 		if (format->quote_style && used_atom[at].atom_type == ATOM_RAW &&
 		    used_atom[at].u.raw_data.option == RAW_BARE)

@@ -1735,8 +1742,8 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj
 	}
 	if (oid_object_info_extended(the_repository, &oi->oid, &oi->info,
 				     OBJECT_INFO_LOOKUP_REPLACE))
-		return strbuf_addf_ret(err, -1, _("missing object %s for %s"),
-				       oid_to_hex(&oi->oid), ref->refname);
+		return strbuf_addf_ret(err, 1, _("%s missing"),
+				       oid_to_hex(&oi->oid));
 	if (oi->info.disk_sizep && oi->disk_size < 0)
 		BUG("Object size is less than zero.");
 

index d4531fef5f91..3db07729f46d 100644
@@ -78,6 +78,7 @@ struct ref_format {
 	 */
 	const char *format;
 	const char *rest;
+	int cat_file_mode;
 	int quote_style;
 	int use_rest;
 	int use_color;

@@ -86,7 +87,7 @@ struct ref_format {
 	int need_color_reset_at_eol;
 };
 
-#define REF_FORMAT_INIT { NULL, NULL, 0, 0, -1 }
+#define REF_FORMAT_INIT { NULL, NULL, 0, 0, 0, -1 }
 
 /*  Macros for checking --merged and --no-merged options */
 #define _OPT_MERGED_NO_MERGED(option, filter, h) \

index 40edf9dab534..3553f84a00c1 100755
@@ -41,7 +41,7 @@ test_expect_success 'Missing objects are reported correctly' '
 	r=refs/heads/missing &&
 	echo $MISSING >.git/$r &&
 	test_when_finished "rm -f .git/$r" &&
-	echo "fatal: missing object $MISSING for $r" >missing-err &&
+	echo "fatal: $MISSING missing" >missing-err &&
 	test_must_fail git for-each-ref 2>err &&
 	test_cmp missing-err err &&
 	(

gitgitgadget

From: ZheNing Hu <[email protected]>

Reuse the err buffer in batch_object_write(), as the buffer scratch does. This will reduce the overhead of multiple allocations of memory of the err buffer.

---
 builtin/cat-file.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

index 026d9405636a..6766efb06f41 100644
@@ -212,32 +212,33 @@ static void batch_write(struct batch_options *opt, const void *data, int len)
 
 static void batch_object_write(const char *obj_name,
 			       struct strbuf *scratch,
+			       struct strbuf *err,
 			       struct batch_options *opt,
 			       struct expand_data *data)
 {
 	int ret = 0;
-	struct strbuf err = STRBUF_INIT;
 	struct ref_array_item item = { data->oid, data->rest };
 
 	strbuf_reset(scratch);
+	strbuf_reset(err);
 
-	ret = format_ref_array_item(&item, &opt->format, scratch, &err);
+	ret = format_ref_array_item(&item, &opt->format, scratch, err);
 	if (!ret) {
 		strbuf_addch(scratch, '\n');
 		batch_write(opt, scratch->buf, scratch->len);
 	} else if (ret < 0) {
-		die("%s\n", err.buf);
+		die("%s\n", err->buf);
 	} else {
 		/* when ret > 0 , don't call die and print the err to stdout*/
-		printf("%s\n", err.buf);
+		printf("%s\n", err->buf);
 		fflush(stdout);
 	}
 	free_array_item_internal(&item);
-	strbuf_release(&err);
 }
 
 static void batch_one_object(const char *obj_name,
 			     struct strbuf *scratch,
+			     struct strbuf *err,
 			     struct batch_options *opt,
 			     struct expand_data *data)
 {

@@ -291,7 +292,7 @@ static void batch_one_object(const char *obj_name,
 		return;
 	}
 
-	batch_object_write(obj_name, scratch, opt, data);
+	batch_object_write(obj_name, scratch, err, opt, data);
 }
 
 struct object_cb_data {

@@ -299,13 +300,14 @@ struct object_cb_data {
 	struct expand_data *expand;
 	struct oidset *seen;
 	struct strbuf *scratch;
+	struct strbuf *err;
 };
 
 static int batch_object_cb(const struct object_id *oid, void *vdata)
 {
 	struct object_cb_data *data = vdata;
 	oidcpy(&data->expand->oid, oid);
-	batch_object_write(NULL, data->scratch, data->opt, data->expand);
+	batch_object_write(NULL, data->scratch, data->err, data->opt, data->expand);
 	return 0;
 }
 

@@ -361,6 +363,7 @@ static int batch_objects(struct batch_options *opt, const struct option *options
 {
 	struct strbuf input = STRBUF_INIT;
 	struct strbuf output = STRBUF_INIT;
+	struct strbuf err = STRBUF_INIT;
 	struct strbuf format = STRBUF_INIT;
 	struct expand_data data;
 	int save_warning;

@@ -389,6 +392,7 @@ static int batch_objects(struct batch_options *opt, const struct option *options
 		cb.opt = opt;
 		cb.expand = &data;
 		cb.scratch = &output;
+		cb.err = &err;
 
 		if (opt->unordered) {
 			struct oidset seen = OIDSET_INIT;

@@ -413,6 +417,7 @@ static int batch_objects(struct batch_options *opt, const struct option *options
 
 		strbuf_release(&format);
 		strbuf_release(&output);
+		strbuf_release(&err);
 		return 0;
 	}
 

@@ -441,12 +446,13 @@ static int batch_objects(struct batch_options *opt, const struct option *options
 			data.rest = p;
 		}
 
-		batch_one_object(input.buf, &output, opt, &data);
+		batch_one_object(input.buf, &output, &err, opt, &data);
 	}
 
 	strbuf_release(&format);
 	strbuf_release(&input);
 	strbuf_release(&output);
+	strbuf_release(&err);
 	warn_on_object_refname_ambiguity = save_warning;
 	return retval;
 }

gitgitgadget

From: ZheNing Hu <[email protected]>

After cat-file reuses the ref-filter logic, we re-implement the functions of --textconv and --filters options.

Add members use_textconv and use_filters in struct ref_format, and use global variables use_filters and use_textconv in ref-filter.c, so that we can filter the content of the object in get_object(). Use actual_oi to record the real expand_data:
it may point to the original oi or the act_oi processed by textconv_object() or convert_to_working_tree(). grab_values() will grab the contents of actual_oi and grab_common_values() to grab the contents of origin oi, this ensures that %(objectsize) still uses the size of the unfiltered data.

In get_object(), we made an optimization: Firstly, get the size and type of the object instead of directly getting the object data. If using --textconv, after successfully obtaining the filtered object data, an extra oid_object_info_extended() will be skipped, which can reduce the cost of object data copy; If using --filter, the data of the object first will be getted first, and then convert_to_working_tree() will be used to get the filtered object data.

---
 builtin/cat-file.c |  5 ++++
 ref-filter.c       | 66 ++++++++++++++++++++++++++++++++++++++++++++--
 ref-filter.h       |  4 ++-
 3 files changed, 72 insertions(+), 3 deletions(-)

index 6766efb06f41..2978b8990e8d 100644
@@ -377,6 +377,11 @@ static int batch_objects(struct batch_options *opt, const struct option *options
 	if (opt->print_contents)
 		strbuf_addstr(&format, "\n%(raw)");
 	opt->format.format = format.buf;
+	if (opt->cmdmode == 'c')
+		opt->format.use_textconv = 1;
+	if (opt->cmdmode == 'w')
+		opt->format.use_filters = 1;
+
 	if (verify_ref_format(&opt->format))
 		usage_with_options(cat_file_usage, options);
 

index 3db07729f46d..56a5ec4c9eaa 100644
@@ -80,6 +80,8 @@ struct ref_format {
 	const char *rest;
 	int cat_file_mode;
 	int quote_style;
+	int use_textconv;
+	int use_filters;
 	int use_rest;
 	int use_color;
 

@@ -87,7 +89,7 @@ struct ref_format {
 	int need_color_reset_at_eol;
 };
 
-#define REF_FORMAT_INIT { NULL, NULL, 0, 0, 0, -1 }
+#define REF_FORMAT_INIT { NULL, NULL, 0, 0, 0, 0, 0, -1 }
 
 /*  Macros for checking --merged and --no-merged options */
 #define _OPT_MERGED_NO_MERGED(option, filter, h) \

gitgitgadget

Hmph, does anybody have 5a5b5f78?

The way to deal with this and avoid resending the same patches (assuming that this is not a 9-patch series, but only 5 of them are new) is to rebase your topic on 723bc66d (ref-filter: add %(rest) atom, 2021-06-09), which will allow you to discard the 4 earlier patches, and force push, with base set to 723bc66d, I think, but I am not a GGG user, so there may need an extra step or two on top of that.

The "get" the value correctly, I think. What you are teaching them is to pass the return value from get_object() through the callchain to their callers.

The readers will be helped if you say what kind of errors get_object() wants to tell its callers, not just "-1" is for error, which is what populate_value() assumes to be sufficient. In other words, which non-zero returns from get_object() are interesting and why?

As a file local static function, the horrible name free_array_item() was tolerable. But before exposing a name like that to the outside world, think twice if that is specific enough, and it is not. There are 47 different kinds of "array"s we use in the system, but this new helper function only works with ref_array_item and not on items in any other kinds of arrays.

Oh, 5a5b5f78 is in gitgitgadget.git and 723bc66d in git.git, their commit messages are different. Later I will rebase my new patch to zh/xxx.

Thanks.
--
ZheNing Hu

Nice goal and feature to have.

Most of the functions that deal with this already use a strbuf in some way, before we had a const char *, now there's a size_t to go along with it, why not simply use a strbuf in the struct for the data? You'll then get the size and \0 handling for free, and any functions to deal with conversion can stick to the strbuf API, there seems to be a lot of back and forth now.

Perl at least deals with that just fine, and to the extent that it doesn't any new problems here would have nothing to do with \0 being in the data. Perl doesn't have a notion of "binary has \0 in it", it always supports \0, it has a notion of "is it utf-8 or not?", so any encoding problems wouldn't be new. I'd think that the same would be true of Python, but I'm not sure.

    git update-ref ... $(git hash-object) ?

On Sat, Jun 12 2021, ZheNing Hu via GitGitGadget wrote:

Subject: s/objet/object/

Yes, this is exactly what I meant.

As stated in 765337a, We can just print the error without exiting if the return value of format_ref_array_item() is greater than 0. Therefore, the current patch is to make get_object() return a value other than -1 when an error occurs.

Makes sense.

You are right, It is correct to only return 0 here at the moment.

Thanks.
--
ZheNing Hu

I left some comments, but saw after the fact that I'd replied to the v1 E-Mails by accident, but anyway, the comments were all on things that are also in v2, so it worked out in the end. Sorry about the confusion.

If somebody throws OBJ_NONE at us by mistake, we do not want to handle such an object and try to extract the subject member from it anyway, no?

The intent of the code here, before the patch, is that "what we do after the control flow passes this point is about the body, subject, trailers, and contents request, so everybody else should go to the next iteration". The caller used to give us an object compatible with these four types of requests, now the caller may throw others, hence "by the way, we know these four kinds of requests make sense only for tags and commits, so everybody else should go to the next iteration, too" would be a natural thing to add. So in that sense, I prefer it over "we know these four types of requests do not make sense for blobs and trees".

During an earlier iteration long time ago, as we knew Perl is capable of handling sequence of bytes including NUL, it was decided to start more strict by rejecting binary for all languages, which can later be loosened, to limit the scope of the initial implementation.

That's horrible. Thanks for noticing.

We'd want to catch segfaults from both hash-object and update-ref. One way to do so may be

	O=$(git hash-object -w blob1) &&
	git update-ref refs/myblobs/blob1 "$O"

Thanks for a review.

Well, this is indeed ignored by me.

So free_array_item() --> free_ref_array_item() and free_array_item() --> free_ref_array_item().

Yeah, "internal" is an incorrect naming. The main purpose here is to only free a ref_array_item's value.

Because batch_object_write() use a ref_array_item which is allocated on the stack, and it's member symref is not used at all. ref_array_item's symref and itself do not need free. The original free_array_item() will free all dynamically
allocated content in ref_array_item. It cannot meet our requirements at this time:
only free the ref_array_item's value.

Make sence.

--
ZheNing Hu

OK.

--
ZheNing Hu

Yes, strbuf is a suitable choice when using <str,len> pair. But if replace v->s with strbuf, the possible changes will be larger.

Not python safe. See [1].
Regarding the perl language, I support Junio's point of view: it can be re-supported in the future.

Are you saying that sanitize_pgp is not needed?

I didn’t think about it, just for convenience.

[1]: https://lore.kernel.org/git/CAOLTT8QR_GRm4TYk0E_eazQ+unVQODc-3L+b4V5JUN5jtZR8uA@mail.gmail.com/

Thanks for a review.
--
ZheNing Hu

Ok.

Maybe it's something like this:

@@ -1762,19 +1762,17 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj

                oi->info.typep = NULL;
                oi->info.contentp = temp_contentp;

- if (use_textconv) {
- act_oi = *oi;
+ if (use_textconv && !ref->rest)
+ return strbuf_addf_ret(err, -1, _("missing
path for '%s'"),
+ oid_to_hex(&act_oi.oid));

- if(!ref->rest)
- return strbuf_addf_ret(err, -1,
_("missing path for '%s'"),
- oid_to_hex(&act_oi.oid));
- if (act_oi.type == OBJ_BLOB) {
- if (textconv_object(the_repository,
- ref->rest,
0100644, &act_oi.oid,
- 1, (char
**)(&act_oi.content), &act_oi.size)) {
- actual_oi = &act_oi;
- goto success;
- }
+ if (use_textconv && oi->type == OBJ_BLOB) {
+ act_oi = *oi;
+ if (textconv_object(the_repository,
+ ref->rest, 0100644, &act_oi.oid,
+ 1, (char
**)(&act_oi.content), &act_oi.size)) {
+ actual_oi = &act_oi;

+                               goto success;
                        }
                }
        }

And this part:

@@ -1786,25 +1784,21 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj

                BUG("Object size is less than zero.");

        if (oi->info.contentp) {

- if (use_filters) {
- if(!ref->rest)
- return strbuf_addf_ret(err, -1,
_("missing path for '%s'"),
- oid_to_hex(&oi->oid));
- if (oi->type == OBJ_BLOB) {

-                               struct strbuf strbuf = STRBUF_INIT;
-                               struct checkout_metadata meta;
-                               act_oi = *oi;

-
- init_checkout_metadata(&meta, NULL,
NULL, &act_oi.oid);
- if
(convert_to_working_tree(&the_index, ref->rest, act_oi.content, act_oi.size, &strbuf, &meta)) {
- act_oi.size = strbuf.len;
- act_oi.content =
strbuf_detach(&strbuf, NULL);
- actual_oi = &act_oi;
- } else {
- die("could not convert '%s' %s",
- oid_to_hex(&oi->oid), ref->rest);
- }
- }
+ if (use_filters && !ref->rest)
+ return strbuf_addf_ret(err, -1, _("missing
path for '%s'"),
+ oid_to_hex(&oi->oid));
+ if (use_filters && oi->type == OBJ_BLOB) {

+                       struct strbuf strbuf = STRBUF_INIT;
+                       struct checkout_metadata meta;
+                       act_oi = *oi;

+
+ init_checkout_metadata(&meta, NULL, NULL, &act_oi.oid);
+ if (!convert_to_working_tree(&the_index,
ref->rest, act_oi.content, act_oi.size, &strbuf, &meta))
+ die("could not convert '%s' %s",
+ oid_to_hex(&oi->oid), ref->rest);

+                       act_oi.size = strbuf.len;
+                       act_oi.content = strbuf_detach(&strbuf, NULL);
+                       actual_oi = &act_oi;
                }

I agree, this method is better. We don't need to care about which members so many "0" point to.

Thanks.
--
ZheNing Hu

I agree.

Yeah. This way it will be more readable.

Indeed, inline assignment must be used in this case.

Thanks. I will pay attention to these style issues later. --
ZheNing Hu

It's okay. Your comments are very useful.

Thanks.
--
ZheNing Hu

I for one would like to see it done that way, those changes are usually easy to read. Also it seems a large part of 2/8 is extra new code because we didn't do that, e.g. getting length differently if something is a strbuf or not, passing char*/size_t pairs to new functions etc.

Ah, I'd missed that. Anyway, if it's easy it seems you discovered that Perl deals with it correctly, so we could just have it support this.

No that instead of:

    echo "" >x

You can do:

echo >x

And also that going back and forth between populating different files is confusing, i.e. this:

echo a >x
echo c >y
echo b >>x

is better as:

echo a >x
echo b >>x
echo c >y

*nod*, Junio had a good suggestion.

After some refactoring, I found that there are two problems:
1. There are a lot of codes like this in ref-filter to fill v->s:

v->s = show_ref(...)
v->s = copy_email(...)

It is very difficult to modify here: We know that show_ref() or copy_email() will allocate a block of memory to v->s, but if v->s is a strbuf, what should we do? In copy_email(), we can pass the v->s to copy_email() and use strbuf_add()/strbuf_addstr() instead of xstrdup() and xmemdupz(). But show_ref() will call external functions like shorten_unambiguous_ref(), we don’t know whether it will return us NULL or a dynamically allocated memory. If continue to pass v->s to the inner function, it is not a feasible method. Or we can use strbuf_attach() + strlen(), I'm not sure this is a good method.

2. See:

- for (i = 0; i < used_atom_cnt; i++) {

+       for (i = 0; i < used_atom_cnt; i++) {
                struct atom_value *v = &ref->value[i];

- if (v->s == NULL && used_atom[i].source == SOURCE_NONE)
+ if (v->s.len == 0 && used_atom[i].source == SOURCE_NONE)
return strbuf_addf_ret(err, -1, _("missing
object %s for %s"),

oid_to_hex(&ref->objectname), ref->refname);
        }

In the case of using strbuf, I don’t know how to distinguish between an empty strbuf and NULL. It can be easily distinguished by using c-style "const char*".

Well, it's ok, support for perl will be put in a separate commit.

Thanks, I get what you meant now.

ok.

Thanks.
--
ZheNing Hu

Yes, sometimes it's just too much of a hassle, looking at shorten_unambiguous_ref() which returns a xstrdup()'d value that could indeed be strbuf_attach'd. I haven't tried the conversion myself, perhaps it's too much hassle.

Just a suggestion from reading your patch in isolation.

If you resend this patch, it might be a good idea to add a short version of the above explanations into the commit message.

[...]

Maybe this could also be part of the explanation.

Yeah, thanks for the review anyway!

From: ZheNing Hu <[email protected]>

Only tag and commit objects use grab_sub_body_contents() to grab object contents in the current codebase. We want to teach the function to also handle blobs and trees to get their raw data, without parsing a blob (whose contents looks like a commit or a tag) incorrectly as a commit or a tag.

Skip the block of code that is specific to handling commits and tags early when the given object is of a wrong type to help later addition to handle other types of objects in this function.

---
 ref-filter.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

index 4db0e40ff4c6..5cee6512fbaf 100644
@@ -1356,11 +1356,12 @@ static void append_lines(struct strbuf *out, const char *buf, unsigned long size
 }
 
 /* See grab_values */
-static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)
+static void grab_sub_body_contents(struct atom_value *val, int deref, struct expand_data *data)
 {
 	int i;
 	const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;
 	size_t sublen = 0, bodylen = 0, nonsiglen = 0, siglen = 0;
+	void *buf = data->content;
 
 	for (i = 0; i < used_atom_cnt; i++) {
 		struct used_atom *atom = &used_atom[i];

@@ -1371,10 +1372,13 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)
 			continue;
 		if (deref)
 			name++;
-		if (strcmp(name, "body") &&
-		    !starts_with(name, "subject") &&
-		    !starts_with(name, "trailers") &&
-		    !starts_with(name, "contents"))
+
+		if ((data->type != OBJ_TAG &&
+		     data->type != OBJ_COMMIT) ||
+		    (strcmp(name, "body") &&
+		     !starts_with(name, "subject") &&
+		     !starts_with(name, "trailers") &&
+		     !starts_with(name, "contents")))
 			continue;
 		if (!subpos)
 			find_subpos(buf,

@@ -1438,17 +1442,19 @@ static void fill_missing_values(struct atom_value *val)
  * pointed at by the ref itself; otherwise it is the object the
  * ref (which is a tag) refers to.
  */
-static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf)
+static void grab_values(struct atom_value *val, int deref, struct object *obj, struct expand_data *data)
 {
+	void *buf = data->content;
+
 	switch (obj->type) {
 	case OBJ_TAG:
 		grab_tag_values(val, deref, obj);
-		grab_sub_body_contents(val, deref, buf);
+		grab_sub_body_contents(val, deref, data);
 		grab_person("tagger", val, deref, buf);
 		break;
 	case OBJ_COMMIT:
 		grab_commit_values(val, deref, obj);
-		grab_sub_body_contents(val, deref, buf);
+		grab_sub_body_contents(val, deref, data);
 		grab_person("author", val, deref, buf);
 		grab_person("committer", val, deref, buf);
 		break;

@@ -1678,7 +1684,7 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj
 			return strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"),
 					       oid_to_hex(&oi->oid), ref->refname);
 		}
-		grab_values(ref->value, deref, *obj, oi->content);
+		grab_values(ref->value, deref, *obj, oi);
 	}
 
 	grab_common_values(ref->value, deref, oi);

gitgitgadget

This patch series make cat-file reuse ref-filter logic. As part of the content related to zh/ref-filter-raw-data has been modified, this patch currently sets the base on zh/ref-filter-atom-type until zh/ref-filter-raw-data is stable And will rebase to it.

f72ad9c~1..39a0d93 of this patch belongs to zh/ref-filter-raw-data. Change from last version:

1. Let --format=%(raw) re-support --perl.
2. Correct some errors in the test.
3. Add the reason why strbuf is not used to replace atom_value's member s in the commit message.

39a0d93..86ac3bc of this patch is another part. Change from last version:

1. Modify free_array_item_internal() to free_ref_array_item_value().
2. Change some errors of the code style.
3. Change some commit messages.

ZheNing Hu (10):
[GSOC] ref-filter: add obj-type check in grab contents [GSOC] ref-filter: add %(raw) atom
[GSOC] ref-filter: --format=%(raw) re-support --perl [GSOC] ref-filter: use non-const ref_format in *_atom_parser() [GSOC] ref-filter: add %(rest) atom
[GSOC] ref-filter: pass get_object() return value to their callers [GSOC] ref-filter: introduce free_ref_array_item_value() function [GSOC] cat-file: reuse ref-filter logic
[GSOC] cat-file: reuse err buf in batch_object_write() [GSOC] cat-file: re-implement --textconv, --filters options

 Documentation/git-cat-file.txt     |   6 +
 Documentation/git-for-each-ref.txt |   9 +
 builtin/cat-file.c                 | 267 ++++++-----------------
 builtin/tag.c                      |   2 +-
 quote.c                            |  17 ++
 quote.h                            |   1 +
 ref-filter.c                       | 331 +++++++++++++++++++++++------
 ref-filter.h                       |  14 +-
 t/t1006-cat-file.sh                | 252 ++++++++++++++++++++++
 t/t3203-branch-output.sh           |   4 +
 t/t6300-for-each-ref.sh            | 235 ++++++++++++++++++++
 t/t6301-for-each-ref-errors.sh     |   2 +-
 t/t7004-tag.sh                     |   4 +
 t/t7030-verify-tag.sh              |   4 +
 14 files changed, 872 insertions(+), 276 deletions(-)

base-commit: 1197f1a46360d3ae96bd9c15908a3a6f8e562207
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-980%2Fadlternative%2Fcat-file-batch-refactor-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-980/adlternative/cat-file-batch-refactor-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/980

Range-diff vs v2:

  1:  48d256db5c34 =  1:  f72ad9cc5e8b [GSOC] ref-filter: add obj-type check in grab contents
  2:  abee6a03becb !  2:  ab497d66c116 [GSOC] ref-filter: add %(raw) atom
     @@ Commit message
          `100644 one\0...`, only the `100644 one` will be added to the buffer,
          which is incorrect.
      
     -    Therefore, add a new member in `struct atom_value`: `s_size`, which
     -    can record raw object size, it can help us add raw object data to
     -    the buffer or compare two buffers which contain raw object data.
     +    Therefore, we need to find a way to record the length of the
     +    atom_value's member `s`. Although strbuf can already record the
     +    string and its length, if we want to replace the type of atom_value's
     +    member `s` with strbuf, many places in ref-filter that are filled
     +    with dynamically allocated mermory in `v->s` are not easy to replace.
     +    At the same time, we need to check if `v->s == NULL` in
     +    populate_value(), and strbuf cannot easily distinguish NULL and empty
     +    strings, but c-style "const char *" can do it. So add a new member in
     +    `struct atom_value`: `s_size`, which can record raw object size, it
     +    can help us add raw object data to the buffer or compare two buffers
     +    which contain raw object data.
      
          Beyond, `--format=%(raw)` cannot be used with `--python`, `--shell`,
     -    `--tcl`, `--perl` because if our binary raw data is passed to a variable
     -    in the host language, the host language may not support arbitrary binary
     -    data in the variables of its string type.
     +    `--tcl`, `--perl` because if our binary raw data is passed to a
     +    variable in the host language, the host language may not support
     +    arbitrary binary data in the variables of its string type.
      
          Mentored-by: Christian Couder <[email protected]>
          Mentored-by: Hariom Verma <[email protected]>
     +    Helped-by: Ævar Arnfjörð Bjarmason <[email protected]>
          Helped-by: Felipe Contreras <[email protected]>
          Helped-by: Phillip Wood <[email protected]>
          Helped-by: Junio C Hamano <[email protected]>
     @@ t/t6300-for-each-ref.sh: test_atom tag contents 'Tagging at 1151968727
      +	git cat-file commit refs/tags/testtag^{} >expected &&
      +	git for-each-ref --format="%(*raw)" refs/tags/testtag >actual &&
      +	sanitize_pgp <expected >expected.clean &&
     ++	echo >>expected.clean &&
      +	sanitize_pgp <actual >actual.clean &&
     -+	echo "" >>expected.clean &&
      +	test_cmp expected.clean actual.clean
      +'
      +
     @@ t/t6300-for-each-ref.sh: test_atom refs/tags/signed-empty contents:body ''
      +	git cat-file tag refs/tags/signed-empty >expected &&
      +	git for-each-ref --format="%(raw)" refs/tags/signed-empty >actual &&
      +	sanitize_pgp <expected >expected.clean &&
     ++	echo >>expected.clean &&
      +	sanitize_pgp <actual >actual.clean &&
     -+	echo "" >>expected.clean &&
      +	test_cmp expected.clean actual.clean
      +'
      +
     @@ t/t6300-for-each-ref.sh: test_atom refs/tags/signed-short contents:signature "$s
      +	git cat-file tag refs/tags/signed-short >expected &&
      +	git for-each-ref --format="%(raw)" refs/tags/signed-short >actual &&
      +	sanitize_pgp <expected >expected.clean &&
     ++	echo >>expected.clean &&
      +	sanitize_pgp <actual >actual.clean &&
     -+	echo "" >>expected.clean &&
      +	test_cmp expected.clean actual.clean
      +'
      +
     @@ t/t6300-for-each-ref.sh: test_atom refs/tags/signed-long contents "subject line
      +	git cat-file tag refs/tags/signed-long >expected &&
      +	git for-each-ref --format="%(raw)" refs/tags/signed-long >actual &&
      +	sanitize_pgp <expected >expected.clean &&
     ++	echo >>expected.clean &&
      +	sanitize_pgp <actual >actual.clean &&
     -+	echo "" >>expected.clean &&
      +	test_cmp expected.clean actual.clean
      +'
      +
     @@ t/t6300-for-each-ref.sh: test_atom refs/mytrees/first contents:body ""
       
      +test_expect_success 'basic atom: refs/mytrees/first raw' '
      +	git cat-file tree refs/mytrees/first >expected &&
     -+	echo "" >>expected &&
     ++	echo >>expected &&
      +	git for-each-ref --format="%(raw)" refs/mytrees/first >actual &&
      +	test_cmp expected actual &&
      +	git cat-file -s refs/mytrees/first >expected &&
     @@ t/t6300-for-each-ref.sh: test_atom refs/myblobs/first contents:body ""
       
      +test_expect_success 'basic atom: refs/myblobs/first raw' '
      +	git cat-file blob refs/myblobs/first >expected &&
     -+	echo "" >>expected &&
     ++	echo >>expected &&
      +	git for-each-ref --format="%(raw)" refs/myblobs/first >actual &&
      +	test_cmp expected actual &&
      +	git cat-file -s refs/myblobs/first >expected &&
     @@ t/t6300-for-each-ref.sh: test_atom refs/myblobs/first contents:body ""
      +	printf "\0 \0a\0 " >blob6 &&
      +	printf "  " >blob7 &&
      +	>blob8 &&
     -+	git hash-object blob1 -w | xargs git update-ref refs/myblobs/blob1 &&
     -+	git hash-object blob2 -w | xargs git update-ref refs/myblobs/blob2 &&
     -+	git hash-object blob3 -w | xargs git update-ref refs/myblobs/blob3 &&
     -+	git hash-object blob4 -w | xargs git update-ref refs/myblobs/blob4 &&
     -+	git hash-object blob5 -w | xargs git update-ref refs/myblobs/blob5 &&
     -+	git hash-object blob6 -w | xargs git update-ref refs/myblobs/blob6 &&
     -+	git hash-object blob7 -w | xargs git update-ref refs/myblobs/blob7 &&
     -+	git hash-object blob8 -w | xargs git update-ref refs/myblobs/blob8
     ++	obj=$(git hash-object -w blob1) &&
     ++        git update-ref refs/myblobs/blob1 "$obj" &&
     ++	obj=$(git hash-object -w blob2) &&
     ++        git update-ref refs/myblobs/blob2 "$obj" &&
     ++	obj=$(git hash-object -w blob3) &&
     ++        git update-ref refs/myblobs/blob3 "$obj" &&
     ++	obj=$(git hash-object -w blob4) &&
     ++        git update-ref refs/myblobs/blob4 "$obj" &&
     ++	obj=$(git hash-object -w blob5) &&
     ++        git update-ref refs/myblobs/blob5 "$obj" &&
     ++	obj=$(git hash-object -w blob6) &&
     ++        git update-ref refs/myblobs/blob6 "$obj" &&
     ++	obj=$(git hash-object -w blob7) &&
     ++        git update-ref refs/myblobs/blob7 "$obj" &&
     ++	obj=$(git hash-object -w blob8) &&
     ++        git update-ref refs/myblobs/blob8 "$obj"
      +'
      +
      +test_expect_success 'Verify sorts with raw' '
     @@ t/t6300-for-each-ref.sh: test_atom refs/myblobs/first contents:body ""
      +		refs/myblobs/ refs/heads/ >actual &&
      +	test_cmp expected actual
      +'
     ++
      +test_expect_success 'validate raw atom with %(if:notequals)' '
      +	cat >expected <<-EOF &&
      +	refs/heads/ambiguous
     @@ t/t6300-for-each-ref.sh: test_atom refs/myblobs/first contents:body ""
      +	test_cmp expected actual
      +'
      +
     -+test_expect_success '%(raw) with --python must failed' '
     ++test_expect_success '%(raw) with --python must fail' '
      +	test_must_fail git for-each-ref --format="%(raw)" --python
      +'
      +
     -+test_expect_success '%(raw) with --tcl must failed' '
     ++test_expect_success '%(raw) with --tcl must fail' '
      +	test_must_fail git for-each-ref --format="%(raw)" --tcl
      +'
      +
     -+test_expect_success '%(raw) with --perl must failed' '
     ++test_expect_success '%(raw) with --perl must fail' '
      +	test_must_fail git for-each-ref --format="%(raw)" --perl
      +'
      +
     -+test_expect_success '%(raw) with --shell must failed' '
     ++test_expect_success '%(raw) with --shell must fail' '
      +	test_must_fail git for-each-ref --format="%(raw)" --shell
      +'
      +
     -+test_expect_success '%(raw) with --shell and --sort=raw must failed' '
     ++test_expect_success '%(raw) with --shell and --sort=raw must fail' '
      +	test_must_fail git for-each-ref --format="%(raw)" --sort=raw --shell
      +'
      +
  -:  ------------ >  3:  b54dbc431e04 [GSOC] ref-filter: --format=%(raw) re-support --perl
  3:  c99d1d070a18 =  4:  9fbbb3c492f5 [GSOC] ref-filter: use non-const ref_format in *_atom_parser()
  4:  5a5b5f78aeea !  5:  39a0d93c7bc1 [GSOC] ref-filter: add %(rest) atom
     @@ ref-filter.c: static struct {
       	 * Please update $__git_ref_fieldlist in git-completion.bash
       	 * when you add new atoms
      @@ ref-filter.c: int verify_ref_format(struct ref_format *format)
     - 		at = parse_ref_filter_atom(format, sp + 2, ep, &err);
       		if (at < 0)
       			die("%s", err.buf);
     + 
      +		if (used_atom[at].atom_type == ATOM_REST)
      +			die("this command reject atom %%(%.*s)", (int)(ep - sp - 2), sp + 2);
      +
     - 		if (format->quote_style && used_atom[at].atom_type == ATOM_RAW &&
     - 		    used_atom[at].u.raw_data.option == RAW_BARE)
     - 			die(_("--format=%.*s cannot be used with"
     + 		if ((format->quote_style == QUOTE_PYTHON ||
     + 		     format->quote_style == QUOTE_SHELL ||
     + 		     format->quote_style == QUOTE_TCL) &&
      @@ ref-filter.c: static int populate_value(struct ref_array_item *ref, struct strbuf *err)
       			v->handler = else_atom_handler;
       			v->s = xstrdup("");
     @@ t/t3203-branch-output.sh: test_expect_success 'git branch --format option' '
       	test_cmp expect actual
       '
       
     -+test_expect_success 'git branch with --format=%(rest) must failed' '
     ++test_expect_success 'git branch with --format=%(rest) must fail' '
      +	test_must_fail git branch --format="%(rest)" >actual
      +'
      +
     @@ t/t6300-for-each-ref.sh: test_expect_success 'basic atom: head contents:trailers
       	test_cmp expect actual.clean
       '
       
     -+test_expect_success 'basic atom: rest must failed' '
     ++test_expect_success 'basic atom: rest must fail' '
      +	test_must_fail git for-each-ref --format="%(rest)" refs/heads/main
      +'
      +
     @@ t/t7004-tag.sh: test_expect_success '--format should list tags as per format giv
       	test_cmp expect actual
       '
       
     -+test_expect_success 'git tag -l with --format="%(rest)" must failed' '
     ++test_expect_success 'git tag -l with --format="%(rest)" must fail' '
      +	test_must_fail git tag -l --format="%(rest)" "v1*"
      +'
      +
     @@ t/t7030-verify-tag.sh: test_expect_success GPG 'verifying tag with --format' '
       	test_cmp expect actual
       '
       
     -+test_expect_success GPG 'verifying tag with --format="%(rest)" must failed' '
     ++test_expect_success GPG 'verifying tag with --format="%(rest)" must fail' '
      +	test_must_fail git verify-tag --format="%(rest)" "fourth-signed"
      +'
      +

5: 49063372e003 ! 6: 35a376db1fc1 [GSOC] ref-filter: teach get_object() return useful value
@@ Metadata
Author: ZheNing Hu <[email protected]>

## Commit message ##
- [GSOC] ref-filter: teach get_object() return useful value
+ [GSOC] ref-filter: pass get_object() return value to their callers

- Let populate_value(), get_ref_atom_value() and
- format_ref_array_item() get the return value of get_object()
- correctly. This can help us later let cat-file --batch get the
- correct error message and return value of get_object().
+ Since in the refactor of git cat-file --batch later,
+ oid_object_info_extended() in get_object() will be used to obtain
+ the info of an object with it's oid. When the object cannot be
+ obtained in the git repository, cat-file --batch expects to output
+ "<oid> missing" and continue the next oid query instead of letting
+ Git exit. In other error conditions, Git should exit normally. So we
+ can achieve this function by passing the return value of get_object().

+ Helped-by: Ævar Arnfjörð Bjarmason <[email protected]>

## ref-filter.c ##
-@@ ref-filter.c: static char *get_worktree_path(const struct used_atom *atom, const struct ref_ar
- static int populate_value(struct ref_array_item *ref, struct strbuf *err)
+@@ ref-filter.c: static int populate_value(struct ref_array_item *ref, struct strbuf *err)
{
struct object *obj;
-- int i;
-+ int i, ret = 0;
+ int i;
++ int ret = 0;
struct object_info empty = OBJECT_INFO_INIT;

CALLOC_ARRAY(ref->value, used_atom_cnt);
@@ ref-filter.c: static int populate_value(struct ref_array_item *ref, struct strbu
oi.oid = ref->objectname;
- if (get_object(ref, 0, &obj, &oi, err))
- return -1;
-+ if ((ret = get_object(ref, 0, &obj, &oi, err)))
++ ret = get_object(ref, 0, &obj, &oi, err);
++ if (ret)
+ return ret;

/*
* If there is no atom that wants to know about tagged
-@@ ref-filter.c: static int populate_value(struct ref_array_item *ref, struct strbuf *err)
- static int get_ref_atom_value(struct ref_array_item *ref, int atom,
+@@ ref-filter.c: static int get_ref_atom_value(struct ref_array_item *ref, int atom,
struct atom_value **v, struct strbuf *err)
{
-+ int ret = 0;
-+
if (!ref->value) {
- if (populate_value(ref, err))
- return -1;
-+ if ((ret = populate_value(ref, err)))
++ int ret = populate_value(ref, err);
++
++ if (ret)
+ return ret;
fill_missing_values(ref->value);
}
@@ ref-filter.c: int format_ref_array_item(struct ref_array_item *info,
}
}
if (*cp) {
-@@ ref-filter.c: int format_ref_array_item(struct ref_array_item *info,
- }
- strbuf_addbuf(final_buf, &state.stack->output);
- pop_stack_element(&state.stack);
-- return 0;
-+ return ret;
- }
-
- void pretty_print_ref(const char *name, const struct object_id *oid,
6: d2f2563eb76a ! 7: 8c1d683ec6e9 [GSOC] ref-filter: introduce free_array_item_internal() function
@@ Metadata
Author: ZheNing Hu <[email protected]>

## Commit message ##
- [GSOC] ref-filter: introduce free_array_item_internal() function
+ [GSOC] ref-filter: introduce free_ref_array_item_value() function

- Introduce free_array_item_internal() for freeing ref_array_item value.
+ When we use ref_array_item which is not dynamically allocated and
+ want to free the space of its member "value" after the end of use,
+ free_array_item() does not meet our needs, because it tries to free
+ ref_array_item itself and its member "symref".
+
+ Introduce free_ref_array_item_value() for freeing ref_array_item value.
It will be called internally by free_array_item(), and it will help
- cat-file --batch free ref_array_item's memory later.
+ cat-file --batch free ref_array_item's value memory later.

+ Helped-by: Junio C Hamano <[email protected]>

@@ ref-filter.c: static int ref_filter_handler(const char *refname, const struct ob

-/* Free memory allocated for a ref_array_item */
-static void free_array_item(struct ref_array_item *item)
-+void free_array_item_internal(struct ref_array_item *item)
++void free_ref_array_item_value(struct ref_array_item *item)
{
- free((char *)item->symref);
if (item->value) {
@@ ref-filter.c: static int ref_filter_handler(const char *refname, const struct ob
+static void free_array_item(struct ref_array_item *item)
+{
+ free((char *)item->symref);
-+ free_array_item_internal(item);
++ free_ref_array_item_value(item);
free(item);
}

@@ ref-filter.h: struct ref_format {
int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int type); /* Clear all memory allocated to ref_array */
void ref_array_clear(struct ref_array *array);
-+/* Free array item's value */
-+void free_array_item_internal(struct ref_array_item *item);
++/* Free ref_array_item's value */
++void free_ref_array_item_value(struct ref_array_item *item);
/* Used to verify if the given format is correct and to parse out the used atoms */ int verify_ref_format(struct ref_format *format);
/* Sort the given ref_array as per the ref_sorting provided */
7: 765337a46ab0 ! 8: bd534a266a40 [GSOC] cat-file: reuse ref-filter logic
@@ Commit message
verify_ref_format() to check atoms.
4. Use has_object_file() in batch_one_object() to check
whether the input object exists.
- 5. Use format_ref_array_item() in batch_object_write() to
+ 5. Let get_object() return 1 and print "<oid> missing" instead
+ of returning -1 and printing "missing object <oid> for <refname>",
+ this can help format_ref_array_item() just report that the
+ object is missing without letting Git exit.
+ 6. Use format_ref_array_item() in batch_object_write() to
get the formatted data corresponding to the object. If the
return value of format_ref_array_item() is equals to zero,
use batch_write() to print object data; else if the return
value less than zero, use die() to print the error message
and exit; else return value greater than zero, only print the
error message, but not exit.
- 6. Let get_object() return 1 and print "<oid> missing" instead
- of returning -1 and printing "missing object <oid> for <refname>",
- this can help format_ref_array_item() just report that the
- object is missing without letting Git exit.
+ 7. Use free_ref_array_item_value() to free ref_array_item's
+ value.

Most of the atoms in for-each-ref --format are now supported, such as %(tree), %(parent), %(author), %(tagger), %(if),
@@ builtin/cat-file.c: static void batch_write(struct batch_options *opt, const voi
+ printf("%s\n", err.buf);
+ fflush(stdout);
}
-+ free_array_item_internal(&item);
++ free_ref_array_item_value(&item);
+ strbuf_release(&err);
}

@@ builtin/cat-file.c: int cmd_cat_file(int argc, const char **argv, const char *pr

## ref-filter.c ##
@@ ref-filter.c: int verify_ref_format(struct ref_format *format)
- at = parse_ref_filter_atom(format, sp + 2, ep, &err);
if (at < 0)
die("%s", err.buf);
+
- if (used_atom[at].atom_type == ATOM_REST)
- die("this command reject atom %%(%.*s)", (int)(ep - sp - 2), sp + 2);
+ if ((!format->cat_file_mode && used_atom[at].atom_type == ATOM_REST) ||
@@ ref-filter.c: int verify_ref_format(struct ref_format *format)
+ used_atom[at].atom_type == ATOM_WORKTREEPATH)))
+ die(_("this command reject atom %%(%.*s)"), (int)(ep - sp - 2), sp + 2);

- if (format->quote_style && used_atom[at].atom_type == ATOM_RAW &&
- used_atom[at].u.raw_data.option == RAW_BARE)
+ if ((format->quote_style == QUOTE_PYTHON ||
+ format->quote_style == QUOTE_SHELL ||
@@ ref-filter.c: static int get_object(struct ref_array_item *ref, int deref, struct object **obj
}
if (oid_object_info_extended(the_repository, &oi->oid, &oi->info,
@@ t/t1006-cat-file.sh: test_expect_success 'cat-file --unordered works' '
+batch_test_atom() {
+ if test "$3" = "fail"
+ then
-+ test_expect_${4:-success} $PREREQ "basic atom: $1 $2 mast failed" "
++ test_expect_${4:-success} $PREREQ "basic atom: $1 $2 must fail" "
+ test_must_fail git cat-file --batch-check='$2' >bad <<-EOF
+ $1
+ EOF
8: 058b304686fd ! 9: b66ab0f2d569 [GSOC] cat-file: reuse err buf in batch_objet_write()
@@ Metadata
Author: ZheNing Hu <[email protected]>

## Commit message ##
- [GSOC] cat-file: reuse err buf in batch_objet_write()
+ [GSOC] cat-file: reuse err buf in batch_object_write()

Reuse the err buffer in batch_object_write(), as the
buffer scratch does. This will reduce the overhead
@@ builtin/cat-file.c: static void batch_write(struct batch_options *opt, const voi
+ printf("%s\n", err->buf);
fflush(stdout);
}
- free_array_item_internal(&item);
+ free_ref_array_item_value(&item);
- strbuf_release(&err);
}

  9:  cbf7d51933ea ! 10:  86ac3bcaecea [GSOC] cat-file: re-implement --textconv, --filters options
     @@ builtin/cat-file.c: static int batch_objects(struct batch_options *opt, const st
       	opt->format.format = format.buf;
      +	if (opt->cmdmode == 'c')
      +		opt->format.use_textconv = 1;
     -+	if (opt->cmdmode == 'w')
     ++	else if (opt->cmdmode == 'w')
      +		opt->format.use_filters = 1;
      +
       	if (verify_ref_format(&opt->format))
     @@ ref-filter.c: int verify_ref_format(struct ref_format *format)
      +		use_filters = format->use_filters;
      +		use_textconv = format->use_textconv;
      +
     - 		if (format->quote_style && used_atom[at].atom_type == ATOM_RAW &&
     - 		    used_atom[at].u.raw_data.option == RAW_BARE)
     - 			die(_("--format=%.*s cannot be used with"
     + 		if ((format->quote_style == QUOTE_PYTHON ||
     + 		     format->quote_style == QUOTE_SHELL ||
     + 		     format->quote_style == QUOTE_TCL) &&
      @@ ref-filter.c: static int get_object(struct ref_array_item *ref, int deref, struct object **obj
       {
       	/* parse_object_buffer() will set eaten to 0 if free() will be needed */
     @@ ref-filter.c: static int get_object(struct ref_array_item *ref, int deref, struc
      +		oi->info.typep = NULL;
      +		oi->info.contentp = temp_contentp;
      +
     -+		if (use_textconv) {
     ++		if (use_textconv && !ref->rest)
     ++			return strbuf_addf_ret(err, -1, _("missing path for '%s'"),
     ++					       oid_to_hex(&act_oi.oid));
     ++		if (use_textconv && oi->type == OBJ_BLOB) {
      +			act_oi = *oi;
     -+
     -+			if(!ref->rest)
     -+				return strbuf_addf_ret(err, -1, _("missing path for '%s'"),
     -+						       oid_to_hex(&act_oi.oid));
     -+			if (act_oi.type == OBJ_BLOB) {
     -+				if (textconv_object(the_repository,
     -+						    ref->rest, 0100644, &act_oi.oid,
     -+						    1, (char **)(&act_oi.content), &act_oi.size)) {
     -+					actual_oi = &act_oi;
     -+					goto success;
     -+				}
     ++			if (textconv_object(the_repository,
     ++					    ref->rest, 0100644, &act_oi.oid,
     ++					    1, (char **)(&act_oi.content), &act_oi.size)) {
     ++				actual_oi = &act_oi;
     ++				goto success;
      +			}
      +		}
       	}
     @@ ref-filter.c: static int get_object(struct ref_array_item *ref, int deref, struc

       	if (oi->info.contentp) {
      -		*obj = parse_object_buffer(the_repository, &oi->oid, oi->type, oi->size, oi->content, &eaten);
     -+		if (use_filters) {
     -+			if(!ref->rest)
     -+				return strbuf_addf_ret(err, -1, _("missing path for '%s'"),
     -+						       oid_to_hex(&oi->oid));
     -+			if (oi->type == OBJ_BLOB) {
     -+				struct strbuf strbuf = STRBUF_INIT;
     -+				struct checkout_metadata meta;
     -+				act_oi = *oi;
     ++		if (use_filters && !ref->rest)
     ++			return strbuf_addf_ret(err, -1, _("missing path for '%s'"),
     ++					       oid_to_hex(&oi->oid));
     ++		if (use_filters && oi->type == OBJ_BLOB) {
     ++			struct strbuf strbuf = STRBUF_INIT;
     ++			struct checkout_metadata meta;
     ++			act_oi = *oi;
      +
     -+				init_checkout_metadata(&meta, NULL, NULL, &act_oi.oid);
     -+				if (convert_to_working_tree(&the_index, ref->rest, act_oi.content, act_oi.size, &strbuf, &meta)) {
     -+					act_oi.size = strbuf.len;
     -+					act_oi.content = strbuf_detach(&strbuf, NULL);
     -+					actual_oi = &act_oi;
     -+				} else {
     -+					die("could not convert '%s' %s",
     -+					    oid_to_hex(&oi->oid), ref->rest);
     -+				}
     -+			}
     ++			init_checkout_metadata(&meta, NULL, NULL, &act_oi.oid);
     ++			if (!convert_to_working_tree(&the_index, ref->rest, act_oi.content, act_oi.size, &strbuf, &meta))
     ++				die("could not convert '%s' %s",
     ++					oid_to_hex(&oi->oid), ref->rest);
     ++			act_oi.size = strbuf.len;
     ++			act_oi.content = strbuf_detach(&strbuf, NULL);
     ++			actual_oi = &act_oi;
      +		}
      +
      +success:
     @@ ref-filter.h: struct ref_format {
       };
       
      -#define REF_FORMAT_INIT { NULL, NULL, 0, 0, 0, -1 }
     -+#define REF_FORMAT_INIT { NULL, NULL, 0, 0, 0, 0, 0, -1 }
     ++#define REF_FORMAT_INIT { .use_color = -1 }
       
       /*  Macros for checking --merged and --no-merged options */
       #define _OPT_MERGED_NO_MERGED(option, filter, h) \

gitgitgadget

From: ZheNing Hu <[email protected]>

Use non-const ref_format in *_atom_parser(), which can help us modify the members of ref_format in *_atom_parser().

---
 builtin/tag.c |  2 +-
 ref-filter.c  | 44 ++++++++++++++++++++++----------------------
 ref-filter.h  |  4 ++--
 3 files changed, 25 insertions(+), 25 deletions(-)

index 82fcfc098242..452558ec9575 100644
@@ -146,7 +146,7 @@ static int verify_tag(const char *name, const char *ref,
 		      const struct object_id *oid, void *cb_data)
 {
 	int flags;
-	const struct ref_format *format = cb_data;
+	struct ref_format *format = cb_data;
 	flags = GPG_VERIFY_VERBOSE;
 
 	if (format->format)

index baf72a718965..74fb423fc89f 100644
@@ -116,7 +116,7 @@ void ref_array_sort(struct ref_sorting *sort, struct ref_array *array);
 void ref_sorting_set_sort_flags_all(struct ref_sorting *sorting, unsigned int mask, int on);
 /*  Based on the given format and quote_style, fill the strbuf */
 int format_ref_array_item(struct ref_array_item *info,
-			  const struct ref_format *format,
+			  struct ref_format *format,
 			  struct strbuf *final_buf,
 			  struct strbuf *error_buf);
 /*  Parse a single sort specifier and add it to the list */

@@ -137,7 +137,7 @@ void setup_ref_filter_porcelain_msg(void);
  * name must be a fully qualified refname.
  */
 void pretty_print_ref(const char *name, const struct object_id *oid,
-		      const struct ref_format *format);
+		      struct ref_format *format);
 
 /*
  * Push a single ref onto the array; this can be used to construct your own

gitgitgadget

From: ZheNing Hu <[email protected]>

Add new formatting option %(raw), which will print the raw object data without any changes. It will help further to migrate all cat-file formatting logic from cat-file to ref-filter.

The raw data of blob, tree objects may contain '\0', but most of the logic in ref-filter depends on the output of the atom being text (specifically, no embedded NULs in it).

E.g. quote_formatting() use strbuf_addstr() or *._quote_buf() add the data to the buffer. The raw data of a tree object is 100644 one\0..., only the 100644 one will be added to the buffer, which is incorrect.

Therefore, we need to find a way to record the length of the atom_value's member s. Although strbuf can already record the string and its length, if we want to replace the type of atom_value's member s with strbuf, many places in ref-filter that are filled with dynamically allocated mermory in v->s are not easy to replace. At the same time, we need to check if v->s == NULL in populate_value(), and strbuf cannot easily distinguish NULL and empty strings, but c-style "const char *" can do it. So add a new member in struct atom_value: s_size, which can record raw object size, it can help us add raw object data to the buffer or compare two buffers which contain raw object data.

Beyond, --format=%(raw) cannot be used with --python, --shell, --tcl, --perl because if our binary raw data is passed to a variable in the host language, the host language may not support arbitrary binary data in the variables of its string type.

---
 Documentation/git-for-each-ref.txt |   9 ++
 ref-filter.c                       | 139 +++++++++++++++----
 t/t6300-for-each-ref.sh            | 216 +++++++++++++++++++++++++++++
 3 files changed, 337 insertions(+), 27 deletions(-)

index 2ae2478de706..7f1f0a1ca3b6 100644
@@ -235,6 +235,15 @@ and `date` to extract the named component.  For email fields (`authoremail`,
 without angle brackets, and `:localpart` to get the part before the `@` symbol
 out of the trimmed email.
 
+The raw data in an object is `raw`.
+
+raw:size::
+	The raw data size of the object.
+
+Note that `--format=%(raw)` can not be used with `--python`, `--shell`, `--tcl`,
+`--perl` because the host language may not support arbitrary binary data in the
+variables of its string type.
+
 The message in a commit or a tag object is `contents`, from which
 `contents:<part>` can be used to extract various parts out of: