Git development
 help / color / mirror / Atom feed
* [RFC/PATCHv8 06/10] Notes API: Allow multiple concurrent notes trees with new struct notes_tree
From: Johan Herland @ 2009-11-20  1:39 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1258681154-2167-1-git-send-email-johan@herland.net>

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

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

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c  |   67 ++++++++++++++++++++++++++++++++++++++-----------------------
 notes.h  |   53 +++++++++++++++++++++++++++++++++++-------------
 pretty.c |    4 +-
 3 files changed, 82 insertions(+), 42 deletions(-)

diff --git a/notes.c b/notes.c
index 9581b98..a5d9736 100644
--- a/notes.c
+++ b/notes.c
@@ -50,9 +50,7 @@ struct leaf_node {
 #define SUBTREE_SHA1_PREFIXCMP(key_sha1, subtree_sha1) \
 	(memcmp(key_sha1, subtree_sha1, subtree_sha1[19]))
 
-static struct int_node root_node;
-
-static int initialized;
+static struct notes_tree default_tree;
 
 static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 		unsigned int n);
@@ -434,14 +432,15 @@ redo:
 	return 0;
 }
 
-void init_notes(const char *notes_ref, int flags)
+void init_notes(struct notes_tree *t, const char *notes_ref, int flags)
 {
 	unsigned char sha1[20], object_sha1[20];
 	unsigned mode;
 	struct leaf_node root_tree;
 
-	assert(!initialized);
-	initialized = 1;
+	if (!t)
+		t = &default_tree;
+	assert(!t->initialized);
 
 	if (!notes_ref) {
 		const char *env = getenv(GIT_NOTES_REF_ENVIRONMENT);
@@ -451,6 +450,10 @@ void init_notes(const char *notes_ref, int flags)
 			notes_ref = GIT_NOTES_DEFAULT_REF;
 	}
 
+	t->root = (struct int_node *) xcalloc(sizeof(struct int_node), 1);
+	t->ref = notes_ref ? xstrdup(notes_ref) : NULL;
+	t->initialized = 1;
+
 	if (flags & NOTES_INIT_EMPTY || !notes_ref ||
 	    read_ref(notes_ref, object_sha1) ||
 	    get_tree_entry(object_sha1, "", sha1, &mode))
@@ -458,44 +461,56 @@ void init_notes(const char *notes_ref, int flags)
 
 	hashclr(root_tree.key_sha1);
 	hashcpy(root_tree.val_sha1, sha1);
-	load_subtree(&root_tree, &root_node, 0);
+	load_subtree(&root_tree, t->root, 0);
 }
 
-void add_note(const unsigned char *object_sha1, const unsigned char *note_sha1)
+void add_note(struct notes_tree *t, const unsigned char *object_sha1,
+		const unsigned char *note_sha1)
 {
 	struct leaf_node *l;
 
-	assert(initialized);
+	if (!t)
+		t = &default_tree;
+	assert(t->initialized);
 	l = (struct leaf_node *) xmalloc(sizeof(struct leaf_node));
 	hashcpy(l->key_sha1, object_sha1);
 	hashcpy(l->val_sha1, note_sha1);
-	note_tree_insert(&root_node, 0, l, PTR_TYPE_NOTE);
+	note_tree_insert(t->root, 0, l, PTR_TYPE_NOTE);
 }
 
-const unsigned char *get_note(const unsigned char *object_sha1)
+const unsigned char *get_note(struct notes_tree *t,
+		const unsigned char *object_sha1)
 {
 	struct leaf_node *found;
 
-	assert(initialized);
-	found = note_tree_find(&root_node, 0, object_sha1);
+	if (!t)
+		t = &default_tree;
+	assert(t->initialized);
+	found = note_tree_find(t->root, 0, object_sha1);
 	return found ? found->val_sha1 : NULL;
 }
 
-int for_each_note(each_note_fn fn, void *cb_data)
+int for_each_note(struct notes_tree *t, each_note_fn fn, void *cb_data)
 {
-	assert(initialized);
-	return for_each_note_helper(&root_node, 0, 0, fn, cb_data);
+	if (!t)
+		t = &default_tree;
+	assert(t->initialized);
+	return for_each_note_helper(t->root, 0, 0, fn, cb_data);
 }
 
-void free_notes(void)
+void free_notes(struct notes_tree *t)
 {
-	note_tree_free(&root_node);
-	memset(&root_node, 0, sizeof(struct int_node));
-	initialized = 0;
+	if (!t)
+		t = &default_tree;
+	if (t->root)
+		note_tree_free(t->root);
+	free(t->root);
+	free(t->ref);
+	memset(t, 0, sizeof(struct notes_tree));
 }
 
-void format_note(const unsigned char *object_sha1, struct strbuf *sb,
-		const char *output_encoding, int flags)
+void format_note(struct notes_tree *t, const unsigned char *object_sha1,
+		struct strbuf *sb, const char *output_encoding, int flags)
 {
 	static const char utf8[] = "utf-8";
 	const unsigned char *sha1;
@@ -503,10 +518,12 @@ void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 	unsigned long linelen, msglen;
 	enum object_type type;
 
-	if (!initialized)
-		init_notes(NULL, 0);
+	if (!t)
+		t = &default_tree;
+	if (!t->initialized)
+		init_notes(t, NULL, 0);
 
-	sha1 = get_note(object_sha1);
+	sha1 = get_note(t, object_sha1);
 	if (!sha1)
 		return;
 
diff --git a/notes.h b/notes.h
index f67bae8..ea1235f 100644
--- a/notes.h
+++ b/notes.h
@@ -2,6 +2,21 @@
 #define NOTES_H
 
 /*
+ * Notes tree object
+ *
+ * Encapsulates the internal notes tree structure associated with a notes ref.
+ * Whenever a struct notes_tree pointer is required below, you may pass NULL in
+ * order to use the default/internal notes tree. E.g. you only need to pass a
+ * non-NULL value if you need to refer to several different notes trees
+ * simultaneously.
+ */
+struct notes_tree {
+	struct int_node *root;
+	char *ref;
+	int initialized;
+};
+
+/*
  * Flags controlling behaviour of notes tree initialization
  *
  * Default behaviour is to initialize the notes tree from the tree object
@@ -10,26 +25,32 @@
 #define NOTES_INIT_EMPTY 1
 
 /*
- * Initialize internal notes tree structure with the notes tree at the given
+ * Initialize the given notes_tree with the notes tree structure at the given
  * ref. If given ref is NULL, the value of the $GIT_NOTES_REF environment
  * variable is used, and if that is missing, the default notes ref is used
  * ("refs/notes/commits").
  *
- * If you need to re-intialize the internal notes tree structure (e.g. loading
- * from a different notes ref), please first de-initialize the current notes
- * tree by calling free_notes().
+ * If you need to re-intialize a notes_tree structure (e.g. when switching from
+ * one notes ref to another), you must first de-initialize the notes_tree
+ * structure by calling free_notes(struct notes_tree *).
+ *
+ * If you pass t == NULL, the default internal notes_tree will be initialized.
+ *
+ * Precondition: The notes_tree structure is zeroed (this can be achieved with
+ * memset(t, 0, sizeof(struct notes_tree)))
  */
-void init_notes(const char *notes_ref, int flags);
+void init_notes(struct notes_tree *t, const char *notes_ref, int flags);
 
-/* Add the given note object to the internal notes tree structure */
-void add_note(const unsigned char *object_sha1,
+/* Add the given note object to the given notes_tree structure */
+void add_note(struct notes_tree *t, const unsigned char *object_sha1,
 		const unsigned char *note_sha1);
 
 /* Get the note object SHA1 containing the note data for the given object */
-const unsigned char *get_note(const unsigned char *object_sha1);
+const unsigned char *get_note(struct notes_tree *t,
+		const unsigned char *object_sha1);
 
 /*
- * Invoke the specified callback function for each note
+ * Invoke the specified callback function for each note in the given notes_tree
  *
  * If the callback returns nonzero, the note walk is aborted, and the return
  * value from the callback is returned from for_each_note().
@@ -43,10 +64,10 @@ const unsigned char *get_note(const unsigned char *object_sha1);
 typedef int each_note_fn(const unsigned char *object_sha1,
 		const unsigned char *note_sha1, const char *note_tree_path,
 		void *cb_data);
-int for_each_note(each_note_fn fn, void *cb_data);
+int for_each_note(struct notes_tree *t, each_note_fn fn, void *cb_data);
 
-/* Free (and de-initialize) the internal notes tree structure */
-void free_notes(void);
+/* Free (and de-initialize) the give notes_tree structure */
+void free_notes(struct notes_tree *t);
 
 /* Flags controlling how notes are formatted */
 #define NOTES_SHOW_HEADER 1
@@ -55,12 +76,14 @@ void free_notes(void);
 /*
  * Fill the given strbuf with the notes associated with the given object.
  *
- * If the internal notes structure is not initialized, it will be auto-
+ * If the given notes_tree structure is not initialized, it will be auto-
  * initialized to the default value (see documentation for init_notes() above).
+ * If the given notes_tree is NULL, the internal/default notes_tree will be
+ * used instead.
  *
  * 'flags' is a bitwise combination of the above formatting flags.
  */
-void format_note(const unsigned char *object_sha1, struct strbuf *sb,
-		const char *output_encoding, int flags);
+void format_note(struct notes_tree *t, const unsigned char *object_sha1,
+		struct strbuf *sb, const char *output_encoding, int flags);
 
 #endif
diff --git a/pretty.c b/pretty.c
index 771d186..eeb2ebd 100644
--- a/pretty.c
+++ b/pretty.c
@@ -775,7 +775,7 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
 		}
 		return 0;	/* unknown %g placeholder */
 	case 'N':
-		format_note(commit->object.sha1, sb, git_log_output_encoding ?
+		format_note(NULL, commit->object.sha1, sb, git_log_output_encoding ?
 			    git_log_output_encoding : git_commit_encoding, 0);
 		return 1;
 	}
@@ -1057,7 +1057,7 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 		strbuf_addch(sb, '\n');
 
 	if (fmt != CMIT_FMT_ONELINE)
-		format_note(commit->object.sha1, sb, encoding,
+		format_note(NULL, commit->object.sha1, sb, encoding,
 			    NOTES_SHOW_HEADER | NOTES_INDENT);
 
 	free(reencoded);
-- 
1.6.4.304.g1365c.dirty

^ permalink raw reply related

* [RFC/PATCHv8 07/10] Refactor notes concatenation into a flexible interface for combining notes
From: Johan Herland @ 2009-11-20  1:39 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1258681154-2167-1-git-send-email-johan@herland.net>

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

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

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

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

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

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

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

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

diff --git a/notes.c b/notes.c
index a5d9736..19ae492 100644
--- a/notes.c
+++ b/notes.c
@@ -127,55 +127,12 @@ static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,
 	return NULL;
 }
 
-/* Create a new blob object by concatenating the two given blob objects */
-static int concatenate_notes(unsigned char *cur_sha1,
-		const unsigned char *new_sha1)
-{
-	char *cur_msg, *new_msg, *buf;
-	unsigned long cur_len, new_len, buf_len;
-	enum object_type cur_type, new_type;
-	int ret;
-
-	/* read in both note blob objects */
-	new_msg = read_sha1_file(new_sha1, &new_type, &new_len);
-	if (!new_msg || !new_len || new_type != OBJ_BLOB) {
-		free(new_msg);
-		return 0;
-	}
-	cur_msg = read_sha1_file(cur_sha1, &cur_type, &cur_len);
-	if (!cur_msg || !cur_len || cur_type != OBJ_BLOB) {
-		free(cur_msg);
-		free(new_msg);
-		hashcpy(cur_sha1, new_sha1);
-		return 0;
-	}
-
-	/* we will separate the notes by a newline anyway */
-	if (cur_msg[cur_len - 1] == '\n')
-		cur_len--;
-
-	/* concatenate cur_msg and new_msg into buf */
-	buf_len = cur_len + 1 + new_len;
-	buf = (char *) xmalloc(buf_len);
-	memcpy(buf, cur_msg, cur_len);
-	buf[cur_len] = '\n';
-	memcpy(buf + cur_len + 1, new_msg, new_len);
-
-	free(cur_msg);
-	free(new_msg);
-
-	/* create a new blob object from buf */
-	ret = write_sha1_file(buf, buf_len, "blob", cur_sha1);
-	free(buf);
-	return ret;
-}
-
 /*
  * To insert a leaf_node:
  * Search to the tree location appropriate for the given leaf_node's key:
  * - If location is unused (NULL), store the tweaked pointer directly there
  * - If location holds a note entry that matches the note-to-be-inserted, then
- *   concatenate the two notes.
+ *   combine the two notes (by calling the given combine_notes function).
  * - If location holds a note entry that matches the subtree-to-be-inserted,
  *   then unpack the subtree-to-be-inserted into the location.
  * - If location holds a matching subtree entry, unpack the subtree at that
@@ -184,7 +141,8 @@ static int concatenate_notes(unsigned char *cur_sha1,
  *   node-to-be-inserted, and store the new int_node into the location.
  */
 static void note_tree_insert(struct int_node *tree, unsigned char n,
-		struct leaf_node *entry, unsigned char type)
+		struct leaf_node *entry, unsigned char type,
+		combine_notes_fn combine_notes)
 {
 	struct int_node *new_node;
 	struct leaf_node *l;
@@ -205,12 +163,11 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
 				if (!hashcmp(l->val_sha1, entry->val_sha1))
 					return;
 
-				if (concatenate_notes(l->val_sha1,
-						entry->val_sha1))
-					die("failed to concatenate note %s "
-					    "into note %s for object %s",
-					    sha1_to_hex(entry->val_sha1),
+				if (combine_notes(l->val_sha1, entry->val_sha1))
+					die("failed to combine notes %s and %s"
+					    " for object %s",
 					    sha1_to_hex(l->val_sha1),
+					    sha1_to_hex(entry->val_sha1),
 					    sha1_to_hex(l->key_sha1));
 				free(entry);
 				return;
@@ -233,7 +190,7 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
 			*p = NULL;
 			load_subtree(l, tree, n);
 			free(l);
-			note_tree_insert(tree, n, entry, type);
+			note_tree_insert(tree, n, entry, type, combine_notes);
 			return;
 		}
 		break;
@@ -243,9 +200,9 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
 	assert(GET_PTR_TYPE(*p) == PTR_TYPE_NOTE ||
 	       GET_PTR_TYPE(*p) == PTR_TYPE_SUBTREE);
 	new_node = (struct int_node *) xcalloc(sizeof(struct int_node), 1);
-	note_tree_insert(new_node, n + 1, l, GET_PTR_TYPE(*p));
+	note_tree_insert(new_node, n + 1, l, GET_PTR_TYPE(*p), combine_notes);
 	*p = SET_PTR_TYPE(new_node, PTR_TYPE_INTERNAL);
-	note_tree_insert(new_node, n + 1, entry, type);
+	note_tree_insert(new_node, n + 1, entry, type, combine_notes);
 }
 
 /* Free the entire notes data contained in the given tree */
