[PATCHv13 00/30] git notes

[PATCHv13 00/30] git notes

[Johan Herland]

Hi,

Here is the 13th iteration of the git-notes series. Changes in this
iteration are as follows:

- Patch #1 has improved its commit message

- Patch #7 (the get_note() API function) improved its header file docs.

- Patch #12 (Builtin-ify git-notes) received code to add some details of the
  current object (the object whose notes are currently being edited)to the
  template text displayed in the editor. Similar details were originally
  present in git-notes.sh, but was lost in the initial builtin-ification.

- Patch #22 (the 'list' subcommand) has learned to fail (instead of segfault)
  when asked to list notes for an object that has none.

- Patch #23 has been inserted to provide --message/--file aliases to the -m/-F
  options. This improves consistency with other git commands.

- Patch #24 (was #23) has been redone to provide the "git notes add" semantics
  suggested by Junio in the discussion following the previous iteration.

- Patch #25 introduces the "git notes append" subcommand which takes on the
  semantics of "git notes add" from the previous iteration.

- Patch #26 deprecates the -m/-F options for "git notes edit", since their
  semantics now overlap with "git notes add -f", and the behaviour is more
  intuitively captured and better documented with "git notes add -f"

- Patch #27 refactors some code in builtin-notes, to allow mixing -m and -F
  options again. This was originally allowed in git-notes.sh, but was
  disabled by the builin-ification. The refactoring also helps with clear
  the way for the next patch...

- Patch #28 introduces the -c/-C options to "git notes add/append". This
  allows re-use of existing notes object when making new note.
  The option names and semantics were inspired by "git commit".

- Patch #29 refactors the argc and exit value handling of builtin-notes, in
  preparation for the next patch

- Patch #30 introduces the "git notes copy" subcommand for copying notes from
  one object to another. This will come in handy when we want to bring notes
  across a commit rewrite (such as rebase/cherry-pick/amend).


There are ongoing discussions on how to further develop and integrate the
notes feature. I try to summarize ideas below, although I don't plan to
put any more stuff into _this_ patch series:

- Better integration with rebase/amend/cherry-pick. Optionally bring
  notes across a commit rewrite. Controlled by command-line options
  and/or config variables. Junio says:
    I used to fix minor issues (styles, decl-after-stmt, etc.) using
    rebase-i long after running "am" in bulk, but these days I find
    myself going back to my "inbox" and fix them in MUA; this is
    only because I know these notes do not propagate across rebases
    and amends -- adjusting the workflow to the tool's limitation is
    not very good.

- Using multiple notes refs simultaneously. Junio says:
    The interface to tell tools to use which notes ref to use should be
    able to say "these refs", not just "this ref" i.e. GIT_NOTES_REF=a:b
    just like PATH=a:b:c...); I am fairly certain that we would want to
    store different kind of information in separate notes trees and
    aggregate them, as we gain experience with notes.
  When showing notes from multiple refs, we should also show from _which_
  note ref a given note originates.
  Of course, when _writing_ notes, only one ref should receive the created
  notes.

- Adding command-line options for specifying notes refs. Junio says:
    There should be an interface to tell tools to use which notes refs via
    command line options; "!alias" does not TAB-complete, and "git lgm"
    above doesn't, either. "git log --notes=notes/amlog --notes=notes/other"
    would probably be the way to go.
  Jakub suggests adding a --notes-ref="a:b:..." option, either to the git
  wrapper, or to all git subcommands that can do stuff with notes.
  Junio adds that the --notes-refs option should DWIM the given ref name
  (i.e. "foo" -> "refs/notes/foo").

- Add a "git notes grep" subcommand: Junio says:
    While reviewing the "inbox", I sometimes wonder if I applied a message
    to somewhere already, but there is no obvious way to grep in the notes
    tree and get the object name that a note is attached to.  Of course I
    know I can "git grep -c johan@herland.net notes/amlog" and it will give
    me something like:

      notes/amlog:65807ee697a28cb30b8ad38ebb8b84cebd3f255d:1
      notes/amlog:c789176020d6a008821e01af8b65f28abc138d4b:1

    but this won't scale and needs scripting to mechanize, once we start
    rebalancing the notes tree with different fan-outs.  The end user (me
    in this case) is interested in "set of objects that match this grep
    criteria", not "the pathnames the notes tree's implementation happens
    to use to store notes for them in the hierarchy".

- Add support for "^{notes}" (or "^@{}") to the rev-parse machinery, so
  that notes can be returned from e.g. "git cat-file --batch". This is
  somewhat complicated when combined with using multiple notes ref
  simultaneously (see above), but can be solved by using (Jakub's
  suggestions:) "^{notes:<namespace>}" (or "^@{<namespace>}").

- Merging notes trees. Junio suspects that we need a specialized merge
  strategy for notes trees. One example of such a strategy is to
  auto-concatenate conflicting notes, instead of forcing the user to
  edit away conflict markers. Note that such a solution should be fairly
  easy to implement on top of the current notes API, by loading both
  notes trees into the same struct notes_tree, while using combine_notes
  == combine_notes_concatenate().

- Matthieu Moy and Sverre Rabbelier suggests to add notes support to
  format-patch: Notes stored in e.g. the "refs/notes/format-patch" ref
  are added to the "comments section" (i.e. following the '---' separator)
  of generated patches. There is also some discussion on whether notes
  should be added as custom headers to the email generated by format-patch.

- Special handling of notes on fetch? Tags are autofollowed; does it make
  sense to do something similar for notes? Guiseppe notes:
    Tags have the idiosincracy of living in the same namespace regardless
    of where they come from, and I believe there a little too high a risk
    of conflicts to do this with notes too. It might make sense to add a
    default fetch of +refs/notes/*:refs/notes/remotes/<remote>/*, but the
    real question is what would pushing do, in case of conflicting notes.

- Note objects that are trees (instead of blobs). Peff originally proposed
  the idea of storing multiple notes for one object in a single notes ref
  by having the note object itself be a tree which contained note blobs
  (with well-defined names). Jon Seymour wanted to expand this idea to
  allow _arbitrary_ tree structures to be stored as notes. However, this
  will cause problems when trying to interpret notes in a consistent
  manner (i.e. how would "git log" display a tree of notes attached to a
  single commit). Problems may also arise when optimizing the notes tree
  structure itself. Peff instead suggests to add another level of
  indirection, and store the tree _SHA1_ in the note (instead of the tree
  object itself), and then teach a few tools (git-fsck, git-metadata?) to
  dereference the SHA1 when needed.

(- Rewrite fast-import notes code to use new notes API with support for
   non-notes.)


Have fun! :)

...Johan


Johan Herland (30):
  Minor cosmetic fixes to notes.c
  Notes API: get_commit_notes() -> format_note() + remove the commit restriction
  Add tests for checking correct handling of $GIT_NOTES_REF and core.notesRef
  Notes API: init_notes(): Initialize the notes tree from the given notes ref
  Notes API: add_note(): Add note objects to the internal notes tree structure
  Notes API: remove_note(): Remove note objects from the notes tree structure
  Notes API: get_note(): Return the note annotating the given object
  Notes API: for_each_note(): Traverse the entire notes tree with a callback
  Notes API: write_notes_tree(): Store the notes tree in the database
  Notes API: Allow multiple concurrent notes trees with new struct notes_tree
  Refactor notes concatenation into a flexible interface for combining notes
  Builtin-ify git-notes
  t3301: Verify successful annotation of non-commits
  t3305: Verify that adding many notes with git-notes triggers increased fanout
  Teach notes code to properly preserve non-notes in the notes tree
  Teach builtin-notes to remove empty notes
  builtin-notes: Add "remove" subcommand for removing existing notes
  t3305: Verify that removing notes triggers automatic fanout consolidation
  Notes API: prune_notes(): Prune notes that belong to non-existing objects
  builtin-notes: Add "prune" subcommand for removing notes for missing objects
  Documentation: Generalize git-notes docs to 'objects' instead of 'commits'
  builtin-notes: Add "list" subcommand for listing note objects
  builtin-notes: Add --message/--file aliases for -m/-F options
  builtin-notes: Add "add" subcommand for adding notes to objects
  builtin-notes: Add "append" subcommand for appending to note objects
  builtin-notes: Deprecate the -m/-F options for "git notes edit"
  builtin-notes: Refactor handling of -F option to allow combining -m and -F
  builtin-notes: Add -c/-C options for reusing notes
  builtin-notes: Misc. refactoring of argc and exit value handling
  builtin-notes: Add "copy" subcommand for copying notes between objects

 Documentation/git-notes.txt                   |   91 +++-
 Makefile                                      |    2 +-
 builtin-notes.c                               |  459 ++++++++++++++
 builtin.h                                     |    3 +
 git-notes.sh => contrib/examples/git-notes.sh |    0
 git.c                                         |    1 +
 notes.c                                       |  843 +++++++++++++++++++++----
 notes.h                                       |  196 ++++++-
 pretty.c                                      |    9 +-
 t/t3301-notes.sh                              |  510 ++++++++++++++-
 t/t3303-notes-subtrees.sh                     |   28 +-
 t/t3304-notes-mixed.sh                        |   36 +-
 t/t3305-notes-fanout.sh                       |   95 +++
 t/t3306-notes-prune.sh                        |   94 +++
 14 files changed, 2171 insertions(+), 196 deletions(-)
 create mode 100644 builtin-notes.c
 rename git-notes.sh => contrib/examples/git-notes.sh (100%)
 create mode 100755 t/t3305-notes-fanout.sh
 create mode 100755 t/t3306-notes-prune.sh

[PATCHv13 01/30] Minor cosmetic fixes to notes.c

[Johan Herland]

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/notes.c b/notes.c
index 023adce..47e38a1 100644
--- a/notes.c
+++ b/notes.c
@@ -1,7 +1,6 @@
 #include "cache.h"
 #include "commit.h"
 #include "notes.h"
-#include "refs.h"
 #include "utf8.h"
 #include "strbuf.h"
 #include "tree-walk.h"
@@ -93,7 +92,7 @@ static void **note_tree_search(struct int_node **tree,
 
 	i = GET_NIBBLE(*n, key_sha1);
 	p = (*tree)->a[i];
-	switch(GET_PTR_TYPE(p)) {
+	switch (GET_PTR_TYPE(p)) {
 	case PTR_TYPE_INTERNAL:
 		*tree = CLR_PTR_TYPE(p);
 		(*n)++;
@@ -195,7 +194,7 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
 
 	assert(GET_PTR_TYPE(entry) == 0); /* no type bits set */
 	l = (struct leaf_node *) CLR_PTR_TYPE(*p);
