Re: [PATCH 5/2] push -s: receiving end

Johan Herland [off-list ref] writes:

quoted

+static void get_note_text(struct strbuf *buf, struct notes_tree *t,
+			  const unsigned char *object)
+{
+	const unsigned char *sha1 = get_note(t, object);
+	char *text;
+	unsigned long len;
+	enum object_type type;
+
+	if (!sha1)
+		return;
+	text = read_sha1_file(sha1, &type, &len);
+	if (text && len && type == OBJ_BLOB)
+		strbuf_add(buf, text, len);
+	free(text);
+}
+

What about adding this function to notes.h as a convenience to other 
users of the notes API?

I actually was hoping to hear that I do not have to do this "check
existing and concatenate", and should let the add_note() function
run its default combine_notes method to do the concatenation.

I found a few things I wasn't quite sure in the notes/notes-merge API, by
the way.

 - The combine_notes callback is run when a note is inserted into the
   in-core notes tree. I felt that this is way too early if you want to
   avoid racing with another process (and the patch tries to wrap
   create-notes-commit with lock-ref/write-ref-sha1 pair), but perhaps
   this is to deal with a case where the calling program calls add_notes()
   on the same object multiple times.

 - create_notes_commit() dies under a few conditions, but some callers
   that are recording advisory/optional notes might want to get an error
   and continue.

I think ideally this patch should handle notes like the following, which
is not quite how I coded it:

 - initialize in-core notes tree;

 - add bunch of notes, without regard to the existing ones, to in-core
   notes tree by calling add_notes();

 - lock the notes ref and read the "parent"; we may want to add "wait and
   retry for a few times until we get the lock" support at lockfile API
   level, but doing it at the application level would be fine.

 - call create-notes-commit, which in turn merges the in-core
   notes with what collides with those already in "parent" by
   calling the combine-notes callback, merges and re-balances
   the notes tree, and makes a notes commit object;

 - update the notes ref with that notes commit, releasing the lock on
   the ref.