@@ -331,7 +288,8 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 				l->key_sha1[19] = (unsigned char) len;
 				type = PTR_TYPE_SUBTREE;
 			}
-			note_tree_insert(node, n, l, type);
+			note_tree_insert(node, n, l, type,
+					combine_notes_concatenate);
 		}
 	}
 	free(buf);
@@ -432,7 +390,59 @@ redo:
 	return 0;
 }
 
-void init_notes(struct notes_tree *t, const char *notes_ref, int flags)
+int combine_notes_concatenate(unsigned char *cur_sha1, const unsigned char *new_sha1)
+{
+	char *cur_msg, *new_msg, *buf;
+	unsigned long cur_len, new_len, buf_len;
+	enum object_type cur_type, new_type;
+	int ret;
+
+	/* read in both note blob objects */
+	new_msg = read_sha1_file(new_sha1, &new_type, &new_len);
+	if (!new_msg || !new_len || new_type != OBJ_BLOB) {
+		free(new_msg);
+		return 0;
+	}
+	cur_msg = read_sha1_file(cur_sha1, &cur_type, &cur_len);
+	if (!cur_msg || !cur_len || cur_type != OBJ_BLOB) {
+		free(cur_msg);
+		free(new_msg);
+		hashcpy(cur_sha1, new_sha1);
+		return 0;
+	}
+
+	/* we will separate the notes by a newline anyway */
+	if (cur_msg[cur_len - 1] == '\n')
+		cur_len--;
+
+	/* concatenate cur_msg and new_msg into buf */
+	buf_len = cur_len + 1 + new_len;
+	buf = (char *) xmalloc(buf_len);
+	memcpy(buf, cur_msg, cur_len);
+	buf[cur_len] = '\n';
+	memcpy(buf + cur_len + 1, new_msg, new_len);
+	free(cur_msg);
+	free(new_msg);
+
+	/* create a new blob object from buf */
+	ret = write_sha1_file(buf, buf_len, "blob", cur_sha1);
+	free(buf);
+	return ret;
+}
+
+int combine_notes_overwrite(unsigned char *cur_sha1, const unsigned char *new_sha1)
+{
+	hashcpy(cur_sha1, new_sha1);
+	return 0;
+}
+
+int combine_notes_ignore(unsigned char *cur_sha1, const unsigned char *new_sha1)
+{
+	return 0;
+}
+
+void init_notes(struct notes_tree *t, const char *notes_ref,
+		combine_notes_fn combine_notes, int flags)
 {
 	unsigned char sha1[20], object_sha1[20];
 	unsigned mode;
@@ -450,8 +460,12 @@ void init_notes(struct notes_tree *t, const char *notes_ref, int flags)
 			notes_ref = GIT_NOTES_DEFAULT_REF;
 	}
 
+	if (!combine_notes)
+		combine_notes = combine_notes_concatenate;
+
 	t->root = (struct int_node *) xcalloc(sizeof(struct int_node), 1);
 	t->ref = notes_ref ? xstrdup(notes_ref) : NULL;
