Git development
 help / color / mirror / Atom feed
* [PATCHv12 15/23] Teach notes code to properly preserve non-notes in the notes tree
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
  To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-1-git-send-email-johan@herland.net>

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 5c49465..f25643e 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.6.6.405.g80ed6

^ permalink raw reply related

* [PATCHv12 17/23] builtin-notes: Add "remove" subcommand for removing existing notes
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
  To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-1-git-send-email-johan@herland.net>

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 5a0234f..88b99a7 100644
--- a/builtin-notes.c
+++ b/builtin-notes.c
@@ -19,6 +19,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
 };
 
@@ -155,9 +156,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[] = {
@@ -176,10 +178,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");
@@ -194,7 +208,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);
 
@@ -208,35 +222,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.6.6.405.g80ed6

^ permalink raw reply related

* [PATCHv12 16/23] Teach builtin-notes to remove empty notes
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
  To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-1-git-send-email-johan@herland.net>

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 a764811..5a0234f 100644
--- a/builtin-notes.c
+++ b/builtin-notes.c
@@ -71,7 +71,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);
@@ -155,7 +155,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 };
@@ -229,8 +230,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.6.6.405.g80ed6

^ permalink raw reply related

* [PATCHv12 12/23] Builtin-ify git-notes
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
  To: gitster; +Cc: git, johan, Stephen Boyd
In-Reply-To: <1264593120-4428-1-git-send-email-johan@herland.net>

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                               |  238 +++++++++++++++++++++++++
 builtin.h                                     |    3 +
 git-notes.sh => contrib/examples/git-notes.sh |    0
 git.c                                         |    1 +
 t/t3301-notes.sh                              |   98 +++++++----
 7 files changed, 308 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..a764811
--- /dev/null
+++ b/builtin-notes.c
@@ -0,0 +1,238 @@
+/*
+ * 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 "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 note contents\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 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));
+
+		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.6.6.405.g80ed6

^ permalink raw reply related

* [PATCHv12 18/23] t3305: Verify that removing notes triggers automatic fanout consolidation
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
  To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-1-git-send-email-johan@herland.net>

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.6.6.405.g80ed6

^ permalink raw reply related

* [PATCHv12 20/23] builtin-notes: Add "prune" subcommand for removing notes for missing objects
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
  To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-1-git-send-email-johan@herland.net>

"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 88b99a7..e84ab7a 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>]",
+	"git notes prune",
 	NULL
 };
 
@@ -159,7 +160,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[] = {
@@ -180,8 +181,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) {
@@ -195,7 +198,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);
 	}
@@ -222,7 +225,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);
@@ -236,12 +239,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.6.6.405.g80ed6

^ permalink raw reply related

* [PATCHv12 19/23] Notes API: prune_notes(): Prune notes that belong to non-existing objects
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
  To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-1-git-send-email-johan@herland.net>

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 f25643e..c0c9c5a 100644
--- a/notes.h
+++ b/notes.h
@@ -158,6 +158,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.6.6.405.g80ed6

^ permalink raw reply related

* [PATCHv12 23/23] builtin-notes: Add "add" subcommand for appending to note objects
From: Johan Herland @ 2010-01-27 11:52 UTC (permalink / raw)
  To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-1-git-send-email-johan@herland.net>

"git notes add" is identical 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 add -m "Acked-by: A U Thor <author@example.com>"

If there is no existing note to append to, "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>
Signed-off-by: Johan Herland <johan@herland.net>
---
 Documentation/git-notes.txt |    5 ++++-
 builtin-notes.c             |   34 +++++++++++++++++++++++++---------
 t/t3301-notes.sh            |   29 +++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 4d29d5f..3fe73e8 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git notes' [list [<object>]]
-'git notes' edit [-F <file> | -m <msg>] [<object>]
+'git notes' (edit | add) [-F <file> | -m <msg>] [<object>]
 'git notes' show [<object>]
 'git notes' remove [<object>]
 'git notes' prune
@@ -44,6 +44,9 @@ list::
 edit::
 	Edit the notes for a given object (defaults to HEAD).
 
+add::
+	Append to the notes for a given object (defaults to HEAD).
+
 show::
 	Show the notes for a given object (defaults to HEAD).
 
diff --git a/builtin-notes.c b/builtin-notes.c
index a322642..4f570a4 100644
--- a/builtin-notes.c
+++ b/builtin-notes.c
@@ -18,7 +18,7 @@
 
 static const char * const git_notes_usage[] = {
 	"git notes [list [<object>]]",
-	"git notes edit [-m <msg> | -F <file>] [<object>]",
+	"git notes (edit | add) [-m <msg> | -F <file>] [<object>]",
 	"git notes show [<object>]",
 	"git notes remove [<object>]",
 	"git notes prune",
@@ -53,7 +53,7 @@ static void write_note_data(int fd, const unsigned char *sha1)
 
 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)
 {
@@ -68,7 +68,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));
 
@@ -82,6 +82,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));
@@ -169,7 +183,7 @@ 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, edit = 0, add = 0, show = 0, remove = 0, prune = 0;
 	int given_object;
 	const char *msgfile = NULL;
 	struct msg_arg msg = { 0, STRBUF_INIT };
@@ -189,6 +203,8 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		list = 1;
 	else if (argc && !strcmp(argv[0], "edit"))
 		edit = 1;
+	else if (argc && !strcmp(argv[0], "add"))
+		add = 1;
 	else if (argc && !strcmp(argv[0], "show"))
 		show = 1;
 	else if (argc && !strcmp(argv[0], "remove"))
@@ -198,10 +214,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 + edit + add + show + remove + prune != 1)
 		usage_with_options(git_notes_usage, options);
 
-	if ((msg.given || msgfile) && !edit) {
+	if ((msg.given || msgfile) && !(edit || add)) {
 		error("cannot use -m/-F options with %s subcommand.", argv[0]);
 		usage_with_options(git_notes_usage, options);
 	}
@@ -250,7 +266,7 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 		return execv_git_cmd(show_args);
 	}
 
-	/* edit/remove/prune command */
+	/* edit/add/remove/prune command */
 
 	if (remove)
 		strbuf_reset(&buf);