-	switch(GET_PTR_TYPE(*p)) {
+	switch (GET_PTR_TYPE(*p)) {
 	case PTR_TYPE_NULL:
 		assert(!*p);
 		*p = SET_PTR_TYPE(entry, type);
@@ -257,7 +256,7 @@ static void note_tree_free(struct int_node *tree)
 	unsigned int i;
 	for (i = 0; i < 16; i++) {
 		void *p = tree->a[i];
-		switch(GET_PTR_TYPE(p)) {
+		switch (GET_PTR_TYPE(p)) {
 		case PTR_TYPE_INTERNAL:
 			note_tree_free(CLR_PTR_TYPE(p));
 			/* fall through */
@@ -274,7 +273,7 @@ static void note_tree_free(struct int_node *tree)
  * - hex_len  - Length of above segment. Must be multiple of 2 between 0 and 40
  * - sha1     - Partial SHA1 value is written here
  * - sha1_len - Max #bytes to store in sha1, Must be >= hex_len / 2, and < 20
- * Returns -1 on error (invalid arguments or invalid SHA1 (not in hex format).
+ * Returns -1 on error (invalid arguments or invalid SHA1 (not in hex format)).
  * Otherwise, returns number of bytes written to sha1 (i.e. hex_len / 2).
  * Pads sha1 with NULs up to sha1_len (not included in returned length).
  */
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 02/30] Notes API: get_commit_notes() -> format_note() + remove the commit restriction

[Johan Herland]

There is really no reason why only commit objects can be annotated. By
changing the struct commit parameter to get_commit_notes() into a sha1 we
gain the ability to annotate any object type. To reflect this in the function
naming as well, we rename get_commit_notes() to format_note().

This patch also fixes comments and variable names throughout notes.c as a
consequence of the removal of the unnecessary 'commit' restriction.

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c  |   33 ++++++++++++++++-----------------
 notes.h  |   11 ++++++++++-
 pretty.c |    8 ++++----
 3 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/notes.c b/notes.c
index 47e38a1..4ee4fec 100644
--- a/notes.c
+++ b/notes.c
@@ -1,5 +1,4 @@
 #include "cache.h"
-#include "commit.h"
 #include "notes.h"
 #include "utf8.h"
 #include "strbuf.h"
@@ -24,10 +23,10 @@ struct int_node {
 /*
  * Leaf nodes come in two variants, note entries and subtree entries,
  * distinguished by the LSb of the leaf node pointer (see above).
- * As a note entry, the key is the SHA1 of the referenced commit, and the
+ * As a note entry, the key is the SHA1 of the referenced object, and the
  * value is the SHA1 of the note object.
  * As a subtree entry, the key is the prefix SHA1 (w/trailing NULs) of the
- * referenced commit, using the last byte of the key to store the length of
+ * referenced object, using the last byte of the key to store the length of
  * the prefix. The value is the SHA1 of the tree object containing the notes
  * subtree.
  */
@@ -210,7 +209,7 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
 				if (concatenate_notes(l->val_sha1,
 						entry->val_sha1))
 					die("failed to concatenate note %s "
-					    "into note %s for commit %s",
+					    "into note %s for object %s",
 					    sha1_to_hex(entry->val_sha1),
 					    sha1_to_hex(l->val_sha1),
 					    sha1_to_hex(l->key_sha1));
@@ -298,7 +297,7 @@ static int get_sha1_hex_segment(const char *hex, unsigned int hex_len,
 static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 		unsigned int n)
 {
-	unsigned char commit_sha1[20];
+	unsigned char object_sha1[20];
 	unsigned int prefix_len;
 	void *buf;
 	struct tree_desc desc;
@@ -311,23 +310,23 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 
 	prefix_len = subtree->key_sha1[19];
 	assert(prefix_len * 2 >= n);
-	memcpy(commit_sha1, subtree->key_sha1, prefix_len);
+	memcpy(object_sha1, subtree->key_sha1, prefix_len);
 	while (tree_entry(&desc, &entry)) {
 		int len = get_sha1_hex_segment(entry.path, strlen(entry.path),
-				commit_sha1 + prefix_len, 20 - prefix_len);
+				object_sha1 + prefix_len, 20 - prefix_len);
 		if (len < 0)
 			continue; /* entry.path is not a SHA1 sum. Skip */
 		len += prefix_len;
 
 		/*
-		 * If commit SHA1 is complete (len == 20), assume note object
-		 * If commit SHA1 is incomplete (len < 20), assume note subtree
+		 * If object SHA1 is complete (len == 20), assume note object
+		 * If object SHA1 is incomplete (len < 20), assume note subtree
 		 */
 		if (len <= 20) {
 			unsigned char type = PTR_TYPE_NOTE;
 			struct leaf_node *l = (struct leaf_node *)
 				xcalloc(sizeof(struct leaf_node), 1);
-			hashcpy(l->key_sha1, commit_sha1);
+			hashcpy(l->key_sha1, object_sha1);
 			hashcpy(l->val_sha1, entry.sha1);
 			if (len < 20) {
 				if (!S_ISDIR(entry.mode))
@@ -343,12 +342,12 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 
 static void initialize_notes(const char *notes_ref_name)
 {
-	unsigned char sha1[20], commit_sha1[20];
+	unsigned char sha1[20], object_sha1[20];
 	unsigned mode;
 	struct leaf_node root_tree;
 
-	if (!notes_ref_name || read_ref(notes_ref_name, commit_sha1) ||
-	    get_tree_entry(commit_sha1, "", sha1, &mode))
+	if (!notes_ref_name || read_ref(notes_ref_name, object_sha1) ||
+	    get_tree_entry(object_sha1, "", sha1, &mode))
 		return;
 
 	hashclr(root_tree.key_sha1);
@@ -356,9 +355,9 @@ static void initialize_notes(const char *notes_ref_name)
 	load_subtree(&root_tree, &root_node, 0);
 }
 
-static unsigned char *lookup_notes(const unsigned char *commit_sha1)
+static unsigned char *lookup_notes(const unsigned char *object_sha1)
 {
-	struct leaf_node *found = note_tree_find(&root_node, 0, commit_sha1);
+	struct leaf_node *found = note_tree_find(&root_node, 0, object_sha1);
 	if (found)
 		return found->val_sha1;
 	return NULL;
@@ -371,7 +370,7 @@ void free_notes(void)
 	initialized = 0;
 }
 
-void get_commit_notes(const struct commit *commit, struct strbuf *sb,
+void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 		const char *output_encoding, int flags)
 {
 	static const char utf8[] = "utf-8";
@@ -390,7 +389,7 @@ void get_commit_notes(const struct commit *commit, struct strbuf *sb,
 		initialized = 1;
 	}
 
-	sha1 = lookup_notes(commit->object.sha1);
+	sha1 = lookup_notes(object_sha1);
 	if (!sha1)
 		return;
 
diff --git a/notes.h b/notes.h
index a1421e3..d745ed1 100644
--- a/notes.h
+++ b/notes.h
@@ -4,10 +4,19 @@
 /* Free (and de-initialize) the internal notes tree structure */
 void free_notes(void);
 
+/* Flags controlling how notes are formatted */
 #define NOTES_SHOW_HEADER 1
 #define NOTES_INDENT 2
 
-void get_commit_notes(const struct commit *commit, struct strbuf *sb,
+/*
+ * Fill the given strbuf with the notes associated with the given object.
+ *
+ * If the internal notes structure is not initialized, it will be auto-
+ * initialized to the default value (see documentation for init_notes() above).
+ *
+ * 'flags' is a bitwise combination of the above formatting flags.
+ */
+void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 		const char *output_encoding, int flags);
 
 #endif
diff --git a/pretty.c b/pretty.c
index d493cad..076b918 100644
--- a/pretty.c
+++ b/pretty.c
@@ -775,8 +775,8 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 		}
 		return 0;	/* unknown %g placeholder */
 	case 'N':
-		get_commit_notes(commit, sb, git_log_output_encoding ?
-			     git_log_output_encoding : git_commit_encoding, 0);
+		format_note(commit->object.sha1, sb, git_log_output_encoding ?
+			    git_log_output_encoding : git_commit_encoding, 0);
 		return 1;
 	}
 
@@ -1095,8 +1095,8 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 		strbuf_addch(sb, '\n');
 
 	if (context->show_notes)
-		get_commit_notes(commit, sb, encoding,
-				 NOTES_SHOW_HEADER | NOTES_INDENT);
+		format_note(commit->object.sha1, sb, encoding,
+			    NOTES_SHOW_HEADER | NOTES_INDENT);
 
 	free(reencoded);
 }
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 03/30] Add tests for checking correct handling of $GIT_NOTES_REF and core.notesRef

[Johan Herland]

Signed-off-by: Johan Herland <johan@herland.net>
---
 t/t3301-notes.sh |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 5d9604b..18aad53 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -206,4 +206,52 @@ do
 	'
 done
 
+test_expect_success 'create other note on a different notes ref (setup)' '
+	: > a5 &&
+	git add a5 &&
+	test_tick &&
+	git commit -m 5th &&
+	GIT_NOTES_REF="refs/notes/other" git notes edit -m "other note"
+'
+
+cat > expect-other << EOF
+commit bd1753200303d0a0344be813e504253b3d98e74d
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:17:13 2005 -0700
+
+    5th
+
+Notes:
+    other note
+EOF
+
+cat > expect-not-other << EOF
+commit bd1753200303d0a0344be813e504253b3d98e74d
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:17:13 2005 -0700
+
+    5th
+EOF
+
+test_expect_success 'Do not show note on other ref by default' '
+	git log -1 > output &&
+	test_cmp expect-not-other output
+'
+
+test_expect_success 'Do show note when ref is given in GIT_NOTES_REF' '
+	GIT_NOTES_REF="refs/notes/other" git log -1 > output &&
+	test_cmp expect-other output
+'
+
+test_expect_success 'Do show note when ref is given in core.notesRef config' '
+	git config core.notesRef "refs/notes/other" &&
+	git log -1 > output &&
+	test_cmp expect-other output
+'
+
+test_expect_success 'Do not show note when core.notesRef is overridden' '
+	GIT_NOTES_REF="refs/notes/wrong" git log -1 > output &&
+	test_cmp expect-not-other output
+'
+
 test_done
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 04/30] Notes API: init_notes(): Initialize the notes tree from the given notes ref

[Johan Herland]

Created by a simple refactoring of initialize_notes().

Also add a new 'flags' parameter, which is a bitwise combination of notes
initialization flags. For now, there is only one flag - NOTES_INIT_EMPTY -
which indicates that the notes tree should not auto-load the contents of
the given (or default) notes ref, but rather should leave the notes tree
initialized to an empty state. This will become useful in the future when
manipulating the notes tree through the notes API.

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |   30 ++++++++++++++++++------------
 notes.h |   20 ++++++++++++++++++++
 2 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/notes.c b/notes.c
index 4ee4fec..3f4ae35 100644
--- a/notes.c
+++ b/notes.c
@@ -340,15 +340,28 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 	free(buf);
 }
 
-static void initialize_notes(const char *notes_ref_name)
+void init_notes(const char *notes_ref, int flags)
 {
 	unsigned char sha1[20], object_sha1[20];
 	unsigned mode;
 	struct leaf_node root_tree;
 
-	if (!notes_ref_name || read_ref(notes_ref_name, object_sha1) ||
-	    get_tree_entry(object_sha1, "", sha1, &mode))
+	assert(!initialized);
+	initialized = 1;
+
+	if (!notes_ref)
+		notes_ref = getenv(GIT_NOTES_REF_ENVIRONMENT);
+	if (!notes_ref)
+		notes_ref = notes_ref_name; /* value of core.notesRef config */
+	if (!notes_ref)
+		notes_ref = GIT_NOTES_DEFAULT_REF;
+
+	if (flags & NOTES_INIT_EMPTY || !notes_ref ||
+	    read_ref(notes_ref, object_sha1))
 		return;
+	if (get_tree_entry(object_sha1, "", sha1, &mode))
+		die("Failed to read notes tree referenced by %s (%s)",
+		    notes_ref, object_sha1);
 
 	hashclr(root_tree.key_sha1);
 	hashcpy(root_tree.val_sha1, sha1);
@@ -379,15 +392,8 @@ void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 	unsigned long linelen, msglen;
 	enum object_type type;
 
-	if (!initialized) {
-		const char *env = getenv(GIT_NOTES_REF_ENVIRONMENT);
-		if (env)
-			notes_ref_name = getenv(GIT_NOTES_REF_ENVIRONMENT);
-		else if (!notes_ref_name)
-			notes_ref_name = GIT_NOTES_DEFAULT_REF;
-		initialize_notes(notes_ref_name);
-		initialized = 1;
-	}
+	if (!initialized)
+		init_notes(NULL, 0);
 
 	sha1 = lookup_notes(object_sha1);
 	if (!sha1)
diff --git a/notes.h b/notes.h
index d745ed1..6b52799 100644
--- a/notes.h
+++ b/notes.h
@@ -1,6 +1,26 @@
 #ifndef NOTES_H
 #define NOTES_H
 
+/*
+ * Flags controlling behaviour of notes tree initialization
+ *
+ * Default behaviour is to initialize the notes tree from the tree object
+ * specified by the given (or default) notes ref.
+ */
+#define NOTES_INIT_EMPTY 1
+
+/*
+ * Initialize internal notes tree structure with the notes tree at the given
+ * ref. If given ref is NULL, the value of the $GIT_NOTES_REF environment
+ * variable is used, and if that is missing, the default notes ref is used
+ * ("refs/notes/commits").
+ *
+ * If you need to re-intialize the internal notes tree structure (e.g. loading
+ * from a different notes ref), please first de-initialize the current notes
+ * tree by calling free_notes().
+ */
+void init_notes(const char *notes_ref, int flags);
+
 /* Free (and de-initialize) the internal notes tree structure */
 void free_notes(void);
 
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 05/30] Notes API: add_note(): Add note objects to the internal notes tree structure

[Johan Herland]

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |   11 +++++++++++
 notes.h |    4 ++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/notes.c b/notes.c
index 3f4ae35..2c0d14e 100644
--- a/notes.c
+++ b/notes.c
@@ -368,6 +368,17 @@ void init_notes(const char *notes_ref, int flags)
 	load_subtree(&root_tree, &root_node, 0);
 }
 
+void add_note(const unsigned char *object_sha1, const unsigned char *note_sha1)
+{
+	struct leaf_node *l;
+
+	assert(initialized);
+	l = (struct leaf_node *) xmalloc(sizeof(struct leaf_node));
+	hashcpy(l->key_sha1, object_sha1);
+	hashcpy(l->val_sha1, note_sha1);
+	note_tree_insert(&root_node, 0, l, PTR_TYPE_NOTE);
+}
+
 static unsigned char *lookup_notes(const unsigned char *object_sha1)
 {
 	struct leaf_node *found = note_tree_find(&root_node, 0, object_sha1);
diff --git a/notes.h b/notes.h
index 6b52799..5f22852 100644
--- a/notes.h
+++ b/notes.h
@@ -21,6 +21,10 @@
  */
 void init_notes(const char *notes_ref, int flags);
 
+/* Add the given note object to the internal notes tree structure */
+void add_note(const unsigned char *object_sha1,
+		const unsigned char *note_sha1);
+
 /* Free (and de-initialize) the internal notes tree structure */
 void free_notes(void);
 
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 06/30] Notes API: remove_note(): Remove note objects from the notes tree structure

[Johan Herland]

This includes adding internal functions for maintaining a healthy notes tree
structure after removing individual notes.

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |   85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 notes.h |    3 ++
 2 files changed, 87 insertions(+), 1 deletions(-)

diff --git a/notes.c b/notes.c
index 2c0d14e..2e82d71 100644
--- a/notes.c
+++ b/notes.c
@@ -44,7 +44,7 @@ struct leaf_node {
 #define CLR_PTR_TYPE(ptr)       ((void *) ((uintptr_t) (ptr) & ~3))
 #define SET_PTR_TYPE(ptr, type) ((void *) ((uintptr_t) (ptr) | (type)))
 
-#define GET_NIBBLE(n, sha1) (((sha1[n >> 1]) >> ((~n & 0x01) << 2)) & 0x0f)
+#define GET_NIBBLE(n, sha1) (((sha1[(n) >> 1]) >> ((~(n) & 0x01) << 2)) & 0x0f)
 
 #define SUBTREE_SHA1_PREFIXCMP(key_sha1, subtree_sha1) \
 	(memcmp(key_sha1, subtree_sha1, subtree_sha1[19]))
@@ -249,6 +249,79 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
 	note_tree_insert(new_node, n + 1, entry, type);
 }
 
+/*
+ * How to consolidate an int_node:
+ * If there are > 1 non-NULL entries, give up and return non-zero.
+ * Otherwise replace the int_node at the given index in the given parent node
+ * with the only entry (or a NULL entry if no entries) from the given tree,
+ * and return 0.
+ */
+static int note_tree_consolidate(struct int_node *tree,
+	struct int_node *parent, unsigned char index)
+{
+	unsigned int i;
+	void *p = NULL;
+
+	assert(tree && parent);
+	assert(CLR_PTR_TYPE(parent->a[index]) == tree);
+
+	for (i = 0; i < 16; i++) {
+		if (GET_PTR_TYPE(tree->a[i]) != PTR_TYPE_NULL) {
+			if (p) /* more than one entry */
+				return -2;
+			p = tree->a[i];
+		}
+	}
+
+	/* replace tree with p in parent[index] */
+	parent->a[index] = p;
+	free(tree);
+	return 0;
+}
+
+/*
+ * To remove a leaf_node:
+ * Search to the tree location appropriate for the given leaf_node's key:
+ * - If location does not hold a matching entry, abort and do nothing.
+ * - Replace the matching leaf_node with a NULL entry (and free the leaf_node).
+ * - Consolidate int_nodes repeatedly, while walking up the tree towards root.
+ */
+static void note_tree_remove(struct int_node *tree, unsigned char n,
+		struct leaf_node *entry)
+{
+	struct leaf_node *l;
+	struct int_node *parent_stack[20];
+	unsigned char i, j;
+	void **p = note_tree_search(&tree, &n, entry->key_sha1);
+
+	assert(GET_PTR_TYPE(entry) == 0); /* no type bits set */
+	if (GET_PTR_TYPE(*p) != PTR_TYPE_NOTE)
+		return; /* type mismatch, nothing to remove */
+	l = (struct leaf_node *) CLR_PTR_TYPE(*p);
+	if (hashcmp(l->key_sha1, entry->key_sha1))
+		return; /* key mismatch, nothing to remove */
+
+	/* we have found a matching entry */
+	free(l);
+	*p = SET_PTR_TYPE(NULL, PTR_TYPE_NULL);
+
+	/* consolidate this tree level, and parent levels, if possible */
+	if (!n)
+		return; /* cannot consolidate top level */
+	/* first, build stack of ancestors between root and current node */
+	parent_stack[0] = &root_node;
+	for (i = 0; i < n; i++) {
+		j = GET_NIBBLE(i, entry->key_sha1);
+		parent_stack[i + 1] = CLR_PTR_TYPE(parent_stack[i]->a[j]);
+	}
+	assert(i == n && parent_stack[i] == tree);
+	/* next, unwind stack until note_tree_consolidate() is done */
+	while (i > 0 &&
+	       !note_tree_consolidate(parent_stack[i], parent_stack[i - 1],
+				      GET_NIBBLE(i - 1, entry->key_sha1)))
+		i--;
+}
+
 /* Free the entire notes data contained in the given tree */
 static void note_tree_free(struct int_node *tree)
 {
@@ -379,6 +452,16 @@ void add_note(const unsigned char *object_sha1, const unsigned char *note_sha1)
 	note_tree_insert(&root_node, 0, l, PTR_TYPE_NOTE);
 }
 
+void remove_note(const unsigned char *object_sha1)
+{
+	struct leaf_node l;
+
+	assert(initialized);
+	hashcpy(l.key_sha1, object_sha1);
+	hashclr(l.val_sha1);
+	return note_tree_remove(&root_node, 0, &l);
+}
+
 static unsigned char *lookup_notes(const unsigned char *object_sha1)
 {
 	struct leaf_node *found = note_tree_find(&root_node, 0, object_sha1);
diff --git a/notes.h b/notes.h
index 5f22852..9e66855 100644
--- a/notes.h
+++ b/notes.h
@@ -25,6 +25,9 @@ void init_notes(const char *notes_ref, int flags);
 void add_note(const unsigned char *object_sha1,
 		const unsigned char *note_sha1);
 
+/* Remove the given note object from the internal notes tree structure */
+void remove_note(const unsigned char *object_sha1);
+
 /* Free (and de-initialize) the internal notes tree structure */
 void free_notes(void);
 
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 07/30] Notes API: get_note(): Return the note annotating the given object

[Johan Herland]

Created by a simple cleanup and rename of lookup_notes().

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |   15 ++++++++-------
 notes.h |    7 +++++++
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/notes.c b/notes.c
index 2e82d71..a0a85b4 100644
--- a/notes.c
+++ b/notes.c
@@ -462,12 +462,13 @@ void remove_note(const unsigned char *object_sha1)
 	return note_tree_remove(&root_node, 0, &l);
 }
 
-static unsigned char *lookup_notes(const unsigned char *object_sha1)
+const unsigned char *get_note(const unsigned char *object_sha1)
 {
-	struct leaf_node *found = note_tree_find(&root_node, 0, object_sha1);
-	if (found)
-		return found->val_sha1;
-	return NULL;
+	struct leaf_node *found;
+
+	assert(initialized);
+	found = note_tree_find(&root_node, 0, object_sha1);
+	return found ? found->val_sha1 : NULL;
 }
 
 void free_notes(void)
@@ -481,7 +482,7 @@ void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 		const char *output_encoding, int flags)
 {
 	static const char utf8[] = "utf-8";
-	unsigned char *sha1;
+	const unsigned char *sha1;
 	char *msg, *msg_p;
 	unsigned long linelen, msglen;
 	enum object_type type;
@@ -489,7 +490,7 @@ void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 	if (!initialized)
 		init_notes(NULL, 0);
 
-	sha1 = lookup_notes(object_sha1);
+	sha1 = get_note(object_sha1);
 	if (!sha1)
 		return;
 
diff --git a/notes.h b/notes.h
index 9e66855..0041aec 100644
--- a/notes.h
+++ b/notes.h
@@ -28,6 +28,13 @@ void add_note(const unsigned char *object_sha1,
 /* Remove the given note object from the internal notes tree structure */
 void remove_note(const unsigned char *object_sha1);
 
+/*
+ * Get the note object SHA1 containing the note data for the given object
+ *
+ * Return NULL if the given object has no notes.
+ */
+const unsigned char *get_note(const unsigned char *object_sha1);
+
 /* Free (and de-initialize) the internal notes tree structure */
 void free_notes(void);
 
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 08/30] Notes API: for_each_note(): Traverse the entire notes tree with a callback

[Johan Herland]

This includes a first attempt at creating an optimal fanout scheme (which
is calculated on-the-fly, while traversing).

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |  133 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 notes.h |   47 ++++++++++++++++++++++
 2 files changed, 180 insertions(+), 0 deletions(-)

diff --git a/notes.c b/notes.c
index a0a85b4..eabd6f3 100644
--- a/notes.c
+++ b/notes.c
@@ -413,6 +413,133 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 	free(buf);
 }
 
+/*
+ * Determine optimal on-disk fanout for this part of the notes tree
+ *
+ * Given a (sub)tree and the level in the internal tree structure, determine
+ * whether or not the given existing fanout should be expanded for this
+ * (sub)tree.
+ *
+ * Values of the 'fanout' variable:
+ * - 0: No fanout (all notes are stored directly in the root notes tree)
+ * - 1: 2/38 fanout
+ * - 2: 2/2/36 fanout
+ * - 3: 2/2/2/34 fanout
+ * etc.
+ */
+static unsigned char determine_fanout(struct int_node *tree, unsigned char n,
+		unsigned char fanout)
+{
+	/*
+	 * The following is a simple heuristic that works well in practice:
+	 * For each even-numbered 16-tree level (remember that each on-disk
+	 * fanout level corresponds to _two_ 16-tree levels), peek at all 16
+	 * entries at that tree level. If all of them are either int_nodes or
+	 * subtree entries, then there are likely plenty of notes below this
+	 * level, so we return an incremented fanout.
+	 */
+	unsigned int i;
+	if ((n % 2) || (n > 2 * fanout))
+		return fanout;
+	for (i = 0; i < 16; i++) {
+		switch (GET_PTR_TYPE(tree->a[i])) {
+		case PTR_TYPE_SUBTREE:
+		case PTR_TYPE_INTERNAL:
+			continue;
+		default:
+			return fanout;
+		}
+	}
+	return fanout + 1;
+}
+
+static void construct_path_with_fanout(const unsigned char *sha1,
+		unsigned char fanout, char *path)
+{
+	unsigned int i = 0, j = 0;
+	const char *hex_sha1 = sha1_to_hex(sha1);
+	assert(fanout < 20);
+	while (fanout) {
+		path[i++] = hex_sha1[j++];
+		path[i++] = hex_sha1[j++];
+		path[i++] = '/';
+		fanout--;
+	}
+	strcpy(path + i, hex_sha1 + j);
+}
+
+static int for_each_note_helper(struct int_node *tree, unsigned char n,
+		unsigned char fanout, int flags, each_note_fn fn,
+		void *cb_data)
+{
+	unsigned int i;
+	void *p;
+	int ret = 0;
+	struct leaf_node *l;
+	static char path[40 + 19 + 1];  /* hex SHA1 + 19 * '/' + NUL */
+
+	fanout = determine_fanout(tree, n, fanout);
+	for (i = 0; i < 16; i++) {
+redo:
+		p = tree->a[i];
+		switch (GET_PTR_TYPE(p)) {
+		case PTR_TYPE_INTERNAL:
+			/* recurse into int_node */
+			ret = for_each_note_helper(CLR_PTR_TYPE(p), n + 1,
+				fanout, flags, fn, cb_data);
+			break;
+		case PTR_TYPE_SUBTREE:
+			l = (struct leaf_node *) CLR_PTR_TYPE(p);
+			/*
+			 * Subtree entries in the note tree represent parts of
+			 * the note tree that have not yet been explored. There
+			 * is a direct relationship between subtree entries at
+			 * level 'n' in the tree, and the 'fanout' variable:
+			 * Subtree entries at level 'n <= 2 * fanout' should be
+			 * preserved, since they correspond exactly to a fanout
+			 * directory in the on-disk structure. However, subtree
+			 * entries at level 'n > 2 * fanout' should NOT be
+			 * preserved, but rather consolidated into the above
+			 * notes tree level. We achieve this by unconditionally
+			 * unpacking subtree entries that exist below the
+			 * threshold level at 'n = 2 * fanout'.
+			 */
+			if (n <= 2 * fanout &&
+			    flags & FOR_EACH_NOTE_YIELD_SUBTREES) {
+				/* invoke callback with subtree */
+				unsigned int path_len =
+					l->key_sha1[19] * 2 + fanout;
+				assert(path_len < 40 + 19);
+				construct_path_with_fanout(l->key_sha1, fanout,
+							   path);
+				/* Create trailing slash, if needed */
+				if (path[path_len - 1] != '/')
+					path[path_len++] = '/';
+				path[path_len] = '\0';
+				ret = fn(l->key_sha1, l->val_sha1, path,
+					 cb_data);
+			}
+			if (n > fanout * 2 ||
+			    !(flags & FOR_EACH_NOTE_DONT_UNPACK_SUBTREES)) {
+				/* unpack subtree and resume traversal */
+				tree->a[i] = NULL;
+				load_subtree(l, tree, n);
+				free(l);
+				goto redo;
+			}
+			break;
+		case PTR_TYPE_NOTE:
+			l = (struct leaf_node *) CLR_PTR_TYPE(p);
+			construct_path_with_fanout(l->key_sha1, fanout, path);
+			ret = fn(l->key_sha1, l->val_sha1, path, cb_data);
+			break;
+		}
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
 void init_notes(const char *notes_ref, int flags)
 {
 	unsigned char sha1[20], object_sha1[20];
@@ -471,6 +598,12 @@ const unsigned char *get_note(const unsigned char *object_sha1)
 	return found ? found->val_sha1 : NULL;
 }
 
+int for_each_note(int flags, each_note_fn fn, void *cb_data)
+{
+	assert(initialized);
+	return for_each_note_helper(&root_node, 0, 0, flags, fn, cb_data);
+}
+
 void free_notes(void)
 {
 	note_tree_free(&root_node);
diff --git a/notes.h b/notes.h
index 0041aec..2131912 100644
--- a/notes.h
+++ b/notes.h
@@ -35,6 +35,53 @@ void remove_note(const unsigned char *object_sha1);
  */
 const unsigned char *get_note(const unsigned char *object_sha1);
 
+/*
+ * Flags controlling behaviour of for_each_note()
+ *
+ * Default behaviour of for_each_note() is to traverse every single note object
+ * in the notes tree, unpacking subtree entries along the way.
+ * The following flags can be used to alter the default behaviour:
+ *
+ * - DONT_UNPACK_SUBTREES causes for_each_note() NOT to unpack and recurse into
+ *   subtree entries while traversing the notes tree. This causes notes within
+ *   those subtrees NOT to be passed to the callback. Use this flag if you
+ *   don't want to traverse _all_ notes, but only want to traverse the parts
+ *   of the notes tree that have already been unpacked (this includes at least
+ *   all notes that have been added/changed).
+ *
+ * - YIELD_SUBTREES causes any subtree entries that are encountered to be
+ *   passed to the callback, before recursing into them. Subtree entries are
+ *   not note objects, but represent intermediate directories in the notes
+ *   tree. When passed to the callback, subtree entries will have a trailing
+ *   slash in their path, which the callback may use to differentiate between
+ *   note entries and subtree entries. Note that already-unpacked subtree
+ *   entries are not part of the notes tree, and will therefore not be yielded.
+ *   If this flag is used together with DONT_UNPACK_SUBTREES, for_each_note()
+ *   will yield the subtree entry, but not recurse into it.
+ */
+#define FOR_EACH_NOTE_DONT_UNPACK_SUBTREES 1
+#define FOR_EACH_NOTE_YIELD_SUBTREES 2
+
+/*
+ * Invoke the specified callback function for each note
+ *
+ * If the callback returns nonzero, the note walk is aborted, and the return
+ * value from the callback is returned from for_each_note(). Hence, a zero
+ * return value from for_each_note() indicates that all notes were walked
+ * successfully.
+ *
+ * IMPORTANT: The callback function is NOT allowed to change the notes tree.
+ * In other words, the following functions can NOT be invoked (on the current
+ * notes tree) from within the callback:
+ * - add_note()
+ * - remove_note()
+ * - free_notes()
+ */
+typedef int each_note_fn(const unsigned char *object_sha1,
+		const unsigned char *note_sha1, char *note_path,
+		void *cb_data);
+int for_each_note(int flags, each_note_fn fn, void *cb_data);
+
 /* Free (and de-initialize) the internal notes tree structure */
 void free_notes(void);
 
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 09/30] Notes API: write_notes_tree(): Store the notes tree in the database

[Johan Herland]

Uses for_each_note() to traverse the notes tree, and produces tree
objects on the fly representing the "on-disk" version of the notes
tree with appropriate fanout.

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |  145 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 notes.h |   38 +++++++++++++++-
 2 files changed, 180 insertions(+), 3 deletions(-)

diff --git a/notes.c b/notes.c
index eabd6f3..b576f7e 100644
--- a/notes.c
+++ b/notes.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "notes.h"
+#include "tree.h"
 #include "utf8.h"
 #include "strbuf.h"
 #include "tree-walk.h"
@@ -540,6 +541,126 @@ redo:
 	return 0;
 }
 
+struct tree_write_stack {
+	struct tree_write_stack *next;
+	struct strbuf buf;
+	char path[2]; /* path to subtree in next, if any */
+};
+
+static inline int matches_tree_write_stack(struct tree_write_stack *tws,
+		const char *full_path)
+{
+	return  full_path[0] == tws->path[0] &&
+		full_path[1] == tws->path[1] &&
+		full_path[2] == '/';
+}
+
+static void write_tree_entry(struct strbuf *buf, unsigned int mode,
+		const char *path, unsigned int path_len, const
+		unsigned char *sha1)
+{
+		strbuf_addf(buf, "%06o %.*s%c", mode, path_len, path, '\0');
+		strbuf_add(buf, sha1, 20);
+}
+
+static void tree_write_stack_init_subtree(struct tree_write_stack *tws,
+		const char *path)
+{
+	struct tree_write_stack *n;
+	assert(!tws->next);
+	assert(tws->path[0] == '\0' && tws->path[1] == '\0');
+	n = (struct tree_write_stack *)
+		xmalloc(sizeof(struct tree_write_stack));
+	n->next = NULL;
+	strbuf_init(&n->buf, 256 * (32 + 40)); /* assume 256 entries per tree */
+	n->path[0] = n->path[1] = '\0';
+	tws->next = n;
+	tws->path[0] = path[0];
+	tws->path[1] = path[1];
+}
+
+static int tree_write_stack_finish_subtree(struct tree_write_stack *tws)
+{
+	int ret;
+	struct tree_write_stack *n = tws->next;
+	unsigned char s[20];
+	if (n) {
+		ret = tree_write_stack_finish_subtree(n);
+		if (ret)
+			return ret;
+		ret = write_sha1_file(n->buf.buf, n->buf.len, tree_type, s);
+		if (ret)
+			return ret;
+		strbuf_release(&n->buf);
+		free(n);
+		tws->next = NULL;
+		write_tree_entry(&tws->buf, 040000, tws->path, 2, s);
+		tws->path[0] = tws->path[1] = '\0';
+	}
+	return 0;
+}
+
+static int write_each_note_helper(struct tree_write_stack *tws,
+		const char *path, unsigned int mode,
+		const unsigned char *sha1)
+{
+	size_t path_len = strlen(path);
+	unsigned int n = 0;
+	int ret;
+
+	/* Determine common part of tree write stack */
+	while (tws && 3 * n < path_len &&
+	       matches_tree_write_stack(tws, path + 3 * n)) {
+		n++;
+		tws = tws->next;
+	}
+
+	/* tws point to last matching tree_write_stack entry */
+	ret = tree_write_stack_finish_subtree(tws);
+	if (ret)
+		return ret;
+
+	/* Start subtrees needed to satisfy path */
+	while (3 * n + 2 < path_len && path[3 * n + 2] == '/') {
+		tree_write_stack_init_subtree(tws, path + 3 * n);
+		n++;
+		tws = tws->next;
+	}
+
+	/* There should be no more directory components in the given path */
+	assert(memchr(path + 3 * n, '/', path_len - (3 * n)) == NULL);
+
+	/* Finally add given entry to the current tree object */
+	write_tree_entry(&tws->buf, mode, path + 3 * n, path_len - (3 * n),
+			 sha1);
+
+	return 0;
+}
+
+struct write_each_note_data {
+	struct tree_write_stack *root;
+};
+
+static int write_each_note(const unsigned char *object_sha1,
+		const unsigned char *note_sha1, char *note_path,
+		void *cb_data)
+{
+	struct write_each_note_data *d =
+		(struct write_each_note_data *) cb_data;
+	size_t note_path_len = strlen(note_path);
+	unsigned int mode = 0100644;
+
+	if (note_path[note_path_len - 1] == '/') {
+		/* subtree entry */
+		note_path_len--;
+		note_path[note_path_len] = '\0';
+		mode = 040000;
+	}
+	assert(note_path_len <= 40 + 19);
+
+	return write_each_note_helper(d->root, note_path, mode, note_sha1);
+}
+
 void init_notes(const char *notes_ref, int flags)
 {
 	unsigned char sha1[20], object_sha1[20];
@@ -604,6 +725,30 @@ int for_each_note(int flags, each_note_fn fn, void *cb_data)
 	return for_each_note_helper(&root_node, 0, 0, flags, fn, cb_data);
 }
 
+int write_notes_tree(unsigned char *result)
+{
+	struct tree_write_stack root;
+	struct write_each_note_data cb_data;
+	int ret;
+
+	assert(initialized);
+
+	/* Prepare for traversal of current notes tree */
+	root.next = NULL; /* last forward entry in list is grounded */
+	strbuf_init(&root.buf, 256 * (32 + 40)); /* assume 256 entries */
+	root.path[0] = root.path[1] = '\0';
+	cb_data.root = &root;
+
+	/* Write tree objects representing current notes tree */
+	ret = for_each_note(FOR_EACH_NOTE_DONT_UNPACK_SUBTREES |
+				FOR_EACH_NOTE_YIELD_SUBTREES,
+			write_each_note, &cb_data) ||
+		tree_write_stack_finish_subtree(&root) ||
+		write_sha1_file(root.buf.buf, root.buf.len, tree_type, result);
+	strbuf_release(&root.buf);
+	return ret;
+}
+
 void free_notes(void)
 {
 	note_tree_free(&root_node);
diff --git a/notes.h b/notes.h
index 2131912..c49b7a5 100644
--- a/notes.h
+++ b/notes.h
@@ -21,11 +21,23 @@
  */
 void init_notes(const char *notes_ref, int flags);
 
-/* Add the given note object to the internal notes tree structure */
+/*
+ * Add the given note object to the internal notes tree structure
+ *
+ * IMPORTANT: The changes made by add_note() to the internal notes tree structure
+ * are not persistent until a subsequent call to write_notes_tree() returns
+ * zero.
+ */
 void add_note(const unsigned char *object_sha1,
 		const unsigned char *note_sha1);
 
-/* Remove the given note object from the internal notes tree structure */
+/*
+ * Remove the given note object from the internal notes tree structure
+ *
+ * IMPORTANT: The changes made by remove_note() to the internal notes tree
+ * structure are not persistent until a subsequent call to write_notes_tree()
+ * returns zero.
+ */
 void remove_note(const unsigned char *object_sha1);
 
 /*
@@ -82,7 +94,27 @@ typedef int each_note_fn(const unsigned char *object_sha1,
 		void *cb_data);
 int for_each_note(int flags, each_note_fn fn, void *cb_data);
 
-/* Free (and de-initialize) the internal notes tree structure */
+/*
+ * Write the internal notes tree structure to the object database
+ *
+ * Creates a new tree object encapsulating the current state of the
+ * internal notes tree, and stores its SHA1 into the 'result' argument.
+ *
+ * Returns zero on success, non-zero on failure.
+ *
+ * IMPORTANT: Changes made to the internal notes tree structure are not
+ * persistent until this function has returned zero. Please also remember
+ * to create a corresponding commit object, and update the appropriate
+ * notes ref.
+ */
+int write_notes_tree(unsigned char *result);
+
+/*
+ * Free (and de-initialize) the internal notes tree structure
+ *
+ * IMPORTANT: Changes made to the notes tree since the last, successful
+ * call to write_notes_tree() will be lost.
+ */
 void free_notes(void);
 
 /* Flags controlling how notes are formatted */
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 10/30] Notes API: Allow multiple concurrent notes trees with new struct notes_tree

[Johan Herland]

The new struct notes_tree encapsulates access to a specific notes tree.
It is provided to allow callers to make use of several different notes trees
simultaneously.

A struct notes_tree * parameter is added to every function in the notes API.
In all cases, NULL can be passed, in which case the fallback "default" notes
tree (default_notes_tree) is used.

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c  |   90 ++++++++++++++++++++++++++++++++++++++-----------------------
 notes.h  |   81 +++++++++++++++++++++++++++++++++++--------------------
 pretty.c |    7 +++--
 3 files changed, 112 insertions(+), 66 deletions(-)

diff --git a/notes.c b/notes.c
index b576f7e..08a369a 100644
--- a/notes.c
+++ b/notes.c
@@ -50,9 +50,7 @@ struct leaf_node {
 #define SUBTREE_SHA1_PREFIXCMP(key_sha1, subtree_sha1) \
 	(memcmp(key_sha1, subtree_sha1, subtree_sha1[19]))
 
-static struct int_node root_node;
-
-static int initialized;
+struct notes_tree default_notes_tree;
 
 static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 		unsigned int n);
@@ -287,8 +285,8 @@ static int note_tree_consolidate(struct int_node *tree,
  * - Replace the matching leaf_node with a NULL entry (and free the leaf_node).
  * - Consolidate int_nodes repeatedly, while walking up the tree towards root.
  */
-static void note_tree_remove(struct int_node *tree, unsigned char n,
-		struct leaf_node *entry)
+static void note_tree_remove(struct notes_tree *t, struct int_node *tree,
+		unsigned char n, struct leaf_node *entry)
 {
 	struct leaf_node *l;
 	struct int_node *parent_stack[20];
@@ -310,7 +308,7 @@ static void note_tree_remove(struct int_node *tree, unsigned char n,
 	if (!n)
 		return; /* cannot consolidate top level */
 	/* first, build stack of ancestors between root and current node */
-	parent_stack[0] = &root_node;
+	parent_stack[0] = t->root;
 	for (i = 0; i < n; i++) {
 		j = GET_NIBBLE(i, entry->key_sha1);
 		parent_stack[i + 1] = CLR_PTR_TYPE(parent_stack[i]->a[j]);
@@ -661,14 +659,15 @@ static int write_each_note(const unsigned char *object_sha1,
 	return write_each_note_helper(d->root, note_path, mode, note_sha1);
 }
 
-void init_notes(const char *notes_ref, int flags)
+void init_notes(struct notes_tree *t, const char *notes_ref, int flags)
 {
 	unsigned char sha1[20], object_sha1[20];
 	unsigned mode;
 	struct leaf_node root_tree;
 
-	assert(!initialized);
-	initialized = 1;
+	if (!t)
+		t = &default_notes_tree;
+	assert(!t->initialized);
 
 	if (!notes_ref)
 		notes_ref = getenv(GIT_NOTES_REF_ENVIRONMENT);
@@ -677,6 +676,10 @@ void init_notes(const char *notes_ref, int flags)
 	if (!notes_ref)
 		notes_ref = GIT_NOTES_DEFAULT_REF;
 
+	t->root = (struct int_node *) xcalloc(sizeof(struct int_node), 1);
+	t->ref = notes_ref ? xstrdup(notes_ref) : NULL;
+	t->initialized = 1;
+
 	if (flags & NOTES_INIT_EMPTY || !notes_ref ||
 	    read_ref(notes_ref, object_sha1))
 		return;
@@ -686,52 +689,65 @@ void init_notes(const char *notes_ref, int flags)
 
 	hashclr(root_tree.key_sha1);
 	hashcpy(root_tree.val_sha1, sha1);
-	load_subtree(&root_tree, &root_node, 0);
+	load_subtree(&root_tree, t->root, 0);
 }
 
-void add_note(const unsigned char *object_sha1, const unsigned char *note_sha1)
+void add_note(struct notes_tree *t, const unsigned char *object_sha1,
+		const unsigned char *note_sha1)
 {
 	struct leaf_node *l;
 
-	assert(initialized);
+	if (!t)
+		t = &default_notes_tree;
+	assert(t->initialized);
 	l = (struct leaf_node *) xmalloc(sizeof(struct leaf_node));
 	hashcpy(l->key_sha1, object_sha1);
 	hashcpy(l->val_sha1, note_sha1);
-	note_tree_insert(&root_node, 0, l, PTR_TYPE_NOTE);
+	note_tree_insert(t->root, 0, l, PTR_TYPE_NOTE);
 }
 
-void remove_note(const unsigned char *object_sha1)
+void remove_note(struct notes_tree *t, const unsigned char *object_sha1)
 {
 	struct leaf_node l;
 
-	assert(initialized);
+	if (!t)
+		t = &default_notes_tree;
+	assert(t->initialized);
 	hashcpy(l.key_sha1, object_sha1);
 	hashclr(l.val_sha1);
-	return note_tree_remove(&root_node, 0, &l);
+	return note_tree_remove(t, t->root, 0, &l);
 }
 
-const unsigned char *get_note(const unsigned char *object_sha1)
+const unsigned char *get_note(struct notes_tree *t,
+		const unsigned char *object_sha1)
 {
 	struct leaf_node *found;
 
-	assert(initialized);
-	found = note_tree_find(&root_node, 0, object_sha1);
+	if (!t)
+		t = &default_notes_tree;
+	assert(t->initialized);
+	found = note_tree_find(t->root, 0, object_sha1);
 	return found ? found->val_sha1 : NULL;
 }
 
-int for_each_note(int flags, each_note_fn fn, void *cb_data)
+int for_each_note(struct notes_tree *t, int flags, each_note_fn fn,
+		void *cb_data)
 {
-	assert(initialized);
-	return for_each_note_helper(&root_node, 0, 0, flags, fn, cb_data);
+	if (!t)
+		t = &default_notes_tree;
+	assert(t->initialized);
+	return for_each_note_helper(t->root, 0, 0, flags, fn, cb_data);
 }
 
-int write_notes_tree(unsigned char *result)
+int write_notes_tree(struct notes_tree *t, unsigned char *result)
 {
 	struct tree_write_stack root;
 	struct write_each_note_data cb_data;
 	int ret;
 
-	assert(initialized);
+	if (!t)
+		t = &default_notes_tree;
+	assert(t->initialized);
 
 	/* Prepare for traversal of current notes tree */
 	root.next = NULL; /* last forward entry in list is grounded */
@@ -740,7 +756,7 @@ int write_notes_tree(unsigned char *result)
 	cb_data.root = &root;
 
 	/* Write tree objects representing current notes tree */
-	ret = for_each_note(FOR_EACH_NOTE_DONT_UNPACK_SUBTREES |
+	ret = for_each_note(t, FOR_EACH_NOTE_DONT_UNPACK_SUBTREES |
 				FOR_EACH_NOTE_YIELD_SUBTREES,
 			write_each_note, &cb_data) ||
 		tree_write_stack_finish_subtree(&root) ||
@@ -749,15 +765,19 @@ int write_notes_tree(unsigned char *result)
 	return ret;
 }
 
-void free_notes(void)
+void free_notes(struct notes_tree *t)
 {
-	note_tree_free(&root_node);
-	memset(&root_node, 0, sizeof(struct int_node));
-	initialized = 0;
+	if (!t)
+		t = &default_notes_tree;
+	if (t->root)
+		note_tree_free(t->root);
+	free(t->root);
+	free(t->ref);
+	memset(t, 0, sizeof(struct notes_tree));
 }
 
-void format_note(const unsigned char *object_sha1, struct strbuf *sb,
-		const char *output_encoding, int flags)
+void format_note(struct notes_tree *t, const unsigned char *object_sha1,
+		struct strbuf *sb, const char *output_encoding, int flags)
 {
 	static const char utf8[] = "utf-8";
 	const unsigned char *sha1;
@@ -765,10 +785,12 @@ void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 	unsigned long linelen, msglen;
 	enum object_type type;
 
-	if (!initialized)
-		init_notes(NULL, 0);
+	if (!t)
+		t = &default_notes_tree;
+	if (!t->initialized)
+		init_notes(t, NULL, 0);
 
-	sha1 = get_note(object_sha1);
+	sha1 = get_note(t, object_sha1);
 	if (!sha1)
 		return;
 
diff --git a/notes.h b/notes.h
index c49b7a5..12acc38 100644
--- a/notes.h
+++ b/notes.h
@@ -2,6 +2,21 @@
 #define NOTES_H
 
 /*
+ * Notes tree object
+ *
+ * Encapsulates the internal notes tree structure associated with a notes ref.
+ * Whenever a struct notes_tree pointer is required below, you may pass NULL in
+ * order to use the default/internal notes tree. E.g. you only need to pass a
+ * non-NULL value if you need to refer to several different notes trees
+ * simultaneously.
+ */
+extern struct notes_tree {
+	struct int_node *root;
+	char *ref;
+	int initialized;
+} default_notes_tree;
+
+/*
  * Flags controlling behaviour of notes tree initialization
  *
  * Default behaviour is to initialize the notes tree from the tree object
@@ -10,48 +25,54 @@
 #define NOTES_INIT_EMPTY 1
 
 /*
- * Initialize internal notes tree structure with the notes tree at the given
+ * Initialize the given notes_tree with the notes tree structure at the given
  * ref. If given ref is NULL, the value of the $GIT_NOTES_REF environment
  * variable is used, and if that is missing, the default notes ref is used
  * ("refs/notes/commits").
  *
- * If you need to re-intialize the internal notes tree structure (e.g. loading
- * from a different notes ref), please first de-initialize the current notes
- * tree by calling free_notes().
+ * If you need to re-intialize a notes_tree structure (e.g. when switching from
+ * one notes ref to another), you must first de-initialize the notes_tree
+ * structure by calling free_notes(struct notes_tree *).
+ *
+ * If you pass t == NULL, the default internal notes_tree will be initialized.
+ *
+ * Precondition: The notes_tree structure is zeroed (this can be achieved with
+ * memset(t, 0, sizeof(struct notes_tree)))
  */
-void init_notes(const char *notes_ref, int flags);
+void init_notes(struct notes_tree *t, const char *notes_ref, int flags);
 
 /*
- * Add the given note object to the internal notes tree structure
+ * Add the given note object to the given notes_tree structure
  *
- * IMPORTANT: The changes made by add_note() to the internal notes tree structure
+ * IMPORTANT: The changes made by add_note() to the given notes_tree structure
  * are not persistent until a subsequent call to write_notes_tree() returns
  * zero.
  */
-void add_note(const unsigned char *object_sha1,
+void add_note(struct notes_tree *t, const unsigned char *object_sha1,
 		const unsigned char *note_sha1);
 
 /*
- * Remove the given note object from the internal notes tree structure
+ * Remove the given note object from the given notes_tree structure
  *
- * IMPORTANT: The changes made by remove_note() to the internal notes tree
+ * IMPORTANT: The changes made by remove_note() to the given notes_tree
  * structure are not persistent until a subsequent call to write_notes_tree()
  * returns zero.
  */
-void remove_note(const unsigned char *object_sha1);
+void remove_note(struct notes_tree *t, const unsigned char *object_sha1);
 
 /*
  * Get the note object SHA1 containing the note data for the given object
  *
  * Return NULL if the given object has no notes.
  */
-const unsigned char *get_note(const unsigned char *object_sha1);
+const unsigned char *get_note(struct notes_tree *t,
+		const unsigned char *object_sha1);
 
 /*
  * Flags controlling behaviour of for_each_note()
  *
  * Default behaviour of for_each_note() is to traverse every single note object
- * in the notes tree, unpacking subtree entries along the way.
+ * in the given notes tree, unpacking subtree entries along the way.
  * The following flags can be used to alter the default behaviour:
  *
  * - DONT_UNPACK_SUBTREES causes for_each_note() NOT to unpack and recurse into
@@ -75,7 +96,7 @@ const unsigned char *get_note(const unsigned char *object_sha1);
 #define FOR_EACH_NOTE_YIELD_SUBTREES 2
 
 /*
- * Invoke the specified callback function for each note
+ * Invoke the specified callback function for each note in the given notes_tree
  *
  * If the callback returns nonzero, the note walk is aborted, and the return
  * value from the callback is returned from for_each_note(). Hence, a zero
@@ -92,30 +113,30 @@ const unsigned char *get_note(const unsigned char *object_sha1);
 typedef int each_note_fn(const unsigned char *object_sha1,
 		const unsigned char *note_sha1, char *note_path,
 		void *cb_data);
-int for_each_note(int flags, each_note_fn fn, void *cb_data);
+int for_each_note(struct notes_tree *t, int flags, each_note_fn fn,
+		void *cb_data);
 
 /*
- * Write the internal notes tree structure to the object database
+ * Write the given notes_tree structure to the object database
  *
- * Creates a new tree object encapsulating the current state of the
- * internal notes tree, and stores its SHA1 into the 'result' argument.
+ * Creates a new tree object encapsulating the current state of the given
+ * notes_tree, and stores its SHA1 into the 'result' argument.
  *
  * Returns zero on success, non-zero on failure.
  *
- * IMPORTANT: Changes made to the internal notes tree structure are not
- * persistent until this function has returned zero. Please also remember
- * to create a corresponding commit object, and update the appropriate
- * notes ref.
+ * IMPORTANT: Changes made to the given notes_tree are not persistent until
+ * this function has returned zero. Please also remember to create a
+ * corresponding commit object, and update the appropriate notes ref.
  */
-int write_notes_tree(unsigned char *result);
+int write_notes_tree(struct notes_tree *t, unsigned char *result);
 
 /*
- * Free (and de-initialize) the internal notes tree structure
+ * Free (and de-initialize) the given notes_tree structure
  *
- * IMPORTANT: Changes made to the notes tree since the last, successful
+ * IMPORTANT: Changes made to the given notes_tree since the last, successful
  * call to write_notes_tree() will be lost.
  */
-void free_notes(void);
+void free_notes(struct notes_tree *t);
 
 /* Flags controlling how notes are formatted */
 #define NOTES_SHOW_HEADER 1
@@ -124,12 +145,14 @@ void free_notes(void);
 /*
  * Fill the given strbuf with the notes associated with the given object.
  *
- * If the internal notes structure is not initialized, it will be auto-
+ * If the given notes_tree structure is not initialized, it will be auto-
  * initialized to the default value (see documentation for init_notes() above).
+ * If the given notes_tree is NULL, the internal/default notes_tree will be
+ * used instead.
  *
  * 'flags' is a bitwise combination of the above formatting flags.
  */
-void format_note(const unsigned char *object_sha1, struct strbuf *sb,
-		const char *output_encoding, int flags);
+void format_note(struct notes_tree *t, const unsigned char *object_sha1,
+		struct strbuf *sb, const char *output_encoding, int flags);
 
 #endif
diff --git a/pretty.c b/pretty.c
index 076b918..f999485 100644
--- a/pretty.c
+++ b/pretty.c
@@ -775,8 +775,9 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 		}
 		return 0;	/* unknown %g placeholder */
 	case 'N':
-		format_note(commit->object.sha1, sb, git_log_output_encoding ?
-			    git_log_output_encoding : git_commit_encoding, 0);
+		format_note(NULL, commit->object.sha1, sb,
+			    git_log_output_encoding ? git_log_output_encoding
+						    : git_commit_encoding, 0);
 		return 1;
 	}
 
@@ -1095,7 +1096,7 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 		strbuf_addch(sb, '\n');
 
 	if (context->show_notes)
-		format_note(commit->object.sha1, sb, encoding,
+		format_note(NULL, commit->object.sha1, sb, encoding,
 			    NOTES_SHOW_HEADER | NOTES_INDENT);
 
 	free(reencoded);
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 11/30] Refactor notes concatenation into a flexible interface for combining notes

[Johan Herland]

When adding a note to an object that already has an existing note, the
current solution is to concatenate the contents of the two notes. However,
the caller may instead wish to _overwrite_ the existing note with the new
note, or maybe even _ignore_ the new note, and keep the existing one. There
might also be other ways of combining notes that are only known to the
caller.

Therefore, instead of unconditionally concatenating notes, we let the caller
specify how to combine notes, by passing in a pointer to a function for
combining notes. The caller may choose to implement its own function for
notes combining, but normally one of the following three conveniently
supplied notes combination functions will be sufficient:

- combine_notes_concatenate() combines the two notes by appending the
  contents of the new note to the contents of the existing note.

- combine_notes_overwrite() replaces the existing note with the new note.

- combine_notes_ignore() keeps the existing note, and ignores the new note.

A combine_notes function can be passed to init_notes() to choose a default
combine_notes function for that notes tree. If NULL is given, the notes tree
falls back to combine_notes_concatenate() as the ultimate default.

A combine_notes function can also be passed directly to add_note(), to
control the notes combining behaviour for a note addition in particular.
If NULL is passed, the combine_notes function registered for the given
notes tree is used.

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |  138 ++++++++++++++++++++++++++++++++++++--------------------------
 notes.h |   34 +++++++++++++++-
 2 files changed, 112 insertions(+), 60 deletions(-)

diff --git a/notes.c b/notes.c
index 08a369a..dc4e4f6 100644
--- a/notes.c
+++ b/notes.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "notes.h"
+#include "blob.h"
 #include "tree.h"
 #include "utf8.h"
 #include "strbuf.h"
@@ -127,55 +128,12 @@ static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,
 	return NULL;
 }
 
-/* Create a new blob object by concatenating the two given blob objects */
-static int concatenate_notes(unsigned char *cur_sha1,
-		const unsigned char *new_sha1)
-{
-	char *cur_msg, *new_msg, *buf;
-	unsigned long cur_len, new_len, buf_len;
-	enum object_type cur_type, new_type;
-	int ret;
-
-	/* read in both note blob objects */
-	new_msg = read_sha1_file(new_sha1, &new_type, &new_len);
-	if (!new_msg || !new_len || new_type != OBJ_BLOB) {
-		free(new_msg);
-		return 0;
-	}
-	cur_msg = read_sha1_file(cur_sha1, &cur_type, &cur_len);
-	if (!cur_msg || !cur_len || cur_type != OBJ_BLOB) {
-		free(cur_msg);
-		free(new_msg);
-		hashcpy(cur_sha1, new_sha1);
-		return 0;
-	}
-
-	/* we will separate the notes by a newline anyway */
-	if (cur_msg[cur_len - 1] == '\n')
-		cur_len--;
-
-	/* concatenate cur_msg and new_msg into buf */
-	buf_len = cur_len + 1 + new_len;
-	buf = (char *) xmalloc(buf_len);
-	memcpy(buf, cur_msg, cur_len);
-	buf[cur_len] = '\n';
-	memcpy(buf + cur_len + 1, new_msg, new_len);
-
-	free(cur_msg);
-	free(new_msg);
-
-	/* create a new blob object from buf */
-	ret = write_sha1_file(buf, buf_len, "blob", cur_sha1);
-	free(buf);
-	return ret;
-}
-
 /*
  * To insert a leaf_node:
  * Search to the tree location appropriate for the given leaf_node's key:
  * - If location is unused (NULL), store the tweaked pointer directly there
  * - If location holds a note entry that matches the note-to-be-inserted, then
- *   concatenate the two notes.
+ *   combine the two notes (by calling the given combine_notes function).
  * - If location holds a note entry that matches the subtree-to-be-inserted,
  *   then unpack the subtree-to-be-inserted into the location.
  * - If location holds a matching subtree entry, unpack the subtree at that
@@ -184,7 +142,8 @@ static int concatenate_notes(unsigned char *cur_sha1,
  *   node-to-be-inserted, and store the new int_node into the location.
  */
 static void note_tree_insert(struct int_node *tree, unsigned char n,
-		struct leaf_node *entry, unsigned char type)
+		struct leaf_node *entry, unsigned char type,
+		combine_notes_fn combine_notes)
 {
 	struct int_node *new_node;
 	struct leaf_node *l;
@@ -205,12 +164,11 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
 				if (!hashcmp(l->val_sha1, entry->val_sha1))
 					return;
 
-				if (concatenate_notes(l->val_sha1,
-						entry->val_sha1))
-					die("failed to concatenate note %s "
-					    "into note %s for object %s",
-					    sha1_to_hex(entry->val_sha1),
+				if (combine_notes(l->val_sha1, entry->val_sha1))
+					die("failed to combine notes %s and %s"
+					    " for object %s",
 					    sha1_to_hex(l->val_sha1),
+					    sha1_to_hex(entry->val_sha1),
 					    sha1_to_hex(l->key_sha1));
 				free(entry);
 				return;
@@ -233,7 +191,7 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
 			*p = NULL;
 			load_subtree(l, tree, n);
 			free(l);
-			note_tree_insert(tree, n, entry, type);
+			note_tree_insert(tree, n, entry, type, combine_notes);
 			return;
 		}
 		break;
@@ -243,9 +201,9 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
 	assert(GET_PTR_TYPE(*p) == PTR_TYPE_NOTE ||
 	       GET_PTR_TYPE(*p) == PTR_TYPE_SUBTREE);
 	new_node = (struct int_node *) xcalloc(sizeof(struct int_node), 1);
-	note_tree_insert(new_node, n + 1, l, GET_PTR_TYPE(*p));
+	note_tree_insert(new_node, n + 1, l, GET_PTR_TYPE(*p), combine_notes);
 	*p = SET_PTR_TYPE(new_node, PTR_TYPE_INTERNAL);
-	note_tree_insert(new_node, n + 1, entry, type);
+	note_tree_insert(new_node, n + 1, entry, type, combine_notes);
 }
 
 /*
@@ -406,7 +364,8 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 				l->key_sha1[19] = (unsigned char) len;
 				type = PTR_TYPE_SUBTREE;
 			}
-			note_tree_insert(node, n, l, type);
+			note_tree_insert(node, n, l, type,
+					 combine_notes_concatenate);
 		}
 	}
 	free(buf);
@@ -659,7 +618,64 @@ static int write_each_note(const unsigned char *object_sha1,
 	return write_each_note_helper(d->root, note_path, mode, note_sha1);
 }
 
-void init_notes(struct notes_tree *t, const char *notes_ref, int flags)
+int combine_notes_concatenate(unsigned char *cur_sha1,
+		const unsigned char *new_sha1)
+{
+	char *cur_msg = NULL, *new_msg = NULL, *buf;
+	unsigned long cur_len, new_len, buf_len;
+	enum object_type cur_type, new_type;
+	int ret;
+
+	/* read in both note blob objects */
+	if (!is_null_sha1(new_sha1))
+		new_msg = read_sha1_file(new_sha1, &new_type, &new_len);
+	if (!new_msg || !new_len || new_type != OBJ_BLOB) {
+		free(new_msg);
+		return 0;
+	}
+	if (!is_null_sha1(cur_sha1))
+		cur_msg = read_sha1_file(cur_sha1, &cur_type, &cur_len);
+	if (!cur_msg || !cur_len || cur_type != OBJ_BLOB) {
+		free(cur_msg);
+		free(new_msg);
+		hashcpy(cur_sha1, new_sha1);
+		return 0;
+	}
+
+	/* we will separate the notes by a newline anyway */
+	if (cur_msg[cur_len - 1] == '\n')
+		cur_len--;
+
+	/* concatenate cur_msg and new_msg into buf */
+	buf_len = cur_len + 1 + new_len;
+	buf = (char *) xmalloc(buf_len);
+	memcpy(buf, cur_msg, cur_len);
+	buf[cur_len] = '\n';
+	memcpy(buf + cur_len + 1, new_msg, new_len);
+	free(cur_msg);
+	free(new_msg);
+
+	/* create a new blob object from buf */
+	ret = write_sha1_file(buf, buf_len, blob_type, cur_sha1);
+	free(buf);
+	return ret;
+}
+
+int combine_notes_overwrite(unsigned char *cur_sha1,
+		const unsigned char *new_sha1)
+{
+	hashcpy(cur_sha1, new_sha1);
+	return 0;
+}
+
+int combine_notes_ignore(unsigned char *cur_sha1,
+		const unsigned char *new_sha1)
+{
+	return 0;
+}
+
+void init_notes(struct notes_tree *t, const char *notes_ref,
+		combine_notes_fn combine_notes, int flags)
 {
 	unsigned char sha1[20], object_sha1[20];
 	unsigned mode;
@@ -676,8 +692,12 @@ void init_notes(struct notes_tree *t, const char *notes_ref, int flags)
 	if (!notes_ref)
 		notes_ref = GIT_NOTES_DEFAULT_REF;
 
+	if (!combine_notes)
+		combine_notes = combine_notes_concatenate;
+
 	t->root = (struct int_node *) xcalloc(sizeof(struct int_node), 1);
 	t->ref = notes_ref ? xstrdup(notes_ref) : NULL;
+	t->combine_notes = combine_notes;
 	t->initialized = 1;
 
 	if (flags & NOTES_INIT_EMPTY || !notes_ref ||
@@ -693,17 +713,19 @@ void init_notes(struct notes_tree *t, const char *notes_ref, int flags)
 }
 
 void add_note(struct notes_tree *t, const unsigned char *object_sha1,
-		const unsigned char *note_sha1)
+		const unsigned char *note_sha1, combine_notes_fn combine_notes)
 {
 	struct leaf_node *l;
 
 	if (!t)
 		t = &default_notes_tree;
 	assert(t->initialized);
+	if (!combine_notes)
+		combine_notes = t->combine_notes;
 	l = (struct leaf_node *) xmalloc(sizeof(struct leaf_node));
 	hashcpy(l->key_sha1, object_sha1);
 	hashcpy(l->val_sha1, note_sha1);
-	note_tree_insert(t->root, 0, l, PTR_TYPE_NOTE);
+	note_tree_insert(t->root, 0, l, PTR_TYPE_NOTE, combine_notes);
 }
 
 void remove_note(struct notes_tree *t, const unsigned char *object_sha1)
@@ -788,7 +810,7 @@ void format_note(struct notes_tree *t, const unsigned char *object_sha1,
 	if (!t)
 		t = &default_notes_tree;
 	if (!t->initialized)
-		init_notes(t, NULL, 0);
+		init_notes(t, NULL, NULL, 0);
 
 	sha1 = get_note(t, object_sha1);
 	if (!sha1)
diff --git a/notes.h b/notes.h
index 12acc38..20d6e17 100644
--- a/notes.h
+++ b/notes.h
@@ -2,6 +2,30 @@
 #define NOTES_H
 
 /*
+ * Function type for combining two notes annotating the same object.
+ *
+ * When adding a new note annotating the same object as an existing note, it is
+ * up to the caller to decide how to combine the two notes. The decision is
+ * made by passing in a function of the following form. The function accepts
+ * two SHA1s -- of the existing note and the new note, respectively. The
+ * function then combines the notes in whatever way it sees fit, and writes the
+ * resulting SHA1 into the first SHA1 argument (cur_sha1). A non-zero return
+ * value indicates failure.
+ *
+ * The two given SHA1s must both be non-NULL and different from each other.
+ *
+ * The default combine_notes function (you get this when passing NULL) is
+ * combine_notes_concatenate(), which appends the contents of the new note to
+ * the contents of the existing note.
+ */
+typedef int combine_notes_fn(unsigned char *cur_sha1, const unsigned char *new_sha1);
+
+/* Common notes combinators */
+int combine_notes_concatenate(unsigned char *cur_sha1, const unsigned char *new_sha1);
+int combine_notes_overwrite(unsigned char *cur_sha1, const unsigned char *new_sha1);
+int combine_notes_ignore(unsigned char *cur_sha1, const unsigned char *new_sha1);
+
+/*
  * Notes tree object
  *
  * Encapsulates the internal notes tree structure associated with a notes ref.
@@ -13,6 +37,7 @@
 extern struct notes_tree {
 	struct int_node *root;
 	char *ref;
+	combine_notes_fn *combine_notes;
 	int initialized;
 } default_notes_tree;
 
@@ -36,10 +61,15 @@ extern struct notes_tree {
  *
  * If you pass t == NULL, the default internal notes_tree will be initialized.
  *
+ * The combine_notes function that is passed becomes the default combine_notes
+ * function for the given notes_tree. If NULL is passed, the default
+ * combine_notes function is combine_notes_concatenate().
+ *
  * Precondition: The notes_tree structure is zeroed (this can be achieved with
  * memset(t, 0, sizeof(struct notes_tree)))
  */
-void init_notes(struct notes_tree *t, const char *notes_ref, int flags);
+void init_notes(struct notes_tree *t, const char *notes_ref,
+		combine_notes_fn combine_notes, int flags);
 
 /*
  * Add the given note object to the given notes_tree structure
@@ -49,7 +79,7 @@ void init_notes(struct notes_tree *t, const char *notes_ref, int flags);
  * zero.
  */
 void add_note(struct notes_tree *t, const unsigned char *object_sha1,
-		const unsigned char *note_sha1);
+		const unsigned char *note_sha1, combine_notes_fn combine_notes);
 
 /*
  * Remove the given note object from the given notes_tree structure
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 12/30] Builtin-ify git-notes

[Johan Herland]

The builtin-ification includes some minor behavioural changes to the
command-line interface: It is no longer allowed to mix the -m and -F
arguments, and it is not allowed to use multiple -F options.

As part of the builtin-ification, we add the commit_notes() function
to the builtin API. This function (together with the notes.h API) can
be easily used from other builtins to manipulate the notes tree.

Also includes needed changes to t3301.

This patch has been improved by the following contributions:
- Stephen Boyd: Use die() instead of fprintf(stderr, ...) followed by exit(1)

Cc: Stephen Boyd <bebarino@gmail.com>
Signed-off-by: Johan Herland <johan@herland.net>
---
 Documentation/git-notes.txt                   |    6 +-
 Makefile                                      |    2 +-
 builtin-notes.c                               |  280 +++++++++++++++++++++++++
 builtin.h                                     |    3 +
 git-notes.sh => contrib/examples/git-notes.sh |    0
 git.c                                         |    1 +
 t/t3301-notes.sh                              |   98 ++++++---
 7 files changed, 350 insertions(+), 40 deletions(-)
 create mode 100644 builtin-notes.c
 rename git-notes.sh => contrib/examples/git-notes.sh (100%)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index d4487ca..0d1ada6 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -37,14 +37,12 @@ OPTIONS
 -------
 -m <msg>::
 	Use the given note message (instead of prompting).
-	If multiple `-m` (or `-F`) options are given, their
-	values are concatenated as separate paragraphs.
+	If multiple `-m` options are given, their values
+	are concatenated as separate paragraphs.
 
 -F <file>::
 	Take the note message from the given file.  Use '-' to
 	read the note message from the standard input.
-	If multiple `-F` (or `-m`) options are given, their
-	values are concatenated as separate paragraphs.
 
 
 Author
diff --git a/Makefile b/Makefile
index c0dbee2..1f95f93 100644
--- a/Makefile
+++ b/Makefile
@@ -353,7 +353,6 @@ SCRIPT_SH += git-merge-one-file.sh
 SCRIPT_SH += git-merge-resolve.sh
 SCRIPT_SH += git-mergetool.sh
 SCRIPT_SH += git-mergetool--lib.sh
-SCRIPT_SH += git-notes.sh
 SCRIPT_SH += git-parse-remote.sh
 SCRIPT_SH += git-pull.sh
 SCRIPT_SH += git-quiltimport.sh
@@ -671,6 +670,7 @@ BUILTIN_OBJS += builtin-mktag.o
 BUILTIN_OBJS += builtin-mktree.o
 BUILTIN_OBJS += builtin-mv.o
 BUILTIN_OBJS += builtin-name-rev.o
+BUILTIN_OBJS += builtin-notes.o
 BUILTIN_OBJS += builtin-pack-objects.o
 BUILTIN_OBJS += builtin-pack-redundant.o
 BUILTIN_OBJS += builtin-pack-refs.o
diff --git a/builtin-notes.c b/builtin-notes.c
new file mode 100644
index 0000000..89aa6e0
--- /dev/null
+++ b/builtin-notes.c
@@ -0,0 +1,280 @@
+/*
+ * Builtin "git notes"
+ *
+ * Copyright (c) 2010 Johan Herland <johan@herland.net>
+ *
+ * Based on git-notes.sh by Johannes Schindelin,
+ * and builtin-tag.c by Kristian Høgsberg and Carlos Rica.
+ */
+
+#include "cache.h"
+#include "builtin.h"
+#include "notes.h"
+#include "blob.h"
+#include "commit.h"
+#include "refs.h"
+#include "exec_cmd.h"
+#include "run-command.h"
+#include "parse-options.h"
+
+static const char * const git_notes_usage[] = {
+	"git notes edit [-m <msg> | -F <file>] [<object>]",
+	"git notes show [<object>]",
+	NULL
+};
+
+static const char note_template[] =
+	"\n"
+	"#\n"
+	"# Write/edit the notes for the following object:\n"
+	"#\n";
+
+static void write_note_data(int fd, const unsigned char *sha1)
+{
+	unsigned long size;
+	enum object_type type;
+	char *buf = read_sha1_file(sha1, &type, &size);
+	if (buf) {
+		if (size)
+			write_or_die(fd, buf, size);
+		free(buf);
+	}
+}
+
+static void write_commented_object(int fd, const unsigned char *object)
+{
+	const char *show_args[5] =
+		{"show", "--stat", "--no-notes", sha1_to_hex(object), NULL};
+	struct child_process show;
+	struct strbuf buf = STRBUF_INIT;
+	FILE *show_out;
+
+	/* Invoke "git show --stat --no-notes $object" */
+	memset(&show, 0, sizeof(show));
+	show.argv = show_args;
+	show.no_stdin = 1;
+	show.out = -1;
+	show.err = 0;
+	show.git_cmd = 1;
+	if (start_command(&show))
+		die("unable to start 'show' for object '%s'",
+		    sha1_to_hex(object));
+
+	/* Open the output as FILE* so strbuf_getline() can be used. */
+	show_out = xfdopen(show.out, "r");
+	if (show_out == NULL)
+		die_errno("can't fdopen 'show' output fd");
+
+	/* Prepend "# " to each output line and write result to 'fd' */
+	while (strbuf_getline(&buf, show_out, '\n') != EOF) {
+		write_or_die(fd, "# ", 2);
+		write_or_die(fd, buf.buf, buf.len);
+		write_or_die(fd, "\n", 1);
+	}
+	strbuf_release(&buf);
+	if (fclose(show_out))
+		die_errno("failed to close pipe to 'show' for object '%s'",
+			  sha1_to_hex(object));
+	if (finish_command(&show))
+		die("failed to finish 'show' for object '%s'",
+		    sha1_to_hex(object));
+}
+
+static void create_note(const unsigned char *object,
+			struct strbuf *buf,
+			int skip_editor,
+			const unsigned char *prev,
+			unsigned char *result)
+{
+	char *path = NULL;
+
+	if (!skip_editor) {
+		int fd;
+
+		/* write the template message before editing: */
+		path = git_pathdup("NOTES_EDITMSG");
+		fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0600);
+		if (fd < 0)
+			die_errno("could not create file '%s'", path);
+
+		if (prev)
+			write_note_data(fd, prev);
+		write_or_die(fd, note_template, strlen(note_template));
+
+		write_commented_object(fd, object);
+
+		close(fd);
+
+		if (launch_editor(path, buf, NULL)) {
+			die("Please supply the note contents using either -m" \
+			    " or -F option");
+		}
+	}
+
+	stripspace(buf, 1);
+
+	if (!skip_editor && !buf->len) {
+		fprintf(stderr, "Removing note for object %s\n",
+			sha1_to_hex(object));
+		hashclr(result);
+	} else {
+		if (write_sha1_file(buf->buf, buf->len, blob_type, result)) {
+			error("unable to write note object");
+			if (path)
+				error("The note contents has been left in %s",
+				      path);
+			exit(128);
+		}
+	}
+
+	if (path) {
+		unlink_or_warn(path);
+		free(path);
+	}
+}
+
+struct msg_arg {
+	int given;
+	struct strbuf buf;
+};
+
+static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
+{
+	struct msg_arg *msg = opt->value;
+
+	if (!arg)
+		return -1;
+	if (msg->buf.len)
+		strbuf_addstr(&(msg->buf), "\n\n");
+	strbuf_addstr(&(msg->buf), arg);
+	msg->given = 1;
+	return 0;
+}
+
+int commit_notes(struct notes_tree *t, const char *msg)
+{
+	struct commit_list *parent;
+	unsigned char tree_sha1[20], prev_commit[20], new_commit[20];
+	struct strbuf buf = STRBUF_INIT;
+
+	if (!t)
+		t = &default_notes_tree;
+	if (!t->initialized || !t->ref || !*t->ref)
+		die("Cannot commit uninitialized/unreferenced notes tree");
+
+	/* Prepare commit message and reflog message */
+	strbuf_addstr(&buf, "notes: "); /* commit message starts at index 7 */
+	strbuf_addstr(&buf, msg);
+	if (buf.buf[buf.len - 1] != '\n')
+		strbuf_addch(&buf, '\n'); /* Make sure msg ends with newline */
+
+	/* Convert notes tree to tree object */
+	if (write_notes_tree(t, tree_sha1))
+		die("Failed to write current notes tree to database");
+
+	/* Create new commit for the tree object */
+	if (!read_ref(t->ref, prev_commit)) { /* retrieve parent commit */
+		parent = xmalloc(sizeof(*parent));
+		parent->item = lookup_commit(prev_commit);
+		parent->next = NULL;
+	} else {
+		hashclr(prev_commit);
+		parent = NULL;
+	}
+	if (commit_tree(buf.buf + 7, tree_sha1, parent, new_commit, NULL))
+		die("Failed to commit notes tree to database");
+
+	/* Update notes ref with new commit */
+	update_ref(buf.buf, t->ref, new_commit, prev_commit, 0, DIE_ON_ERR);
+
+	strbuf_release(&buf);
+	return 0;
+}
+
+int cmd_notes(int argc, const char **argv, const char *prefix)
+{
+	struct strbuf buf = STRBUF_INIT;
+	struct notes_tree *t;
+	unsigned char object[20], new_note[20];
+	const unsigned char *note;
+	const char *object_ref;
+	int edit = 0, show = 0;
+	const char *msgfile = NULL;
+	struct msg_arg msg = { 0, STRBUF_INIT };
+	struct option options[] = {
+		OPT_GROUP("Notes edit options"),
+		OPT_CALLBACK('m', NULL, &msg, "msg",
+			     "note contents as a string", parse_msg_arg),
+		OPT_FILENAME('F', NULL, &msgfile, "note contents in a file"),
+		OPT_END()
+	};
+
+	git_config(git_default_config, NULL);
+
+	argc = parse_options(argc, argv, prefix, options, git_notes_usage, 0);
+
+	if (argc && !strcmp(argv[0], "edit"))
+		edit = 1;
+	else if (argc && !strcmp(argv[0], "show"))
+		show = 1;
+
+	if (edit + show != 1)
+		usage_with_options(git_notes_usage, options);
+
+	object_ref = argc == 2 ? argv[1] : "HEAD";
+	if (argc > 2) {
+		error("too many parameters");
+		usage_with_options(git_notes_usage, options);
+	}
+
+	if (get_sha1(object_ref, object))
+		die("Failed to resolve '%s' as a valid ref.", object_ref);
+
+	init_notes(NULL, NULL, NULL, 0);
+	t = &default_notes_tree;
+
+	if (prefixcmp(t->ref, "refs/notes/"))
+		die("Refusing to %s notes in %s (outside of refs/notes/)",
+		    edit ? "edit" : "show", t->ref);
+
+	note = get_note(t, object);
+
+	/* show command */
+
+	if (show && !note) {
+		error("No note found for object %s.", sha1_to_hex(object));
+		return 1;
+	} else if (show) {
+		const char *show_args[3] = {"show", sha1_to_hex(note), NULL};
+		return execv_git_cmd(show_args);
+	}
+
+	/* edit command */
+
+	if (msg.given || msgfile) {
+		if (msg.given && msgfile) {
+			error("mixing -m and -F options is not allowed.");
+			usage_with_options(git_notes_usage, options);
+		}
+		if (msg.given)
+			strbuf_addbuf(&buf, &(msg.buf));
+		else {
+			if (!strcmp(msgfile, "-")) {
+				if (strbuf_read(&buf, 0, 1024) < 0)
+					die_errno("cannot read '%s'", msgfile);
+			} else {
+				if (strbuf_read_file(&buf, msgfile, 1024) < 0)
+					die_errno("could not open or read '%s'",
+						msgfile);
+			}
+		}
+	}
+
+	create_note(object, &buf, msg.given || msgfile, note, new_note);
+	add_note(t, object, new_note, combine_notes_overwrite);
+	commit_notes(t, "Note added by 'git notes edit'");
+
+	free_notes(t);
+	strbuf_release(&buf);
+	return 0;
+}
diff --git a/builtin.h b/builtin.h
index e8202f3..cdf9847 100644
--- a/builtin.h
+++ b/builtin.h
@@ -5,6 +5,7 @@
 #include "strbuf.h"
 #include "cache.h"
 #include "commit.h"
+#include "notes.h"
 
 extern const char git_version_string[];
 extern const char git_usage_string[];
@@ -18,6 +19,7 @@ extern int fmt_merge_msg(int merge_summary, struct strbuf *in,
 extern int commit_tree(const char *msg, unsigned char *tree,
 		struct commit_list *parents, unsigned char *ret,
 		const char *author);
+extern int commit_notes(struct notes_tree *t, const char *msg);
 extern int check_pager_config(const char *cmd);
 
 extern int cmd_add(int argc, const char **argv, const char *prefix);
@@ -78,6 +80,7 @@ extern int cmd_mktag(int argc, const char **argv, const char *prefix);
 extern int cmd_mktree(int argc, const char **argv, const char *prefix);
 extern int cmd_mv(int argc, const char **argv, const char *prefix);
 extern int cmd_name_rev(int argc, const char **argv, const char *prefix);
+extern int cmd_notes(int argc, const char **argv, const char *prefix);
 extern int cmd_pack_objects(int argc, const char **argv, const char *prefix);
 extern int cmd_pack_redundant(int argc, const char **argv, const char *prefix);
 extern int cmd_patch_id(int argc, const char **argv, const char *prefix);
diff --git a/git-notes.sh b/contrib/examples/git-notes.sh
similarity index 100%
rename from git-notes.sh
rename to contrib/examples/git-notes.sh
diff --git a/git.c b/git.c
index b3e23f1..32f76e1 100644
--- a/git.c
+++ b/git.c
@@ -343,6 +343,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "mktree", cmd_mktree, RUN_SETUP },
 		{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
 		{ "name-rev", cmd_name_rev, RUN_SETUP },
+		{ "notes", cmd_notes, RUN_SETUP },
 		{ "pack-objects", cmd_pack_objects, RUN_SETUP },
 		{ "pack-redundant", cmd_pack_redundant, RUN_SETUP },
 		{ "patch-id", cmd_patch_id },
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 18aad53..10f62f4 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -12,8 +12,8 @@ echo "$MSG" > "$1"
 echo "$MSG" >& 2
 EOF
 chmod a+x fake_editor.sh
-VISUAL=./fake_editor.sh
-export VISUAL
+GIT_EDITOR=./fake_editor.sh
+export GIT_EDITOR
 
 test_expect_success 'cannot annotate non-existing HEAD' '
 	(MSG=3 && export MSG && test_must_fail git notes edit)
@@ -56,8 +56,17 @@ test_expect_success 'handle empty notes gracefully' '
 
 test_expect_success 'create notes' '
 	git config core.notesRef refs/notes/commits &&
+	MSG=b0 git notes edit &&
+	test ! -f .git/NOTES_EDITMSG &&
+	test 1 = $(git ls-tree refs/notes/commits | wc -l) &&
+	test b0 = $(git notes show) &&
+	git show HEAD^ &&
+	test_must_fail git notes show HEAD^
+'
+
+test_expect_success 'edit existing notes' '
 	MSG=b1 git notes edit &&
-	test ! -f .git/new-notes &&
+	test ! -f .git/NOTES_EDITMSG &&
 	test 1 = $(git ls-tree refs/notes/commits | wc -l) &&
 	test b1 = $(git notes show) &&
 	git show HEAD^ &&
@@ -110,19 +119,16 @@ test_expect_success 'show multi-line notes' '
 	git log -2 > output &&
 	test_cmp expect-multiline output
 '
-test_expect_success 'create -m and -F notes (setup)' '
+test_expect_success 'create -F notes (setup)' '
 	: > a4 &&
 	git add a4 &&
 	test_tick &&
 	git commit -m 4th &&
 	echo "xyzzy" > note5 &&
-	git notes edit -m spam -F note5 -m "foo
-bar
-baz"
+	git notes edit -F note5
 '
 
-whitespace="    "
-cat > expect-m-and-F << EOF
+cat > expect-F << EOF
 commit 15023535574ded8b1a89052b32673f84cf9582b8
 Author: A U Thor <author@example.com>
 Date:   Thu Apr 7 15:16:13 2005 -0700
@@ -130,21 +136,15 @@ Date:   Thu Apr 7 15:16:13 2005 -0700
     4th
 
 Notes:
-    spam
-$whitespace
     xyzzy
-$whitespace
-    foo
-    bar
-    baz
 EOF
 
-printf "\n" >> expect-m-and-F
-cat expect-multiline >> expect-m-and-F
+printf "\n" >> expect-F
+cat expect-multiline >> expect-F
 
-test_expect_success 'show -m and -F notes' '
+test_expect_success 'show -F notes' '
 	git log -3 > output &&
-	test_cmp expect-m-and-F output
+	test_cmp expect-F output
 '
 
 cat >expect << EOF
@@ -164,13 +164,7 @@ test_expect_success 'git log --pretty=raw does not show notes' '
 cat >>expect <<EOF
 
 Notes:
-    spam
-$whitespace
     xyzzy
-$whitespace
-    foo
-    bar
-    baz
 EOF
 test_expect_success 'git log --show-notes' '
 	git log -1 --pretty=raw --show-notes >output &&
@@ -179,17 +173,17 @@ test_expect_success 'git log --show-notes' '
 
 test_expect_success 'git log --no-notes' '
 	git log -1 --no-notes >output &&
-	! grep spam output
+	! grep xyzzy output
 '
 
 test_expect_success 'git format-patch does not show notes' '
 	git format-patch -1 --stdout >output &&
-	! grep spam output
+	! grep xyzzy output
 '
 
 test_expect_success 'git format-patch --show-notes does show notes' '
 	git format-patch --show-notes -1 --stdout >output &&
-	grep spam output
+	grep xyzzy output
 '
 
 for pretty in \
@@ -202,19 +196,22 @@ do
 	esac
 	test_expect_success "git show $pretty does$not show notes" '
 		git show $p >output &&
-		eval "$negate grep spam output"
+		eval "$negate grep xyzzy output"
 	'
 done
 
-test_expect_success 'create other note on a different notes ref (setup)' '
+test_expect_success 'create -m notes (setup)' '
 	: > a5 &&
 	git add a5 &&
 	test_tick &&
 	git commit -m 5th &&
-	GIT_NOTES_REF="refs/notes/other" git notes edit -m "other note"
+	git notes edit -m spam -m "foo
+bar
+baz"
 '
 
-cat > expect-other << EOF
+whitespace="    "
+cat > expect-m << EOF
 commit bd1753200303d0a0344be813e504253b3d98e74d
 Author: A U Thor <author@example.com>
 Date:   Thu Apr 7 15:17:13 2005 -0700
@@ -222,15 +219,46 @@ Date:   Thu Apr 7 15:17:13 2005 -0700
     5th
 
 Notes:
+    spam
+$whitespace
+    foo
+    bar
+    baz
+EOF
+
+printf "\n" >> expect-m
+cat expect-F >> expect-m
+
+test_expect_success 'show -m notes' '
+	git log -4 > output &&
+	test_cmp expect-m output
+'
+
+test_expect_success 'create other note on a different notes ref (setup)' '
+	: > a6 &&
+	git add a6 &&
+	test_tick &&
+	git commit -m 6th &&
+	GIT_NOTES_REF="refs/notes/other" git notes edit -m "other note"
+'
+
+cat > expect-other << EOF
+commit 387a89921c73d7ed72cd94d179c1c7048ca47756
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:18:13 2005 -0700
+
+    6th
+
+Notes:
     other note
 EOF
 
 cat > expect-not-other << EOF
-commit bd1753200303d0a0344be813e504253b3d98e74d
+commit 387a89921c73d7ed72cd94d179c1c7048ca47756
 Author: A U Thor <author@example.com>
-Date:   Thu Apr 7 15:17:13 2005 -0700
+Date:   Thu Apr 7 15:18:13 2005 -0700
 
-    5th
+    6th
 EOF
 
 test_expect_success 'Do not show note on other ref by default' '
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 13/30] t3301: Verify successful annotation of non-commits

[Johan Herland]

Adds a testcase verifying that git-notes works successfully on
tree, blob, and tag objects.

Signed-off-by: Johan Herland <johan@herland.net>
---
 t/t3301-notes.sh |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 10f62f4..fd5e593 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -282,4 +282,21 @@ test_expect_success 'Do not show note when core.notesRef is overridden' '
 	test_cmp expect-not-other output
 '
 
+test_expect_success 'Allow notes on non-commits (trees, blobs, tags)' '
+	echo "Note on a tree" > expect
+	git notes edit -m "Note on a tree" HEAD: &&
+	git notes show HEAD: > actual &&
+	test_cmp expect actual &&
+	echo "Note on a blob" > expect
+	filename=$(git ls-tree --name-only HEAD | head -n1) &&
+	git notes edit -m "Note on a blob" HEAD:$filename &&
+	git notes show HEAD:$filename > actual &&
+	test_cmp expect actual &&
+	echo "Note on a tag" > expect
+	git tag -a -m "This is an annotated tag" foobar HEAD^ &&
+	git notes edit -m "Note on a tag" foobar &&
+	git notes show foobar > actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 14/30] t3305: Verify that adding many notes with git-notes triggers increased fanout

[Johan Herland]

Add a test verifying that the notes code automatically restructures the
notes tree into a deeper fanout level, when many notes are added with
"git notes".

Signed-off-by: Johan Herland <johan@herland.net>
---
 t/t3305-notes-fanout.sh |   50 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 50 insertions(+), 0 deletions(-)
 create mode 100755 t/t3305-notes-fanout.sh

diff --git a/t/t3305-notes-fanout.sh b/t/t3305-notes-fanout.sh
new file mode 100755
index 0000000..823b0ff
--- /dev/null
+++ b/t/t3305-notes-fanout.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+test_description='Test that adding many notes triggers automatic fanout restructuring'
+
+. ./test-lib.sh
+
+test_expect_success 'creating many notes with git-notes' '
+	num_notes=300 &&
+	i=0 &&
+	while test $i -lt $num_notes
+	do
+		i=$(($i + 1)) &&
+		test_tick &&
+		echo "file for commit #$i" > file &&
+		git add file &&
+		git commit -q -m "commit #$i" &&
+		git notes edit -m "note #$i" || return 1
+	done
+'
+
+test_expect_success 'many notes created correctly with git-notes' '
+	git log | grep "^    " > output &&
+	i=300 &&
+	while test $i -gt 0
+	do
+		echo "    commit #$i" &&
+		echo "    note #$i" &&
+		i=$(($i - 1));
+	done > expect &&
+	test_cmp expect output
+'
+
+test_expect_success 'many notes created with git-notes triggers fanout' '
+	# Expect entire notes tree to have a fanout == 1
+	git ls-tree -r --name-only refs/notes/commits |
+	while read path
+	do
+		case "$path" in
+		??/??????????????????????????????????????)
+			: true
+			;;
+		*)
+			echo "Invalid path \"$path\"" &&
+			return 1
+			;;
+		esac
+	done
+'
+
+test_done
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 15/30] Teach notes code to properly preserve non-notes in the notes tree