+	t->combine_notes = combine_notes;
 	t->initialized = 1;
 
 	if (flags & NOTES_INIT_EMPTY || !notes_ref ||
@@ -465,17 +479,19 @@ void init_notes(struct notes_tree *t, const char *notes_ref, int flags)
 }
 
 void add_note(struct notes_tree *t, const unsigned char *object_sha1,
-		const unsigned char *note_sha1)
+		const unsigned char *note_sha1, combine_notes_fn combine_notes)
 {
 	struct leaf_node *l;
 
 	if (!t)
 		t = &default_tree;
 	assert(t->initialized);
+	if (!combine_notes)
+		combine_notes = t->combine_notes;
 	l = (struct leaf_node *) xmalloc(sizeof(struct leaf_node));
 	hashcpy(l->key_sha1, object_sha1);
 	hashcpy(l->val_sha1, note_sha1);
-	note_tree_insert(t->root, 0, l, PTR_TYPE_NOTE);
+	note_tree_insert(t->root, 0, l, PTR_TYPE_NOTE, combine_notes);
 }
 
 const unsigned char *get_note(struct notes_tree *t,
@@ -521,7 +537,7 @@ void format_note(struct notes_tree *t, const unsigned char *object_sha1,
 	if (!t)
 		t = &default_tree;
 	if (!t->initialized)
-		init_notes(t, NULL, 0);
+		init_notes(t, NULL, NULL, 0);
 
 	sha1 = get_note(t, object_sha1);
 	if (!sha1)
diff --git a/notes.h b/notes.h
index ea1235f..047a282 100644
--- a/notes.h
+++ b/notes.h
@@ -2,6 +2,30 @@
 #define NOTES_H
 
 /*
+ * Function type for combining two notes annotating the same object.
+ *
+ * When adding a new note annotating the same object as an existing note, it is
+ * up to the caller to decide how to combine the two notes. The decision is
+ * made by passing in a function of the following form. The function accepts
+ * two SHA1s -- of the existing note and the new note, respectively. The
+ * function then combines the notes in whatever way it sees fit, and writes the
+ * resulting SHA1 into the first SHA1 argument (cur_sha1). A non-zero return
+ * value indicates failure.
+ *
+ * The two given SHA1s are guaranteed to be non-NULL and different.
+ *
+ * The default combine_notes function (you get this when passing NULL) is
+ * combine_notes_concatenate(), which appends the contents of the new note to
+ * the contents of the existing note.
+ */
+typedef int combine_notes_fn(unsigned char *cur_sha1, const unsigned char *new_sha1);
+
+/* Common notes combinators */
+int combine_notes_concatenate(unsigned char *cur_sha1, const unsigned char *new_sha1);
+int combine_notes_overwrite(unsigned char *cur_sha1, const unsigned char *new_sha1);
+int combine_notes_ignore(unsigned char *cur_sha1, const unsigned char *new_sha1);
+
+/*
  * Notes tree object
  *
  * Encapsulates the internal notes tree structure associated with a notes ref.
@@ -13,6 +37,7 @@
 struct notes_tree {
 	struct int_node *root;
 	char *ref;
+	combine_notes_fn *combine_notes;
 	int initialized;
 };
 
@@ -36,14 +61,19 @@ struct notes_tree {
  *
  * If you pass t == NULL, the default internal notes_tree will be initialized.
  *
+ * The combine_notes function that is passed becomes the default combine_notes
+ * function for the given notes_tree. If NULL is passed, the default
+ * combine_notes function is combine_notes_concatenate().
+ *
  * Precondition: The notes_tree structure is zeroed (this can be achieved with
  * memset(t, 0, sizeof(struct notes_tree)))
  */
-void init_notes(struct notes_tree *t, const char *notes_ref, int flags);
+void init_notes(struct notes_tree *t, const char *notes_ref,
+		combine_notes_fn combine_notes, int flags);
 
 /* Add the given note object to the given notes_tree structure */
 void add_note(struct notes_tree *t, const unsigned char *object_sha1,
-		const unsigned char *note_sha1);
+		const unsigned char *note_sha1, combine_notes_fn combine_notes);
 
 /* Get the note object SHA1 containing the note data for the given object */
 const unsigned char *get_note(struct notes_tree *t,
-- 
1.6.4.304.g1365c.dirty

^ permalink raw reply related

* [RFC/PATCHv8 09/10] Rename t9301 to t9350, to make room for more fast-import tests
From: Johan Herland @ 2009-11-20  1:39 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1258681154-2167-1-git-send-email-johan@herland.net>

Signed-off-by: Johan Herland <johan@herland.net>
---
 t/{t9301-fast-export.sh => t9350-fast-export.sh} |    0
 1 files changed, 0 insertions(+), 0 deletions(-)
 rename t/{t9301-fast-export.sh => t9350-fast-export.sh} (100%)

diff --git a/t/t9301-fast-export.sh b/t/t9350-fast-export.sh
similarity index 100%
rename from t/t9301-fast-export.sh
rename to t/t9350-fast-export.sh
-- 
1.6.4.304.g1365c.dirty

^ permalink raw reply

* [RFC/PATCHv8 08/10] fast-import: Proper notes tree manipulation using the notes API
From: Johan Herland @ 2009-11-20  1:39 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1258681154-2167-1-git-send-email-johan@herland.net>

This patch teaches 'git fast-import' to use the notes API to organize
the manipulation of note objects through a fast-import stream.
Note objects are added both to the notes tree and the "regular" tree
through the 'N' command, and when we're about to store the "regular" tree
object for the current commit, we walk through the notes tree to adjust all
the note object locations in the stored tree.

Unloading of a fast-import branch also unloads the corresponding notes
tree (if any), and the notes tree is (re)loaded when needed.

Signed-off-by: Johan Herland <johan@herland.net>
---

This patch is substantially different from the previous iteration.
Unloading (and reloading) the notes tree along with its corresponding
branch was relatively straightforward to fix, but avoiding the
destroying and re-adding of all the notes in every commit was much
harder. After 3-4 attempts at a simpler (but fundamentally broken)
approach, I finally landed on this. I'm not satisfied with the amount
of code introduced by this patch, and would be happy if someone found
a better/shorter/more elegant way to solve this problem.


...Johan


 fast-import.c          |  297 +++++++++++++++++++++++++++++++++++++++++++++++-
 t/t9300-fast-import.sh |  156 +++++++++++++++++++++++--
 2 files changed, 435 insertions(+), 18 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index dd3c99d..be3cbf9 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -156,6 +156,7 @@ Format of STDIN stream:
 #include "csum-file.h"
 #include "quote.h"
 #include "exec_cmd.h"
+#include "notes.h"

 #define PACK_ID_BITS 16
 #define MAX_PACK_ID ((1<<PACK_ID_BITS)-1)
@@ -246,6 +247,7 @@ struct branch
 	struct tree_entry branch_tree;
 	uintmax_t last_commit;
 	unsigned active : 1;
+	unsigned has_notes : 1;
 	unsigned pack_id : PACK_ID_BITS;
 	unsigned char sha1[20];
 };
@@ -277,6 +279,33 @@ struct recent_command
 	char *buf;
 };

+struct notes_tree_list
+{
+	struct notes_tree tree;
+	struct notes_tree_list *next;
+};
+
+struct tree_entry_replace
+{
+	struct tree_entry_replace *next;
+	char *old_path;
+	char *new_path;
+};
+
+struct reconcile_notes_tree_helper_data
+{
+	struct tree_entry *root;
+	struct tree_entry_replace **replace_list;
+};
+
+struct reconcile_note_and_tree_entry_data
+{
+	const unsigned char *object_sha1;
+	const unsigned char *note_sha1;
+	const char *note_tree_path;
+	struct tree_entry_replace **replace_list;
+};
+
 /* Configured limits on output */
 static unsigned long max_depth = 10;
 static off_t max_packsize = (1LL << 32) - 1;
@@ -341,6 +370,9 @@ static struct branch *active_branches;
 static struct tag *first_tag;
 static struct tag *last_tag;

+/* Notes data */
+static struct notes_tree_list *notes_trees;
+
 /* Input stream parsing */
 static whenspec_type whenspec = WHENSPEC_RAW;
 static struct strbuf command_buf = STRBUF_INIT;
@@ -694,6 +726,7 @@ static struct branch *new_branch(const char *name)
 	b->branch_tree.versions[0].mode = S_IFDIR;
 	b->branch_tree.versions[1].mode = S_IFDIR;
 	b->active = 0;
+	b->has_notes = 0;
 	b->pack_id = MAX_PACK_ID;
 	branch_table[hc] = b;
 	branch_count++;
@@ -1816,6 +1849,221 @@ static void parse_new_blob(void)
 	store_object(OBJ_BLOB, &buf, &last_blob, NULL, next_mark);
 }

+static struct notes_tree *notes_tree_create(const char *branch_name)
+{
+	struct notes_tree_list *ret = (struct notes_tree_list *)
+		xcalloc(sizeof(struct notes_tree_list), 1);
+	init_notes(&ret->tree, branch_name, combine_notes_overwrite,
+		NOTES_INIT_EMPTY);
+	ret->next = notes_trees;
+	notes_trees = ret;
+	return &ret->tree;
+}
+
+static struct notes_tree *notes_tree_get(struct branch *b)
+{
+	struct notes_tree_list *cur = notes_trees;
+	while (cur && strcmp(cur->tree.ref, b->name))
+		cur = cur->next;
+	return cur ? &cur->tree : NULL;
+}
+
+static void add_to_replace_list(
+		struct tree_entry_replace **replace_list,
+		const char *old_path, const char *new_path)
+{
+	struct tree_entry_replace *r = (struct tree_entry_replace *)
+		xmalloc(sizeof(struct tree_entry_replace));
+	r->next = (*replace_list)->next;
+	r->old_path = xstrdup(old_path);
+	r->new_path = xstrdup(new_path);
+	(*replace_list)->next = r;
+	*replace_list = r;
+}
+
+static void process_replace_list(struct tree_entry_replace *r,
+		struct tree_entry *root)
+{
+	struct tree_entry_replace *tmp;
+	struct tree_entry e;
+	while (r) {
+		if (r->old_path && r->new_path) {
+			/* rename old_path to new_path */
+			memset(&e, 0, sizeof(e));
+			tree_content_remove(root, r->old_path, &e);
+			if (!e.versions[1].mode)
+				die("Path %s not in branch", r->old_path);
+			tree_content_set(root, r->new_path, e.versions[1].sha1,
+					 e.versions[1].mode, e.tree);
+		}
+		tmp = r;
+		r = r->next;
+		free(tmp);
+	}
+}
+
+/*
+ * Find tree entries below given root that "match" the given SHA1 sum, and
+ * invoke the given callback for each matching entry.
+ */
+static int do_for_each_tree_entry_matching_sha1(
+		struct tree_entry *root,
+		const char *hex_sha1,
+		unsigned int match_len,
+		char *path,
+		unsigned int path_len,
+		int cb_fn(struct tree_entry *e, const char *path,
+			  unsigned int path_len, void *cb_data),
+		void *cb_data)
+{
+	struct tree_content *t = root->tree;
+	struct tree_entry *e;
+	unsigned int i;
+	int result;
+
+	for (i = 0; i < t->entry_count; i++) {
+		e = t->entries[i];
+		if (match_len + e->name->str_len > 40 ||
+		    memcmp(hex_sha1 + match_len, e->name->str_dat,
+			e->name->str_len))
+			continue;
+
+		/* partial or full match = append to path */
+		if (path_len)
+			path[path_len++] = '/';
+		memcpy(path + path_len, e->name->str_dat,
+			e->name->str_len);
+		path_len += e->name->str_len;
+		assert(path_len < 80);
+		match_len += e->name->str_len;
+		assert(match_len <= 40);
+
+		if (match_len == 40) {
+			/* full match */
+			assert(path_len < 80);
+			path[path_len] = '\0';
+			if ((result = cb_fn(e, path, path_len, cb_data)))
+				return result;
+		}
+		else {
+			/* partial match */
+			assert(match_len < 40);
+			if (S_ISDIR(e->versions[1].mode)) {
+				if (!e->tree)
+					load_tree(e);
+				result = do_for_each_tree_entry_matching_sha1(
+					e, hex_sha1, match_len, path, path_len,
+					cb_fn, cb_data);
+				if (result)
+					return result;
+			}
+		}
+
+		path_len -= e->name->str_len;
+		if (path_len)
+			path_len--;
+		match_len -= e->name->str_len;
+	}
+	return 0;
+}
+
+static int for_each_tree_entry_matching_sha1(
+		struct tree_entry *root,
+		const unsigned char *sha1,
+		int cb_fn(struct tree_entry *e, const char *path,
+			  unsigned int path_len, void *cb_data),
+		void *cb_data)
+{
+	char hex_sha1[40], path[80];
+	memcpy(hex_sha1, sha1_to_hex(sha1), 40);
+	return do_for_each_tree_entry_matching_sha1(root, hex_sha1, 0, path, 0,
+					       cb_fn, cb_data);
+}
+
+static int reconcile_note_and_tree_entry(struct tree_entry *e,
+		const char *path, unsigned int path_len, void *cb_data)
+{
+	struct reconcile_note_and_tree_entry_data *d =
+		(struct reconcile_note_and_tree_entry_data *) cb_data;
+	int same_path = !strcmp(path, d->note_tree_path);
+	int same_sha1 = !hashcmp(e->versions[1].sha1, d->note_sha1);
+	if (same_sha1 && !same_path)
+		add_to_replace_list(d->replace_list, path, d->note_tree_path);
+	/* TODO: handle the other combinations of same_path and same_sha1 */
+	return 0;
+}
+
+static int reconcile_notes_tree_helper(
+		const unsigned char *object_sha1,
+		const unsigned char *note_sha1,
+		const char *note_tree_path,
+		void *cb_data)
+{
+	/*
+	 * Coordinate reconciliation by traversing the tree and invoking the
+	 * reconcile_note_and_tree_entry() helper for each tree_entry whose
+	 * path matches the object_sha1 of the current note.
+	 */
+	struct reconcile_notes_tree_helper_data *d =
+		(struct reconcile_notes_tree_helper_data *) cb_data;
+	struct reconcile_note_and_tree_entry_data r =
+		{ object_sha1, note_sha1, note_tree_path, d->replace_list };
+	return for_each_tree_entry_matching_sha1(d->root, object_sha1,
+		reconcile_note_and_tree_entry, &r);
+}
+
+static void do_notes_tree_load(struct tree_entry *root,
+		const char *sha1_prefix, unsigned int sha1_prefix_len,
+		struct notes_tree *notes)
+{
+	struct tree_content *t = root->tree;
+	unsigned int i;
+	char hex_sha1[40];
+
+	if (sha1_prefix_len)
+		memcpy(hex_sha1, sha1_prefix, sha1_prefix_len);
+	for (i = 0; i < t->entry_count; i++) {
+		unsigned char sha1[20];
+		struct tree_entry *e = t->entries[i];
+		if (e->name->str_len + sha1_prefix_len > 40)
+			continue;
+		memcpy(hex_sha1 + sha1_prefix_len, e->name->str_dat,
+			e->name->str_len);
+		if (sha1_prefix_len + e->name->str_len == 40 &&
+		    !get_sha1_hex(hex_sha1, sha1))
+			add_note(notes, sha1, e->versions[1].sha1, NULL);
+		if (sha1_prefix_len + e->name->str_len >= 40 ||
+		    !S_ISDIR(e->versions[1].mode))
+			continue;
+		if (!e->tree)
+			load_tree(e);
+		do_notes_tree_load(e, hex_sha1,
+			sha1_prefix_len + e->name->str_len, notes);
+	}
+}
+
+static void notes_tree_load(struct notes_tree *notes, struct tree_entry *root)
+{
+	do_notes_tree_load(root, NULL, 0, notes);
+}
+
+static void notes_tree_unload(const char *branch_name)
+{
+	struct notes_tree_list *cur = notes_trees, *prev = NULL;
+	while (cur && strcmp(cur->tree.ref, branch_name)) {
+		prev = cur;
+		cur = cur->next;
+	}
+	if (!cur)
+		return;
+	if (prev)
+		prev->next = cur->next;
+	else
+		notes_trees = cur->next;
+	free_notes(&cur->tree);
+	free(cur);
+}
+
 static void unload_one_branch(void)
 {
 	while (cur_active_branches
@@ -1844,6 +2092,8 @@ static void unload_one_branch(void)
 			release_tree_content_recursive(e->branch_tree.tree);
 			e->branch_tree.tree = NULL;
 		}
+		if (e->has_notes)
+			notes_tree_unload(e->name);
 		cur_active_branches--;
 	}
 }