@@ -268,8 +284,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, add,
+			    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 13f307a..b40e5a1 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -316,6 +316,35 @@ test_expect_success 'list specific note with "git notes list <object>"' '
 	test_cmp expect output
 '
 
+cat > expect << EOF
+Initial set of notes
+
+More notes added with git notes add
+EOF
+
+test_expect_success 'Append to existing note with "git notes add"' '
+	git notes edit -m "Initial set of notes" &&
+	git notes add -m "More notes added with git notes add" &&
+	git notes show > output &&
+	test_cmp expect output
+'
+
+test_expect_success 'Appending empty string does not change existing note' '
+	git notes add -m "" &&
+	git notes show > output &&
+	test_cmp expect output
+'
+
+test_expect_success 'git notes add == edit when there is no existing note' '
+	git notes remove HEAD &&
+	! git notes show &&
+	git notes add -m "Initial set of notes
+
+More notes added with git notes add" &&
+	git notes show > output &&
+	test_cmp expect output
+'
+
 test_expect_success 'create other note on a different notes ref (setup)' '
 	: > a6 &&
 	git add a6 &&
-- 
1.6.6.405.g80ed6

^ permalink raw reply related

* [PATCHv12 21/23] Documentation: Generalize git-notes docs to 'objects' instead of 'commits'
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
  To: gitster; +Cc: git, johan, Johannes Schindelin
In-Reply-To: <1264593120-4428-1-git-send-email-johan@herland.net>

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.6.6.405.g80ed6

^ permalink raw reply related

* [PATCHv12 22/23] builtin-notes: Add "list" subcommand for listing note objects
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
  To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-1-git-send-email-johan@herland.net>

"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>, if any.

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             |   33 +++++++++++++++++++++++++++++----
 t/t3301-notes.sh            |   24 ++++++++++++++++++++++++
 3 files changed, 65 insertions(+), 5 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 e84ab7a..a322642 100644
--- a/builtin-notes.c
+++ b/builtin-notes.c
@@ -17,6 +17,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>]",
@@ -30,6 +31,14 @@ static const char note_template[] =
 	"# Write/edit the note contents\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;
