Re: [PATCH] ref-filter: add "notes" atom

[Junio C Hamano <gitster@pobox.com>]

Kousik Sanagavarapu <five231003@gmail.com> writes:

> +static void grab_notes_values(struct atom_value *val, int deref,
> +			      struct object *obj)
> +{
> +	for (int i = 0; i < used_atom_cnt; i++) {
> +		struct used_atom *atom = &used_atom[i];
> +		const char *name = atom->name;
> +		struct atom_value *v = &val[i];
> +
> +		struct child_process cmd = CHILD_PROCESS_INIT;
> +		struct strbuf out = STRBUF_INIT;
> +		struct strbuf err = STRBUF_INIT;
> +
> +		if (atom->atom_type != ATOM_NOTES)
> +			continue;
> +
> +		if (!!deref != (*name == '*'))
> +			continue;
> +
> +		cmd.git_cmd = 1;
> +		strvec_push(&cmd.args, "notes");
> +		if (atom->u.notes_refname) {
> +			strvec_push(&cmd.args, "--ref");
> +			strvec_push(&cmd.args, atom->u.notes_refname);
> +		}
> +		strvec_push(&cmd.args, "show");
> +		strvec_push(&cmd.args, oid_to_hex(&obj->oid));
> +		if (pipe_command(&cmd, NULL, 0, &out, 0, &err, 0) < 0) {
> +			error(_("failed to run 'notes'"));
> +			v->s = xstrdup("");
> +			continue;
> +		}
> +		strbuf_rtrim(&out);
> +		v->s = strbuf_detach(&out, NULL);
> +
> +		strbuf_release(&err);
> +	}
> +}

I suspect that this was written to mimick what is done for describe.

The describe codepath has a (semi-)valid reason to fork out to a
subprocess, as computation of describe smudges the object flags of
in-core object database and it is not trivial to call into the
helper functions twice.

But showing notes for a single commit is merely an internal call to
get_note() away, so unless the note object is not a blob (which
should be absolutely rare), spawning a subprocess for each and every
ref tip feels a bit heavier than acceptable.  We'd probably need to
maintain a table of notes_trees, one per <note-ref> used as
%(notes:<note-ref>) in the format string, and init_notes() on them
while parsing the atoms, and in this codepath it would be a look-up
of notes_tree from the table based on the u.notes_refname by calling
get_note() to learn the object name, plus reading the object
contents into the v->s member when the note object is a blob (and
fallback the above code when it is not a blob, which is a rare-case,
if we really want to handle them).