@@ -2010,7 +2260,7 @@ static void file_change_cr(struct branch *b, int rename)
 		leaf.tree);
 }

-static void note_change_n(struct branch *b)
+static void note_change_n(struct branch *b, struct notes_tree *notes)
 {
 	const char *p = command_buf.buf + 2;
 	static struct strbuf uq = STRBUF_INIT;
@@ -2080,8 +2330,16 @@ static void note_change_n(struct branch *b)
 			    typename(type), command_buf.buf);
 	}

+	assert(notes);
+	add_note(notes, commit_sha1, sha1, NULL);
+
+	/*
+	 * Also add a corresponding entry to the branch_tree. This entry is
+	 * a placeholder for the final note entry in the tree, and will be
+	 * renamed into the correct location before committing the tree.
+	 */
 	tree_content_set(&b->branch_tree, sha1_to_hex(commit_sha1), sha1,
-		S_IFREG | 0644, NULL);
+			 S_IFREG | 0644, NULL);
 }

 static void file_change_deleteall(struct branch *b)
@@ -2213,6 +2471,7 @@ static void parse_new_commit(void)
 	char *committer = NULL;
 	struct hash_list *merge_list = NULL;
 	unsigned int merge_count;
+	struct notes_tree *notes = NULL;

 	/* Obtain the branch name from the rest of our command */
 	sp = strchr(command_buf.buf, ' ') + 1;
@@ -2243,6 +2502,10 @@ static void parse_new_commit(void)
 		load_branch(b);
 	}