@@ -160,7 +169,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[] = {
@@ -175,7 +185,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;
@@ -183,8 +195,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) {
@@ -197,7 +211,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);
@@ -215,6 +230,16 @@ int cmd_notes(int argc, const char **argv, const char *prefix)
 
 	note = get_note(t, object);
 
+	/* list command */
+
+	if (list) {
+		if (given_object) {
+			puts(sha1_to_hex(note));
+			return 0;
+		} else
+			return for_each_note(t, 0, list_each_note, NULL);
+	}
+
 	/* show command */
 
 	if (show && !note) {
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index d29daac..13f307a 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -292,6 +292,30 @@ 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
+'
+
 test_expect_success 'create other note on a different notes ref (setup)' '
 	: > a6 &&
 	git add a6 &&
-- 
1.6.6.405.g80ed6

^ permalink raw reply related

* [PATCHv12 00/23] git notes
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
  To: gitster; +Cc: git, johan

Hi,

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

- Rebased onto 67bc740... (Merge branch 'jc/maint-limit-note-output') to adjust
  for the fixes limiting the eagerness of log and friends to show notes.

- Patches #2, #3, #10, #12, #17 and #20 resolved conflicts while rebasing.

- Patch #12 has been changed to call die() instead of fprintf(stderr, ...)
  followed by exit(1). Thanks to Stephen Boyd for the suggestion.

- Patches #17 and #18 have been changed to add a new "git notes remove"
  subcommand instead of a "git notes edit -d" option.

- Patch #19 has renamed gc_notes() to prune_notes().

- Patch #20 (which added "git gc --notes") has been replaced by a patch adding
  the equivalent "git notes prune" subcommand.

- Patch #21 is a new patch revamping the git-notes documentation to talk about
  'object' notes instead of 'commit' notes.

- Patch #22 is a new patch adding the "git notes list" command for listing
  notes and the objects they annotate.

- Patch #23 is a new patch adding the "git notes add" command for appending
  contents to notes (instead of editing/replacing).


STILL TODO:

- Suggestion by Matthieu Moy and Sverre Rabbelier:
  Add notes support to git-format-patch, where note contents in
  refs/notes/format-patch are added to the "comments section"
  (i.e. following the '---' separator) of generated patches.

- Better integration with rebase/amend/cherry-pick. Optionally bring
  notes across a commit rewrite. Controlled by command-line options
  and/or config variables. Add "git notes move" and "git notes copy"
  to suit. 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.

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

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

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

- Handle note objects that are not blobs, but trees

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


Have fun! :)

...Johan


Johan Herland (23):
  Minor non-functional 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 "add" subcommand for appending to note objects

 Documentation/git-notes.txt                   |   60 ++-
 Makefile                                      |    2 +-
 builtin-notes.c                               |  301 +++++++++
 builtin.h                                     |    3 +
 git-notes.sh => contrib/examples/git-notes.sh |    0
 git.c                                         |    1 +
 notes.c                                       |  843 +++++++++++++++++++++----
 notes.h                                       |  192 ++++++-
 pretty.c                                      |    9 +-
 t/t3301-notes.sh                              |  262 +++++++-
 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, 1737 insertions(+), 189 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

^ permalink raw reply

* [PATCHv12 02/23] Notes API: get_commit_notes() -> format_note() + remove the commit restriction
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
  To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-1-git-send-email-johan@herland.net>

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.6.6.405.g80ed6

^ permalink raw reply related

* [PATCHv12 04/23] Notes API: init_notes(): Initialize the notes tree from the given notes ref
From: Johan Herland @ 2010-01-27 11:51 UTC (permalink / raw)
  To: gitster; +Cc: git, johan
In-Reply-To: <1264593120-4428-1-git-send-email-johan@herland.net>

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.6.6.405.g80ed6

^ permalink raw reply related

* Re: git notes: notes
From: Johan Herland @ 2010-01-27 11:55 UTC (permalink / raw)
  To: git; +Cc: Joey Hess, Johannes Schindelin
In-Reply-To: <20100121040533.GA13597@gnu.kitenet.net>

