Kousik Sanagavarapu <five231003@xxxxxxxxx> 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).