+	/* retrieve notes tree */
+	if (b->has_notes)
+		notes = notes_tree_get(b);
+
 	/* file_change* */
 	while (command_buf.len > 0) {
 		if (!prefixcmp(command_buf.buf, "M "))
@@ -2253,10 +2516,22 @@ static void parse_new_commit(void)
 			file_change_cr(b, 1);
 		else if (!prefixcmp(command_buf.buf, "C "))
 			file_change_cr(b, 0);
-		else if (!prefixcmp(command_buf.buf, "N "))
-			note_change_n(b);
-		else if (!strcmp("deleteall", command_buf.buf))
+		else if (!prefixcmp(command_buf.buf, "N ")) {
+			if (!notes) {
+				notes = notes_tree_create(b->name);
+				notes_tree_load(notes, &b->branch_tree);
+				b->has_notes = 1;
+			}
+			note_change_n(b, notes);
+		}
+		else if (!strcmp("deleteall", command_buf.buf)) {
+			if (notes) {
+				notes_tree_unload(b->name);
+				b->has_notes = 0;
+				notes = NULL;
+			}
 			file_change_deleteall(b);
+		}
 		else {
 			unread_command_buf = 1;
 			break;
@@ -2265,6 +2540,18 @@ static void parse_new_commit(void)
 			break;
 	}

+	if (notes) {
+		/* reconcile diffs between b->branch_tree and the notes tree */
+		struct reconcile_notes_tree_helper_data d;
+		struct tree_entry_replace *replace_list =
+			xcalloc(1, sizeof(struct tree_entry_replace));
+		struct tree_entry_replace *anchor = replace_list;
+		d.root = &b->branch_tree;
+		d.replace_list = &replace_list;
+		for_each_note(notes, reconcile_notes_tree_helper, &d);
+		process_replace_list(anchor, &b->branch_tree);
+	}
+
 	/* build the tree and the commit */
 	store_tree(&b->branch_tree);
 	hashcpy(b->branch_tree.versions[0].sha1,
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index b49815d..bf8c509 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1092,9 +1092,12 @@ test_expect_success 'P: fail on blob mark in gitlink' '
 ### series Q (notes)
 ###

-note1_data="Note for the first commit"
-note2_data="Note for the second commit"
-note3_data="Note for the third commit"
+note1_data="The first note for the first commit"
+note2_data="The first note for the second commit"
+note3_data="The first note for the third commit"
+note1b_data="The second note for the first commit"
+note1c_data="The third note for the first commit"
+note2b_data="The second note for the second commit"

 test_tick
 cat >input <<INPUT_END
@@ -1169,7 +1172,45 @@ data <<EOF
 $note3_data
 EOF

+commit refs/notes/foobar
+mark :10
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+notes (:10)
+COMMIT
+
+N inline :3
+data <<EOF
+$note1b_data
+EOF
+
+commit refs/notes/foobar2
+mark :11
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+notes (:11)
+COMMIT
+
+N inline :3
+data <<EOF
+$note1c_data
+EOF
+
+commit refs/notes/foobar
+mark :12
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+notes (:12)
+COMMIT
+
+deleteall
+N inline :5
+data <<EOF
+$note2b_data
+EOF
+
 INPUT_END
+
 test_expect_success \
 	'Q: commit notes' \
 	'git fast-import <input &&
@@ -1224,8 +1265,8 @@ committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
 notes (:9)
 EOF
 test_expect_success \
-	'Q: verify notes commit' \
-	'git cat-file commit refs/notes/foobar | sed 1d >actual &&
+	'Q: verify first notes commit' \
+	'git cat-file commit refs/notes/foobar~2 | sed 1d >actual &&
 	test_cmp expect actual'

 cat >expect.unsorted <<EOF
@@ -1235,23 +1276,112 @@ cat >expect.unsorted <<EOF
 EOF
 cat expect.unsorted | sort >expect
 test_expect_success \
-	'Q: verify notes tree' \
-	'git cat-file -p refs/notes/foobar^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
+	'Q: verify first notes tree' \
+	'git cat-file -p refs/notes/foobar~2^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
 	 test_cmp expect actual'

 echo "$note1_data" >expect
 test_expect_success \
-	'Q: verify note for first commit' \
-	'git cat-file blob refs/notes/foobar:$commit1 >actual && test_cmp expect actual'
+	'Q: verify first note for first commit' \
+	'git cat-file blob refs/notes/foobar~2:$commit1 >actual && test_cmp expect actual'

 echo "$note2_data" >expect
 test_expect_success \
-	'Q: verify note for second commit' \
-	'git cat-file blob refs/notes/foobar:$commit2 >actual && test_cmp expect actual'
+	'Q: verify first note for second commit' \
+	'git cat-file blob refs/notes/foobar~2:$commit2 >actual && test_cmp expect actual'
+
+echo "$note3_data" >expect
+test_expect_success \
+	'Q: verify first note for third commit' \
+	'git cat-file blob refs/notes/foobar~2:$commit3 >actual && test_cmp expect actual'
+
+cat >expect <<EOF
+parent `git rev-parse --verify refs/notes/foobar~2`
+author $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+
+notes (:10)
+EOF
+test_expect_success \
+	'Q: verify second notes commit' \
+	'git cat-file commit refs/notes/foobar^ | sed 1d >actual &&
+	test_cmp expect actual'
+
+cat >expect.unsorted <<EOF
+100644 blob $commit1
+100644 blob $commit2
+100644 blob $commit3
+EOF
+cat expect.unsorted | sort >expect
+test_expect_success \
+	'Q: verify second notes tree' \
+	'git cat-file -p refs/notes/foobar^^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
+	 test_cmp expect actual'
+
+echo "$note1b_data" >expect
+test_expect_success \
+	'Q: verify second note for first commit' \
+	'git cat-file blob refs/notes/foobar^:$commit1 >actual && test_cmp expect actual'
+
+echo "$note2_data" >expect
+test_expect_success \
+	'Q: verify first note for second commit' \
+	'git cat-file blob refs/notes/foobar^:$commit2 >actual && test_cmp expect actual'

 echo "$note3_data" >expect
 test_expect_success \
-	'Q: verify note for third commit' \
-	'git cat-file blob refs/notes/foobar:$commit3 >actual && test_cmp expect actual'
+	'Q: verify first note for third commit' \
+	'git cat-file blob refs/notes/foobar^:$commit3 >actual && test_cmp expect actual'
+
+cat >expect <<EOF
+author $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+
+notes (:11)
+EOF
+test_expect_success \
+	'Q: verify third notes commit' \
+	'git cat-file commit refs/notes/foobar2 | sed 1d >actual &&
+	test_cmp expect actual'
+
+cat >expect.unsorted <<EOF
+100644 blob $commit1
+EOF
+cat expect.unsorted | sort >expect
+test_expect_success \
+	'Q: verify third notes tree' \
+	'git cat-file -p refs/notes/foobar2^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
+	 test_cmp expect actual'
+
+echo "$note1c_data" >expect
+test_expect_success \
+	'Q: verify third note for first commit' \
+	'git cat-file blob refs/notes/foobar2:$commit1 >actual && test_cmp expect actual'
+
+cat >expect <<EOF
+parent `git rev-parse --verify refs/notes/foobar^`
+author $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+
+notes (:12)
+EOF
+test_expect_success \
+	'Q: verify fourth notes commit' \
+	'git cat-file commit refs/notes/foobar | sed 1d >actual &&
+	test_cmp expect actual'
+
+cat >expect.unsorted <<EOF
+100644 blob $commit2
+EOF
+cat expect.unsorted | sort >expect
+test_expect_success \
+	'Q: verify fourth notes tree' \
+	'git cat-file -p refs/notes/foobar^{tree} | sed "s/ [0-9a-f]*	/ /" >actual &&
+	 test_cmp expect actual'
+
+echo "$note2b_data" >expect
+test_expect_success \
+	'Q: verify second note for second commit' \
+	'git cat-file blob refs/notes/foobar:$commit2 >actual && test_cmp expect actual'

 test_done
--
1.6.4.304.g1365c.dirty

^ permalink raw reply related

* [RFC/PATCHv8 10/10] Add more testcases to test fast-import of notes
From: Johan Herland @ 2009-11-20  1:39 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1258681154-2167-1-git-send-email-johan@herland.net>

This patch adds testcases verifying correct behaviour in several scenarios
regarding fast-import of notes:
- using a mixture of 'N' and 'M' commands
- updating existing notes
- concatenation of notes
- 'deleteall' also removes notes
- fanout schemes is added/removed when needed
- git-fast-import's branch unload/reload preserves notes

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

diff --git a/t/t9301-fast-import-notes.sh b/t/t9301-fast-import-notes.sh
new file mode 100755
index 0000000..5a08a76
--- /dev/null
+++ b/t/t9301-fast-import-notes.sh
@@ -0,0 +1,578 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Johan Herland
+#
+
+test_description='test git fast-import of notes objects'
+. ./test-lib.sh
+
+
+test_tick
+cat >input <<INPUT_END
+commit refs/heads/master
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+first commit
+COMMIT
+
+M 644 inline foo
+data <<EOF
+file foo in first commit
+EOF
+
+M 755 inline bar
+data <<EOF
+file bar in first commit
+EOF
+
+M 644 inline baz/xyzzy
+data <<EOF
+file baz/xyzzy in first commit
+EOF
+
+commit refs/heads/master
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+second commit
+COMMIT
+
+M 644 inline foo
+data <<EOF
+file foo in second commit
+EOF
+
+M 755 inline baz/xyzzy
+data <<EOF
+file baz/xyzzy in second commit
+EOF
+
+commit refs/heads/master
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+third commit
+COMMIT
+
+M 644 inline foo
+data <<EOF
+file foo in third commit
+EOF
+
+commit refs/heads/master
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+fourth commit
+COMMIT
+
+M 755 inline bar
+data <<EOF
+file bar in fourth commit
+EOF
+
+INPUT_END
+
+test_expect_success 'set up master branch' '
+
+	git fast-import <input &&
+	git whatchanged master
+'
+
+commit4=$(git rev-parse refs/heads/master)
+commit3=$(git rev-parse "$commit4^")
+commit2=$(git rev-parse "$commit4~2")
+commit1=$(git rev-parse "$commit4~3")
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/test
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+first notes commit
+COMMIT
+
+M 644 inline $commit1
+data <<EOF
+first note for first commit
+EOF
+
+M 755 inline $commit2
+data <<EOF
+first note for second commit
+EOF
+
+INPUT_END
+
+cat >expect <<EXPECT_END
+    fourth commit
+    third commit
+    second commit
+    first note for second commit
+    first commit
+    first note for first commit
+EXPECT_END
+
+test_expect_success 'add notes with simple M command' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/test git log | grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/test
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+second notes commit
+COMMIT
+
+from refs/notes/test^0
+N inline $commit3
+data <<EOF
+first note for third commit
+EOF
+
+N inline $commit4
+data <<EOF
+first note for fourth commit
+EOF
+
+INPUT_END
+
+cat >expect <<EXPECT_END
+    fourth commit
+    first note for fourth commit
+    third commit
+    first note for third commit
+    second commit
+    first note for second commit
+    first commit
+    first note for first commit
+EXPECT_END
+
+test_expect_success 'add notes with simple N command' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/test git log | grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/test
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+third notes commit
+COMMIT
+
+from refs/notes/test^0
+N inline $commit1
+data <<EOF
+second note for first commit
+EOF
+
+N inline $commit2
+data <<EOF
+second note for second commit
+EOF
+
+N inline $commit3
+data <<EOF
+second note for third commit
+EOF
+
+N inline $commit4
+data <<EOF
+second note for fourth commit
+EOF
+
+INPUT_END
+
+cat >expect <<EXPECT_END
+    fourth commit
+    second note for fourth commit
+    third commit
+    second note for third commit
+    second commit
+    second note for second commit
+    first commit
+    second note for first commit
+EXPECT_END
+
+test_expect_success 'update existing notes with N command' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/test git log | grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/test
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+fourth notes commit
+COMMIT
+
+from refs/notes/test^0
+M 644 inline $(echo "$commit3" | sed "s|^..|&/|")
+data <<EOF
+prefix of note for third commit
+EOF
+
+M 644 inline $(echo "$commit4" | sed "s|^..|&/|")
+data <<EOF
+prefix of note for fourth commit
+EOF
+
+M 644 inline $(echo "$commit4" | sed "s|^\(..\)\(..\)|\1/\2/|")
+data <<EOF
+pre-prefix of note for fourth commit
+EOF
+
+N inline $commit1
+data <<EOF
+third note for first commit
+EOF
+
+N inline $commit2
+data <<EOF
+third note for second commit
+EOF
+
+N inline $commit3
+data <<EOF
+third note for third commit
+EOF
+
+N inline $commit4
+data <<EOF
+third note for fourth commit
+EOF
+
+
+INPUT_END
+
+cat >expect <<EXPECT_END
+    fourth commit
+    pre-prefix of note for fourth commit
+    prefix of note for fourth commit
+    third note for fourth commit
+    third commit
+    prefix of note for third commit
+    third note for third commit
+    second commit
+    third note for second commit
+    first commit
+    third note for first commit
+EXPECT_END
+
+test_expect_success 'add concatentation notes with M command' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/test git log | grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/test
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+fifth notes commit
+COMMIT
+
+from refs/notes/test^0
+deleteall
+
+INPUT_END
+
+cat >expect <<EXPECT_END
+    fourth commit
+    third commit
+    second commit
+    first commit
+EXPECT_END
+
+test_expect_success 'verify that deleteall also removes notes' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/test git log | grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/test
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+sixth notes commit
+COMMIT
+
+from refs/notes/test^0
+M 644 inline $commit1
+data <<EOF
+third note for first commit
+EOF
+
+M 644 inline $commit3
+data <<EOF
+third note for third commit
+EOF
+
+N inline $commit1
+data <<EOF
+fourth note for first commit
+EOF
+
+N inline $commit3
+data <<EOF
+fourth note for third commit
+EOF
+
+INPUT_END
+
+cat >expect <<EXPECT_END
+    fourth commit
+    third commit
+    fourth note for third commit
+    second commit
+    first commit
+    fourth note for first commit
+EXPECT_END
+
+test_expect_success 'verify that N commands override M commands' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/test git log | grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+# Write fast-import commands to create the given number of commits
+fast_import_commits () {
+	my_ref=$1
+	my_num_commits=$2
+	my_append_to_file=$3
+	my_i=0
+	while test $my_i -lt $my_num_commits
+	do
+		my_i=$(($my_i + 1))
+		test_tick
+		cat >>"$my_append_to_file" <<INPUT_END
+commit $my_ref
+mark :$my_i
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+commit #$my_i
+COMMIT
+
+M 644 inline file
+data <<EOF
+file contents in commit #$my_i
+EOF
+
+INPUT_END
+	done
+}
+
+# Write fast-import commands to create the given number of notes annotating
+# the commits created by fast_import_commits()
+fast_import_notes () {
+	my_notes_ref=$1
+	my_num_commits=$2
+	my_append_to_file=$3
+	my_note_append=$4
+	test_tick
+	cat >>"$my_append_to_file" <<INPUT_END
+commit $my_notes_ref
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+committing $my_num_commits notes
+COMMIT
+
+INPUT_END
+
+	my_i=0
+	while test $my_i -lt $my_num_commits
+	do
+		my_i=$(($my_i + 1))
+		cat >>"$my_append_to_file" <<INPUT_END
+N inline :$my_i
+data <<EOF
+note for commit #$my_i$my_note_append
+EOF
+
+INPUT_END
+	done
+}
+
+
+rm input expect
+num_commits=400
+# Create lots of commits
+fast_import_commits "refs/heads/many_commits" $num_commits input
+# Create one note per above commit
+fast_import_notes "refs/notes/many_notes" $num_commits input
+# Finally create the expected output from all these notes and commits
+i=$num_commits
+while test $i -gt 0
+do
+	cat >>expect <<EXPECT_END
+    commit #$i
+    note for commit #$i
+EXPECT_END
+	i=$(($i - 1))
+done
+
+test_expect_success 'add lots of commits and notes' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/many_notes git log refs/heads/many_commits |
+	    grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_expect_success 'verify that lots of notes trigger a fanout scheme' '
+
+	# None of the entries in the top-level notes tree should be a full SHA1
+	git cat-file -p refs/notes/many_notes: |
+	while read mode type sha1 path
+	do
+		path_len=$(expr length "$path") &&
+		if test $path_len -ge 40
+		then
+			return 1
+		fi
+	done
+
+'
+
+commit1=$(git rev-parse "refs/heads/many_commits")
+commit2=$(git rev-parse "$commit1^")
+commit3=$(git rev-parse "$commit2^")
+commit4=$(git rev-parse "$commit3^")
+
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/many_notes
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+replacing many notes with few notes
+COMMIT
+
+from refs/notes/many_notes^0
+deleteall
+N inline $commit1
+data <<EOF
+note for commit #$num_commits
+EOF
+
+N inline $commit2
+data <<EOF
+note for commit #$(($num_commits - 1))
+EOF
+
+N inline $commit3
+data <<EOF
+note for commit #$(($num_commits - 2))
+EOF
+
+N inline $commit4
+data <<EOF
+note for commit #$(($num_commits - 3))
+EOF
+
+INPUT_END
+
+i=$num_commits
+rm expect
+while test $i -gt 0
+do
+	cat >>expect <<EXPECT_END
+    commit #$i
+EXPECT_END
+	if test $i -gt $(($num_commits - 4))
+	then
+		cat >>expect <<EXPECT_END
+    note for commit #$i
+EXPECT_END
+	fi
+	i=$(($i - 1))
+done
+
+test_expect_success 'remove lots of notes' '
+
+	git fast-import <input &&
+	GIT_NOTES_REF=refs/notes/many_notes git log refs/heads/many_commits |
+	    grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_expect_success 'verify that removing notes trigger fanout consolidation' '
+
+	# All entries in the top-level notes tree should be a full SHA1
+	git cat-file -p refs/notes/many_notes: |
+	while read mode type sha1 path
+	do
+		path_len=$(expr length "$path") &&
+		if test $path_len -ne 40
+		then
+			return 1
+		fi
+	done
+
+'
+
+rm input expect
+num_notes_refs=10
+num_commits=16
+some_commits=8
+# Create commits
+fast_import_commits "refs/heads/more_commits" $num_commits input
+# Create one note per above commit per notes ref
+i=0
+while test $i -lt $num_notes_refs
+do
+	i=$(($i + 1))
+	fast_import_notes "refs/notes/more_notes_$i" $num_commits input
+done
+# Trigger branch reloading in git-fast-import by repeating the note creation
+i=0
+while test $i -lt $num_notes_refs
+do
+	i=$(($i + 1))
+	fast_import_notes "refs/notes/more_notes_$i" $some_commits input " (2)"
+done
+# Finally create the expected output from the notes in refs/notes/more_notes_1
+i=$num_commits
+while test $i -gt 0
+do
+	note_data="note for commit #$i"
+	if test $i -le $some_commits
+	then
+		note_data="$note_data (2)"
+	fi
+	cat >>expect <<EXPECT_END
+    commit #$i
+    $note_data
+EXPECT_END
+	i=$(($i - 1))
+done
+
+test_expect_success "add notes to $num_commits commits in each of $num_notes_refs refs" '
+
+	git fast-import --active-branches=5 <input &&
+	GIT_NOTES_REF=refs/notes/more_notes_1 git log refs/heads/more_commits |
+	    grep "^    " > actual &&
+	test_cmp expect actual
+
+'
+
+test_done
-- 
1.6.4.304.g1365c.dirty

^ permalink raw reply related

* Re: [RFC/PATCHv7 22/22] fast-import: Proper notes tree manipulation using the notes API
From: Johan Herland @ 2009-11-20  1:43 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: git, gitster, Johannes.Schindelin, trast, tavestbo, git,
	chriscool, sam
In-Reply-To: <20091009142524.GQ9261@spearce.org>

On Friday 09 October 2009, Shawn O. Pearce wrote:
> Johan Herland <johan@herland.net> wrote:
> > This patch teaches 'git fast-import' to use the notes API to organize
> > the manipulation of note objects through a fast-import stream. Note
> > objects are added to the notes tree through the 'N' command, and when
> > we're about to store the tree object for the current commit, we walk
> > through the notes tree and insert all the notes into the stored tree.
> 
> Some high level comments about this patch:
> 
> - You don't destroy the struct notes_tree during unload_one_branch()
>   which means notes trees stay in memory even if the branch table
>   is overflowing.  I think you should discard the notes tree when
>   a branch unloads, and recreate it when the branch loads.
> 
> - Destroying and adding back all notes is OK with ~20k notes, but
>   doing that with ~150k-~800k notes is going to slow down a lot,
>   losing the "fast" part.

Thanks for the comments. I've tried to address them in the 8th iteration of 
this series (Patch 8/10 to be more precise), just submitted to the mailing 
list.


...Johan

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

^ permalink raw reply

* Re: Hey - A Conceptual Simplication....
From: Dmitry Potapov @ 2009-11-20  1:48 UTC (permalink / raw)
  To: George Dennie
  Cc: 'Jason Sewall', 'Jakub Narebski',
	'Jan Krüger', git
In-Reply-To: <009401ca68bc$7e4b12b0$7ae13810$@com>

On Wed, Nov 18, 2009 at 09:03:31PM -0500, George Dennie wrote:
> 
> For example, the functional notion of the repository seems well
> defined: a growing web of immutable commits each created as either an
> isolated commit or more typically an update and/or merger of one or
> more pre-existing commits. 

In Git, commits are not immutable. One thing that many Git users do
is git-rebase, which in essense is re-writing or re-ordering exising
commits. So, you can change history in Git, but you should never change
the published history. (Of course, that leads to the question what is
considered as published history. For instance, commits merged on the
proposed-updates branch are usually not considered to be "published",
so they can be re-written or discarded later).

So, the correct way to use Git is to find the right balance between
the need to clean up after mistakes (using git-rebase) and not doing
too much, so you will not lose important history or create problems
for other peoples.

> 
> The notion of a shapeless commit is curious. Intuitively, I consider a
> commit as capturing the state of my work at a transactional boundary
> (i.e. a successful unit test...or even lunch break).

No, it is not what Git commits were intended for. In Git, a commit is
a change intended to achieve some goal. Basically, you send a patch
to maintainer, and you should explain what this patch does and why it
is useful... If your explanation is "I have a lunch break now", it is
very bad explanation, thus a bad patch.


Dmitry

^ permalink raw reply

* Re: Hey - A Conceptual Simplication....
From: david @ 2009-11-20  1:55 UTC (permalink / raw)
  To: Dmitry Potapov
  Cc: George Dennie, 'Jason Sewall', 'Jakub Narebski',
	'Jan Krüger', git
In-Reply-To: <20091120014843.GB22556@dpotapov.dyndns.org>

On Fri, 20 Nov 2009, Dmitry Potapov wrote:

> On Wed, Nov 18, 2009 at 09:03:31PM -0500, George Dennie wrote:
>>
>> For example, the functional notion of the repository seems well
>> defined: a growing web of immutable commits each created as either an
>> isolated commit or more typically an update and/or merger of one or
>> more pre-existing commits.
>
> In Git, commits are not immutable. One thing that many Git users do
> is git-rebase, which in essense is re-writing or re-ordering exising
> commits. So, you can change history in Git, but you should never change
> the published history. (Of course, that leads to the question what is
> considered as published history. For instance, commits merged on the
> proposed-updates branch are usually not considered to be "published",
> so they can be re-written or discarded later).
>
> So, the correct way to use Git is to find the right balance between
> the need to clean up after mistakes (using git-rebase) and not doing
> too much, so you will not lose important history or create problems
> for other peoples.

the typical advice is to clean up before you make changes public, but not 
afterwords.

David Lang

>>
>> The notion of a shapeless commit is curious. Intuitively, I consider a
>> commit as capturing the state of my work at a transactional boundary
>> (i.e. a successful unit test...or even lunch break).
>
> No, it is not what Git commits were intended for. In Git, a commit is
> a change intended to achieve some goal. Basically, you send a patch
> to maintainer, and you should explain what this patch does and why it
> is useful... If your explanation is "I have a lunch break now", it is
> very bad explanation, thus a bad patch.
>
>
> Dmitry
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: Hey - A Conceptual Simplication....
From: Dmitry Potapov @ 2009-11-20  2:31 UTC (permalink / raw)
  To: George Dennie
  Cc: git, B.Steinbrink, 'Jason Sewall',
	'Jakub Narebski', 'Jan Krüger', torvalds
In-Reply-To: <00d401ca6954$a29fa020$e7dee060$@com>

On Thu, Nov 19, 2009 at 03:12:35PM -0500, George Dennie wrote:
> 
> I think an important piece of conceptual information missing from the docs
> is a concise list of the conceptual properties defining the context of the
> working tree, index, and repository during normal use. This itemization
> would go far in explaining the synergies between the various commands. 

Speaking about "normal use"... I suggest you read about Git workflows:

$ git help gitworkflows

> 
> Functionally, all the commands merely manipulate these properties. If these
> properties were summarize in context one would expect that would represent a
> very complete functional model of Git. A user could review the description
> figure what they wanted to do and then find the command(s) to accomplish it.

It is like to say that driving a car merely means to manipulate its
components, so if these components were summarized, it would be all
that one needs to know to drive a car...

While I don't dispute that basic understanding of key Git concepts is
important, understanding of a typical Git workflow cannot be deduced
from knowledge of separate parts. Now if I were to describe Git just in
a few words, I would say that Git repository is just a DAG of objects,
the working tree is the place where you work, and the index is what
helps you to create fine-grained commits and do merges. But it says
very little (if anything) about how to use it.


Dmitry

^ permalink raw reply

* Re: Hey - A Conceptual Simplication....
From: Björn Steinbrink @ 2009-11-20  2:35 UTC (permalink / raw)
  To: Dmitry Potapov
  Cc: George Dennie, 'Jason Sewall', 'Jakub Narebski',
	'Jan Krüger', git
In-Reply-To: <20091120014843.GB22556@dpotapov.dyndns.org>

On 2009.11.20 04:48:44 +0300, Dmitry Potapov wrote:
> On Wed, Nov 18, 2009 at 09:03:31PM -0500, George Dennie wrote:
> > 
> > For example, the functional notion of the repository seems well
> > defined: a growing web of immutable commits each created as either an
> > isolated commit or more typically an update and/or merger of one or
> > more pre-existing commits. 
> 
> In Git, commits are not immutable.

Commit _are_ immutable. Like all git objects (blob, tree, commits, tag).
"Rewriting" history actually means creating a new history (adding
objects), and then changing a ref (most often a branch head) to
reference the new instead of the old history.

Björn

^ permalink raw reply

* Re: Hey - A Conceptual Simplication....
From: Dmitry Potapov @ 2009-11-20  2:56 UTC (permalink / raw)
  To: david
  Cc: George Dennie, 'Jason Sewall', 'Jakub Narebski',
	'Jan Krüger', git
In-Reply-To: <alpine.DEB.2.00.0911191754540.10307@asgard.lang.hm>

On Thu, Nov 19, 2009 at 05:55:21PM -0800, david@lang.hm wrote:
> On Fri, 20 Nov 2009, Dmitry Potapov wrote:
>
>> So, the correct way to use Git is to find the right balance between
>> the need to clean up after mistakes (using git-rebase) and not doing
>> too much, so you will not lose important history or create problems
>> for other peoples.
>
> the typical advice is to clean up before you make changes public, but not 
> afterwords.

True, except patches may get additional clean up or improvements based
on review feedback, or even get some small fix-ups while they live on
'pu'. But re-writing something that other people may base their work on
is clearly wrong. On the other hand, rebasing a large series of patches
even if it has never been published may be a wrong way to go, because
you replace well tested states with some others, which were not tested.
So if it is a long and complex series of patches, chances are high that
you can break something in it. So, it requires some judgement when to
use git-rebase and when git-merge.


Dmitry

^ permalink raw reply

* Re: Hey - A Conceptual Simplication....
From: Dmitry Potapov @ 2009-11-20  3:08 UTC (permalink / raw)
  To: Björn Steinbrink
  Cc: George Dennie, 'Jason Sewall', 'Jakub Narebski',
	'Jan Krüger', git
In-Reply-To: <20091120023540.GA17796@atjola.homenet>

On Fri, Nov 20, 2009 at 03:35:40AM +0100, Björn Steinbrink wrote:
> On 2009.11.20 04:48:44 +0300, Dmitry Potapov wrote:
> > On Wed, Nov 18, 2009 at 09:03:31PM -0500, George Dennie wrote:
> > > 
> > > For example, the functional notion of the repository seems well
> > > defined: a growing web of immutable commits each created as either an
> > > isolated commit or more typically an update and/or merger of one or
> > > more pre-existing commits. 
> > 
> > In Git, commits are not immutable.
> 
> Commit _are_ immutable. Like all git objects (blob, tree, commits, tag).
> "Rewriting" history actually means creating a new history (adding
> objects), and then changing a ref (most often a branch head) to
> reference the new instead of the old history.

I stand corrected. All objects in Git repository are actually immutable,
but because references can be changed (and tools like git-rebase change
it automatically), it _appears_ like editing existing commits, but in
fact old commits do not disappear immediately. Even if there is no other
branches or tags that refer to old commits, git-reflog stores references
to them for 30 days after that the garbage collector can remove them.


Dmitry

^ permalink raw reply

* Re: [PATCH 1/2] t/lib-http.sh: Restructure finding of default httpd  location
From: Jay Soffian @ 2009-11-20  3:14 UTC (permalink / raw)
  To: Tarmigan Casebolt; +Cc: git, peff, drizzd, gitster, spearce
In-Reply-To: <1258680123-28684-1-git-send-email-tarmigan+git@gmail.com>

On Thu, Nov 19, 2009 at 8:22 PM, Tarmigan Casebolt
<tarmigan+git@gmail.com> wrote:
> uname might not be the best way to determine the default location for
> httpd since different Linux distributions apparently put httpd in
> different places, so we test a couple different locations for httpd,
> and use the first one that we come across.  We do the same for the
> modules directory.

Perhaps testing the distribution and looking in the known location for
that distribution then? That said, going through a list of well known
locations should work too.

> +for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'
> +do
> +       test -x "$DEFAULT_HTTPD_PATH" && break
> +done

Unfortunately this leaves DEFAULT_HTTPD_PATH as the last item in the
list even if the test does not pass. You can add an empty item to the
end of the list if you want to do this way.

> +for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \
> +                                 '/usr/lib/apache2/modules' \
> +                                 '/usr/lib64/httpd/modules' \
> +                                 '/usr/lib/httpd/modules'
> +do
> +       test -d "$DEFAULT_HTTPD_MODULE_PATH" && break
> +done

Ditto.

j.

^ permalink raw reply

* Re: [PATCH 1/2] t/lib-http.sh: Restructure finding of default httpd  location
From: Tarmigan @ 2009-11-20  3:30 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, peff, drizzd, gitster, spearce
In-Reply-To: <76718490911191914n23d067b8teb17907de9ec83d5@mail.gmail.com>

On Thu, Nov 19, 2009 at 7:14 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> On Thu, Nov 19, 2009 at 8:22 PM, Tarmigan Casebolt
> <tarmigan+git@gmail.com> wrote:
>> uname might not be the best way to determine the default location for
>> httpd since different Linux distributions apparently put httpd in
>> different places, so we test a couple different locations for httpd,
>> and use the first one that we come across.  We do the same for the
>> modules directory.
>
> Perhaps testing the distribution and looking in the known location for
> that distribution then? That said, going through a list of well known
> locations should work too.

Is there a nice way to test the distribution?  Seems to me like doing
that might be more complicated and also more fragile.

>> +for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'
>> +do
>> +       test -x "$DEFAULT_HTTPD_PATH" && break
>> +done
>
> Unfortunately this leaves DEFAULT_HTTPD_PATH as the last item in the
> list even if the test does not pass. You can add an empty item to the
> end of the list if you want to do this way.

Yes.  I think this is how it was before though too, and it is caught
later in the script with the LIB_HTTPD_PATH setting and testing.

>> +for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \
>> +                                 '/usr/lib/apache2/modules' \
>> +                                 '/usr/lib64/httpd/modules' \
>> +                                 '/usr/lib/httpd/modules'
>> +do
>> +       test -d "$DEFAULT_HTTPD_MODULE_PATH" && break
>> +done
>
> Ditto.

Yes.  Again, this is still more thorough than before, but in this case
the script does not check later.  Perhaps the script should test this
value and test_done if it's not a directory?

Thanks,
Tarmigan

^ permalink raw reply

* Re: [PATCH 0/2] jn/gitweb-blame fixes
From: Stephen Boyd @ 2009-11-20  4:05 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <4B05EA37.7060704@gmail.com>

Stephen Boyd wrote:
> Jakub Narebski wrote:
>>
>> Thanks for working on this.  Also it is nice to have incremental blame
>> tested for another browser, beside Mozilla 1.17.2 and Konqueror 3.5.3
>
> For those following along, Opera-10.10 has been tested and works.

Ok. I tried using the version of incremental blame that's in next 
(e206d62 gitweb: Colorize 'blame_incremental' view during processing, 
2009-09-01) on IE8 with no success. The page loads and the file is shown 
with line numbers, but the progress is stuck at 0% (with the &nbsp; 
showing too).

I then tried with my two patches applied on top of e206d62 on IE8 and 
still no success. The page loads and the file is show with the line 
numbers but still stuck at 0%, but the &nbsp; is gone at least.

Do you have access to IE8 to confirm?

^ permalink raw reply

* Re: [spf:guess] Re: git-svn not fetching all revisions
From: Sam Vilain @ 2009-11-20  4:12 UTC (permalink / raw)
  To: Marcus Better; +Cc: git
In-Reply-To: <4B0534FF.5040001@better.se>

Marcus Better wrote:
> Ok, that works, but how can I now sync with the svn repository with git
> svn rebase/dcommit? I think the filter-branch rewriting confuses git-svn.
>

A quick hint: git-svn only relies on the last piece of metadata it can
see in 'git log'; so make sure that one looks right and you should be
fine...

Sam

^ permalink raw reply

* Re: [ANNOUNCE] codeBeamer MR - Easy ACL for Git
From: Sitaram Chamarty @ 2009-11-20  4:56 UTC (permalink / raw)
  To: Intland Software; +Cc: Petr Baudis, git
In-Reply-To: <4B054D0A.5030802@intland.com>

On Thu, Nov 19, 2009 at 7:20 PM, Intland Software <marketing@intland.com> wrote:
> Petr Baudis wrote:
>>
>> I think a lot of people wonder now, how does this compare to existing
>> solutions; from your announcement I thought it's something like
>> Gitosis/Gitolite, but in fact it seems more similar to Gitorious or
>> GitHub (if it was publicly available, of course); perhaps it would be
>
> All right, some quick comparisons with codeBeamer Managed Repository (MR).
>
> * MR against Gitosis

I think you meant "versus" :-)

> In terms of access control, MR has the concept of "role", and it makes our
> security model more fine grained. Permissions can be set by role. One user
> account can have multiple roles. Roles are project-dependent. When you add a
> group to a project, you can assign multiple roles to the group (which is
> equivalent with assigning those roles to each group member one by one).
> On the other hand, MR has a much broader scope than Gitosis. MR helps you to
> manage your repos, to track your tasks/bugs/issues, to follow commit
> activities, to browse repos in the web, can be extended using its APIs, etc.
> (And you don't have to install and maintain Git extensions for this.)

> * MR against Gitolite
> Pretty much the same applies here as well.

Conceptually, gitolite can do the roles stuff you mentioned,
if I understood it correctly.  Of course, gitolite's access
config is in plain text.

The web-based control, issue tracking, etc., are all on a
different plane from what gitosis/gitolite aim to be.  So
much so that I might even disagree with Pasky on the need to
mention these two products in your website.  Here's one
perspective (in round figures):

gitolite:   1600 lines of shell+perl, 1600 lines of doc
gitosis:    3300 lines of python
MR:         150 MB binary download

I don't honestly see any way to even *begin* to compare :-)

You should stick to gitorious, github, and -- here's a new
one for you -- indefero.

-- 
Sitaram

^ permalink raw reply

* Re: Hey - A Conceptual Simplication....
From: Junio C Hamano @ 2009-11-20  6:27 UTC (permalink / raw)
  To: Jakub Narebski
  Cc: George Dennie, git, B.Steinbrink, 'Jason Sewall',
	'Jan Krüger', torvalds
In-Reply-To: <200911200149.19528.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> If you didn't find sufficient description of underlying concepts behind
> git in "Git User's Manual" (distributed with Git), "Git Community Book"
> or "Pro Git", take a look at the following documents:
>
>  * "Git for Computer Scientists"
>  * "Git From Bottom's Up"
>  * "The Git Parable"
> ...
> It is documented, see referenced mentioned above.

I actually would want ourselves step back a bit and make sure that anybody
who is completely new to git won't get confused with the concepts after
s/he reads our "Git User's Manual" and nothing else.  Listing five or six
documents and "you'll find information somewhere among these" *might* be
the best thing we could do at this very second, but we should strive to do
better than that.

^ permalink raw reply

* Re: Hey - A Conceptual Simplication....
From: Junio C Hamano @ 2009-11-20  6:33 UTC (permalink / raw)
  To: Dmitry Potapov; +Cc: George Dennie, 'Jan Krüger', git
In-Reply-To: <20091120013545.GA22556@dpotapov.dyndns.org>

Dmitry Potapov <dpotapov@gmail.com> writes:

> It is more difficult to make this mistake with Git than many others
> VCSes, because Git shows the list of files that are changed but not
> committed as well as the list of untracked files when you try to commit
> something.

Not really in practice.  Too many people carry their existing practice of
using -m to write a useless single liner commit log message that they
acquired while using their previous SCM.  Arguably, useless log messages
are less of a problem on systems like CVS/SVN because they do not do
useful log summarization such as "log -- paths..." or "shortlog", so they
can be excused for learning the practice in the first place, though.

That incidentally is exactly why earlier we (mostly me and Linus)
recommended people not to teach "commit -m" to new people, but of course
nobody listened ;-).

^ permalink raw reply

* Re: [PATCH] git-count-objects: Fix a disk-space under-estimate on Cygwin
From: Junio C Hamano @ 2009-11-20  7:00 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: GIT Mailing-list
In-Reply-To: <4B059280.40902@ramsay1.demon.co.uk>

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---

Could you write a concise summary in the commit log message to explain why
it is a correct fix?  Your detailed analysis after three dash lines was an
amusing read, but people who will be reading "git log" output 6 months
down the road won't have an access to it.  Something like...

    Cygwin has st_blocks in struct stat, but at least on NTFS, the field
    counts in blocks of st_blksize bytes, not in 512-byte blocks.

The Makefile already explains what NO_ST_BLOCKS_IN_STRUCT_STAT is about:

    # Define NO_ST_BLOCKS_IN_STRUCT_STAT if your platform does not have st_blocks
    # field that counts the on-disk footprint in 512-byte blocks.

so the above explanation should be enough.

Note that this is not any standard compliance.  POSIX cops out like this
in the <sys/stat.h> description:

    The unit for the st_blocks member of the stat structure is not defined
    within POSIX.1-2008. In some implementations it is 512 bytes. It may
    differ on a file system basis. There is no correlation between values
    of the st_blocks and st_blksize, and the f_bsize (from
    <sys/statvfs.h>) structure members.

IOW, a system like Cygwin that does not use 512-byte is not in violation.

> diff --git a/Makefile b/Makefile
> index 5d5976f..5624563 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -783,6 +783,10 @@ ifeq ($(uname_O),Cygwin)
>  	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
>  	NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
>  	OLD_ICONV = UnfortunatelyYes
> +	# The st_blocks field is not in units of 512 bytes, as the code
> +	# expects, which leads to an under-estimate of the disk space used.
> +	# In order to use an alternate algorithm, we claim to lack st_blocks.
> +	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease

This comment is redundant, as the explanation of the Makefile variable
already said the same thing.

Also it is somewhat wrong to say "claim to lack".  The Makefile variable
merely means "do not use this field".  The reason may be that the platform
lacks it, or it may have it but it does not count in 512-byte blocks.  It
does not matter---either way the field is not usable.

^ permalink raw reply

* Re: [PATCH 2/2] diffcore-break: save cnt_data for other phases
From: Junio C Hamano @ 2009-11-20  7:32 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20091119152234.GA6877@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I don't know if it is worth refactoring though.

I would rather not touch it.

> Also, I don't see where we ever actually free the cnt_data. After we run
> the whole O(n^2) loop, we should be able to drop cnt_data entirely. I
> think in practice it doesn't matter all that much, since it isn't that
> big, and afterwards we just generate the patch and exit (unless we are
> doing diffcore-break, too, which will actually free all of the data).

Originally, each phase of the diffcore pipeline was designed to be
independent from other phases, and keeping things in core was done as an
optimization for smallish trees so that later phases in the diffcore
pipeline can reuse prepopulated data.  If this were year 2006, I would
have said that keeping it in core would be better because we could add new
phases after it later, even though there is nobody who uses cnt_data after
diffcore-rename in the diffcore pipeline right now,

But it seems that our performance is more than adequate without such
keeping-things-around for smallish trees anyway, and it is wiser to
optimize for heavier workload.  More importantly, nobody seems to be
planning new phases in the pipeline, so I agree it is a good idea to drop
cnt_data once the rename detection phase is done with it.

^ permalink raw reply

* Re: [ANNOUNCE] codeBeamer MR - Easy ACL for Git
From: Petr Baudis @ 2009-11-20  7:47 UTC (permalink / raw)
  To: Sitaram Chamarty; +Cc: Intland Software, git
In-Reply-To: <2e24e5b90911192056t706071ble163a53741017ef@mail.gmail.com>

  Hi!

On Fri, Nov 20, 2009 at 10:26:32AM +0530, Sitaram Chamarty wrote:
> On Thu, Nov 19, 2009 at 7:20 PM, Intland Software <marketing@intland.com> wrote:
> > * MR against Gitolite
> > Pretty much the same applies here as well.
> 
> Conceptually, gitolite can do the roles stuff you mentioned,
> if I understood it correctly.  Of course, gitolite's access
> config is in plain text.
> 
> The web-based control, issue tracking, etc., are all on a
> different plane from what gitosis/gitolite aim to be.  So
> much so that I might even disagree with Pasky on the need to
> mention these two products in your website.

I brought Gitosis/Gitolite up because I got the impression that MR was
marketed primarily as a Git ACL tool, the other things being sort of
mirror; maybe my impession was wrong, but I still think the comparison
in ACL capabilities is useful.

> You should stick to gitorious, github, and -- here's a new
> one for you -- indefero.

Hmm, I didn't even know about this one, thanks for the pointer. Looks
like this suddenly is a very popular area. High competition is good!

(BTW, if you don't care about wikis and issue tracking, but you do care
about simplicity and light-weightness, you should best stick to Girocco!
;-)

-- 
				Petr "Pasky" Baudis
A lot of people have my books on their bookshelves.
That's the problem, they need to read them. -- Don Knuth

^ permalink raw reply

* Re: [PATCH] git-count-objects: Fix a disk-space under-estimate on Cygwin
From: Junio C Hamano @ 2009-11-20  7:49 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list
In-Reply-To: <7vd43d8yva.fsf@alter.siamese.dyndns.org>

When estimating the on-disk footprint of a file, we either used st_blocks
that is counted in 512-byte blocks (traditional unix behaviour), or on
platforms that do not have such st_blocks field in struct stat, simply the
file size itself.  Building with NO_ST_BLOCKS_IN_STRUCT_STAT will choose
the latter implementaion.

POSIX.1 says in its <sys/stat.h> description on this issue:

    The unit for the st_blocks member of the stat structure is not
    defined within POSIX.1-2008. In some implementations it is 512
    bytes. It may differ on a file system basis. There is no
    correlation between values of the st_blocks and st_blksize,
    and the f_bsize (from <sys/statvfs.h>) structure members.

Even though the above explicitly states st_blksize does not have any
correlation, at least on one system (Cygwin on NTFS), the st_blocks field
seems to count in blocks of st_blksize bytes.  A new Makefile variable
ST_BLOCKS_COUNTS_IN_BLKSIZE chooses to use this for the on-disk footprint.

---

Not signed-off yet, but this might be a workable alternative.  Testing on
wider platforms (especially not on "Linux with ext?") might find this
alternative useful.  I dunno.

 Makefile          |    8 +++++++-
 git-compat-util.h |    4 +++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 79195c8..5f2c5de 100644
--- a/Makefile
+++ b/Makefile
@@ -148,6 +148,9 @@ all::
 # Define USE_STDEV below if you want git to care about the underlying device
 # change being considered an inode change from the update-index perspective.
 #
+# Define ST_BLOCKS_COUNTS_IN_ST_BLKSIZE if your platform has st_blocks
+# field that counts the on-disk footprint in st_blksize-byte blocks.
+#
 # Define NO_ST_BLOCKS_IN_STRUCT_STAT if your platform does not have st_blocks
 # field that counts the on-disk footprint in 512-byte blocks.
 #
@@ -776,7 +779,7 @@ ifeq ($(uname_O),Cygwin)
 	NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
 	NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
 	OLD_ICONV = UnfortunatelyYes
-	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
+	ST_BLOCKS_COUNTS_IN_ST_BLKSIZE = YesPlease
 	# There are conflicting reports about this.
 	# On some boxes NO_MMAP is needed, and not so elsewhere.
 	# Try commenting this out if you suspect MMAP is more efficient
@@ -1126,6 +1129,9 @@ endif
 ifdef NO_D_INO_IN_DIRENT
 	BASIC_CFLAGS += -DNO_D_INO_IN_DIRENT
 endif
+ifdef ST_BLOCKS_COUNTS_IN_ST_BLKSIZE
+	BASIC_CFLAGS += -DST_BLOCKS_COUNTS_IN_ST_BLKSIZE
+endif
 ifdef NO_ST_BLOCKS_IN_STRUCT_STAT
 	BASIC_CFLAGS += -DNO_ST_BLOCKS_IN_STRUCT_STAT
 endif
diff --git a/git-compat-util.h b/git-compat-util.h
index ef60803..bba9c9e 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -240,7 +240,9 @@ extern int git_munmap(void *start, size_t length);
 
 #endif /* NO_MMAP */
 
-#ifdef NO_ST_BLOCKS_IN_STRUCT_STAT
+#ifdef ST_BLOCKS_COUNTS_IN_ST_BLKSIZE
+#define on_disk_bytes(st) ((st).st_blocks * (st).st_blksize)
+#elif defined(NO_ST_BLOCKS_IN_STRUCT_STAT) && NO_ST_BLOCKS_IN_STRUCT_STAT
 #define on_disk_bytes(st) ((st).st_size)
 #else
 #define on_disk_bytes(st) ((st).st_blocks * 512)

^ permalink raw reply related

* Re: [PATCH] gitk: Use git-difftool for external diffs
From: Junio C Hamano @ 2009-11-20  7:53 UTC (permalink / raw)
  To: Paul Mackerras; +Cc: David Aguilar, peff, sam, git
In-Reply-To: <19205.50406.91209.309984@cargo.ozlabs.ibm.com>

Paul Mackerras <paulus@samba.org> writes:

> I have no problem with using git difftool if the underlying git is new
> enough, I just don't want gitk to explode when it isn't.

I somehow doubt there are many users who use pre 1.6.3 git but keep their
gitk separately updated to very recent version, so personally I wouldn't
worry too much about this.

> Also, I don't think we should remove the ability for the user to
> choose which external diff tool to use.

This is a larger concern.  Does "difftool" allow use of an arbitrary tool
like how gitk does (I have a suspicion that it is not as flexible)?

^ permalink raw reply

* Re: [PATCH 2/2] t/lib-http.sh: Enable httpd tests by default.
From: Junio C Hamano @ 2009-11-20  8:03 UTC (permalink / raw)
  To: Tarmigan Casebolt; +Cc: git, peff, jaysoffian, drizzd, gitster, spearce
In-Reply-To: <1258680123-28684-2-git-send-email-tarmigan+git@gmail.com>

Tarmigan Casebolt <tarmigan+git@gmail.com> writes:

> With smart http, git over http is likely to become much more common.
> To increase testing of smart http, enable the http tests by default.

Sorry, but no test that listens to network ports should be enabled by
default; otherwise it will break automated, unattended tests people have
already set up randomly, depending on when the port happens to be
available for use by the tests.

^ 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