On Thursday 21 January 2010, Joey Hess wrote:
> Johannes Schindelin wrote:
> > I do not really care as long as there is a nice way to edit the
> > complete note interactively.
> >
> > Of course, I _do_ expect people to get confused just like they do with
> > the current inconsistencies: "git commit -m" does not really append,
> > but set the commit message, even if you amend a commit.
> >
> > So maybe you want to use a different command line option for that.
> 
> Maybe: git notes add [-m|-F]

Thanks for the suggestion. I've added this to the new iteration of the 
jh/notes series.


...Johan

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

^ permalink raw reply

* Re: [PATCHv11 20/20] builtin-gc: Teach the new --notes option to garbage-collect notes
From: Johan Herland @ 2010-01-27 12:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Stephen Boyd
In-Reply-To: <7vtyufp6r9.fsf@alter.siamese.dyndns.org>

On Thursday 21 January 2010, Junio C Hamano wrote:
> Stephen Boyd <bebarino@gmail.com> writes:
> > On Sun, Jan 17, 2010 at 1:04 PM, Johan Herland wrote:
> >> The new --notes option triggers a call to gc_notes(), which removes
> >> note objects that reference non-existing objects.
> >
> > Shouldn't notes be unconditionally garbage collected? Or maybe there's
> > a reason why notes should be treated differently that isn't written
> > here.
> 
> A few thoughts, inspired by this patch, but on other things around
> "notes":
> 
>  - This is more about pruning notes regarding objects that are no longer
>    availabe from _the tip_ of the notes tree.  It doesn't run (and I am
>    not suggesting to make it it to run) filter-branch to eradicate all
>    such stale notes from the notes commit ancestry, so in that sense this
>    is not really a garbage collection.

I've removed this patch from the next iteration of the series...

>  - We would want to have "notes prune" subcommand that lets us do the
>    pruning without all the other "gc" operation.  "git gc" would of
>  course call the same underlying code "notes prune" would use if we want
>  to be able to trigger "notes prune" from it.

...and replaced it with "git notes prune".

>  - Because there is no public interface to list objects that are
>  annotated with the most recent notes tree, the only thing this pruning
>  affects is the look-up overhead of "log --show-notes".  As such, it
>  might be better to later add tree rebalancing in "notes prune" and at
>  that point, it will become even less about garbage collection and more
>  about performance ("notes optimize?").
> 
>  - Do we want to have a public interface to list objects that are
>    annotated with notes?  "git notes" probably deserves a bit more
>    subcommands other than "(edit|show)" to help users, e.g. "list" and
>    "remove".

Added in the next iteration of the series.

>  - If this were "notes optimize" (this is just me thinking aloud),
>  perhaps we would want to do this at some key places (e.g. when you
>    automatically rebalance the tree).

There is currently no explicit rebalancing of the tree. The only thing "git 
notes prune" does, is call remove_note() for all notes referencing 
unreachable objects. Instead the tree is always kept in "balance" by 
remove_note() calling note_tree_consolidate() for the part of the tree 
affected by the removed note.


...Johan

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

^ permalink raw reply

* Re: Testing if a certain commit is in the current branch
From: Frans Pop @ 2010-01-27 13:26 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Junio C Hamano, git
In-Reply-To: <20100127101943.GA4470@progeny.tock>

On Wednesday 27 January 2010, Jonathan Nieder wrote:
> Frans Pop wrote:
> > The use case is a wrapper script for kernel builds in which some
> > variation are needed (due to changes in the build system) when
> > building older kernels, e.g. during bisections or when building older
> > stable updates. So I test for the presence of commits that introduced
> > relevant changes.
>
> Neat idea.  How about this (untested)?
>
>  git rev-list ^HEAD --no-walk commit1 commit2 ...

Does not seem to work. But I'd be interested if you find a variant that 
does as it's very fast.

Thanks,
FJP

^ permalink raw reply

* Re: Testing if a certain commit is in the current branch
From: Petr Baudis @ 2010-01-27 15:08 UTC (permalink / raw)
  To: Frans Pop; +Cc: Junio C Hamano, git
In-Reply-To: <201001270936.14935.elendil@planet.nl>

