notes: Don't create (empty) commit when removing non-existing notes

Extend remove_note() in the notes API to return whether or not a note was
actually removed. Use this in 'git notes remove' to skip the creation of
a notes commit when no notes were actually removed.

Also add a test illustrating the change in behavior.

Signed-off-by: Johan Herland <johan@herland.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/notes.c b/builtin/notes.c
index fbc347c9f09e0da4ba43982a5f7b29882080f150..6d07aac80cc2bfd042eee905abb5086a29e1d90a 100644 (file)
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -769,6 +769,7 @@ static int remove_cmd(int argc, const char **argv, const char *prefix)
        const char *object_ref;
        struct notes_tree *t;
        unsigned char object[20];
+       int retval;
 
        argc = parse_options(argc, argv, prefix, options,
                             git_notes_remove_usage, 0);
@@ -785,12 +786,17 @@ static int remove_cmd(int argc, const char **argv, const char *prefix)
 
        t = init_notes_check("remove");
 
-       fprintf(stderr, "Removing note for object %s\n", sha1_to_hex(object));
-       remove_note(t, object);
+       retval = remove_note(t, object);
+       if (retval)
+               fprintf(stderr, "Object %s has no note\n", sha1_to_hex(object));
+       else {
+               fprintf(stderr, "Removing note for object %s\n",
+                       sha1_to_hex(object));
 
-       commit_notes(t, "Notes removed by 'git notes remove'");
+               commit_notes(t, "Notes removed by 'git notes remove'");
+       }
        free_notes(t);
-       return 0;
+       return retval;
 }
 
 static int prune(int argc, const char **argv, const char *prefix)

diff --git a/notes.c b/notes.c
index 7fd203560aaded5ebbd80d67c805df5a707aa60d..70d00135eb5b67cd6f21b416cde2ae4a1967fd0a 100644 (file)
--- a/notes.c
+++ b/notes.c
@@ -263,11 +263,13 @@ static int note_tree_consolidate(struct int_node *tree,
  * 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.
+ * - Copy the matching entry's value into the given entry.
  * - 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 notes_tree *t, 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];
@@ -282,6 +284,7 @@ static void note_tree_remove(struct notes_tree *t, struct int_node *tree,
                return; /* key mismatch, nothing to remove */
 
        /* we have found a matching entry */
+       hashcpy(entry->val_sha1, l->val_sha1);
        free(l);
        *p = SET_PTR_TYPE(NULL, PTR_TYPE_NULL);
 
@@ -1003,17 +1006,20 @@ void add_note(struct notes_tree *t, const unsigned char *object_sha1,
        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)
+int remove_note(struct notes_tree *t, const unsigned char *object_sha1)
 {
        struct leaf_node l;
 
        if (!t)
                t = &default_notes_tree;
        assert(t->initialized);
-       t->dirty = 1;
        hashcpy(l.key_sha1, object_sha1);
        hashclr(l.val_sha1);
        note_tree_remove(t, t->root, 0, &l);
+       if (is_null_sha1(l.val_sha1)) // no note was removed
+               return 1;
+       t->dirty = 1;
+       return 0;
 }
 
 const unsigned char *get_note(struct notes_tree *t,

diff --git a/notes.h b/notes.h
index 65fc3a66b2f575dd078cc390e7f5ea5cbebffc96..5106761534cfb6c5b97baede700e99b0aa5dfc3b 100644 (file)
--- a/notes.h
+++ b/notes.h
@@ -89,8 +89,10 @@ void add_note(struct notes_tree *t, const unsigned char *object_sha1,
  * 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.
+ *
+ * Return 0 if a note was removed; 1 if there was no note to remove.
  */
-void remove_note(struct notes_tree *t, const unsigned char *object_sha1);
+int remove_note(struct notes_tree *t, const unsigned char *object_sha1);
 
 /*
  * Get the note object SHA1 containing the note data for the given object

diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 96b75813d73b3fd34a3c96aa9e977594319adb1d..a2b79a0430b2468241a578aaad479a09db774dd5 100755 (executable)
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -365,6 +365,13 @@ c18dc024e14f08d18d14eea0d747ff692d66d6a3 1584215f1d29c65e99c6c6848626553fdd07fd7
 c9c6af7f78bc47490dbf3e822cf2f3c24d4b9061 268048bfb8a1fb38e703baceb8ab235421bf80c5
 EOF
 
+test_expect_success 'removing non-existing note should not create new commit' '
+       git rev-parse --verify refs/notes/commits > before_commit &&
+       test_must_fail git notes remove HEAD^ &&
+       git rev-parse --verify refs/notes/commits > after_commit &&
+       test_cmp before_commit after_commit
+'
+
 test_expect_success 'list notes with "git notes list"' '
        git notes list > output &&
        test_cmp expect output