[Johan Herland]

The note tree structure allows for non-note entries to coexist with note
entries in a notes tree. Although we certainly expect there to be very
few non-notes in a notes tree, we should still support them to a certain
degree.

This patch teaches the notes code to preserve non-notes when updating the
notes tree with write_notes_tree(). Non-notes are not affected by fanout
restructuring.

For non-notes to be handled correctly, we can no longer allow subtree
entries that do not match the fanout structure produced by the notes code
itself. This means that fanouts like 4/36, 6/34, 8/32, 4/4/32, etc. are
no longer recognized as note subtrees; only 2-based fanouts are allowed
(2/38, 2/2/36, 2/2/2/34, etc.). Since the notes code has never at any point
_produced_ non-2-based fanouts, it is highly unlikely that this change will
cause problems for anyone.

The patch also adds some tests verifying the correct handling of non-notes
in a notes tree.

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c                   |  219 +++++++++++++++++++++++++++++++++++++--------
 notes.h                   |    1 +
 t/t3303-notes-subtrees.sh |   28 ++++---
 t/t3304-notes-mixed.sh    |   36 +++++++-
 4 files changed, 233 insertions(+), 51 deletions(-)

diff --git a/notes.c b/notes.c
index dc4e4f6..d432517 100644
--- a/notes.c
+++ b/notes.c
@@ -37,6 +37,21 @@ struct leaf_node {
 	unsigned char val_sha1[20];
 };
 