On Wed, Jan 27, 2010 at 09:36:14AM +0100, Frans Pop wrote:
> On Wednesday 27 January 2010, Junio C Hamano wrote:
> > Frans Pop <elendil@planet.nl> writes:
> > > I'm currently using the following command for this:
> > >    git log --pretty=format:%H | grep -q "^<commit id>"
> > >
> > > Problem is that with large repos that can be quite slow.
> > >
> > > Is there a faster way to do the test?
> >
> > test "$(git merge-base HEAD $commit)" = "$(git rev-parse $commit)"
> 
> Great! If the commit ID is not present that only takes 1 sec versus 11 secs 
> for my test. (If the commit _is_ present and fairly recent my test can be 
> faster, but 11 secs delay when it's not present hurts more.)

Isn't something very wrong if grepping the log output is faster than
simple merge-base call? Can you post exact numbers?

				Petr "Pasky" Baudis

^ permalink raw reply

* Re: Testing if a certain commit is in the current branch
From: Frans Pop @ 2010-01-27 15:39 UTC (permalink / raw)
  To: Dale Farnsworth; +Cc: git
In-Reply-To: <E1Na9ca-0004x2-Gf@xyzzy.farnsworth.org>

On Wednesday 27 January 2010, Dale Farnsworth wrote:
> I've been using the following.  It produces output only if the commit
> is NOT in HEAD.
>
> git rev-list -1 ^HEAD commitID

Nice. That does work.

Returns 0 in both cases and you'd need to allow for nonexisting commit IDs, 
so you'd get:
   test -z "$(git rev-list -1 ^HEAD <commit id> 2>/dev/null)"

Time needed looks to be slightly faster than Junio's test.

Thanks,
FJP

^ permalink raw reply

* Re: Testing if a certain commit is in the current branch
From: Frans Pop @ 2010-01-27 15:27 UTC (permalink / raw)
  To: Petr Baudis; +Cc: Junio C Hamano, git
In-Reply-To: <20100127150834.GG9553@machine.or.cz>

On Wednesday 27 January 2010, Petr Baudis wrote:
> > Great! If the commit ID is not present that only takes 1 sec versus 11
> > secs for my test. (If the commit _is_ present and fairly recent my
> > test can be faster, but 11 secs delay when it's not present hurts
> > more.)
>
> Isn't something very wrong if grepping the log output is faster than
> simple merge-base call? Can you post exact numbers?

It's only faster if the commit is present very early in the log.
The reason is probably that because of the '-q' option it does not actually 
read the full log, but breaks out as soon as it's found a match.

In my test I had an elapsed time of ~0.5 sec (versus 1 sec for Junio's 
test). And here's a grep for the very first commit:

$ time git log --pretty=format:%H | grep -q 48404cb50b0b547

real    0m0.010s
user    0m0.000s
sys     0m0.012s

^ permalink raw reply

* Re: [PATCH] filter-branch fix and tests
From: Michal Sojka @ 2010-01-27 15:49 UTC (permalink / raw)
  To: Johannes Schindelin, git
In-Reply-To: <alpine.DEB.1.00.1001261939420.4641@intel-tinevez-2-302>

Hi Johannes,

On Tuesday 26 of January 2010 19:40:47 Johannes Schindelin wrote:
> On Mon, 25 Jan 2010, Michal Sojka wrote:
> > this is resend of the patch I sent two weeks ago. I've extended the
> > commit message to address Johannes' question and in the second patch
> > I've added tests for the fix.
> 
> From a quick look, it does not seem that I will be happy with your "fix";
> you changed the commit message, but did not address the issue.
> 
> Unfortunately, I do not have time to review anything that takes longer
> than 30 minutes in total, and today not even that, so that will have to
> wait.

Thanks for your reply. I do not want to steal your time, but I may not
understand what is the issue. I extended my previous patch with tests
(see bellow) so that all the use cases which might be IMHO affected by
my "fix" are covered. If you can think of another use case let me
know.

The fastest way to test this patch is:
GIT_SKIP_TESTS='t7003.[12]? t7003.[2-9]' ./t7003-filter-branch.sh -d

>From 849b105541ba4b5e3592de6769922b1264be0c77 Mon Sep 17 00:00:00 2001
From: Michal Sojka <sojkam1@fel.cvut.cz>
Date: Wed, 27 Jan 2010 15:57:17 +0100
Subject: [PATCH] filter-branch: Add tests for submodules

There are three important tests:
1) 'rewrite submodule with another content' passes only with the
   previous patch applied.

