Re: [PATCH] ref-filter: add "notes" atom
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 20, 2024, 23:51 UTC
- Message-ID
- <xmqqserlh3ak.fsf@gitster.g>
- In-Reply-To
- <Zz4Wr1YY7HxRARoc@five231003>
Kousik Sanagavarapu <five231003@gmail.com> writes:
Show 37 quoted lines
> +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).