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

> 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).