2) 'checkout submodule during rewrite' demonstrates that it is not
   possible to replace a submodule revision in tree-filter by checking
   the submodule out and reseting the submodule's HEAD. Fails both
   with and without the previous patch. This is because filter-branch
   sets GIT_WORKING_TREE to "." which causes clone (called from
   git-submodule) to fail.

3) 'replace submodule revision' shows that replacing submodule
   revision is possible by direct index manipulation. Succeeds both
   with and without the previous patch.

Signed-off-by: Michal Sojka <sojkam1@fel.cvut.cz>
---
 t/t7003-filter-branch.sh |   48 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 48 insertions(+), 0 deletions(-)

diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index 9503875..a218d7a 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -306,4 +306,52 @@ test_expect_success '--remap-to-ancestor with filename filters' '
 	test $orig_invariant = $(git rev-parse invariant)
 '
 
+test_expect_success 'setup submodule' '
+	rm -rf * .*
+	git init &&
+	test_commit file &&
+	mkdir submod &&
+	submodurl="$PWD/submod"
+	( cd submod &&
+	  git init &&
+	  test_commit file-in-submod ) &&
+	git submodule add "$submodurl"
+	git commit -m "added submodule" &&
+	test_commit add-file &&
+	( cd submod && test_commit add-in-submodule ) &&
+	git add submod &&
+	git commit -m "changed submodule"
+	git branch original HEAD
+'
+
+orig_head=`git show-ref --hash HEAD`
+export orig_head
+
+test_expect_success 'rewrite submodule with another content' '
+	git filter-branch --tree-filter "test -d submod && {
+					 rm -rf submod &&
+					 git rm -rf --quiet submod &&
+					 mkdir submod &&
+					 : > submod/file
+					 } || : &&
+					 test $orig_head != `git show-ref --hash HEAD`"
+'
+
+test_expect_failure 'checkout submodule during rewrite' '
+	git reset --hard original &&
+	git filter-branch -f --tree-filter \
+	    "git submodule update --init &&
+	     cd submod &&
+	     git reset --hard origin/master" HEAD
+'
+
+test_expect_success 'replace submodule revision' '
+	git reset --hard original &&
+	git filter-branch -f --tree-filter \
+	    "git ls-files --error-unmatch -- submod > /dev/null 2>&1 &&
+	     git update-index --cacheinfo 160000 0123456789012345678901234567890123456789 submod ||
+	     : &&
+	     test $orig_head != `git show-ref --hash HEAD`"
+'
+
 test_done
-- 
1.6.6

^ permalink raw reply related

* Re: [PATCH not-for-mainline] Implement git-vcs-p4
From: Ilari Liusvaara @ 2010-01-27 15:56 UTC (permalink / raw)
  To: Tor Arvid Lund; +Cc: Daniel Barkalow, git
In-Reply-To: <1a6be5fa1001270318h4ac8ac3bnaba48787a5b3efa6@mail.gmail.com>

On Wed, Jan 27, 2010 at 12:18:35PM +0100, Tor Arvid Lund wrote:

