Re: [PATCH] notes: Use get_sha1_committish instead of read_ref in init_notes()

[Johan Herland <johan@herland.net>]

On Wed, Jun 17, 2015 at 5:18 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Mike Hommey <mh@glandium.org> writes:
> > I'm tempted to make init_notes itself do the check, based on the value
> > it is given for a "read_only" argument.
>
> Yeah, that would be one sensible way to go after making sure that
> everything goes thru this interface.

Agreed. Furthermore, consider adding the read_only flag (or however
you choose to encode it internally) to struct notes_tree, so that the
API functions that _manipulate_ notes trees can immediately bail out
when used on a read-only tree (i.e. we want them to fail as early as
possible).

> > On the other hand, some commands
> > do their ref resolving themselves already.
>
> Again, as long as they do not bypass the "read-only" safety you are
> suggesting to add to init_notes(), that is OK.

Agreed. An alternative to adding a simple read_only flag argument is
to modify the const char *notes_ref argument into two separate
arguments: const char *notes_treeish, and const char *update_ref,
where the latter should be NULL for read-only trees. That said,
currently the logic for actually updating notes ref lives outside the
notes.h API (see commit_notes() in notes-utils.h/c), so there might be
room for more consolidation/refactoring here...


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net