+/*
+ * A notes tree may contain entries that are not notes, and that do not follow
+ * the naming conventions of notes. There are typically none/few of these, but
+ * we still need to keep track of them. Keep a simple linked list sorted alpha-
+ * betically on the non-note path. The list is populated when parsing tree
+ * objects in load_subtree(), and the non-notes are correctly written back into
+ * the tree objects produced by write_notes_tree().
+ */
+struct non_note {
+	struct non_note *next; /* grounded (last->next == NULL) */
+	char *path;
+	unsigned int mode;
+	unsigned char sha1[20];
+};
+
 #define PTR_TYPE_NULL     0
 #define PTR_TYPE_INTERNAL 1
 #define PTR_TYPE_NOTE     2
@@ -53,8 +68,8 @@ struct leaf_node {
 
 struct notes_tree default_notes_tree;
 
-static void load_subtree(struct leaf_node *subtree, struct int_node *node,
-		unsigned int n);
+static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
+		struct int_node *node, unsigned int n);
 
 /*
  * Search the tree until the appropriate location for the given key is found:
@@ -71,7 +86,7 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
  *      - an unused leaf node (NULL)
  *      In any case, set *tree and *n, and return pointer to the tree location.
  */
-static void **note_tree_search(struct int_node **tree,
+static void **note_tree_search(struct notes_tree *t, struct int_node **tree,
 		unsigned char *n, const unsigned char *key_sha1)
 {
 	struct leaf_node *l;
@@ -83,9 +98,9 @@ static void **note_tree_search(struct int_node **tree,
 		if (!SUBTREE_SHA1_PREFIXCMP(key_sha1, l->key_sha1)) {
 			/* unpack tree and resume search */
 			(*tree)->a[0] = NULL;
-			load_subtree(l, *tree, *n);
+			load_subtree(t, l, *tree, *n);
 			free(l);
-			return note_tree_search(tree, n, key_sha1);
+			return note_tree_search(t, tree, n, key_sha1);
 		}
 	}
 
@@ -95,15 +110,15 @@ static void **note_tree_search(struct int_node **tree,
 	case PTR_TYPE_INTERNAL:
 		*tree = CLR_PTR_TYPE(p);
 		(*n)++;
-		return note_tree_search(tree, n, key_sha1);
+		return note_tree_search(t, tree, n, key_sha1);
 	case PTR_TYPE_SUBTREE:
 		l = (struct leaf_node *) CLR_PTR_TYPE(p);
 		if (!SUBTREE_SHA1_PREFIXCMP(key_sha1, l->key_sha1)) {
 			/* unpack tree and resume search */
 			(*tree)->a[i] = NULL;
-			load_subtree(l, *tree, *n);
+			load_subtree(t, l, *tree, *n);
 			free(l);
-			return note_tree_search(tree, n, key_sha1);
+			return note_tree_search(t, tree, n, key_sha1);
 		}
 		/* fall through */
 	default:
@@ -116,10 +131,11 @@ static void **note_tree_search(struct int_node **tree,
  * Search to the tree location appropriate for the given key:
  * If a note entry with matching key, return the note entry, else return NULL.
  */
-static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,
+static struct leaf_node *note_tree_find(struct notes_tree *t,
+		struct int_node *tree, unsigned char n,
 		const unsigned char *key_sha1)
 {
-	void **p = note_tree_search(&tree, &n, key_sha1);
+	void **p = note_tree_search(t, &tree, &n, key_sha1);
 	if (GET_PTR_TYPE(*p) == PTR_TYPE_NOTE) {
 		struct leaf_node *l = (struct leaf_node *) CLR_PTR_TYPE(*p);
 		if (!hashcmp(key_sha1, l->key_sha1))
@@ -141,13 +157,13 @@ static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,
  * - Else, create a new int_node, holding both the node-at-location and the
  *   node-to-be-inserted, and store the new int_node into the location.
  */
-static void note_tree_insert(struct int_node *tree, unsigned char n,
-		struct leaf_node *entry, unsigned char type,
+static void note_tree_insert(struct notes_tree *t, struct int_node *tree,
+		unsigned char n, struct leaf_node *entry, unsigned char type,
 		combine_notes_fn combine_notes)
 {
 	struct int_node *new_node;
 	struct leaf_node *l;
-	void **p = note_tree_search(&tree, &n, entry->key_sha1);
+	void **p = note_tree_search(t, &tree, &n, entry->key_sha1);
 
 	assert(GET_PTR_TYPE(entry) == 0); /* no type bits set */
 	l = (struct leaf_node *) CLR_PTR_TYPE(*p);
@@ -178,7 +194,7 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
 			if (!SUBTREE_SHA1_PREFIXCMP(l->key_sha1,
 						    entry->key_sha1)) {
 				/* unpack 'entry' */
-				load_subtree(entry, tree, n);
+				load_subtree(t, entry, tree, n);
 				free(entry);
 				return;
 			}
@@ -189,9 +205,10 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
 		if (!SUBTREE_SHA1_PREFIXCMP(entry->key_sha1, l->key_sha1)) {
 			/* unpack 'l' and restart insert */
 			*p = NULL;
-			load_subtree(l, tree, n);
+			load_subtree(t, l, tree, n);
 			free(l);
-			note_tree_insert(tree, n, entry, type, combine_notes);
+			note_tree_insert(t, tree, n, entry, type,
+					 combine_notes);
 			return;
 		}
 		break;
@@ -201,9 +218,10 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
 	assert(GET_PTR_TYPE(*p) == PTR_TYPE_NOTE ||
 	       GET_PTR_TYPE(*p) == PTR_TYPE_SUBTREE);
 	new_node = (struct int_node *) xcalloc(sizeof(struct int_node), 1);
-	note_tree_insert(new_node, n + 1, l, GET_PTR_TYPE(*p), combine_notes);
+	note_tree_insert(t, new_node, n + 1, l, GET_PTR_TYPE(*p),
+			 combine_notes);
 	*p = SET_PTR_TYPE(new_node, PTR_TYPE_INTERNAL);
-	note_tree_insert(new_node, n + 1, entry, type, combine_notes);
+	note_tree_insert(t, new_node, n + 1, entry, type, combine_notes);
 }
 
 /*
@@ -249,7 +267,7 @@ static void note_tree_remove(struct notes_tree *t, struct int_node *tree,
 	struct leaf_node *l;
 	struct int_node *parent_stack[20];
 	unsigned char i, j;
-	void **p = note_tree_search(&tree, &n, entry->key_sha1);
+	void **p = note_tree_search(t, &tree, &n, entry->key_sha1);
 
 	assert(GET_PTR_TYPE(entry) == 0); /* no type bits set */
 	if (GET_PTR_TYPE(*p) != PTR_TYPE_NOTE)
@@ -324,14 +342,67 @@ static int get_sha1_hex_segment(const char *hex, unsigned int hex_len,
 	return len;
 }
 
-static void load_subtree(struct leaf_node *subtree, struct int_node *node,
-		unsigned int n)
+static int non_note_cmp(const struct non_note *a, const struct non_note *b)
+{
+	return strcmp(a->path, b->path);
+}
+
+static void add_non_note(struct notes_tree *t, const char *path,
+		unsigned int mode, const unsigned char *sha1)
+{
+	struct non_note *p = t->prev_non_note, *n;
+	n = (struct non_note *) xmalloc(sizeof(struct non_note));
+	n->next = NULL;
+	n->path = xstrdup(path);
+	n->mode = mode;
+	hashcpy(n->sha1, sha1);
+	t->prev_non_note = n;
+
+	if (!t->first_non_note) {
+		t->first_non_note = n;
+		return;
+	}
+
+	if (non_note_cmp(p, n) < 0)
+		; /* do nothing  */
+	else if (non_note_cmp(t->first_non_note, n) <= 0)
+		p = t->first_non_note;
+	else {
+		/* n sorts before t->first_non_note */
+		n->next = t->first_non_note;
+		t->first_non_note = n;
+		return;
+	}
+
+	/* n sorts equal or after p */
+	while (p->next && non_note_cmp(p->next, n) <= 0)
+		p = p->next;
+
+	if (non_note_cmp(p, n) == 0) { /* n ~= p; overwrite p with n */
+		assert(strcmp(p->path, n->path) == 0);
+		p->mode = n->mode;
+		hashcpy(p->sha1, n->sha1);
+		free(n);
+		t->prev_non_note = p;
+		return;
+	}
+
+	/* n sorts between p and p->next */
+	n->next = p->next;
+	p->next = n;
+}
+
+static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
+		struct int_node *node, unsigned int n)
 {
 	unsigned char object_sha1[20];
 	unsigned int prefix_len;
 	void *buf;
 	struct tree_desc desc;
 	struct name_entry entry;
+	int len, path_len;
+	unsigned char type;
+	struct leaf_node *l;
 
 	buf = fill_tree_descriptor(&desc, subtree->val_sha1);
 	if (!buf)
@@ -342,31 +413,68 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 	assert(prefix_len * 2 >= n);
 	memcpy(object_sha1, subtree->key_sha1, prefix_len);
 	while (tree_entry(&desc, &entry)) {
-		int len = get_sha1_hex_segment(entry.path, strlen(entry.path),
+		path_len = strlen(entry.path);
+		len = get_sha1_hex_segment(entry.path, path_len,
 				object_sha1 + prefix_len, 20 - prefix_len);
 		if (len < 0)
-			continue; /* entry.path is not a SHA1 sum. Skip */
+			goto handle_non_note; /* entry.path is not a SHA1 */
 		len += prefix_len;
 
 		/*
 		 * If object SHA1 is complete (len == 20), assume note object
-		 * If object SHA1 is incomplete (len < 20), assume note subtree
+		 * If object SHA1 is incomplete (len < 20), and current
+		 * component consists of 2 hex chars, assume note subtree
 		 */
 		if (len <= 20) {
-			unsigned char type = PTR_TYPE_NOTE;
-			struct leaf_node *l = (struct leaf_node *)
+			type = PTR_TYPE_NOTE;
+			l = (struct leaf_node *)
 				xcalloc(sizeof(struct leaf_node), 1);
 			hashcpy(l->key_sha1, object_sha1);
 			hashcpy(l->val_sha1, entry.sha1);
 			if (len < 20) {
-				if (!S_ISDIR(entry.mode))
-					continue; /* entry cannot be subtree */
+				if (!S_ISDIR(entry.mode) || path_len != 2)
+					goto handle_non_note; /* not subtree */
 				l->key_sha1[19] = (unsigned char) len;
 				type = PTR_TYPE_SUBTREE;
 			}
-			note_tree_insert(node, n, l, type,
+			note_tree_insert(t, node, n, l, type,
 					 combine_notes_concatenate);
 		}
+		continue;
+
+handle_non_note:
+		/*
+		 * Determine full path for this non-note entry:
+		 * The filename is already found in entry.path, but the
+		 * directory part of the path must be deduced from the subtree
+		 * containing this entry. We assume here that the overall notes
+		 * tree follows a strict byte-based progressive fanout
+		 * structure (i.e. using 2/38, 2/2/36, etc. fanouts, and not
+		 * e.g. 4/36 fanout). This means that if a non-note is found at
+		 * path "dead/beef", the following code will register it as
+		 * being found on "de/ad/beef".
+		 * On the other hand, if you use such non-obvious non-note
+		 * paths in the middle of a notes tree, you deserve what's
+		 * coming to you ;). Note that for non-notes that are not
+		 * SHA1-like at the top level, there will be no problems.
+		 *
+		 * To conclude, it is strongly advised to make sure non-notes
+		 * have at least one non-hex character in the top-level path
+		 * component.
+		 */
+		{
+			char non_note_path[PATH_MAX];
+			char *p = non_note_path;
+			const char *q = sha1_to_hex(subtree->key_sha1);
+			int i;
+			for (i = 0; i < prefix_len; i++) {
+				*p++ = *q++;
+				*p++ = *q++;
+				*p++ = '/';
+			}
+			strcpy(p, entry.path);
+			add_non_note(t, non_note_path, entry.mode, entry.sha1);
+		}
 	}
 	free(buf);
 }
@@ -426,9 +534,9 @@ static void construct_path_with_fanout(const unsigned char *sha1,
 	strcpy(path + i, hex_sha1 + j);
 }
 
-static int for_each_note_helper(struct int_node *tree, unsigned char n,
-		unsigned char fanout, int flags, each_note_fn fn,
-		void *cb_data)
+static int for_each_note_helper(struct notes_tree *t, struct int_node *tree,
+		unsigned char n, unsigned char fanout, int flags,
+		each_note_fn fn, void *cb_data)
 {
 	unsigned int i;
 	void *p;
@@ -443,7 +551,7 @@ redo:
 		switch (GET_PTR_TYPE(p)) {
 		case PTR_TYPE_INTERNAL:
 			/* recurse into int_node */
-			ret = for_each_note_helper(CLR_PTR_TYPE(p), n + 1,
+			ret = for_each_note_helper(t, CLR_PTR_TYPE(p), n + 1,
 				fanout, flags, fn, cb_data);
 			break;
 		case PTR_TYPE_SUBTREE:
@@ -481,7 +589,7 @@ redo:
 			    !(flags & FOR_EACH_NOTE_DONT_UNPACK_SUBTREES)) {
 				/* unpack subtree and resume traversal */
 				tree->a[i] = NULL;
-				load_subtree(l, tree, n);
+				load_subtree(t, l, tree, n);
 				free(l);
 				goto redo;
 			}
@@ -596,8 +704,29 @@ static int write_each_note_helper(struct tree_write_stack *tws,
 
 struct write_each_note_data {
 	struct tree_write_stack *root;
+	struct non_note *next_non_note;
 };
 
+static int write_each_non_note_until(const char *note_path,
+		struct write_each_note_data *d)
+{
+	struct non_note *n = d->next_non_note;
+	int cmp, ret;
+	while (n && (!note_path || (cmp = strcmp(n->path, note_path)) <= 0)) {
+		if (note_path && cmp == 0)
+			; /* do nothing, prefer note to non-note */
+		else {
+			ret = write_each_note_helper(d->root, n->path, n->mode,
+						     n->sha1);
+			if (ret)
+				return ret;
+		}
+		n = n->next;
+	}
+	d->next_non_note = n;
+	return 0;
+}
+
 static int write_each_note(const unsigned char *object_sha1,
 		const unsigned char *note_sha1, char *note_path,
 		void *cb_data)
@@ -615,7 +744,9 @@ static int write_each_note(const unsigned char *object_sha1,
 	}
 	assert(note_path_len <= 40 + 19);
 
-	return write_each_note_helper(d->root, note_path, mode, note_sha1);
+	/* Weave non-note entries into note entries */
+	return  write_each_non_note_until(note_path, d) ||
+		write_each_note_helper(d->root, note_path, mode, note_sha1);
 }
 
 int combine_notes_concatenate(unsigned char *cur_sha1,
@@ -696,6 +827,8 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
 		combine_notes = combine_notes_concatenate;
 
 	t->root = (struct int_node *) xcalloc(sizeof(struct int_node), 1);
+	t->first_non_note = NULL;
+	t->prev_non_note = NULL;
 	t->ref = notes_ref ? xstrdup(notes_ref) : NULL;
 	t->combine_notes = combine_notes;
 	t->initialized = 1;
@@ -709,7 +842,7 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
 
 	hashclr(root_tree.key_sha1);
 	hashcpy(root_tree.val_sha1, sha1);
-	load_subtree(&root_tree, t->root, 0);
+	load_subtree(t, &root_tree, t->root, 0);
 }
 
 void add_note(struct notes_tree *t, const unsigned char *object_sha1,
@@ -725,7 +858,7 @@ void add_note(struct notes_tree *t, const unsigned char *object_sha1,
 	l = (struct leaf_node *) xmalloc(sizeof(struct leaf_node));
 	hashcpy(l->key_sha1, object_sha1);
 	hashcpy(l->val_sha1, note_sha1);
-	note_tree_insert(t->root, 0, l, PTR_TYPE_NOTE, combine_notes);
+	note_tree_insert(t, t->root, 0, l, PTR_TYPE_NOTE, combine_notes);
 }
 
 void remove_note(struct notes_tree *t, const unsigned char *object_sha1)
@@ -748,7 +881,7 @@ const unsigned char *get_note(struct notes_tree *t,
 	if (!t)
 		t = &default_notes_tree;
 	assert(t->initialized);
-	found = note_tree_find(t->root, 0, object_sha1);
+	found = note_tree_find(t, t->root, 0, object_sha1);
 	return found ? found->val_sha1 : NULL;
 }
 
@@ -758,7 +891,7 @@ int for_each_note(struct notes_tree *t, int flags, each_note_fn fn,
 	if (!t)
 		t = &default_notes_tree;
 	assert(t->initialized);
-	return for_each_note_helper(t->root, 0, 0, flags, fn, cb_data);
+	return for_each_note_helper(t, t->root, 0, 0, flags, fn, cb_data);
 }
 
 int write_notes_tree(struct notes_tree *t, unsigned char *result)
@@ -776,11 +909,13 @@ int write_notes_tree(struct notes_tree *t, unsigned char *result)
 	strbuf_init(&root.buf, 256 * (32 + 40)); /* assume 256 entries */
 	root.path[0] = root.path[1] = '\0';
 	cb_data.root = &root;
+	cb_data.next_non_note = t->first_non_note;
 
 	/* Write tree objects representing current notes tree */
 	ret = for_each_note(t, FOR_EACH_NOTE_DONT_UNPACK_SUBTREES |
 				FOR_EACH_NOTE_YIELD_SUBTREES,
 			write_each_note, &cb_data) ||
+		write_each_non_note_until(NULL, &cb_data) ||
 		tree_write_stack_finish_subtree(&root) ||
 		write_sha1_file(root.buf.buf, root.buf.len, tree_type, result);
 	strbuf_release(&root.buf);
@@ -794,6 +929,12 @@ void free_notes(struct notes_tree *t)
 	if (t->root)
 		note_tree_free(t->root);
 	free(t->root);
+	while (t->first_non_note) {
+		t->prev_non_note = t->first_non_note->next;
+		free(t->first_non_note->path);
+		free(t->first_non_note);
+		t->first_non_note = t->prev_non_note;
+	}
 	free(t->ref);
 	memset(t, 0, sizeof(struct notes_tree));
 }
diff --git a/notes.h b/notes.h
index 20d6e17..f98578f 100644
--- a/notes.h
+++ b/notes.h
@@ -36,6 +36,7 @@ int combine_notes_ignore(unsigned char *cur_sha1, const unsigned char *new_sha1)
  */
 extern struct notes_tree {
 	struct int_node *root;
+	struct non_note *first_non_note, *prev_non_note;
 	char *ref;
 	combine_notes_fn *combine_notes;
 	int initialized;
diff --git a/t/t3303-notes-subtrees.sh b/t/t3303-notes-subtrees.sh
index edc4bc8..75ec187 100755
--- a/t/t3303-notes-subtrees.sh
+++ b/t/t3303-notes-subtrees.sh
@@ -95,12 +95,12 @@ INPUT_END
 test_expect_success 'test notes in 2/38-fanout' 'test_sha1_based "s|^..|&/|"'
 test_expect_success 'verify notes in 2/38-fanout' 'verify_notes'
 
-test_expect_success 'test notes in 4/36-fanout' 'test_sha1_based "s|^....|&/|"'
-test_expect_success 'verify notes in 4/36-fanout' 'verify_notes'
-
 test_expect_success 'test notes in 2/2/36-fanout' 'test_sha1_based "s|^\(..\)\(..\)|\1/\2/|"'
 test_expect_success 'verify notes in 2/2/36-fanout' 'verify_notes'
 
+test_expect_success 'test notes in 2/2/2/34-fanout' 'test_sha1_based "s|^\(..\)\(..\)\(..\)|\1/\2/\3/|"'
+test_expect_success 'verify notes in 2/2/2/34-fanout' 'verify_notes'
+
 test_same_notes () {
 	(
 		start_note_commit &&
@@ -128,14 +128,17 @@ INPUT_END
 	git fast-import --quiet
 }
 
-test_expect_success 'test same notes in 4/36-fanout and 2/38-fanout' 'test_same_notes "s|^..|&/|" "s|^....|&/|"'
-test_expect_success 'verify same notes in 4/36-fanout and 2/38-fanout' 'verify_notes'
+test_expect_success 'test same notes in no fanout and 2/38-fanout' 'test_same_notes "s|^..|&/|" ""'
+test_expect_success 'verify same notes in no fanout and 2/38-fanout' 'verify_notes'
+
+test_expect_success 'test same notes in no fanout and 2/2/36-fanout' 'test_same_notes "s|^\(..\)\(..\)|\1/\2/|" ""'
+test_expect_success 'verify same notes in no fanout and 2/2/36-fanout' 'verify_notes'
 
 test_expect_success 'test same notes in 2/38-fanout and 2/2/36-fanout' 'test_same_notes "s|^\(..\)\(..\)|\1/\2/|" "s|^..|&/|"'
 test_expect_success 'verify same notes in 2/38-fanout and 2/2/36-fanout' 'verify_notes'
 
-test_expect_success 'test same notes in 4/36-fanout and 2/2/36-fanout' 'test_same_notes "s|^\(..\)\(..\)|\1/\2/|" "s|^....|&/|"'
-test_expect_success 'verify same notes in 4/36-fanout and 2/2/36-fanout' 'verify_notes'
+test_expect_success 'test same notes in 2/2/2/34-fanout and 2/2/36-fanout' 'test_same_notes "s|^\(..\)\(..\)|\1/\2/|" "s|^\(..\)\(..\)\(..\)|\1/\2/\3/|"'
+test_expect_success 'verify same notes in 2/2/2/34-fanout and 2/2/36-fanout' 'verify_notes'
 
 test_concatenated_notes () {
 	(
@@ -176,13 +179,16 @@ verify_concatenated_notes () {
     test_cmp expect output
 }
 
-test_expect_success 'test notes in 4/36-fanout concatenated with 2/38-fanout' 'test_concatenated_notes "s|^..|&/|" "s|^....|&/|"'
-test_expect_success 'verify notes in 4/36-fanout concatenated with 2/38-fanout' 'verify_concatenated_notes'
+test_expect_success 'test notes in no fanout concatenated with 2/38-fanout' 'test_concatenated_notes "s|^..|&/|" ""'
+test_expect_success 'verify notes in no fanout concatenated with 2/38-fanout' 'verify_concatenated_notes'
+
+test_expect_success 'test notes in no fanout concatenated with 2/2/36-fanout' 'test_concatenated_notes "s|^\(..\)\(..\)|\1/\2/|" ""'
+test_expect_success 'verify notes in no fanout concatenated with 2/2/36-fanout' 'verify_concatenated_notes'
 
 test_expect_success 'test notes in 2/38-fanout concatenated with 2/2/36-fanout' 'test_concatenated_notes "s|^\(..\)\(..\)|\1/\2/|" "s|^..|&/|"'
 test_expect_success 'verify notes in 2/38-fanout concatenated with 2/2/36-fanout' 'verify_concatenated_notes'
 
-test_expect_success 'test notes in 4/36-fanout concatenated with 2/2/36-fanout' 'test_concatenated_notes "s|^\(..\)\(..\)|\1/\2/|" "s|^....|&/|"'
-test_expect_success 'verify notes in 4/36-fanout concatenated with 2/2/36-fanout' 'verify_concatenated_notes'
+test_expect_success 'test notes in 2/2/36-fanout concatenated with 2/2/2/34-fanout' 'test_concatenated_notes "s|^\(..\)\(..\)\(..\)|\1/\2/\3/|" "s|^\(..\)\(..\)|\1/\2/|"'
+test_expect_success 'verify notes in 2/2/36-fanout concatenated with 2/2/2/34-fanout' 'verify_concatenated_notes'
 
 test_done
diff --git a/t/t3304-notes-mixed.sh b/t/t3304-notes-mixed.sh
index 256687f..c975a6d 100755
--- a/t/t3304-notes-mixed.sh
+++ b/t/t3304-notes-mixed.sh
@@ -131,6 +131,17 @@ data <<EOF
 another non-note with SHA1-like name
 EOF
 
+M 644 inline de/adbeefdeadbeefdeadbeefdeadbeefdeadbeef
+data <<EOF
+This is actually a valid note, albeit to a non-existing object.
+It is needed in order to trigger the "mishandling" of the dead/beef non-note.
+EOF
+
+M 644 inline dead/beef
+data <<EOF
+yet another non-note with SHA1-like name
+EOF
+
 INPUT_END
 	git fast-import --quiet <input &&
 	git config core.notesRef refs/notes/commits
@@ -158,6 +169,9 @@ EXPECT_END
 cat >expect_nn3 <<EXPECT_END
 another non-note with SHA1-like name
 EXPECT_END
+cat >expect_nn4 <<EXPECT_END
+yet another non-note with SHA1-like name
+EXPECT_END
 
 test_expect_success "verify contents of non-notes" '
 
@@ -166,7 +180,27 @@ test_expect_success "verify contents of non-notes" '
 	git cat-file -p refs/notes/commits:deadbeef > actual_nn2 &&
 	test_cmp expect_nn2 actual_nn2 &&
 	git cat-file -p refs/notes/commits:de/adbeef > actual_nn3 &&
-	test_cmp expect_nn3 actual_nn3
+	test_cmp expect_nn3 actual_nn3 &&
+	git cat-file -p refs/notes/commits:dead/beef > actual_nn4 &&
+	test_cmp expect_nn4 actual_nn4
+'
+
+test_expect_success "git-notes preserves non-notes" '
+
+	test_tick &&
+	git notes edit -m "foo bar"
+'
+
+test_expect_success "verify contents of non-notes after git-notes" '
+
+	git cat-file -p refs/notes/commits:foobar/non-note.txt > actual_nn1 &&
+	test_cmp expect_nn1 actual_nn1 &&
+	git cat-file -p refs/notes/commits:deadbeef > actual_nn2 &&
+	test_cmp expect_nn2 actual_nn2 &&
+	git cat-file -p refs/notes/commits:de/adbeef > actual_nn3 &&
+	test_cmp expect_nn3 actual_nn3 &&
+	git cat-file -p refs/notes/commits:dead/beef > actual_nn4 &&
+	test_cmp expect_nn4 actual_nn4
 '
 
 test_done
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 16/30] Teach builtin-notes to remove empty notes

[Johan Herland]

When the result of editing a note is an empty string, the associated note
entry should be deleted from the notes tree.

This allows deleting notes by invoking either "git notes -m ''" or
"git notes -F /dev/null".

Signed-off-by: Johan Herland <johan@herland.net>
---
 builtin-notes.c  |   15 +++++++++++----
 t/t3301-notes.sh |   31 +++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/builtin-notes.c b/builtin-notes.c
index 89aa6e0..7b4cb13 100644
--- a/builtin-notes.c
+++ b/builtin-notes.c
@@ -113,7 +113,7 @@ static void create_note(const unsigned char *object,
 
 	stripspace(buf, 1);
 
-	if (!skip_editor && !buf->len) {
+	if (!buf->len) {
 		fprintf(stderr, "Removing note for object %s\n",
 			sha1_to_hex(object));
 		hashclr(result);
@@ -197,7 +197,8 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	struct notes_tree *t;
 	unsigned char object[20], new_note[20];
 	const unsigned char *note;
-	const char *object_ref;
+	const char *object_ref, *logmsg;
+
 	int edit = 0, show = 0;
 	const char *msgfile = NULL;
 	struct msg_arg msg = { 0, STRBUF_INIT };
@@ -271,8 +272,14 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	}
 
 	create_note(object, &buf, msg.given || msgfile, note, new_note);
-	add_note(t, object, new_note, combine_notes_overwrite);
-	commit_notes(t, "Note added by 'git notes edit'");
+	if (is_null_sha1(new_note)) {
+		remove_note(t, object);
+		logmsg = "Note removed by 'git notes edit'";
+	} else {
+		add_note(t, object, new_note, combine_notes_overwrite);
+		logmsg = "Note added by 'git notes edit'";
+	}
+	commit_notes(t, logmsg);
 
 	free_notes(t);
 	strbuf_release(&buf);
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index fd5e593..fe59e73 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -234,6 +234,37 @@ test_expect_success 'show -m notes' '
 	test_cmp expect-m output
 '
 
+test_expect_success 'remove note with -F /dev/null (setup)' '
+	git notes edit -F /dev/null
+'
+
+cat > expect-rm-F << EOF
+commit bd1753200303d0a0344be813e504253b3d98e74d
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:17:13 2005 -0700
+
+    5th
+EOF
+
+printf "\n" >> expect-rm-F
+cat expect-F >> expect-rm-F
+
+test_expect_success 'verify note removal with -F /dev/null' '
+	git log -4 > output &&
+	test_cmp expect-rm-F output &&
+	! git notes show
+'
+
+test_expect_success 'do not create empty note with -m "" (setup)' '
+	git notes edit -m ""
+'
+
+test_expect_success 'verify non-creation of note with -m ""' '
+	git log -4 > output &&
+	test_cmp expect-rm-F output &&
+	! git notes show
+'
+
 test_expect_success 'create other note on a different notes ref (setup)' '
 	: > a6 &&
 	git add a6 &&
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 17/30] builtin-notes: Add "remove" subcommand for removing existing notes

[Johan Herland]

Using "git notes remove" is equivalent to specifying an empty note message.

The patch includes tests verifying correct behaviour of the new subcommand.

Signed-off-by: Johan Herland <johan@herland.net>
---
 Documentation/git-notes.txt |   15 ++++++---
 builtin-notes.c             |   65 +++++++++++++++++++++++-------------------
 t/t3301-notes.sh            |   27 ++++++++++++++++++
 3 files changed, 73 insertions(+), 34 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 0d1ada6..a52d23a 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -8,14 +8,14 @@ git-notes - Add/inspect commit notes
 SYNOPSIS
 --------
 [verse]
-'git notes' (edit [-F <file> | -m <msg>] | show) [commit]
+'git notes' (edit [-F <file> | -m <msg>] | show | remove) [commit]
 
 DESCRIPTION
 -----------
-This command allows you to add notes to commit messages, without
-changing the commit.  To discern these notes from the message stored
-in the commit object, the notes are indented like the message, after
-an unindented line saying "Notes:".
+This command allows you to add/remove notes to/from commit messages,
+without changing the commit.  To discern these notes from the message
+stored in the commit object, the notes are indented like the message,
+after an unindented line saying "Notes:".
 
 To disable commit notes, you have to set the config variable
 core.notesRef to the empty string.  Alternatively, you can set it
@@ -32,6 +32,11 @@ edit::
 show::
 	Show the notes for a given commit (defaults to HEAD).
 
+remove::
+	Remove the notes for a given commit (defaults to HEAD).
+	This is equivalent to specifying an empty note message to
+	the `edit` subcommand.
+
 
 OPTIONS
 -------
diff --git a/builtin-notes.c b/builtin-notes.c
index 7b4cb13..7c40075 100644
--- a/builtin-notes.c
+++ b/builtin-notes.c
@@ -20,6 +20,7 @@
 static const char * const git_notes_usage[] = {
 	"git notes edit [-m <msg> | -F <file>] [<object>]",
 	"git notes show [<object>]",
+	"git notes remove [<object>]",
 	NULL
 };
 
@@ -197,9 +198,10 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	struct notes_tree *t;
 	unsigned char object[20], new_note[20];
 	const unsigned char *note;
-	const char *object_ref, *logmsg;
+	const char *object_ref;
+	char logmsg[100];
 
-	int edit = 0, show = 0;
+	int edit = 0, show = 0, remove = 0;
 	const char *msgfile = NULL;
 	struct msg_arg msg = { 0, STRBUF_INIT };
 	struct option options[] = {
@@ -218,10 +220,22 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		edit = 1;
 	else if (argc && !strcmp(argv[0], "show"))
 		show = 1;
+	else if (argc && !strcmp(argv[0], "remove"))
+		remove = 1;
 
-	if (edit + show != 1)
+	if (edit + show + remove != 1)
 		usage_with_options(git_notes_usage, options);
 
+	if ((msg.given || msgfile) && !edit) {
+		error("cannot use -m/-F options with %s subcommand.", argv[0]);
+		usage_with_options(git_notes_usage, options);
+	}
+
+	if (msg.given && msgfile) {
+		error("mixing -m and -F options is not allowed.");
+		usage_with_options(git_notes_usage, options);
+	}
+
 	object_ref = argc == 2 ? argv[1] : "HEAD";
 	if (argc > 2) {
 		error("too many parameters");
@@ -236,7 +250,7 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 
 	if (prefixcmp(t->ref, "refs/notes/"))
 		die("Refusing to %s notes in %s (outside of refs/notes/)",
-		    edit ? "edit" : "show", t->ref);
+		    argv[0], t->ref);
 
 	note = get_note(t, object);
 
@@ -250,35 +264,28 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		return execv_git_cmd(show_args);
 	}
 
-	/* edit command */
-
-	if (msg.given || msgfile) {
-		if (msg.given && msgfile) {
-			error("mixing -m and -F options is not allowed.");
-			usage_with_options(git_notes_usage, options);
-		}
-		if (msg.given)
-			strbuf_addbuf(&buf, &(msg.buf));
-		else {
-			if (!strcmp(msgfile, "-")) {
-				if (strbuf_read(&buf, 0, 1024) < 0)
-					die_errno("cannot read '%s'", msgfile);
-			} else {
-				if (strbuf_read_file(&buf, msgfile, 1024) < 0)
-					die_errno("could not open or read '%s'",
-						msgfile);
-			}
-		}
+	/* edit/remove command */
+
+	if (remove)
+		strbuf_reset(&buf);
+	else if (msg.given)
+		strbuf_addbuf(&buf, &(msg.buf));
+	else if (msgfile) {
+		if (!strcmp(msgfile, "-")) {
+			if (strbuf_read(&buf, 0, 1024) < 0)
+				die_errno("cannot read '%s'", msgfile);
+		} else if (strbuf_read_file(&buf, msgfile, 1024) < 0)
+			die_errno("could not open or read '%s'", msgfile);
 	}
 
-	create_note(object, &buf, msg.given || msgfile, note, new_note);
-	if (is_null_sha1(new_note)) {
+	create_note(object, &buf, msg.given || msgfile || remove, note,
+		    new_note);
+	if (is_null_sha1(new_note))
 		remove_note(t, object);
-		logmsg = "Note removed by 'git notes edit'";
-	} else {
+	else
 		add_note(t, object, new_note, combine_notes_overwrite);
-		logmsg = "Note added by 'git notes edit'";
-	}
+	snprintf(logmsg, sizeof(logmsg), "Note %s by 'git notes %s'",
+		 is_null_sha1(new_note) ? "removed" : "added", argv[0]);
 	commit_notes(t, logmsg);
 
 	free_notes(t);
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index fe59e73..d29daac 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -265,6 +265,33 @@ test_expect_success 'verify non-creation of note with -m ""' '
 	! git notes show
 '
 
+test_expect_success 'remove note with "git notes remove" (setup)' '
+	git notes remove HEAD^
+'
+
+cat > expect-rm-remove << EOF
+commit bd1753200303d0a0344be813e504253b3d98e74d
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:17:13 2005 -0700
+
+    5th
+
+commit 15023535574ded8b1a89052b32673f84cf9582b8
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:16:13 2005 -0700
+
+    4th
+EOF
+
+printf "\n" >> expect-rm-remove
+cat expect-multiline >> expect-rm-remove
+
+test_expect_success 'verify note removal with "git notes remove"' '
+	git log -4 > output &&
+	test_cmp expect-rm-remove output &&
+	! git notes show HEAD^
+'
+
 test_expect_success 'create other note on a different notes ref (setup)' '
 	: > a6 &&
 	git add a6 &&
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 18/30] t3305: Verify that removing notes triggers automatic fanout consolidation

[Johan Herland]

Signed-off-by: Johan Herland <johan@herland.net>
---
 t/t3305-notes-fanout.sh |   47 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 46 insertions(+), 1 deletions(-)

diff --git a/t/t3305-notes-fanout.sh b/t/t3305-notes-fanout.sh
index 823b0ff..c6d263b 100755
--- a/t/t3305-notes-fanout.sh
+++ b/t/t3305-notes-fanout.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='Test that adding many notes triggers automatic fanout restructuring'
+test_description='Test that adding/removing many notes triggers automatic fanout restructuring'
 
 . ./test-lib.sh
 
@@ -47,4 +47,49 @@ test_expect_success 'many notes created with git-notes triggers fanout' '
 	done
 '
 
+test_expect_success 'deleting most notes with git-notes' '
+	num_notes=250 &&
+	i=0 &&
+	git rev-list HEAD |
+	while read sha1
+	do
+		i=$(($i + 1)) &&
+		if test $i -gt $num_notes
+		then
+			break
+		fi &&
+		test_tick &&
+		git notes remove "$sha1"
+	done
+'
+
+test_expect_success 'most notes deleted correctly with git-notes' '
+	git log HEAD~250 | grep "^    " > output &&
+	i=50 &&
+	while test $i -gt 0
+	do
+		echo "    commit #$i" &&
+		echo "    note #$i" &&
+		i=$(($i - 1));
+	done > expect &&
+	test_cmp expect output
+'
+
+test_expect_success 'deleting most notes triggers fanout consolidation' '
+	# Expect entire notes tree to have a fanout == 0
+	git ls-tree -r --name-only refs/notes/commits |
+	while read path
+	do
+		case "$path" in
+		????????????????????????????????????????)
+			: true
+			;;
+		*)
+			echo "Invalid path \"$path\"" &&
+			return 1
+			;;
+		esac
+	done
+'
+
 test_done
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 19/30] Notes API: prune_notes(): Prune notes that belong to non-existing objects

[Johan Herland]

When an object is made unreachable by Git, any notes that annotate that object
are not automagically made unreachable, since all notes are always trivially
reachable from a notes ref. In order to remove notes for non-existing objects,
we therefore need to add functionality for traversing the notes tree and
explicitly removing references to notes that annotate non-reachable objects.
Thus the notes objects themselves also become unreachable, and are removed
by a later garbage collect.

prune_notes() performs this traversal (by using for_each_note() internally),
and removes the notes in question from the notes tree.

Note that the effect of prune_notes() is not persistent unless a subsequent
call to write_notes_tree() is made.

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |   39 +++++++++++++++++++++++++++++++++++++++
 notes.h |   12 ++++++++++++
 2 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/notes.c b/notes.c
index d432517..3ba3e6d 100644
--- a/notes.c
+++ b/notes.c
@@ -749,6 +749,29 @@ static int write_each_note(const unsigned char *object_sha1,
 		write_each_note_helper(d->root, note_path, mode, note_sha1);
 }
 
+struct note_delete_list {
+	struct note_delete_list *next;
+	const unsigned char *sha1;
+};
+
+static int prune_notes_helper(const unsigned char *object_sha1,
+		const unsigned char *note_sha1, char *note_path,
+		void *cb_data)
+{
+	struct note_delete_list **l = (struct note_delete_list **) cb_data;
+	struct note_delete_list *n;
+
+	if (has_sha1_file(object_sha1))
+		return 0; /* nothing to do for this note */
+
+	/* failed to find object => prune this note */
+	n = (struct note_delete_list *) xmalloc(sizeof(*n));
+	n->next = *l;
+	n->sha1 = object_sha1;
+	*l = n;
+	return 0;
+}
+
 int combine_notes_concatenate(unsigned char *cur_sha1,
 		const unsigned char *new_sha1)
 {
@@ -922,6 +945,22 @@ int write_notes_tree(struct notes_tree *t, unsigned char *result)
 	return ret;
 }
 
+void prune_notes(struct notes_tree *t)
+{
+	struct note_delete_list *l = NULL;
+
+	if (!t)
+		t = &default_notes_tree;
+	assert(t->initialized);
+
+	for_each_note(t, 0, prune_notes_helper, &l);
+
+	while (l) {
+		remove_note(t, l->sha1);
+		l = l->next;
+	}
+}
+
 void free_notes(struct notes_tree *t)
 {
 	if (!t)
diff --git a/notes.h b/notes.h
index f98578f..bad03cc 100644
--- a/notes.h
+++ b/notes.h
@@ -162,6 +162,18 @@ int for_each_note(struct notes_tree *t, int flags, each_note_fn fn,
 int write_notes_tree(struct notes_tree *t, unsigned char *result);
 
 /*
+ * Remove all notes annotating non-existing objects from the given notes tree
+ *
+ * All notes in the given notes_tree that are associated with objects that no
+ * longer exist in the database, are removed from the notes tree.
+ *
+ * IMPORTANT: The changes made by prune_notes() to the given notes_tree
+ * structure are not persistent until a subsequent call to write_notes_tree()
+ * returns zero.
+ */
+void prune_notes(struct notes_tree *t);
+
+/*
  * Free (and de-initialize) the given notes_tree structure
  *
  * IMPORTANT: Changes made to the given notes_tree since the last, successful
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 20/30] builtin-notes: Add "prune" subcommand for removing notes for missing objects

[Johan Herland]

"git notes prune" will remove all notes that annotate unreachable/non-
existing objects.

The patch includes tests verifying correct behaviour of the new subcommand.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johan Herland <johan@herland.net>
---
 Documentation/git-notes.txt |    4 +-
 builtin-notes.c             |   28 ++++++++-----
 t/t3306-notes-prune.sh      |   94 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 115 insertions(+), 11 deletions(-)
 create mode 100755 t/t3306-notes-prune.sh

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index a52d23a..3973f90 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -8,7 +8,7 @@ git-notes - Add/inspect commit notes
 SYNOPSIS
 --------
 [verse]
-'git notes' (edit [-F <file> | -m <msg>] | show | remove) [commit]
+'git notes' (edit [-F <file> | -m <msg>] | show | remove | prune) [commit]
 
 DESCRIPTION
 -----------
@@ -37,6 +37,8 @@ remove::
 	This is equivalent to specifying an empty note message to
 	the `edit` subcommand.
 
+prune::
+	Remove all notes for non-existing/unreachable objects.
 
 OPTIONS
 -------
diff --git a/builtin-notes.c b/builtin-notes.c
index 7c40075..48bc455 100644
--- a/builtin-notes.c
+++ b/builtin-notes.c
@@ -21,6 +21,7 @@ static const char * const git_notes_usage[] = {
 	"git notes edit [-m <msg> | -F <file>] [<object>]",
 	"git notes show [<object>]",
 	"git notes remove [<object>]",
+	"git notes prune",
 	NULL
 };
 
@@ -201,7 +202,7 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	const char *object_ref;
 	char logmsg[100];
 
-	int edit = 0, show = 0, remove = 0;
+	int edit = 0, show = 0, remove = 0, prune = 0;
 	const char *msgfile = NULL;
 	struct msg_arg msg = { 0, STRBUF_INIT };
 	struct option options[] = {
@@ -222,8 +223,10 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		show = 1;
 	else if (argc && !strcmp(argv[0], "remove"))
 		remove = 1;
+	else if (argc && !strcmp(argv[0], "prune"))
+		prune = 1;
 
-	if (edit + show + remove != 1)
+	if (edit + show + remove + prune != 1)
 		usage_with_options(git_notes_usage, options);
 
 	if ((msg.given || msgfile) && !edit) {
@@ -237,7 +240,7 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	}
 
 	object_ref = argc == 2 ? argv[1] : "HEAD";
-	if (argc > 2) {
+	if (argc > 2 || (prune && argc > 1)) {
 		error("too many parameters");
 		usage_with_options(git_notes_usage, options);
 	}
@@ -264,7 +267,7 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		return execv_git_cmd(show_args);
 	}
 
-	/* edit/remove command */
+	/* edit/remove/prune command */
 
 	if (remove)
 		strbuf_reset(&buf);
@@ -278,12 +281,17 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 			die_errno("could not open or read '%s'", msgfile);
 	}
 
-	create_note(object, &buf, msg.given || msgfile || remove, note,
-		    new_note);
-	if (is_null_sha1(new_note))
-		remove_note(t, object);
-	else
-		add_note(t, object, new_note, combine_notes_overwrite);
+	if (prune) {
+		hashclr(new_note);
+		prune_notes(t);
+	} else {
+		create_note(object, &buf, msg.given || msgfile || remove, note,
+			    new_note);
+		if (is_null_sha1(new_note))
+			remove_note(t, object);
+		else
+			add_note(t, object, new_note, combine_notes_overwrite);
+	}
 	snprintf(logmsg, sizeof(logmsg), "Note %s by 'git notes %s'",
 		 is_null_sha1(new_note) ? "removed" : "added", argv[0]);
 	commit_notes(t, logmsg);
diff --git a/t/t3306-notes-prune.sh b/t/t3306-notes-prune.sh
new file mode 100755
index 0000000..b0adc7e
--- /dev/null
+++ b/t/t3306-notes-prune.sh
@@ -0,0 +1,94 @@
+#!/bin/sh
+
+test_description='Test git notes prune'
+
+. ./test-lib.sh
+
+test_expect_success 'setup: create a few commits with notes' '
+
+	: > file1 &&
+	git add file1 &&
+	test_tick &&
+	git commit -m 1st &&
+	git notes edit -m "Note #1" &&
+	: > file2 &&
+	git add file2 &&
+	test_tick &&
+	git commit -m 2nd &&
+	git notes edit -m "Note #2" &&
+	: > file3 &&
+	git add file3 &&
+	test_tick &&
+	git commit -m 3rd &&
+	git notes edit -m "Note #3"
+'
+
+cat > expect <<END_OF_LOG
+commit 5ee1c35e83ea47cd3cc4f8cbee0568915fbbbd29
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:15:13 2005 -0700
+
+    3rd
+
+Notes:
+    Note #3
+
+commit 08341ad9e94faa089d60fd3f523affb25c6da189
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:14:13 2005 -0700
+
+    2nd
+
+Notes:
+    Note #2
+
+commit ab5f302035f2e7aaf04265f08b42034c23256e1f
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:13:13 2005 -0700
+
+    1st
+
+Notes:
+    Note #1
+END_OF_LOG
+
+test_expect_success 'verify commits and notes' '
+
+	git log > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'remove some commits' '
+
+	git reset --hard HEAD~2 &&
+	git reflog expire --expire=now HEAD &&
+	git gc --prune=now
+'
+
+test_expect_success 'verify that commits are gone' '
+
+	! git cat-file -p 5ee1c35e83ea47cd3cc4f8cbee0568915fbbbd29 &&
+	! git cat-file -p 08341ad9e94faa089d60fd3f523affb25c6da189 &&
+	git cat-file -p ab5f302035f2e7aaf04265f08b42034c23256e1f
+'
+
+test_expect_success 'verify that notes are still present' '
+
+	git notes show 5ee1c35e83ea47cd3cc4f8cbee0568915fbbbd29 &&
+	git notes show 08341ad9e94faa089d60fd3f523affb25c6da189 &&
+	git notes show ab5f302035f2e7aaf04265f08b42034c23256e1f
+'
+
+test_expect_success 'prune notes' '
+
+	git notes prune
+'
+
+test_expect_success 'verify that notes are gone' '
+
+	! git notes show 5ee1c35e83ea47cd3cc4f8cbee0568915fbbbd29 &&
+	! git notes show 08341ad9e94faa089d60fd3f523affb25c6da189 &&
+	git notes show ab5f302035f2e7aaf04265f08b42034c23256e1f
+'
+
+test_done
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 21/30] Documentation: Generalize git-notes docs to 'objects' instead of 'commits'

[Johan Herland]

Notes can annotate arbitrary objects (not only commits), but this is not
reflected in the current documentation.

This patch rewrites the git-notes documentation to talk about 'objects'
instead of 'commits'. However, the discussion on commit notes and how
they are displayed by 'git log' is largely preserved.

Finally, I add myself to the Author/Documentation credits, since most of
the lines in the git-notes code and docs are blamed on me.

Cc: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johan Herland <johan@herland.net>
---
 Documentation/git-notes.txt |   35 ++++++++++++++++++++---------------
 1 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 3973f90..84db2a4 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -3,37 +3,41 @@ git-notes(1)
 
 NAME
 ----
-git-notes - Add/inspect commit notes
+git-notes - Add/inspect object notes
 
 SYNOPSIS
 --------
 [verse]
-'git notes' (edit [-F <file> | -m <msg>] | show | remove | prune) [commit]
+'git notes' (edit [-F <file> | -m <msg>] | show | remove | prune) [object]
 
 DESCRIPTION
 -----------
-This command allows you to add/remove notes to/from commit messages,
-without changing the commit.  To discern these notes from the message
-stored in the commit object, the notes are indented like the message,
-after an unindented line saying "Notes:".
+This command allows you to add/remove notes to/from objects, without
+changing the objects themselves.
 
-To disable commit notes, you have to set the config variable
-core.notesRef to the empty string.  Alternatively, you can set it
-to a different ref, something like "refs/notes/bugzilla".  This setting
-can be overridden by the environment variable "GIT_NOTES_REF".
+A typical use of notes is to extend a commit message without having
+to change the commit itself. Such commit notes can be shown by `git log`
+along with the original commit message. To discern these notes from the
+message stored in the commit object, the notes are indented like the
+message, after an unindented line saying "Notes:".
+
+To disable notes, you have to set the config variable core.notesRef to
+the empty string.  Alternatively, you can set it to a different ref,
+something like "refs/notes/bugzilla".  This setting can be overridden
+by the environment variable "GIT_NOTES_REF".
 
 
 SUBCOMMANDS
 -----------
 
 edit::
-	Edit the notes for a given commit (defaults to HEAD).
+	Edit the notes for a given object (defaults to HEAD).
 
 show::
-	Show the notes for a given commit (defaults to HEAD).
+	Show the notes for a given object (defaults to HEAD).
 
 remove::
-	Remove the notes for a given commit (defaults to HEAD).
+	Remove the notes for a given object (defaults to HEAD).
 	This is equivalent to specifying an empty note message to
 	the `edit` subcommand.
 
@@ -54,11 +58,12 @@ OPTIONS
 
 Author
 ------
-Written by Johannes Schindelin <johannes.schindelin@gmx.de>
+Written by Johannes Schindelin <johannes.schindelin@gmx.de> and
+Johan Herland <johan@herland.net>
 
 Documentation
 -------------
-Documentation by Johannes Schindelin
+Documentation by Johannes Schindelin and Johan Herland
 
 GIT
 ---
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 22/30] builtin-notes: Add "list" subcommand for listing note objects

[Johan Herland]

"git notes list" will list all note objects in the current notes ref (in the
format "<note object> <annotated object>"). "git notes list <object>" will
list the note object associated with the given <object>, or fail loudly if
the given <object> has no associated notes.

If no arguments are given to "git notes", it defaults to the "list"
subcommand. This is for pseudo-compatibility with "git tag" and "git branch".

The patch includes tests verifying correct behaviour of the new subcommand.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johan Herland <johan@herland.net>
---
 Documentation/git-notes.txt |   13 ++++++++++++-
 builtin-notes.c             |   37 ++++++++++++++++++++++++++++++++-----
 t/t3301-notes.sh            |   32 ++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 84db2a4..4d29d5f 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -8,7 +8,12 @@ git-notes - Add/inspect object notes
 SYNOPSIS
 --------
 [verse]
-'git notes' (edit [-F <file> | -m <msg>] | show | remove | prune) [object]
+'git notes' [list [<object>]]
+'git notes' edit [-F <file> | -m <msg>] [<object>]
+'git notes' show [<object>]
+'git notes' remove [<object>]
+'git notes' prune
+
 
 DESCRIPTION
 -----------
@@ -30,6 +35,12 @@ by the environment variable "GIT_NOTES_REF".
 SUBCOMMANDS
 -----------
 
+list::
+	List the notes object for a given object. If no object is
+	given, show a list of all note objects and the objects they
+	annotate (in the format "<note object> <annotated object>").
+	This is the default subcommand if no subcommand is given.
+
 edit::
 	Edit the notes for a given object (defaults to HEAD).
 
diff --git a/builtin-notes.c b/builtin-notes.c
index 48bc455..b808534 100644
--- a/builtin-notes.c
+++ b/builtin-notes.c
@@ -18,6 +18,7 @@
 #include "parse-options.h"
 
 static const char * const git_notes_usage[] = {
+	"git notes [list [<object>]]",
 	"git notes edit [-m <msg> | -F <file>] [<object>]",
 	"git notes show [<object>]",
 	"git notes remove [<object>]",
@@ -31,6 +32,14 @@ static const char note_template[] =
 	"# Write/edit the notes for the following object:\n"
 	"#\n";
 
+static int list_each_note(const unsigned char *object_sha1,
+		const unsigned char *note_sha1, char *note_path,
+		void *cb_data)
+{
+	printf("%s %s\n", sha1_to_hex(note_sha1), sha1_to_hex(object_sha1));
+	return 0;
+}
+
 static void write_note_data(int fd, const unsigned char *sha1)
 {
 	unsigned long size;
@@ -202,7 +211,8 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	const char *object_ref;
 	char logmsg[100];
 
-	int edit = 0, show = 0, remove = 0, prune = 0;
+	int list = 0, edit = 0, show = 0, remove = 0, prune = 0;
+	int given_object;
 	const char *msgfile = NULL;
 	struct msg_arg msg = { 0, STRBUF_INIT };
 	struct option options[] = {
@@ -217,7 +227,9 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 
 	argc = parse_options(argc, argv, prefix, options, git_notes_usage, 0);
 
-	if (argc && !strcmp(argv[0], "edit"))
+	if (argc && !strcmp(argv[0], "list"))
+		list = 1;
+	else if (argc && !strcmp(argv[0], "edit"))
 		edit = 1;
 	else if (argc && !strcmp(argv[0], "show"))
 		show = 1;
@@ -225,8 +237,10 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		remove = 1;
 	else if (argc && !strcmp(argv[0], "prune"))
 		prune = 1;
+	else if (!argc)
+		list = 1; /* Default to 'list' if no other subcommand given */
 
-	if (edit + show + remove + prune != 1)
+	if (list + edit + show + remove + prune != 1)
 		usage_with_options(git_notes_usage, options);
 
 	if ((msg.given || msgfile) && !edit) {
@@ -239,7 +253,8 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		usage_with_options(git_notes_usage, options);
 	}
 
-	object_ref = argc == 2 ? argv[1] : "HEAD";
+	given_object = argc == 2;
+	object_ref = given_object ? argv[1] : "HEAD";
 	if (argc > 2 || (prune && argc > 1)) {
 		error("too many parameters");
 		usage_with_options(git_notes_usage, options);
@@ -257,9 +272,21 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 
 	note = get_note(t, object);
 
+	/* list command */
+
+	if (list) {
+		if (given_object) {
+			if (note) {
+				puts(sha1_to_hex(note));
+				return 0;
+			}
+		} else
+			return for_each_note(t, 0, list_each_note, NULL);
+	}
+
 	/* show command */
 
-	if (show && !note) {
+	if ((list || show) && !note) {
 		error("No note found for object %s.", sha1_to_hex(object));
 		return 1;
 	} else if (show) {
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index d29daac..768a1cb 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -292,6 +292,38 @@ test_expect_success 'verify note removal with "git notes remove"' '
 	! git notes show HEAD^
 '
 
+cat > expect << EOF
+c18dc024e14f08d18d14eea0d747ff692d66d6a3 1584215f1d29c65e99c6c6848626553fdd07fd75
+c9c6af7f78bc47490dbf3e822cf2f3c24d4b9061 268048bfb8a1fb38e703baceb8ab235421bf80c5
+EOF
+
+test_expect_success 'list notes with "git notes list"' '
+	git notes list > output &&
+	test_cmp expect output
+'
+
+test_expect_success 'list notes with "git notes"' '
+	git notes > output &&
+	test_cmp expect output
+'
+
+cat > expect << EOF
+c18dc024e14f08d18d14eea0d747ff692d66d6a3
+EOF
+
+test_expect_success 'list specific note with "git notes list <object>"' '
+	git notes list HEAD^^ > output &&
+	test_cmp expect output
+'
+
+cat > expect << EOF
+EOF
+
+test_expect_success 'listing non-existing notes fails' '
+	test_must_fail git notes list HEAD > output &&
+	test_cmp expect output
+'
+
 test_expect_success 'create other note on a different notes ref (setup)' '
 	: > a6 &&
 	git add a6 &&
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 23/30] builtin-notes: Add --message/--file aliases for -m/-F options

[Johan Herland]

Signed-off-by: Johan Herland <johan@herland.net>
---
 Documentation/git-notes.txt |    2 ++
 builtin-notes.c             |    4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 4d29d5f..8969f6f 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -58,11 +58,13 @@ prune::
 OPTIONS
 -------
 -m <msg>::
+--message=<msg>::
 	Use the given note message (instead of prompting).
 	If multiple `-m` options are given, their values
 	are concatenated as separate paragraphs.
 
 -F <file>::
+--file=<file>::
 	Take the note message from the given file.  Use '-' to
 	read the note message from the standard input.
 
diff --git a/builtin-notes.c b/builtin-notes.c
index b808534..ec959bc 100644
--- a/builtin-notes.c
+++ b/builtin-notes.c
@@ -217,9 +217,9 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	struct msg_arg msg = { 0, STRBUF_INIT };
 	struct option options[] = {
 		OPT_GROUP("Notes edit options"),
-		OPT_CALLBACK('m', NULL, &msg, "msg",
+		OPT_CALLBACK('m', "message", &msg, "msg",
 			     "note contents as a string", parse_msg_arg),
-		OPT_FILENAME('F', NULL, &msgfile, "note contents in a file"),
+		OPT_FILENAME('F', "file", &msgfile, "note contents in a file"),
 		OPT_END()
 	};
 
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 24/30] builtin-notes: Add "add" subcommand for adding notes to objects

[Johan Herland]

"git notes add" is identical to "git notes edit" except that instead of
editing existing notes for a given object, you can only add notes to an
object that currently has none. If "git notes add" finds existing notes
for the given object, the addition is aborted. However, if the new
-f/--force option is used, "git notes add" will _overwrite_ the existing
notes with the new notes contents.

If there is no existing notes for the given object. "git notes add" is
identical to "git notes edit" (i.e. it adds a new note).

The patch includes tests verifying correct behaviour of the new subcommand.

Suggested-by: Joey Hess <joey@kitenet.net>
Improved-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johan Herland <johan@herland.net>
---
 Documentation/git-notes.txt |   11 ++++++++
 builtin-notes.c             |   30 ++++++++++++++++++++---
 t/t3301-notes.sh            |   55 +++++++++++++++++++++++++++++--------------
 3 files changed, 74 insertions(+), 22 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 8969f6f..94e12b5 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -9,6 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git notes' [list [<object>]]
+'git notes' add [-f] [-F <file> | -m <msg>] [<object>]
 'git notes' edit [-F <file> | -m <msg>] [<object>]
 'git notes' show [<object>]
 'git notes' remove [<object>]
@@ -41,6 +42,11 @@ list::
 	annotate (in the format "<note object> <annotated object>").
 	This is the default subcommand if no subcommand is given.
 
+add::
+	Add notes for a given object (defaults to HEAD). Abort if the
+	object already has notes, abort. (use `-f` to overwrite an
+	existing note).
+
 edit::
 	Edit the notes for a given object (defaults to HEAD).
 
@@ -57,6 +63,11 @@ prune::
 
 OPTIONS
 -------
+-f::
+--force::
+	When adding notes to an object that already has notes,
+	overwrite the existing notes (instead of aborting).
+
 -m <msg>::
 --message=<msg>::
 	Use the given note message (instead of prompting).
diff --git a/builtin-notes.c b/builtin-notes.c
index ec959bc..006edf6 100644
--- a/builtin-notes.c
+++ b/builtin-notes.c
@@ -19,6 +19,7 @@
 
 static const char * const git_notes_usage[] = {
 	"git notes [list [<object>]]",
+	"git notes add [-f] [-m <msg> | -F <file>] [<object>]",
 	"git notes edit [-m <msg> | -F <file>] [<object>]",
 	"git notes show [<object>]",
 	"git notes remove [<object>]",
@@ -211,7 +212,8 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	const char *object_ref;
 	char logmsg[100];
 
-	int list = 0, edit = 0, show = 0, remove = 0, prune = 0;
+	int list = 0, add = 0, edit = 0, show = 0, remove = 0, prune = 0,
+	    force = 0;
 	int given_object;
 	const char *msgfile = NULL;
 	struct msg_arg msg = { 0, STRBUF_INIT };
@@ -220,6 +222,7 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		OPT_CALLBACK('m', "message", &msg, "msg",
 			     "note contents as a string", parse_msg_arg),
 		OPT_FILENAME('F', "file", &msgfile, "note contents in a file"),
+		OPT_BOOLEAN('f', "force", &force, "replace existing notes"),
 		OPT_END()
 	};
 
@@ -229,6 +232,8 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 
 	if (argc && !strcmp(argv[0], "list"))
 		list = 1;
+	else if (argc && !strcmp(argv[0], "add"))
+		add = 1;
 	else if (argc && !strcmp(argv[0], "edit"))
 		edit = 1;
 	else if (argc && !strcmp(argv[0], "show"))
@@ -240,10 +245,10 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	else if (!argc)
 		list = 1; /* Default to 'list' if no other subcommand given */
 
-	if (list + edit + show + remove + prune != 1)
+	if (list + add + edit + show + remove + prune != 1)
 		usage_with_options(git_notes_usage, options);
 
-	if ((msg.given || msgfile) && !edit) {
+	if ((msg.given || msgfile) && !(add || edit)) {
 		error("cannot use -m/-F options with %s subcommand.", argv[0]);
 		usage_with_options(git_notes_usage, options);
 	}
@@ -253,6 +258,11 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		usage_with_options(git_notes_usage, options);
 	}
 
+	if (force && !add) {
+		error("cannot use -f option with %s subcommand.", argv[0]);
+		usage_with_options(git_notes_usage, options);
+	}
+
 	given_object = argc == 2;
 	object_ref = given_object ? argv[1] : "HEAD";
 	if (argc > 2 || (prune && argc > 1)) {
@@ -294,7 +304,19 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		return execv_git_cmd(show_args);
 	}
 
-	/* edit/remove/prune command */
+	/* add/edit/remove/prune command */
+
+	if (add && note) {
+		if (force)
+			fprintf(stderr, "Overwriting existing notes for object %s\n",
+				sha1_to_hex(object));
+		else {
+			error("Cannot add notes. Found existing notes for object %s. "
+			      "Use '-f' to overwrite existing notes",
+			      sha1_to_hex(object));
+			return 1;
+		}
+	}
 
 	if (remove)
 		strbuf_reset(&buf);
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 768a1cb..df458ca 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -16,7 +16,7 @@ GIT_EDITOR=./fake_editor.sh
 export GIT_EDITOR
 
 test_expect_success 'cannot annotate non-existing HEAD' '
-	(MSG=3 && export MSG && test_must_fail git notes edit)
+	(MSG=3 && export MSG && test_must_fail git notes add)
 '
 
 test_expect_success setup '
@@ -32,18 +32,18 @@ test_expect_success setup '
 
 test_expect_success 'need valid notes ref' '
 	(MSG=1 GIT_NOTES_REF=/ && export MSG GIT_NOTES_REF &&
-	 test_must_fail git notes edit) &&
+	 test_must_fail git notes add) &&
 	(MSG=2 GIT_NOTES_REF=/ && export MSG GIT_NOTES_REF &&
 	 test_must_fail git notes show)
 '
 
-test_expect_success 'refusing to edit in refs/heads/' '
+test_expect_success 'refusing to add notes in refs/heads/' '
 	(MSG=1 GIT_NOTES_REF=refs/heads/bogus &&
 	 export MSG GIT_NOTES_REF &&
-	 test_must_fail git notes edit)
+	 test_must_fail git notes add)
 '
 
-test_expect_success 'refusing to edit in refs/remotes/' '
+test_expect_success 'refusing to edit notes in refs/remotes/' '
 	(MSG=1 GIT_NOTES_REF=refs/remotes/bogus &&
 	 export MSG GIT_NOTES_REF &&
 	 test_must_fail git notes edit)
@@ -56,16 +56,34 @@ test_expect_success 'handle empty notes gracefully' '
 
 test_expect_success 'create notes' '
 	git config core.notesRef refs/notes/commits &&
-	MSG=b0 git notes edit &&
+	MSG=b4 git notes add &&
 	test ! -f .git/NOTES_EDITMSG &&
 	test 1 = $(git ls-tree refs/notes/commits | wc -l) &&
-	test b0 = $(git notes show) &&
+	test b4 = $(git notes show) &&
 	git show HEAD^ &&
 	test_must_fail git notes show HEAD^
 '
 
 test_expect_success 'edit existing notes' '
-	MSG=b1 git notes edit &&
+	MSG=b3 git notes edit &&
+	test ! -f .git/NOTES_EDITMSG &&
+	test 1 = $(git ls-tree refs/notes/commits | wc -l) &&
+	test b3 = $(git notes show) &&
+	git show HEAD^ &&
+	test_must_fail git notes show HEAD^
+'
+
+test_expect_success 'cannot add note where one exists' '
+	! MSG=b2 git notes add &&
+	test ! -f .git/NOTES_EDITMSG &&
+	test 1 = $(git ls-tree refs/notes/commits | wc -l) &&
+	test b3 = $(git notes show) &&
+	git show HEAD^ &&
+	test_must_fail git notes show HEAD^
+'
+
+test_expect_success 'can overwrite existing note with "git notes add -f"' '
+	MSG=b1 git notes add -f &&
 	test ! -f .git/NOTES_EDITMSG &&
 	test 1 = $(git ls-tree refs/notes/commits | wc -l) &&
 	test b1 = $(git notes show) &&
@@ -89,6 +107,7 @@ test_expect_success 'show notes' '
 	git log -1 > output &&
 	test_cmp expect output
 '
+
 test_expect_success 'create multi-line notes (setup)' '
 	: > a3 &&
 	git add a3 &&
@@ -96,7 +115,7 @@ test_expect_success 'create multi-line notes (setup)' '
 	git commit -m 3rd &&
 	MSG="b3
 c3c3c3c3
-d3d3d3" git notes edit
+d3d3d3" git notes add
 '
 
 cat > expect-multiline << EOF
@@ -125,7 +144,7 @@ test_expect_success 'create -F notes (setup)' '
 	test_tick &&
 	git commit -m 4th &&
 	echo "xyzzy" > note5 &&
-	git notes edit -F note5
+	git notes add -F note5
 '
 
 cat > expect-F << EOF
@@ -205,7 +224,7 @@ test_expect_success 'create -m notes (setup)' '
 	git add a5 &&
 	test_tick &&
 	git commit -m 5th &&
-	git notes edit -m spam -m "foo
+	git notes add -m spam -m "foo
 bar
 baz"
 '
@@ -234,8 +253,8 @@ test_expect_success 'show -m notes' '
 	test_cmp expect-m output
 '
 
-test_expect_success 'remove note with -F /dev/null (setup)' '
-	git notes edit -F /dev/null
+test_expect_success 'remove note with add -f -F /dev/null (setup)' '
+	git notes add -f -F /dev/null
 '
 
 cat > expect-rm-F << EOF
@@ -256,7 +275,7 @@ test_expect_success 'verify note removal with -F /dev/null' '
 '
 
 test_expect_success 'do not create empty note with -m "" (setup)' '
-	git notes edit -m ""
+	git notes add -m ""
 '
 
 test_expect_success 'verify non-creation of note with -m ""' '
@@ -329,7 +348,7 @@ test_expect_success 'create other note on a different notes ref (setup)' '
 	git add a6 &&
 	test_tick &&
 	git commit -m 6th &&
-	GIT_NOTES_REF="refs/notes/other" git notes edit -m "other note"
+	GIT_NOTES_REF="refs/notes/other" git notes add -m "other note"
 '
 
 cat > expect-other << EOF
@@ -374,17 +393,17 @@ test_expect_success 'Do not show note when core.notesRef is overridden' '
 
 test_expect_success 'Allow notes on non-commits (trees, blobs, tags)' '
 	echo "Note on a tree" > expect
-	git notes edit -m "Note on a tree" HEAD: &&
+	git notes add -m "Note on a tree" HEAD: &&
 	git notes show HEAD: > actual &&
 	test_cmp expect actual &&
 	echo "Note on a blob" > expect
 	filename=$(git ls-tree --name-only HEAD | head -n1) &&
-	git notes edit -m "Note on a blob" HEAD:$filename &&
+	git notes add -m "Note on a blob" HEAD:$filename &&
 	git notes show HEAD:$filename > actual &&
 	test_cmp expect actual &&
 	echo "Note on a tag" > expect
 	git tag -a -m "This is an annotated tag" foobar HEAD^ &&
-	git notes edit -m "Note on a tag" foobar &&
+	git notes add -m "Note on a tag" foobar &&
 	git notes show foobar > actual &&
 	test_cmp expect actual
 '
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 25/30] builtin-notes: Add "append" subcommand for appending to note objects

[Johan Herland]

"git notes append" is equivalent to "git notes edit" except that instead
of editing existing notes contents, you can only append to it. This is
useful for quickly adding annotations like e.g.:
	git notes append -m "Acked-by: A U Thor <author@example.com>"

"git notes append" takes the same -m/-F options as "git notes add".

If there is no existing note to append to, "git notes append" is identical
to "git notes add" (i.e. it adds a new note).

The patch includes tests verifying correct behaviour of the new subcommand.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johan Herland <johan@herland.net>
---
 Documentation/git-notes.txt |    5 +++++
 builtin-notes.c             |   35 ++++++++++++++++++++++++++---------
 t/t3301-notes.sh            |   36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 94e12b5..35dd8fa 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -10,6 +10,7 @@ SYNOPSIS
 [verse]
 'git notes' [list [<object>]]
 'git notes' add [-f] [-F <file> | -m <msg>] [<object>]
+'git notes' append [-F <file> | -m <msg>] [<object>]
 'git notes' edit [-F <file> | -m <msg>] [<object>]
 'git notes' show [<object>]
 'git notes' remove [<object>]
@@ -47,6 +48,10 @@ add::
 	object already has notes, abort. (use `-f` to overwrite an
 	existing note).
 
+append::
+	Append to the notes of an existing object (defaults to HEAD).
+	Creates a new notes object if needed.
+
 edit::
 	Edit the notes for a given object (defaults to HEAD).
 
diff --git a/builtin-notes.c b/builtin-notes.c
index 006edf6..c88df00 100644
--- a/builtin-notes.c
+++ b/builtin-notes.c
@@ -20,6 +20,7 @@
 static const char * const git_notes_usage[] = {
 	"git notes [list [<object>]]",
 	"git notes add [-f] [-m <msg> | -F <file>] [<object>]",
+	"git notes append [-m <msg> | -F <file>] [<object>]",
 	"git notes edit [-m <msg> | -F <file>] [<object>]",
 	"git notes show [<object>]",
 	"git notes remove [<object>]",
@@ -94,7 +95,7 @@ static void write_commented_object(int fd, const unsigned char *object)
 
 static void create_note(const unsigned char *object,
 			struct strbuf *buf,
-			int skip_editor,
+			int skip_editor, int append_only,
 			const unsigned char *prev,
 			unsigned char *result)
 {
@@ -109,7 +110,7 @@ static void create_note(const unsigned char *object,
 		if (fd < 0)
 			die_errno("could not create file '%s'", path);
 
-		if (prev)
+		if (prev && !append_only)
 			write_note_data(fd, prev);
 		write_or_die(fd, note_template, strlen(note_template));
 
@@ -125,6 +126,20 @@ static void create_note(const unsigned char *object,
 
 	stripspace(buf, 1);
 
+	if (prev && append_only) {
+		/* Append buf to previous note contents */
+		unsigned long size;
+		enum object_type type;
+		char *prev_buf = read_sha1_file(prev, &type, &size);
+
+		strbuf_grow(buf, size + 1);
+		if (buf->len && prev_buf && size)
+			strbuf_insert(buf, 0, "\n", 1);
+		if (prev_buf && size)
+			strbuf_insert(buf, 0, prev_buf, size);
+		free(prev_buf);
+	}
+
 	if (!buf->len) {
 		fprintf(stderr, "Removing note for object %s\n",
 			sha1_to_hex(object));
@@ -212,8 +227,8 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	const char *object_ref;
 	char logmsg[100];
 
-	int list = 0, add = 0, edit = 0, show = 0, remove = 0, prune = 0,
-	    force = 0;
+	int list = 0, add = 0, append = 0, edit = 0, show = 0, remove = 0,
+	    prune = 0, force = 0;
 	int given_object;
 	const char *msgfile = NULL;
 	struct msg_arg msg = { 0, STRBUF_INIT };
@@ -234,6 +249,8 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		list = 1;
 	else if (argc && !strcmp(argv[0], "add"))
 		add = 1;
+	else if (argc && !strcmp(argv[0], "append"))
+		append = 1;
 	else if (argc && !strcmp(argv[0], "edit"))
 		edit = 1;
 	else if (argc && !strcmp(argv[0], "show"))
@@ -245,10 +262,10 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	else if (!argc)
 		list = 1; /* Default to 'list' if no other subcommand given */
 
-	if (list + add + edit + show + remove + prune != 1)
+	if (list + add + append + edit + show + remove + prune != 1)
 		usage_with_options(git_notes_usage, options);
 
-	if ((msg.given || msgfile) && !(add || edit)) {
+	if ((msg.given || msgfile) && !(add || append || edit)) {
 		error("cannot use -m/-F options with %s subcommand.", argv[0]);
 		usage_with_options(git_notes_usage, options);
 	}
@@ -304,7 +321,7 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		return execv_git_cmd(show_args);
 	}
 
-	/* add/edit/remove/prune command */
+	/* add/append/edit/remove/prune command */
 
 	if (add && note) {
 		if (force)
@@ -334,8 +351,8 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		hashclr(new_note);
 		prune_notes(t);
 	} else {
-		create_note(object, &buf, msg.given || msgfile || remove, note,
-			    new_note);
+		create_note(object, &buf, msg.given || msgfile || remove,
+			    append, note, new_note);
 		if (is_null_sha1(new_note))
 			remove_note(t, object);
 		else
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index df458ca..290ed63 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -343,6 +343,42 @@ test_expect_success 'listing non-existing notes fails' '
 	test_cmp expect output
 '
 
+cat > expect << EOF
+Initial set of notes
+
+More notes appended with git notes append
+EOF
+
+test_expect_success 'append to existing note with "git notes append"' '
+	git notes add -m "Initial set of notes" &&
+	git notes append -m "More notes appended with git notes append" &&
+	git notes show > output &&
+	test_cmp expect output
+'
+
+test_expect_success 'appending empty string does not change existing note' '
+	git notes append -m "" &&
+	git notes show > output &&
+	test_cmp expect output
+'
+
+test_expect_success 'git notes append == add when there is no existing note' '
+	git notes remove HEAD &&
+	test_must_fail git notes list HEAD &&
+	git notes append -m "Initial set of notes
+
+More notes appended with git notes append" &&
+	git notes show > output &&
+	test_cmp expect output
+'
+
+test_expect_success 'appending empty string to non-existing note does not create note' '
+	git notes remove HEAD &&
+	test_must_fail git notes list HEAD &&
+	git notes append -m "" &&
+	test_must_fail git notes list HEAD
+'
+
 test_expect_success 'create other note on a different notes ref (setup)' '
 	: > a6 &&
 	git add a6 &&
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 26/30] builtin-notes: Deprecate the -m/-F options for "git notes edit"

[Johan Herland]

The semantics for "git notes edit -m/-F" overlap with those for
"git notes add -f", and the behaviour (i.e. overwriting existing
notes with the given message/file) is more intuitively captured
by (and better documented with) "git notes add -f".

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Johan Herland <johan@herland.net>
---
 Documentation/git-notes.txt |    2 +-
 builtin-notes.c             |   10 ++++++++--
 t/t3304-notes-mixed.sh      |    2 +-
 t/t3305-notes-fanout.sh     |    2 +-
 t/t3306-notes-prune.sh      |    6 +++---
 5 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 35dd8fa..53c5d90 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git notes' [list [<object>]]
 'git notes' add [-f] [-F <file> | -m <msg>] [<object>]
 'git notes' append [-F <file> | -m <msg>] [<object>]
-'git notes' edit [-F <file> | -m <msg>] [<object>]
+'git notes' edit [<object>]
 'git notes' show [<object>]
 'git notes' remove [<object>]
 'git notes' prune
diff --git a/builtin-notes.c b/builtin-notes.c
index c88df00..572b477 100644
--- a/builtin-notes.c
+++ b/builtin-notes.c
@@ -21,7 +21,7 @@ static const char * const git_notes_usage[] = {
 	"git notes [list [<object>]]",
 	"git notes add [-f] [-m <msg> | -F <file>] [<object>]",
 	"git notes append [-m <msg> | -F <file>] [<object>]",
-	"git notes edit [-m <msg> | -F <file>] [<object>]",
+	"git notes edit [<object>]",
 	"git notes show [<object>]",
 	"git notes remove [<object>]",
 	"git notes prune",
@@ -233,7 +233,7 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	const char *msgfile = NULL;
 	struct msg_arg msg = { 0, STRBUF_INIT };
 	struct option options[] = {
-		OPT_GROUP("Notes edit options"),
+		OPT_GROUP("Notes options"),
 		OPT_CALLBACK('m', "message", &msg, "msg",
 			     "note contents as a string", parse_msg_arg),
 		OPT_FILENAME('F', "file", &msgfile, "note contents in a file"),
@@ -270,6 +270,12 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		usage_with_options(git_notes_usage, options);
 	}

+	if ((msg.given || msgfile) && edit) {
+		fprintf(stderr, "The -m and -F options has been deprecated for"
+			" the 'edit' subcommand.\n"
+			"Please use 'git notes add -f -m/-F' instead.\n");
+	}
+
 	if (msg.given && msgfile) {
 		error("mixing -m and -F options is not allowed.");
 		usage_with_options(git_notes_usage, options);
diff --git a/t/t3304-notes-mixed.sh b/t/t3304-notes-mixed.sh
index c975a6d..1709e8c 100755
--- a/t/t3304-notes-mixed.sh
+++ b/t/t3304-notes-mixed.sh
@@ -188,7 +188,7 @@ test_expect_success "verify contents of non-notes" '
 test_expect_success "git-notes preserves non-notes" '

 	test_tick &&
-	git notes edit -m "foo bar"
+	git notes add -f -m "foo bar"
 '

 test_expect_success "verify contents of non-notes after git-notes" '
diff --git a/t/t3305-notes-fanout.sh b/t/t3305-notes-fanout.sh
index c6d263b..b1ea64b 100755
--- a/t/t3305-notes-fanout.sh
+++ b/t/t3305-notes-fanout.sh
@@ -14,7 +14,7 @@ test_expect_success 'creating many notes with git-notes' '
 		echo "file for commit #$i" > file &&
 		git add file &&
 		git commit -q -m "commit #$i" &&
-		git notes edit -m "note #$i" || return 1
+		git notes add -m "note #$i" || return 1
 	done
 '

diff --git a/t/t3306-notes-prune.sh b/t/t3306-notes-prune.sh
index b0adc7e..a0ed035 100755
--- a/t/t3306-notes-prune.sh
+++ b/t/t3306-notes-prune.sh
@@ -10,17 +10,17 @@ test_expect_success 'setup: create a few commits with notes' '
 	git add file1 &&
 	test_tick &&
 	git commit -m 1st &&
-	git notes edit -m "Note #1" &&
+	git notes add -m "Note #1" &&
 	: > file2 &&
 	git add file2 &&
 	test_tick &&
 	git commit -m 2nd &&
-	git notes edit -m "Note #2" &&
+	git notes add -m "Note #2" &&
 	: > file3 &&
 	git add file3 &&
 	test_tick &&
 	git commit -m 3rd &&
-	git notes edit -m "Note #3"
+	git notes add -m "Note #3"
 '

 cat > expect <<END_OF_LOG
--
1.7.0.rc1.141.gd3fd

[PATCHv13 27/30] builtin-notes: Refactor handling of -F option to allow combining -m and -F

[Johan Herland]

By moving the -F option handling into a separate function (parse_file_arg),
we can start allowing several -F options, and mixed usage of -m and -F
options. Each -m/-F given appends to the note message, in the order they are
given on the command-line.

The patch includes tests verifying correct behaviour of the new functionality.

Signed-off-by: Johan Herland <johan@herland.net>
---
 builtin-notes.c  |   94 +++++++++++++++++++++++++++++------------------------
 t/t3301-notes.sh |   23 ++++++++++++-
 2 files changed, 73 insertions(+), 44 deletions(-)

diff --git a/builtin-notes.c b/builtin-notes.c
index 572b477..190c46c 100644
--- a/builtin-notes.c
+++ b/builtin-notes.c
@@ -34,6 +34,11 @@ static const char note_template[] =
 	"# Write/edit the notes for the following object:\n"
 	"#\n";
 
+struct msg_arg {
+	int given;
+	struct strbuf buf;
+};
+
 static int list_each_note(const unsigned char *object_sha1,
 		const unsigned char *note_sha1, char *note_path,
 		void *cb_data)
@@ -93,15 +98,13 @@ static void write_commented_object(int fd, const unsigned char *object)
 		    sha1_to_hex(object));
 }
 
-static void create_note(const unsigned char *object,
-			struct strbuf *buf,
-			int skip_editor, int append_only,
-			const unsigned char *prev,
+static void create_note(const unsigned char *object, struct msg_arg *msg,
+			int append_only, const unsigned char *prev,
 			unsigned char *result)
 {
 	char *path = NULL;
 
-	if (!skip_editor) {
+	if (!msg->given) {
 		int fd;
 
 		/* write the template message before editing: */
@@ -118,34 +121,33 @@ static void create_note(const unsigned char *object,
 
 		close(fd);
 
-		if (launch_editor(path, buf, NULL)) {
+		if (launch_editor(path, &(msg->buf), NULL)) {
 			die("Please supply the note contents using either -m" \
 			    " or -F option");
 		}
+		stripspace(&(msg->buf), 1);
 	}
 
-	stripspace(buf, 1);
-
 	if (prev && append_only) {
 		/* Append buf to previous note contents */
 		unsigned long size;
 		enum object_type type;
 		char *prev_buf = read_sha1_file(prev, &type, &size);
 
-		strbuf_grow(buf, size + 1);
-		if (buf->len && prev_buf && size)
-			strbuf_insert(buf, 0, "\n", 1);
+		strbuf_grow(&(msg->buf), size + 1);
+		if (msg->buf.len && prev_buf && size)
+			strbuf_insert(&(msg->buf), 0, "\n", 1);
 		if (prev_buf && size)
-			strbuf_insert(buf, 0, prev_buf, size);
+			strbuf_insert(&(msg->buf), 0, prev_buf, size);
 		free(prev_buf);
 	}
 
-	if (!buf->len) {
+	if (!msg->buf.len) {
 		fprintf(stderr, "Removing note for object %s\n",
 			sha1_to_hex(object));
 		hashclr(result);
 	} else {
-		if (write_sha1_file(buf->buf, buf->len, blob_type, result)) {
+		if (write_sha1_file(msg->buf.buf, msg->buf.len, blob_type, result)) {
 			error("unable to write note object");
 			if (path)
 				error("The note contents has been left in %s",
@@ -160,20 +162,39 @@ static void create_note(const unsigned char *object,
 	}
 }
 
-struct msg_arg {
-	int given;
-	struct strbuf buf;
-};
-
 static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
 {
 	struct msg_arg *msg = opt->value;
 
 	if (!arg)
 		return -1;
+
+	strbuf_grow(&(msg->buf), strlen(arg) + 2);
 	if (msg->buf.len)
-		strbuf_addstr(&(msg->buf), "\n\n");
+		strbuf_addstr(&(msg->buf), "\n");
 	strbuf_addstr(&(msg->buf), arg);
+	stripspace(&(msg->buf), 0);
+
+	msg->given = 1;
+	return 0;
+}
+
+static int parse_file_arg(const struct option *opt, const char *arg, int unset)
+{
+	struct msg_arg *msg = opt->value;
+
+	if (!arg)
+		return -1;
+
+	if (msg->buf.len)
+		strbuf_addstr(&(msg->buf), "\n");
+	if (!strcmp(arg, "-")) {
+		if (strbuf_read(&(msg->buf), 0, 1024) < 0)
+			die_errno("cannot read '%s'", arg);
+	} else if (strbuf_read_file(&(msg->buf), arg, 1024) < 0)
+		die_errno("could not open or read '%s'", arg);
+	stripspace(&(msg->buf), 0);
+
 	msg->given = 1;
 	return 0;
 }
@@ -220,7 +241,6 @@ int commit_notes(struct notes_tree *t, const char *msg)
 
 int cmd_notes(int argc, const char **argv, const char *prefix)
 {
-	struct strbuf buf = STRBUF_INIT;
 	struct notes_tree *t;
 	unsigned char object[20], new_note[20];
 	const unsigned char *note;
@@ -230,13 +250,13 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	int list = 0, add = 0, append = 0, edit = 0, show = 0, remove = 0,
 	    prune = 0, force = 0;
 	int given_object;
-	const char *msgfile = NULL;
 	struct msg_arg msg = { 0, STRBUF_INIT };
 	struct option options[] = {
 		OPT_GROUP("Notes options"),
-		OPT_CALLBACK('m', "message", &msg, "msg",
+		OPT_CALLBACK('m', "message", &msg, "MSG",
 			     "note contents as a string", parse_msg_arg),
-		OPT_FILENAME('F', "file", &msgfile, "note contents in a file"),
+		OPT_CALLBACK('F', "file", &msg, "FILE",
+			     "note contents in a file", parse_file_arg),
 		OPT_BOOLEAN('f', "force", &force, "replace existing notes"),
 		OPT_END()
 	};
@@ -265,21 +285,17 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	if (list + add + append + edit + show + remove + prune != 1)
 		usage_with_options(git_notes_usage, options);
 
-	if ((msg.given || msgfile) && !(add || append || edit)) {
+	if (msg.given && !(add || append || edit)) {
 		error("cannot use -m/-F options with %s subcommand.", argv[0]);
 		usage_with_options(git_notes_usage, options);
 	}
 
-	if ((msg.given || msgfile) && edit) {
+	if (msg.given && edit) {
 		fprintf(stderr, "The -m and -F options has been deprecated for"
 			" the 'edit' subcommand.\n"
 			"Please use 'git notes add -f -m/-F' instead.\n");
 	}
 
-	if (msg.given && msgfile) {
-		error("mixing -m and -F options is not allowed.");
-		usage_with_options(git_notes_usage, options);
-	}
 
 	if (force && !add) {
 		error("cannot use -f option with %s subcommand.", argv[0]);
@@ -341,24 +357,16 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		}
 	}
 
-	if (remove)
-		strbuf_reset(&buf);
-	else if (msg.given)
-		strbuf_addbuf(&buf, &(msg.buf));
-	else if (msgfile) {
-		if (!strcmp(msgfile, "-")) {
-			if (strbuf_read(&buf, 0, 1024) < 0)
-				die_errno("cannot read '%s'", msgfile);
-		} else if (strbuf_read_file(&buf, msgfile, 1024) < 0)
-			die_errno("could not open or read '%s'", msgfile);
+	if (remove) {
+		msg.given = 1;
+		strbuf_reset(&(msg.buf));
 	}
 
 	if (prune) {
 		hashclr(new_note);
 		prune_notes(t);
 	} else {
-		create_note(object, &buf, msg.given || msgfile || remove,
-			    append, note, new_note);
+		create_note(object, &msg, append, note, new_note);
 		if (is_null_sha1(new_note))
 			remove_note(t, object);
 		else
@@ -369,6 +377,6 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	commit_notes(t, logmsg);
 
 	free_notes(t);
-	strbuf_release(&buf);
+	strbuf_release(&(msg.buf));
 	return 0;
 }
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 290ed63..07090e3 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -284,8 +284,29 @@ test_expect_success 'verify non-creation of note with -m ""' '
 	! git notes show
 '
 
+cat > expect-combine_m_and_F << EOF
+foo
+
+xyzzy
+
+bar
+
+zyxxy
+
+baz
+EOF
+
+test_expect_success 'create note with combination of -m and -F' '
+	echo "xyzzy" > note_a &&
+	echo "zyxxy" > note_b &&
+	git notes add -m "foo" -F note_a -m "bar" -F note_b -m "baz" &&
+	git notes show > output &&
+	test_cmp expect-combine_m_and_F output
+'
+
 test_expect_success 'remove note with "git notes remove" (setup)' '
-	git notes remove HEAD^
+	git notes remove HEAD^ &&
+	git notes remove
 '
 
 cat > expect-rm-remove << EOF
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 28/30] builtin-notes: Add -c/-C options for reusing notes

[Johan Herland]

Inspired by the -c/-C options to "git commit", we teach these options to
"git notes add/append" to allow reuse of note objects.

With this patch in place, it is now easy to copy or move notes between
objects. For example, to copy object A's notes to object B:
	git notes add [-f] -C $(git notes list A) B
To move instead of copying, you simply remove the notes from the source
object afterwards, e.g.:
	git notes remove A

The patch includes tests verifying correct behaviour of the new functionality.

Signed-off-by: Johan Herland <johan@herland.net>
---
 Documentation/git-notes.txt |   12 ++++-
 builtin-notes.c             |   63 +++++++++++++++++++----
 t/t3301-notes.sh            |  116 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 179 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 53c5d90..15de4b3 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -9,8 +9,8 @@ SYNOPSIS
 --------
 [verse]
 'git notes' [list [<object>]]
-'git notes' add [-f] [-F <file> | -m <msg>] [<object>]
-'git notes' append [-F <file> | -m <msg>] [<object>]
+'git notes' add [-f] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
+'git notes' append [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
 'git notes' edit [<object>]
 'git notes' show [<object>]
 'git notes' remove [<object>]
@@ -84,6 +84,14 @@ OPTIONS
 	Take the note message from the given file.  Use '-' to
 	read the note message from the standard input.
 
+-C <object>::
+--reuse-message=<object>::
+	Reuse the note message from the given note object.
+
+-c <object>::
+--reedit-message=<object>::
+	Like '-C', but with '-c' the editor is invoked, so that
+	the user can further edit the note message.
 
 Author
 ------
diff --git a/builtin-notes.c b/builtin-notes.c
index 190c46c..98de115 100644
--- a/builtin-notes.c
+++ b/builtin-notes.c
@@ -19,8 +19,8 @@
 
 static const char * const git_notes_usage[] = {
 	"git notes [list [<object>]]",
-	"git notes add [-f] [-m <msg> | -F <file>] [<object>]",
-	"git notes append [-m <msg> | -F <file>] [<object>]",
+	"git notes add [-f] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]",
+	"git notes append [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]",
 	"git notes edit [<object>]",
 	"git notes show [<object>]",
 	"git notes remove [<object>]",
@@ -36,6 +36,7 @@ static const char note_template[] =
 
 struct msg_arg {
 	int given;
+	int use_editor;
 	struct strbuf buf;
 };
 
@@ -104,7 +105,7 @@ static void create_note(const unsigned char *object, struct msg_arg *msg,
 {
 	char *path = NULL;
 
-	if (!msg->given) {
+	if (msg->use_editor || !msg->given) {
 		int fd;
 
 		/* write the template message before editing: */
@@ -113,13 +114,16 @@ static void create_note(const unsigned char *object, struct msg_arg *msg,
 		if (fd < 0)
 			die_errno("could not create file '%s'", path);
 
-		if (prev && !append_only)
+		if (msg->given)
+			write_or_die(fd, msg->buf.buf, msg->buf.len);
+		else if (prev && !append_only)
 			write_note_data(fd, prev);
 		write_or_die(fd, note_template, strlen(note_template));
 
 		write_commented_object(fd, object);
 
 		close(fd);
+		strbuf_reset(&(msg->buf));
 
 		if (launch_editor(path, &(msg->buf), NULL)) {
 			die("Please supply the note contents using either -m" \
@@ -199,6 +203,40 @@ static int parse_file_arg(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+static int parse_reuse_arg(const struct option *opt, const char *arg, int unset)
+{
+	struct msg_arg *msg = opt->value;
+	char *buf;
+	unsigned char object[20];
+	enum object_type type;
+	unsigned long len;
+
+	if (!arg)
+		return -1;
+
+	if (msg->buf.len)
+		strbuf_addstr(&(msg->buf), "\n");
+
+	if (get_sha1(arg, object))
+		die("Failed to resolve '%s' as a valid ref.", arg);
+	if (!(buf = read_sha1_file(object, &type, &len)) || !len) {
+		free(buf);
+		die("Failed to read object '%s'.", arg);;
+	}
+	strbuf_add(&(msg->buf), buf, len);
+	free(buf);
+
+	msg->given = 1;
+	return 0;
+}
+
+static int parse_reedit_arg(const struct option *opt, const char *arg, int unset)
+{
+	struct msg_arg *msg = opt->value;
+	msg->use_editor = 1;
+	return parse_reuse_arg(opt, arg, unset);
+}
+
 int commit_notes(struct notes_tree *t, const char *msg)
 {
 	struct commit_list *parent;
@@ -250,13 +288,17 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	int list = 0, add = 0, append = 0, edit = 0, show = 0, remove = 0,
 	    prune = 0, force = 0;
 	int given_object;
-	struct msg_arg msg = { 0, STRBUF_INIT };
+	struct msg_arg msg = { 0, 0, STRBUF_INIT };
 	struct option options[] = {
 		OPT_GROUP("Notes options"),
 		OPT_CALLBACK('m', "message", &msg, "MSG",
 			     "note contents as a string", parse_msg_arg),
 		OPT_CALLBACK('F', "file", &msg, "FILE",
 			     "note contents in a file", parse_file_arg),
+		OPT_CALLBACK('c', "reedit-message", &msg, "OBJECT",
+			   "reuse and edit specified note object", parse_reedit_arg),
+		OPT_CALLBACK('C', "reuse-message", &msg, "OBJECT",
+			   "reuse specified note object", parse_reuse_arg),
 		OPT_BOOLEAN('f', "force", &force, "replace existing notes"),
 		OPT_END()
 	};
@@ -286,17 +328,17 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		usage_with_options(git_notes_usage, options);
 
 	if (msg.given && !(add || append || edit)) {
-		error("cannot use -m/-F options with %s subcommand.", argv[0]);
+		error("cannot use -m/-F/-c/-C options with %s subcommand.",
+		      argv[0]);
 		usage_with_options(git_notes_usage, options);
 	}
 
 	if (msg.given && edit) {
-		fprintf(stderr, "The -m and -F options has been deprecated for"
-			" the 'edit' subcommand.\n"
-			"Please use 'git notes add -f -m/-F' instead.\n");
+		fprintf(stderr, "The -m/-F/-c/-C options have been deprecated "
+			"for the 'edit' subcommand.\n"
+			"Please use 'git notes add -f -m/-F/-c/-C' instead.\n");
 	}
 
-
 	if (force && !add) {
 		error("cannot use -f option with %s subcommand.", argv[0]);
 		usage_with_options(git_notes_usage, options);
@@ -359,6 +401,7 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 
 	if (remove) {
 		msg.given = 1;
+		msg.use_editor = 0;
 		strbuf_reset(&(msg.buf));
 	}
 
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 07090e3..6447e5f 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -465,4 +465,120 @@ test_expect_success 'Allow notes on non-commits (trees, blobs, tags)' '
 	test_cmp expect actual
 '
 
+cat > expect << EOF
+commit 2ede89468182a62d0bde2583c736089bcf7d7e92
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:19:13 2005 -0700
+
+    7th
+
+Notes:
+    other note
+EOF
+
+test_expect_success 'create note from other note with "git notes add -C"' '
+	: > a7 &&
+	git add a7 &&
+	test_tick &&
+	git commit -m 7th &&
+	git notes add -C $(git notes list HEAD^) &&
+	git log -1 > actual &&
+	test_cmp expect actual &&
+	test "$(git notes list HEAD)" = "$(git notes list HEAD^)"
+'
+
+test_expect_success 'create note from non-existing note with "git notes add -C" fails' '
+	: > a8 &&
+	git add a8 &&
+	test_tick &&
+	git commit -m 8th &&
+	test_must_fail git notes add -C deadbeef &&
+	test_must_fail git notes list HEAD
+'
+
+cat > expect << EOF
+commit 016e982bad97eacdbda0fcbd7ce5b0ba87c81f1b
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:21:13 2005 -0700
+
+    9th
+
+Notes:
+    yet another note
+EOF
+
+test_expect_success 'create note from other note with "git notes add -c"' '
+	: > a9 &&
+	git add a9 &&
+	test_tick &&
+	git commit -m 9th &&
+	MSG="yet another note" git notes add -c $(git notes list HEAD^^) &&
+	git log -1 > actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'create note from non-existing note with "git notes add -c" fails' '
+	: > a10 &&
+	git add a10 &&
+	test_tick &&
+	git commit -m 10th &&
+	test_must_fail MSG="yet another note" git notes add -c deadbeef &&
+	test_must_fail git notes list HEAD
+'
+
+cat > expect << EOF
+commit 016e982bad97eacdbda0fcbd7ce5b0ba87c81f1b
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:21:13 2005 -0700
+
+    9th
+
+Notes:
+    yet another note
+$whitespace
+    yet another note
+EOF
+
+test_expect_success 'append to note from other note with "git notes append -C"' '
+	git notes append -C $(git notes list HEAD^) HEAD^ &&
+	git log -1 HEAD^ > actual &&
+	test_cmp expect actual
+'
+
+cat > expect << EOF
+commit ffed603236bfa3891c49644257a83598afe8ae5a
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:22:13 2005 -0700
+
+    10th
+
+Notes:
+    other note
+EOF
+
+test_expect_success 'create note from other note with "git notes append -c"' '
+	MSG="other note" git notes append -c $(git notes list HEAD^) &&
+	git log -1 > actual &&
+	test_cmp expect actual
+'
+
+cat > expect << EOF
+commit ffed603236bfa3891c49644257a83598afe8ae5a
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:22:13 2005 -0700
+
+    10th
+
+Notes:
+    other note
+$whitespace
+    yet another note
+EOF
+
+test_expect_success 'append to note from other note with "git notes append -c"' '
+	MSG="yet another note" git notes append -c $(git notes list HEAD) &&
+	git log -1 > actual &&
+	test_cmp expect actual
+'
+
 test_done
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 29/30] builtin-notes: Misc. refactoring of argc and exit value handling

[Johan Herland]

This is in preparation of future patches that add additional subcommands.

Signed-off-by: Johan Herland <johan@herland.net>
---
 builtin-notes.c |   61 ++++++++++++++++++++++++++++++++----------------------
 1 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/builtin-notes.c b/builtin-notes.c
index 98de115..bbf98a9 100644
--- a/builtin-notes.c
+++ b/builtin-notes.c
@@ -287,7 +287,7 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 
 	int list = 0, add = 0, append = 0, edit = 0, show = 0, remove = 0,
 	    prune = 0, force = 0;
-	int given_object;
+	int given_object = 0, i = 1, retval = 0;
 	struct msg_arg msg = { 0, 0, STRBUF_INIT };
 	struct option options[] = {
 		OPT_GROUP("Notes options"),
@@ -321,8 +321,10 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		remove = 1;
 	else if (argc && !strcmp(argv[0], "prune"))
 		prune = 1;
-	else if (!argc)
+	else if (!argc) {
 		list = 1; /* Default to 'list' if no other subcommand given */
+		i = 0;
+	}
 
 	if (list + add + append + edit + show + remove + prune != 1)
 		usage_with_options(git_notes_usage, options);
@@ -344,9 +346,10 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		usage_with_options(git_notes_usage, options);
 	}
 
-	given_object = argc == 2;
-	object_ref = given_object ? argv[1] : "HEAD";
-	if (argc > 2 || (prune && argc > 1)) {
+	given_object = argc > i;
+	object_ref = given_object ? argv[i++] : "HEAD";
+
+	if (argc > i || (prune && given_object)) {
 		error("too many parameters");
 		usage_with_options(git_notes_usage, options);
 	}
@@ -369,34 +372,38 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		if (given_object) {
 			if (note) {
 				puts(sha1_to_hex(note));
-				return 0;
+				goto end;
 			}
-		} else
-			return for_each_note(t, 0, list_each_note, NULL);
+		} else {
+			retval = for_each_note(t, 0, list_each_note, NULL);
+			goto end;
+		}
 	}
 
 	/* show command */
 
 	if ((list || show) && !note) {
 		error("No note found for object %s.", sha1_to_hex(object));
-		return 1;
+		retval = 1;
+		goto end;
 	} else if (show) {
 		const char *show_args[3] = {"show", sha1_to_hex(note), NULL};
-		return execv_git_cmd(show_args);
+		retval = execv_git_cmd(show_args);
+		goto end;
 	}
 
 	/* add/append/edit/remove/prune command */
 
 	if (add && note) {
-		if (force)
-			fprintf(stderr, "Overwriting existing notes for object %s\n",
-				sha1_to_hex(object));
-		else {
-			error("Cannot add notes. Found existing notes for object %s. "
-			      "Use '-f' to overwrite existing notes",
+		if (!force) {
+			error("Cannot add notes. Found existing notes for object"
+			      " %s. Use '-f' to overwrite existing notes",
 			      sha1_to_hex(object));
-			return 1;
+			retval = 1;
+			goto end;
 		}
+		fprintf(stderr, "Overwriting existing notes for object %s\n",
+			sha1_to_hex(object));
 	}
 
 	if (remove) {
@@ -408,18 +415,22 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	if (prune) {
 		hashclr(new_note);
 		prune_notes(t);
-	} else {
+		goto commit;
+	} else
 		create_note(object, &msg, append, note, new_note);
-		if (is_null_sha1(new_note))
-			remove_note(t, object);
-		else
-			add_note(t, object, new_note, combine_notes_overwrite);
-	}
-	snprintf(logmsg, sizeof(logmsg), "Note %s by 'git notes %s'",
+
+	if (is_null_sha1(new_note))
+		remove_note(t, object);
+	else
+		add_note(t, object, new_note, combine_notes_overwrite);
+
+commit:
+	snprintf(logmsg, sizeof(logmsg), "Notes %s by 'git notes %s'",
 		 is_null_sha1(new_note) ? "removed" : "added", argv[0]);
 	commit_notes(t, logmsg);
 
+end:
 	free_notes(t);
 	strbuf_release(&(msg.buf));
-	return 0;
+	return retval;
 }
-- 
1.7.0.rc1.141.gd3fd

[PATCHv13 30/30] builtin-notes: Add "copy" subcommand for copying notes between objects

[Johan Herland]

This is useful for keeping notes to objects that are being rewritten by e.g.
'git commit --amend', 'git rebase', or 'git cherry-pick'.

"git notes copy <from> <to>" is in practice equivalent to
"git notes add -C $(git notes list <from>) <to>", although it is somewhat
more convenient for regular users.

"git notes copy" takes the same -f option as "git add", to overwrite existing
notes at the target (instead of aborting with an error message).

If the <from>-object has no notes, "git notes copy" will abort with an error
message.

The patch includes tests verifying correct behaviour of the new subcommand.

Signed-off-by: Johan Herland <johan@herland.net>
---
 Documentation/git-notes.txt |    8 +++++
 builtin-notes.c             |   39 +++++++++++++++++++++-----
 t/t3301-notes.sh            |   63 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 102 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 15de4b3..14f73b9 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -10,6 +10,7 @@ SYNOPSIS
 [verse]
 'git notes' [list [<object>]]
 'git notes' add [-f] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
+'git notes' copy [-f] <from-object> <to-object>
 'git notes' append [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
 'git notes' edit [<object>]
 'git notes' show [<object>]
@@ -48,6 +49,13 @@ add::
 	object already has notes, abort. (use `-f` to overwrite an
 	existing note).
 
+copy::
+	Copy the notes for the first object onto the second object.
+	Abort if the second object already has notes, or if the first
+	objects has none. (use -f to overwrite existing notes to the
+	second object). This subcommand is equivalent to:
+	`git notes add [-f] -C $(git notes list <from-object>) <to-object>`
+
 append::
 	Append to the notes of an existing object (defaults to HEAD).
 	Creates a new notes object if needed.
diff --git a/builtin-notes.c b/builtin-notes.c
index bbf98a9..123ecad 100644
--- a/builtin-notes.c
+++ b/builtin-notes.c
@@ -20,6 +20,7 @@
 static const char * const git_notes_usage[] = {
 	"git notes [list [<object>]]",
 	"git notes add [-f] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]",
+	"git notes copy [-f] <from-object> <to-object>",
 	"git notes append [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]",
 	"git notes edit [<object>]",
 	"git notes show [<object>]",
@@ -280,13 +281,13 @@ int commit_notes(struct notes_tree *t, const char *msg)
 int cmd_notes(int argc, const char **argv, const char *prefix)
 {
 	struct notes_tree *t;
-	unsigned char object[20], new_note[20];
+	unsigned char object[20], from_obj[20], new_note[20];
 	const unsigned char *note;
 	const char *object_ref;
 	char logmsg[100];
 
-	int list = 0, add = 0, append = 0, edit = 0, show = 0, remove = 0,
-	    prune = 0, force = 0;
+	int list = 0, add = 0, copy = 0, append = 0, edit = 0, show = 0,
+	    remove = 0, prune = 0, force = 0;
 	int given_object = 0, i = 1, retval = 0;
 	struct msg_arg msg = { 0, 0, STRBUF_INIT };
 	struct option options[] = {
@@ -311,6 +312,8 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		list = 1;
 	else if (argc && !strcmp(argv[0], "add"))
 		add = 1;
+	else if (argc && !strcmp(argv[0], "copy"))
+		copy = 1;
 	else if (argc && !strcmp(argv[0], "append"))
 		append = 1;
 	else if (argc && !strcmp(argv[0], "edit"))
@@ -326,7 +329,7 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		i = 0;
 	}
 
-	if (list + add + append + edit + show + remove + prune != 1)
+	if (list + add + copy + append + edit + show + remove + prune != 1)
 		usage_with_options(git_notes_usage, options);
 
 	if (msg.given && !(add || append || edit)) {
@@ -341,11 +344,22 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 			"Please use 'git notes add -f -m/-F/-c/-C' instead.\n");
 	}
 
-	if (force && !add) {
+	if (force && !(add || copy)) {
 		error("cannot use -f option with %s subcommand.", argv[0]);
 		usage_with_options(git_notes_usage, options);
 	}
 
+	if (copy) {
+		const char *from_ref;
+		if (argc < 3) {
+			error("too few parameters");
+			usage_with_options(git_notes_usage, options);
+		}
+		from_ref = argv[i++];
+		if (get_sha1(from_ref, from_obj))
+			die("Failed to resolve '%s' as a valid ref.", from_ref);
+	}
+
 	given_object = argc > i;
 	object_ref = given_object ? argv[i++] : "HEAD";
 
@@ -394,11 +408,11 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 
 	/* add/append/edit/remove/prune command */
 
-	if (add && note) {
+	if ((add || copy) && note) {
 		if (!force) {
-			error("Cannot add notes. Found existing notes for object"
+			error("Cannot %s notes. Found existing notes for object"
 			      " %s. Use '-f' to overwrite existing notes",
-			      sha1_to_hex(object));
+			      argv[0], sha1_to_hex(object));
 			retval = 1;
 			goto end;
 		}
@@ -416,6 +430,15 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		hashclr(new_note);
 		prune_notes(t);
 		goto commit;
+	} else if (copy) {
+		const unsigned char *from_note = get_note(t, from_obj);
+		if (!from_note) {
+			error("Missing notes on source object %s. Cannot copy.",
+			      sha1_to_hex(from_obj));
+			retval = 1;
+			goto end;
+		}
+		hashcpy(new_note, from_note);
 	} else
 		create_note(object, &msg, append, note, new_note);
 
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 6447e5f..90178f9 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -581,4 +581,67 @@ test_expect_success 'append to note from other note with "git notes append -c"'
 	test_cmp expect actual
 '
 
+cat > expect << EOF
+commit 6352c5e33dbcab725fe0579be16aa2ba8eb369be
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:23:13 2005 -0700
+
+    11th
+
+Notes:
+    other note
+$whitespace
+    yet another note
+EOF
+
+test_expect_success 'copy note with "git notes copy"' '
+	: > a11 &&
+	git add a11 &&
+	test_tick &&
+	git commit -m 11th &&
+	git notes copy HEAD^ HEAD &&
+	git log -1 > actual &&
+	test_cmp expect actual &&
+	test "$(git notes list HEAD)" = "$(git notes list HEAD^)"
+'
+
+test_expect_success 'prevent overwrite with "git notes copy"' '
+	test_must_fail git notes copy HEAD~2 HEAD &&
+	git log -1 > actual &&
+	test_cmp expect actual &&
+	test "$(git notes list HEAD)" = "$(git notes list HEAD^)"
+'
+
+cat > expect << EOF
+commit 6352c5e33dbcab725fe0579be16aa2ba8eb369be
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:23:13 2005 -0700
+
+    11th
+
+Notes:
+    yet another note
+$whitespace
+    yet another note
+EOF
+
+test_expect_success 'allow overwrite with "git notes copy -f"' '
+	git notes copy -f HEAD~2 HEAD &&
+	git log -1 > actual &&
+	test_cmp expect actual &&
+	test "$(git notes list HEAD)" = "$(git notes list HEAD~2)"
+'
+
+test_expect_success 'cannot copy note from object without notes' '
+	: > a12 &&
+	git add a12 &&
+	test_tick &&
+	git commit -m 12th &&
+	: > a13 &&
+	git add a13 &&
+	test_tick &&
+	git commit -m 13th &&
+	test_must_fail git notes copy HEAD^ HEAD
+'
+
 test_done
-- 
1.7.0.rc1.141.gd3fd

Re: [PATCHv13 28/30] builtin-notes: Add -c/-C options for reusing notes

[Stephen Boyd]

On 02/13/2010 01:28 PM, Johan Herland wrote:
> @@ -199,6 +203,40 @@ static int parse_file_arg(const struct option *opt, const char *arg, int unset)
>  	return 0;
>  }
>  
> +static int parse_reuse_arg(const struct option *opt, const char *arg, int unset)
> +{
> +	struct msg_arg *msg = opt->value;
> +	char *buf;
> +	unsigned char object[20];
> +	enum object_type type;
> +	unsigned long len;
> +
> +	if (!arg)
> +		return -1;

This is impossible unless you're using the PARSE_OPT_OPTARG flag or
allowing negation (i.e. --no-reuse-mesage). You should probably make the
two callback options PARSE_OPT_NONEG and then drop this if statement.
Same applies to some of the other callbacks not introduced in this patch.

> +
> +	if (msg->buf.len)
> +		strbuf_addstr(&(msg->buf), "\n");
> +

Use strbuf_addch()? I saw this in a couple other patches too.

Re: [PATCHv13 28/30] builtin-notes: Add -c/-C options for reusing notes

[Johan Herland]

On Monday 15 February 2010, Stephen Boyd wrote:
> On 02/13/2010 01:28 PM, Johan Herland wrote:
> > @@ -199,6 +203,40 @@ static int parse_file_arg(const struct option
> > *opt, const char *arg, int unset)
> > 
> >  	return 0;
> >  
> >  }
> > 
> > +static int parse_reuse_arg(const struct option *opt, const char *arg,
> > int unset) +{
> > +	struct msg_arg *msg = opt->value;
> > +	char *buf;
> > +	unsigned char object[20];
> > +	enum object_type type;
> > +	unsigned long len;
> > +
> > +	if (!arg)
> > +		return -1;
> 
> This is impossible unless you're using the PARSE_OPT_OPTARG flag or
> allowing negation (i.e. --no-reuse-mesage). You should probably make the
> two callback options PARSE_OPT_NONEG and then drop this if statement.
> Same applies to some of the other callbacks not introduced in this patch.

Thanks. Fixed locally. Will be part of the next iteration.

> > +
> > +	if (msg->buf.len)
> > +		strbuf_addstr(&(msg->buf), "\n");
> > +
> 
> Use strbuf_addch()? I saw this in a couple other patches too.

Of course. Thanks for noticing. Fixed locally. Will be part of the next 
iteration.


Thanks for the review! :)

...Johan

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

[PATCH] builtin-notes: Minor (mostly parse_options-related) fixes

[Johan Herland]

Use PARSE_OPT_NONEG to disallow --no-<option> for --message, --file,
--reedit-message and --reuse-message. --no-<option> does not make sense
for these options, and specifying PARSE_OPT_NONEG simplifies the code in
the option-handling callbacks.

Also, use strbuf_addch(... '\n') instead of strbuf_addstr(... "\n") in
couple of places.

Finally, improve the short-help by dividing the options into two OPT_GROUPs.

Suggested-by: Stephen Boyd <bebarino@gmail.com>
Signed-off-by: Johan Herland <johan@herland.net>
---

On Tuesday 16 February 2010, Johan Herland wrote:
> On Monday 15 February 2010, Stephen Boyd wrote:
> > On 02/13/2010 01:28 PM, Johan Herland wrote:
> > > @@ -199,6 +203,40 @@ static int parse_file_arg(const struct option

[...]

> > > +	if (!arg)
> > > +		return -1;
> > 
> > This is impossible unless you're using the PARSE_OPT_OPTARG flag or
> > allowing negation (i.e. --no-reuse-mesage). You should probably make
> > the two callback options PARSE_OPT_NONEG and then drop this if
> > statement. Same applies to some of the other callbacks not introduced
> > in this patch.
> 
> Thanks. Fixed locally. Will be part of the next iteration.
> 
> > > +
> > > +	if (msg->buf.len)
> > > +		strbuf_addstr(&(msg->buf), "\n");
> > > +
> > 
> > Use strbuf_addch()? I saw this in a couple other patches too.
> 
> Of course. Thanks for noticing. Fixed locally. Will be part of the next
> iteration.

I never got around to submitting the next jh/notes iteration before it was
merged to 'next', so here are Stephen's suggestions as a separate patch on
top of 'next'. AFAICS it does not conflict with tr/notes-display in 'pu'.


Have fun! :)

...Johan


 builtin-notes.c |   38 +++++++++++++++++---------------------
 1 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/builtin-notes.c b/builtin-notes.c
index 123ecad..feb710a 100644
--- a/builtin-notes.c
+++ b/builtin-notes.c
@@ -171,12 +171,9 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)
 {
 	struct msg_arg *msg = opt->value;
 
-	if (!arg)
-		return -1;
-
 	strbuf_grow(&(msg->buf), strlen(arg) + 2);
 	if (msg->buf.len)
-		strbuf_addstr(&(msg->buf), "\n");
+		strbuf_addch(&(msg->buf), '\n');
 	strbuf_addstr(&(msg->buf), arg);
 	stripspace(&(msg->buf), 0);
 
@@ -188,11 +185,8 @@ static int parse_file_arg(const struct option *opt, const char *arg, int unset)
 {
 	struct msg_arg *msg = opt->value;
 
-	if (!arg)
-		return -1;
-
 	if (msg->buf.len)
-		strbuf_addstr(&(msg->buf), "\n");
+		strbuf_addch(&(msg->buf), '\n');
 	if (!strcmp(arg, "-")) {
 		if (strbuf_read(&(msg->buf), 0, 1024) < 0)
 			die_errno("cannot read '%s'", arg);
@@ -212,11 +206,8 @@ static int parse_reuse_arg(const struct option *opt, const char *arg, int unset)
 	enum object_type type;
 	unsigned long len;
 
-	if (!arg)
-		return -1;
-
 	if (msg->buf.len)
-		strbuf_addstr(&(msg->buf), "\n");
+		strbuf_addch(&(msg->buf), '\n');
 
 	if (get_sha1(arg, object))
 		die("Failed to resolve '%s' as a valid ref.", arg);
@@ -291,15 +282,20 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 	int given_object = 0, i = 1, retval = 0;
 	struct msg_arg msg = { 0, 0, STRBUF_INIT };
 	struct option options[] = {
-		OPT_GROUP("Notes options"),
-		OPT_CALLBACK('m', "message", &msg, "MSG",
-			     "note contents as a string", parse_msg_arg),
-		OPT_CALLBACK('F', "file", &msg, "FILE",
-			     "note contents in a file", parse_file_arg),
-		OPT_CALLBACK('c', "reedit-message", &msg, "OBJECT",
-			   "reuse and edit specified note object", parse_reedit_arg),
-		OPT_CALLBACK('C', "reuse-message", &msg, "OBJECT",
-			   "reuse specified note object", parse_reuse_arg),
+		OPT_GROUP("Notes contents options"),
+		{ OPTION_CALLBACK, 'm', "message", &msg, "MSG",
+			"note contents as a string", PARSE_OPT_NONEG,
+			parse_msg_arg},
+		{ OPTION_CALLBACK, 'F', "file", &msg, "FILE",
+			"note contents in a file", PARSE_OPT_NONEG,
+			parse_file_arg},
+		{ OPTION_CALLBACK, 'c', "reedit-message", &msg, "OBJECT",
+			"reuse and edit specified note object", PARSE_OPT_NONEG,
+			parse_reedit_arg},
+		{ OPTION_CALLBACK, 'C', "reuse-message", &msg, "OBJECT",
+			"reuse specified note object", PARSE_OPT_NONEG,
+			parse_reuse_arg},
+		OPT_GROUP("Other options"),
 		OPT_BOOLEAN('f', "force", &force, "replace existing notes"),
 		OPT_END()
 	};
-- 
1.7.0.rc1.141.gd3fd

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

Re: [PATCH] builtin-notes: Minor (mostly parse_options-related) fixes

[Stephen Boyd]

On 02/24/2010 04:48 PM, Johan Herland wrote:
> Use PARSE_OPT_NONEG to disallow --no-<option> for --message, --file,
> --reedit-message and --reuse-message. --no-<option> does not make sense
> for these options, and specifying PARSE_OPT_NONEG simplifies the code in
> the option-handling callbacks.
>
> Also, use strbuf_addch(... '\n') instead of strbuf_addstr(... "\n") in
> couple of places.
>
> Finally, improve the short-help by dividing the options into two OPT_GROUPs.
>
> Suggested-by: Stephen Boyd <bebarino@gmail.com>
> Signed-off-by: Johan Herland <johan@herland.net>

Acked-by: Stephen Boyd <bebarino@gmail.com>

On a related note, I spent last night splitting up cmd_notes() into
subcommands. This way it's more like builtin-remote.c with respect to
the many subcommands and allows you to do something like 'git notes add
-h' and only see the options for add. I'll send it in a few hours after
I have time to polish it up (and probably rebase onto this change).