> Then, I did 'git fetch', and got a seg fault. I got around it by
> commenting out a line:
> 
> diff --git a/transport.c b/transport.c
> index 7714fdb..5b404f7 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -924,7 +924,7 @@ struct transport *transport_get(struct remote
> *remote, const char *url)
>         ret->url = url;
> 
>         /* In case previous URL had helper forced, reset it. */
> -       remote->foreign_vcs = NULL;
> +/*     remote->foreign_vcs = NULL;*/
> 
>         /* maybe it is a foreign URL? */
>         if (url) {
> 

Hmm... And just commenting out that line will break case if you have
push URL using remote helpers and second one for same remote that
doesn't. 

I'll look into that issue.

-Ilari

^ permalink raw reply

* Re: Testing if a certain commit is in the current branch
From: Dale Farnsworth @ 2010-01-27 15:15 UTC (permalink / raw)
  To: elendil, git
In-Reply-To: <201001271426.06844.elendil@planet.nl>

> >  git rev-list ^HEAD --no-walk commit1 commit2 ...
> 
> Does not seem to work. But I'd be interested if you find a variant that 
> does as it's very fast.

I've been using the following.  It produces output only if the commit
is NOT in HEAD.

git rev-list -1 ^HEAD commitID

-Dale

^ permalink raw reply

* [PATCH] Fix remote.<remote>.vcs
From: Ilari Liusvaara @ 2010-01-27 16:16 UTC (permalink / raw)
  To: git; +Cc: Tor Arvid Lund

remote.<remote>.vcs causes remote->foreign_vcs to be set on entry to
transport_get(). Unfortunately, the code assumed that any such entry
is stale from previous round. Resetting after starting the remote helper
avoids that case (it isn't likely anyone is using VCS setting and multiple
push URLs, as VCS setting is meant for non-URL-based helpers).

Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
 transport.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

Tor, does this also fix the segfault for you? This might work without
breaking some multiple-URL cases (just commenting out the reset will break
them).

diff --git a/transport.c b/transport.c
index 7714fdb..76b22e5 100644
--- a/transport.c
+++ b/transport.c
@@ -923,9 +923,6 @@ struct transport *transport_get(struct remote *remote, const char *url)
 		url = remote->url[0];
 	ret->url = url;
 
-	/* In case previous URL had helper forced, reset it. */
-	remote->foreign_vcs = NULL;
-
 	/* maybe it is a foreign URL? */
 	if (url) {
 		const char *p = url;
@@ -938,6 +935,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
 
 	if (remote && remote->foreign_vcs) {
 		transport_helper_init(ret, remote->foreign_vcs);
+		remote->foreign_vcs = NULL;
 	} else if (!prefixcmp(url, "rsync:")) {
 		ret->get_refs_list = get_refs_via_rsync;
 		ret->fetch = fetch_objs_via_rsync;
-- 
1.7.0.rc0.19.gd681a

^ permalink raw reply related

* Re: [PATCH] filter-branch fix and tests
From: Johannes Sixt @ 2010-01-27 16:18 UTC (permalink / raw)
  To: Michal Sojka; +Cc: Johannes Schindelin, git
In-Reply-To: <201001271649.19287.sojkam1@fel.cvut.cz>

Michal Sojka schrieb:
> +orig_head=`git show-ref --hash HEAD`
> +export orig_head

You place the tree filter in double-quotes. Hence, orig_head will be
interpolated on the filter-branch command line. You don't need to export it.

> +test_expect_success 'rewrite submodule with another content' '
> +	git filter-branch --tree-filter "test -d submod && {
> +					 rm -rf submod &&
> +					 git rm -rf --quiet submod &&
> +					 mkdir submod &&
> +					 : > submod/file
> +					 } || : &&
> +					 test $orig_head != `git show-ref --hash HEAD`"

What is the purpose of the check in the last line?

As long as you have another command after the "} || : &&", you can just
write "}" instead.

> +test_expect_failure 'checkout submodule during rewrite' '
> +	git reset --hard original &&
> +	git filter-branch -f --tree-filter \
> +	    "git submodule update --init &&
> +	     cd submod &&
> +	     git reset --hard origin/master" HEAD

You must not change the directory without changing back. Use a sub-shell.

I'm not sure whether it's worth catering for this use-case anyway.
Replacing a submodule commit should really be done only in the
--index-filter. The tree that --tree-filter checks out is intended only as
a temporary scratch area. It is not intended as a full worktree. In
particular, since 'submodule update --init' changes the configuration, it
is extremly dangerous to call from a filter.

-- Hannes

^ permalink raw reply

* How to email out commit message automatically?
From: Weiwei Shu @ 2010-01-27 16:20 UTC (permalink / raw)
  To: git, shuww2008

Hi All,

The second time to post here.  Git is great.

I would like to email the commit message to my Gmail box automatically
whenever I submit a commit.  Can anybody point me a link on how to do
that?  I've searched it for half day but not successful.  Thanks!


Weiwei

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox