Skip to contentSkip to conversation

RESEND: git notes

PATCH Superseded Johan Herland 4 participants 19 messages Jun 15, 2016

Subsystems (2)

v1 of 13 Superseded Latest is v13 →

lore
Share & tools
lore
#0 lore raw

cover RESEND: git notes

Hi,

Dscho has asked me to take over the responsibility for the js/notes patch series.

The following is a re-roll and resend of the patch series currently in pu, plus my own 2 patches for adding support for "-m" and "-F" to "git notes edit".

On advice from Dscho, I have squashed the current bugfix and cleanup patches in js/notes into the first 4 "main" patches. As a result the original 15 + 2 patch series is now down to 5 (4 + 1) patches.

In sum, these 5 patches produce the exact same result as the original js/notes series (plus my 2 patches).

I have taken the liberty of squashing the various Signed-off-by tags (along with their corresponding patches) into these 5 new patches. I hope this is OK with everybody. If not, I apologize; please tell me, and I will re-send.

Have fun! :)

...Johan

Johan Herland (1):
Teach "-m <msg>" and "-F <file>" to "git notes edit"

Johannes Schindelin (4):
Introduce commit notes
Add a script to edit/inspect notes
Speed up git notes lookup
Add an expensive test for git-notes

 .gitignore                       |    1 +
 Documentation/config.txt         |   13 +++
 Documentation/git-notes.txt      |   60 ++++++++++++++
 Makefile                         |    3 +
 cache.h                          |    4 +
 command-list.txt                 |    1 +
 commit.c                         |    1 +
 config.c                         |    5 +
 environment.c                    |    1 +
 git-notes.sh                     |  121 ++++++++++++++++++++++++++++
 notes.c                          |  160 ++++++++++++++++++++++++++++++++++++++
 notes.h                          |    7 ++
 pretty.c                         |    5 +
 t/t3301-notes.sh                 |  149 +++++++++++++++++++++++++++++++++++
 t/t3302-notes-index-expensive.sh |   98 +++++++++++++++++++++++
 15 files changed, 629 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/git-notes.txt
 create mode 100755 git-notes.sh
 create mode 100644 notes.c
 create mode 100644 notes.h
 create mode 100755 t/t3301-notes.sh
 create mode 100755 t/t3302-notes-index-expensive.sh
#1 lore raw

PATCH 1/5 Introduce commit notes

From: Johannes Schindelin <[email protected]>

Commit notes are blobs which are shown together with the commit message. These blobs are taken from the notes ref, which you can configure by the config variable core.notesRef, which in turn can be overridden by the environment variable GIT_NOTES_REF.

The notes ref is a branch which contains "files" whose names are the names of the corresponding commits (i.e. the SHA-1).

The rationale for putting this information into a ref is this: we want to be able to fetch and possibly union-merge the notes, maybe even look at the date when a note was introduced, and we want to store them efficiently together with the other objects.

[tr: fix core.notesRef documentation]
[tv: fix printing of multi-line notes]

---
 Documentation/config.txt |   13 ++++++++
 Makefile                 |    2 +
 cache.h                  |    4 ++
 commit.c                 |    1 +
 config.c                 |    5 +++
 environment.c            |    1 +
 notes.c                  |   69 ++++++++++++++++++++++++++++++++++++++++++++++
 notes.h                  |    7 ++++
 pretty.c                 |    5 +++
 9 files changed, 107 insertions(+), 0 deletions(-)
 create mode 100644 notes.c
 create mode 100644 notes.h
Documentation/config.txt +13 -0
index 2c03162..c4c039b 100644
@@ -438,6 +438,19 @@ On some file system/operating system combinations, this is unreliable.
 Set this config setting to 'rename' there; However, This will remove the
 check that makes sure that existing object files will not get overwritten.
 
+core.notesRef::
+	When showing commit messages, also show notes which are stored in
+	the given ref.  This ref is expected to contain files named
+	after the full SHA-1 of the commit they annotate.
++
+If such a file exists in the given ref, the referenced blob is read, and
+appended to the commit message, separated by a "Notes:" line.  If the
+given ref itself does not exist, it is not an error, but means that no
+notes should be printed.
++
+This setting defaults to "refs/notes/commits", and can be overridden by
+the `GIT_NOTES_REF` environment variable.
+
 alias.*::
 	Command aliases for the linkgit:git[1] command wrapper - e.g.
 	after defining "alias.last = cat-file commit HEAD", the invocation
Makefile +2 -0
index a579310..8601bdd 100644
@@ -409,6 +409,7 @@ LIB_H += ll-merge.h
 LIB_H += log-tree.h
 LIB_H += mailmap.h
 LIB_H += merge-recursive.h
+LIB_H += notes.h
 LIB_H += object.h
 LIB_H += pack.h
 LIB_H += pack-refs.h
@@ -492,6 +493,7 @@ LIB_OBJS += match-trees.o
 LIB_OBJS += merge-file.o
 LIB_OBJS += merge-recursive.o
 LIB_OBJS += name-hash.o
+LIB_OBJS += notes.o
 LIB_OBJS += object.o
 LIB_OBJS += pack-check.o
 LIB_OBJS += pack-refs.o
cache.h +4 -0
index b8503ad..8a830a6 100644
@@ -371,6 +371,8 @@ static inline enum object_type object_type(unsigned int mode)
 #define GITATTRIBUTES_FILE ".gitattributes"
 #define INFOATTRIBUTES_FILE "info/attributes"
 #define ATTRIBUTE_MACRO_PREFIX "[attr]"
+#define GIT_NOTES_REF_ENVIRONMENT "GIT_NOTES_REF"
+#define GIT_NOTES_DEFAULT_REF "refs/notes/commits"
 
 extern int is_bare_repository_cfg;
 extern int is_bare_repository(void);
@@ -561,6 +563,8 @@ enum object_creation_mode {
 
 extern enum object_creation_mode object_creation_mode;
 
+extern char *notes_ref_name;
+
 #define GIT_REPO_VERSION 0
 extern int repository_format_version;
 extern int check_repository_format(void);
commit.c +1 -0
index aa3b35b..cf72143 100644
@@ -5,6 +5,7 @@
 #include "utf8.h"
 #include "diff.h"
 #include "revision.h"
+#include "notes.h"
 
 int save_commit_buffer = 1;
 
config.c +5 -0
index 1682273..d82b580 100644
@@ -469,6 +469,11 @@ static int git_default_core_config(const char *var, const char *value)
 		return 0;
 	}
 
+	if (!strcmp(var, "core.notesref")) {
+		notes_ref_name = xstrdup(value);
+		return 0;
+	}
+
 	if (!strcmp(var, "core.pager"))
 		return git_config_string(&pager_program, var, value);
 
environment.c +1 -0
index 801a005..f445cfd 100644
@@ -50,6 +50,7 @@ enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 
 /* Parallel index stat data preload? */
 int core_preload_index = 0;
+char *notes_ref_name;
 
 /* This is set by setup_git_dir_gently() and/or git_default_config() */
 char *git_work_tree_cfg;
notes.c new file +69 -0

69 lines not on this page · raw message

notes.h new file +7 -0
new file mode 100644
index 0000000..79d21b6
@@ -0,0 +1,7 @@
+#ifndef NOTES_H
+#define NOTES_H
+
+void get_commit_notes(const struct commit *commit, struct strbuf *sb,
+		const char *output_encoding);
+
+#endif
pretty.c +5 -0
index a0ef356..d30a964 100644
@@ -6,6 +6,7 @@
 #include "string-list.h"
 #include "mailmap.h"
 #include "log-tree.h"
+#include "notes.h"
 #include "color.h"
 
 static char *user_format;
@@ -963,5 +964,9 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 	 */
 	if (fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
 		strbuf_addch(sb, '\n');
+
+	if (fmt != CMIT_FMT_ONELINE)
+		get_commit_notes(commit, sb, encoding);
+
 	free(reencoded);
 }
Signature · 1 line
1.6.3.rc0.1.gf800
#2 lore raw

PATCH 2/5 Add a script to edit/inspect notes

In reply to Johan Herland

From: Johannes Schindelin <[email protected]>

The script 'git notes' allows you to edit and show commit notes, by calling either

git notes show <commit>

or

git notes edit <commit>

[tv: fix printing of multi-line notes]
[mg: test and handle empty notes gracefully]
[tr: only clean up message file when editing]
[tr: use GIT_EDITOR and core.editor over VISUAL/EDITOR] [tr: t3301: fix confusing quoting in test for valid notes ref] [tr: t3301: use test_must_fail instead of !]
[tr: refuse to edit notes outside refs/notes/]
[jc: tests: fix "export var=val"]
[cc: Documentation: fix 'linkgit' macro in "git-notes.txt"] [jh: Minor cleanup and bugfixing in git-notes.sh]

---
 .gitignore                  |    1 +
 Documentation/git-notes.txt |   46 +++++++++++++++++
 Makefile                    |    1 +
 command-list.txt            |    1 +
 git-notes.sh                |   73 +++++++++++++++++++++++++++
 t/t3301-notes.sh            |  114 +++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 236 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/git-notes.txt
 create mode 100755 git-notes.sh
 create mode 100755 t/t3301-notes.sh
.gitignore +1 -0
index 41c0b20..13221e7 100644
@@ -86,6 +86,7 @@ git-mktag
 git-mktree
 git-name-rev
 git-mv
+git-notes
 git-pack-redundant
 git-pack-objects
 git-pack-refs
Documentation/git-notes.txt new file +46 -0

46 lines not on this page · raw message

Makefile +1 -0
index 8601bdd..ab4a2b7 100644
@@ -298,6 +298,7 @@ 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
command-list.txt +1 -0
index fb03a2e..4296941 100644
@@ -74,6 +74,7 @@ git-mktag                               plumbingmanipulators
 git-mktree                              plumbingmanipulators
 git-mv                                  mainporcelain common
 git-name-rev                            plumbinginterrogators
+git-notes                               mainporcelain
 git-pack-objects                        plumbingmanipulators
 git-pack-redundant                      plumbinginterrogators
 git-pack-refs                           ancillarymanipulators
git-notes.sh new file +73 -0

73 lines not on this page · raw message

t/t3301-notes.sh new file +114 -0

114 lines not on this page · raw message

Signature · 1 line
1.6.3.rc0.1.gf800
#3 lore raw

PATCH 3/5 Speed up git notes lookup

In reply to Johan Herland

From: Johannes Schindelin <[email protected]>

To avoid looking up each and every commit in the notes ref's tree object, which is very expensive, speed things up by slurping the tree object's contents into a hash_map.

The idea for the hashmap singleton is from David Reiss, initial benchmarking by Jeff King.

Note: the implementation allows for arbitrary entries in the notes tree object, ignoring those that do not reference a valid object. This allows you to annotate arbitrary branches, or objects.

[jc: fixed an obvious error in initialize_hash_map()]

---
 notes.c |  113 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 102 insertions(+), 11 deletions(-)
notes.c +102 -11
index b0cf553..bd73784 100644
@@ -4,16 +4,112 @@
 #include "refs.h"
 #include "utf8.h"
 #include "strbuf.h"
+#include "tree-walk.h"
+
+struct entry {
+	unsigned char commit_sha1[20];
+	unsigned char notes_sha1[20];
+};
+
+struct hash_map {
+	struct entry *entries;
+	off_t count, size;
+};
 
 static int initialized;
+static struct hash_map hash_map;
+
+static int hash_index(struct hash_map *map, const unsigned char *sha1)
+{
+	int i = ((*(unsigned int *)sha1) % map->size);
+
+	for (;;) {
+		unsigned char *current = map->entries[i].commit_sha1;
+
+		if (!hashcmp(sha1, current))
+			return i;
+
+		if (is_null_sha1(current))
+			return -1 - i;
+
+		if (++i == map->size)
+			i = 0;
+	}
+}
+
+static void add_entry(const unsigned char *commit_sha1,
+		const unsigned char *notes_sha1)
+{
+	int index;
+
+	if (hash_map.count + 1 > hash_map.size >> 1) {
+		int i, old_size = hash_map.size;
+		struct entry *old = hash_map.entries;
+
+		hash_map.size = old_size ? old_size << 1 : 64;
+		hash_map.entries = (struct entry *)
+			xcalloc(sizeof(struct entry), hash_map.size);
+
+		for (i = 0; i < old_size; i++)
+			if (!is_null_sha1(old[i].commit_sha1)) {
+				index = -1 - hash_index(&hash_map,
+						old[i].commit_sha1);
+				memcpy(hash_map.entries + index, old + i,
+					sizeof(struct entry));
+			}
+		free(old);
+	}
+
+	index = hash_index(&hash_map, commit_sha1);
+	if (index < 0) {
+		index = -1 - index;
+		hash_map.count++;
+	}
+
+	hashcpy(hash_map.entries[index].commit_sha1, commit_sha1);
+	hashcpy(hash_map.entries[index].notes_sha1, notes_sha1);
+}
+
+static void initialize_hash_map(const char *notes_ref_name)
+{
+	unsigned char sha1[20], commit_sha1[20];
+	unsigned mode;
+	struct tree_desc desc;
+	struct name_entry entry;
+	void *buf;
+
+	if (!notes_ref_name || read_ref(notes_ref_name, commit_sha1) ||
+	    get_tree_entry(commit_sha1, "", sha1, &mode))
+		return;
+
+	buf = fill_tree_descriptor(&desc, sha1);
+	if (!buf)
+		die("Could not read %s for notes-index", sha1_to_hex(sha1));
+
+	while (tree_entry(&desc, &entry))
+		if (!get_sha1(entry.path, commit_sha1))
+			add_entry(commit_sha1, entry.sha1);
+	free(buf);
+}
+
+static unsigned char *lookup_notes(const unsigned char *commit_sha1)
+{
+	int index;
+
+	if (!hash_map.size)
+		return NULL;
+
+	index = hash_index(&hash_map, commit_sha1);
+	if (index < 0)
+		return NULL;
+	return hash_map.entries[index].notes_sha1;
+}
 
 void get_commit_notes(const struct commit *commit, struct strbuf *sb,
 		const char *output_encoding)
 {
 	static const char *utf8 = "utf-8";
-	struct strbuf name = STRBUF_INIT;
-	const char *hex;
-	unsigned char sha1[20];
+	unsigned char *sha1;
 	char *msg, *msg_p;
 	unsigned long linelen, msglen;
 	enum object_type type;
@@ -24,17 +120,12 @@ void get_commit_notes(const struct commit *commit, struct strbuf *sb,
 			notes_ref_name = getenv(GIT_NOTES_REF_ENVIRONMENT);
 		else if (!notes_ref_name)
 			notes_ref_name = GIT_NOTES_DEFAULT_REF;
-		if (notes_ref_name && read_ref(notes_ref_name, sha1))
-			notes_ref_name = NULL;
+		initialize_hash_map(notes_ref_name);
 		initialized = 1;
 	}
 
-	if (!notes_ref_name)
-		return;
-
-	strbuf_addf(&name, "%s:%s", notes_ref_name,
-			sha1_to_hex(commit->object.sha1));
-	if (get_sha1(name.buf, sha1))
+	sha1 = lookup_notes(commit->object.sha1);
+	if (!sha1)
 		return;
 
 	if (!(msg = read_sha1_file(sha1, &type, &msglen)) || !msglen ||
Signature · 1 line
1.6.3.rc0.1.gf800
#4 lore raw

PATCH 4/5 Add an expensive test for git-notes

In reply to Johan Herland

From: Johannes Schindelin <[email protected]>

git-notes have the potential of being pretty expensive, so test with a lot of commits. A lot. So to make things cheaper, you have to opt-in explicitely, by setting the environment variable GIT_NOTES_TIMING_TESTS.

[jc: tests: fix "export var=val"]

---
 t/t3302-notes-index-expensive.sh |   98 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 98 insertions(+), 0 deletions(-)
 create mode 100755 t/t3302-notes-index-expensive.sh
t/t3302-notes-index-expensive.sh new file +98 -0
new file mode 100755
index 0000000..0ef3e95
@@ -0,0 +1,98 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Johannes E. Schindelin
+#
+
+test_description='Test commit notes index (expensive!)'
+
+. ./test-lib.sh
+
+test -z "$GIT_NOTES_TIMING_TESTS" && {
+	say Skipping timing tests
+	test_done
+	exit
+}
+
+create_repo () {
+	number_of_commits=$1
+	nr=0
+	parent=
+	test -d .git || {
+	git init &&
+	tree=$(git write-tree) &&
+	while [ $nr -lt $number_of_commits ]; do
+		test_tick &&
+		commit=$(echo $nr | git commit-tree $tree $parent) ||
+			return
+		parent="-p $commit"
+		nr=$(($nr+1))
+	done &&
+	git update-ref refs/heads/master $commit &&
+	{
+		GIT_INDEX_FILE=.git/temp; export GIT_INDEX_FILE;
+		git rev-list HEAD | cat -n | sed "s/^[ 	][ 	]*/ /g" |
+		while read nr sha1; do
+			blob=$(echo note $nr | git hash-object -w --stdin) &&
+			echo $sha1 | sed "s/^/0644 $blob 0	/"
+		done | git update-index --index-info &&
+		tree=$(git write-tree) &&
+		test_tick &&
+		commit=$(echo notes | git commit-tree $tree) &&
+		git update-ref refs/notes/commits $commit
+	} &&
+	git config core.notesRef refs/notes/commits
+	}
+}
+
+test_notes () {
+	count=$1 &&
+	git config core.notesRef refs/notes/commits &&
+	git log | grep "^    " > output &&
+	i=1 &&
+	while [ $i -le $count ]; do
+		echo "    $(($count-$i))" &&
+		echo "    note $i" &&
+		i=$(($i+1));
+	done > expect &&
+	git diff expect output
+}
+
+cat > time_notes << \EOF
+	mode=$1
+	i=1
+	while [ $i -lt $2 ]; do
+		case $1 in
+		no-notes)
+			GIT_NOTES_REF=non-existing; export GIT_NOTES_REF
+		;;
+		notes)
+			unset GIT_NOTES_REF
+		;;
+		esac
+		git log >/dev/null
+		i=$(($i+1))
+	done
+EOF
+
+time_notes () {
+	for mode in no-notes notes
+	do
+		echo $mode
+		/usr/bin/time sh ../time_notes $mode $1
+	done
+}
+
+for count in 10 100 1000 10000; do
+
+	mkdir $count
+	(cd $count;
+
+	test_expect_success "setup $count" "create_repo $count"
+
+	test_expect_success 'notes work' "test_notes $count"
+
+	test_expect_success 'notes timing' "time_notes 100"
+	)
+done
+
+test_done
Signature · 1 line
1.6.3.rc0.1.gf800
#5 lore raw
In reply to Johan Herland

Johan Herland venit, vidit, dixit 16.05.2009 03:45:

Quoted 13 lines

Hi,

Dscho has asked me to take over the responsibility for the js/notes patch series.

The following is a re-roll and resend of the patch series currently in pu, plus my own 2 patches for adding support for "-m" and "-F" to "git notes edit".

On advice from Dscho, I have squashed the current bugfix and cleanup patches in js/notes into the first 4 "main" patches. As a result the original 15 + 2 patch series is now down to 5 (4 + 1) patches.

Quoted 7 lines

In sum, these 5 patches produce the exact same result as the original js/notes series (plus my 2 patches).

I have taken the liberty of squashing the various Signed-off-by tags (along with their corresponding patches) into these 5 new patches. I hope this is OK with everybody. If not, I apologize; please tell me, and I will re-send.

Well, effectively you removed me (and others) from the author list :|

I think the issue with the tree difference after this series (compared to the original one) shows that this squashing action makes reviewers' lives more complicated rather than easier. If it were the other way round squashing would be fine, of course.

Michael

Quoted 39 lines

Have fun! :)

...Johan

Johan Herland (1):
Teach "-m <msg>" and "-F <file>" to "git notes edit"

Johannes Schindelin (4):
Introduce commit notes
Add a script to edit/inspect notes
Speed up git notes lookup
Add an expensive test for git-notes

 .gitignore                       |    1 +
 Documentation/config.txt         |   13 +++
 Documentation/git-notes.txt      |   60 ++++++++++++++
 Makefile                         |    3 +
 cache.h                          |    4 +
 command-list.txt                 |    1 +
 commit.c                         |    1 +
 config.c                         |    5 +
 environment.c                    |    1 +
 git-notes.sh                     |  121 ++++++++++++++++++++++++++++
 notes.c                          |  160 ++++++++++++++++++++++++++++++++++++++
 notes.h                          |    7 ++
 pretty.c                         |    5 +
 t/t3301-notes.sh                 |  149 +++++++++++++++++++++++++++++++++++
 t/t3302-notes-index-expensive.sh |   98 +++++++++++++++++++++++
 15 files changed, 629 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/git-notes.txt
 create mode 100755 git-notes.sh
 create mode 100644 notes.c
 create mode 100644 notes.h
 create mode 100755 t/t3301-notes.sh
 create mode 100755 t/t3302-notes-index-expensive.sh
#6 lore raw
Quoted 15 lines · On Saturday 16 May 2009, [email protected] wrote:

Johan Herland venit, vidit, dixit 16.05.2009 03:45:

Quoted 14 lines

Dscho has asked me to take over the responsibility for the js/notes patch series.

The following is a re-roll and resend of the patch series currently in pu, plus my own 2 patches for adding support for "-m" and "-F" to "git notes edit".

On advice from Dscho, I have squashed the current bugfix and cleanup patches in js/notes into the first 4 "main" patches. As a result the original 15 + 2 patch series is now down to 5 (4 + 1) patches.

In sum, these 5 patches produce the exact same result as the original js/notes series (plus my 2 patches).

Quoted 6 lines
Quoted 4 lines

I have taken the liberty of squashing the various Signed-off-by tags (along with their corresponding patches) into these 5 new patches. I hope this is OK with everybody. If not, I apologize; please tell me, and I will re-send.

Well, effectively you removed me (and others) from the author list :|

I know, and I'm sorry. This is not an ideal solution.

When a patch series is cooking in 'pu' and it receives fixes and cleanups from people on the list, traditionally (I believe) Junio expects a new/replacement series that incorporates this feedback to be posted to the list. This results in cleaner series, but at the cost of collapsing authorship down to the primary author.

In this case, the original author (Dscho) has not had the time to do this re-roll, and the bugfixes/cleanups have instead been added to the end of the patch series, resulting in the original js/notes series growing from 4 to 15 patches.

I believe the re-roll I have done results in a nicer and cleaner overall series, and I have tried to mitigate the loss of authorship by adding small notes in the commit messages mentioning the fixes contributed by others. Still, Dscho stands as the primary author for the 4 "main" patches, and I believe this is correct, even if the notes feature has been discussed and developed conceptually by many others (myself included).

Quoted 4 lines

I think the issue with the tree difference after this series (compared to the original one) shows that this squashing action makes reviewers' lives more complicated rather than easier. If it were the other way round squashing would be fine, of course.

When taking over this patch series, I believe I had 4 choices:

1. Leave everything as-is, adding my two patches on top of the existing 15. For reviewers of the notes feature as a whole, I don't believe an ever- growing patch series makes their life easier. Also, I believe a shorter patch series looks nicer after being merged to 'next' (otherwise, why are we doing this whole 're-rolling while in pu'-workflow?)

2. Squash the existing 15-patch series down to 4 patches, and add my 2 patches on top. This would yield a 4+2-patch series that would by tree- identical to the original js/notes series after patch #4. However, everybody's authorship would be lost, except my own (patch 5 & 6), and this strikes me as a bit hypocritical.

3. Like #2, but also squash my first patch (the bugfix/cleanup part) into the first 4 patches. This, I believe, best follows the intent of 'pu' (squashing bugfixes and cleanups into the original series). Unfortunately, it breaks the tree-equality provided in #2.

4. Squash all 17 patches into a 4-patch series. However, I believe the final patch (adding support for '-m' and '-F' to 'git notes edit') is more of a feature patch, than a bugfix/cleanup patch, and thus should not be squashed.

I therefore went with #3, which is - as I said above - not an ideal solution, but AFAICS the best under the circumstances.

...Johan

Signature · 2 lines
Johan Herland, <[email protected]>
www.herland.net
#7 lore raw
In reply to [email protected]

Hi,

Quoted 15 lines · On Sat, 16 May 2009, [email protected] wrote:

Johan Herland venit, vidit, dixit 16.05.2009 03:45:

Quoted 13 lines

Dscho has asked me to take over the responsibility for the js/notes patch series.

The following is a re-roll and resend of the patch series currently in pu, plus my own 2 patches for adding support for "-m" and "-F" to "git notes edit".

On advice from Dscho, I have squashed the current bugfix and cleanup patches in js/notes into the first 4 "main" patches. As a result the original 15 + 2 patch series is now down to 5 (4 + 1) patches.

In sum, these 5 patches produce the exact same result as the original js/notes series (plus my 2 patches).

Actually, I would like this series not to go in as-is, as the most important obstacle for inclusion into 1.6.2 (IIRC) was that performance was pretty lousy.

In the least, it should allow reading fan-out directories, but preferably it would even _write_ fan-out directories.

The only tricky part was how to decide which notes to take if there are ambiguities, but I think a concatenation (or that shallower paths take precedence) are both viable strategies.

Quoted 1 line

Well, effectively you removed me (and others) from the author list :|

FWIW whenever I squashed other people's patches into mine (and that is not limited to the 'notes' branch), I tried to add proper attribution to the commit message. In Open Source, the currency is merit, so attribution is important.

Ciao,
Dscho

#8 lore raw
In reply to Johan Herland
Quoted 10 lines · Johan Herland

The following is a re-roll and resend of the patch series currently in pu, plus my own 2 patches for adding support for "-m" and "-F" to "git notes edit".

On advice from Dscho, I have squashed the current bugfix and cleanup patches in js/notes into the first 4 "main" patches. As a result the original 15 + 2 patch series is now down to 5 (4 + 1) patches.

In sum, these 5 patches produce the exact same result as the original js/notes series (plus my 2 patches).

Thanks, but you did no such thing, actually.

The result from this sequence

git checkout -b jh/notes v1.6.3.1^0
git am ./+jh-notes.mbox ;# these patches
git checkout v1.6.3.1^0
git merge js/notes ;# allow rerere to reapply the resolution git diff -R jh/notes^

should be empty, or should show your improvements over what has been queued in 'pu'.

Here is what I see:

    diff --git b/git-notes.sh a/git-notes.sh
    index 6ec33c9..7c3b8b9 100755
    --- b/git-notes.sh
    +++ a/git-notes.sh
    @@ -20,15 +20,16 @@ edit)
                    die "Refusing to edit notes in $GIT_NOTES_REF (outside of refs/notes/)"
            fi

    -	MESSAGE="$GIT_DIR"/new-notes-$COMMIT
    +	MSG_FILE="$GIT_DIR/new-notes-$COMMIT"
    +	GIT_INDEX_FILE="MSG_FILE.idx"

Renaming of the variable MESSAGE to MSG_FILE may be an improvement, as the former could have puzzled the reader if $MESSAGE contains the message itself, or it has the name of a file that stores the message (the answer is latter).

But I think there is a regression with a missing '$' here.

    +	export GIT_INDEX_FILE
    +
            trap '
    -		test -f "$MESSAGE" && rm "$MESSAGE"
    +		test -f "$MSG_FILE" && rm "$MSG_FILE"
    +		test -f "$GIT_INDEX_FILE" && rm "$GIT_INDEX_FILE"
            ' 0

Another improvement is that the variable definitions were moved up before the trap that uses these definitions. This is a good change.

    -	GIT_NOTES_REF= git log -1 $COMMIT | sed "s/^/#/" > "$MESSAGE"
    -
    -	GIT_INDEX_FILE="$MESSAGE".idx
    -	export GIT_INDEX_FILE
    +	git log -1 $COMMIT | sed "s/^/#/" > "$MSG_FILE"

Dscho's version exports an empty GIT_NOTES_REF, presumably to avoid triggering the notes mechanism, while running this "git log -1", but yours don't. Is there a reason behind this change?

The rest are fallouts from s/MESSAGE/MSG_FILE/ renaming.

    @@ -36,16 +37,16 @@ edit)
            else
                    PARENT="-p $CURRENT_HEAD"
                    git read-tree "$GIT_NOTES_REF" || die "Could not read index"
    -		git cat-file blob :$COMMIT >> "$MESSAGE" 2> /dev/null
    +		git cat-file blob :$COMMIT >> "$MSG_FILE" 2> /dev/null
            fi

            core_editor="$(git config core.editor)"
    -	${GIT_EDITOR:-${core_editor:-${VISUAL:-${EDITOR:-vi}}}} "$MESSAGE"
    +	${GIT_EDITOR:-${core_editor:-${VISUAL:-${EDITOR:-vi}}}} "$MSG_FILE"

    -	grep -v ^# < "$MESSAGE" | git stripspace > "$MESSAGE".processed
    -	mv "$MESSAGE".processed "$MESSAGE"
    -	if [ -s "$MESSAGE" ]; then
    -		BLOB=$(git hash-object -w "$MESSAGE") ||
    +	grep -v ^# < "$MSG_FILE" | git stripspace > "$MSG_FILE".processed
    +	mv "$MSG_FILE".processed "$MSG_FILE"
    +	if [ -s "$MSG_FILE" ]; then
    +		BLOB=$(git hash-object -w "$MSG_FILE") ||
                            die "Could not write into object database"
                    git update-index --add --cacheinfo 0644 $BLOB $COMMIT ||
                            die "Could not write index"

I am a bit reluctant to take this to replace js/notes as-is until I hear answers to these points (and others may have comments too).

#9 lore raw
Quoted 8 lines · Junio C Hamano
Quoted 2 lines · Johan Herland

In sum, these 5 patches produce the exact same result as the original js/notes series (plus my 2 patches).

Thanks, but you did no such thing, actually.

The result from this sequence

Quoted 8 lines

git checkout -b jh/notes v1.6.3.1^0
git am ./+jh-notes.mbox ;# these patches
git checkout v1.6.3.1^0
git merge js/notes ;# allow rerere to reapply the resolution git diff -R jh/notes^

should be empty, or should show your improvements over what has been queued in 'pu'.

Sorry for the screwup. The diff should in any case not be empty, since the first patch (the cleanup/bugfix part) of my original 2 patches (which are NOT in js/notes on 'pu') has been squashed into the first 4 patches of this new series.

Therefore, the diff you quote below is simply this first patch from my original two-part series (the second patch of that series is now the 5th patch in this series - adding '-m' and '-F' support to 'git notes edit').

Quoted 13 lines

Here is what I see:

    diff --git b/git-notes.sh a/git-notes.sh
    index 6ec33c9..7c3b8b9 100755
    --- b/git-notes.sh
    +++ a/git-notes.sh
    @@ -20,15 +20,16 @@ edit)
                    die "Refusing to edit notes in $GIT_NOTES_REF (outside of refs/notes/)" fi

    -	MESSAGE="$GIT_DIR"/new-notes-$COMMIT
    +	MSG_FILE="$GIT_DIR/new-notes-$COMMIT"
    +	GIT_INDEX_FILE="MSG_FILE.idx"
Quoted 4 lines

Renaming of the variable MESSAGE to MSG_FILE may be an improvement, as the former could have puzzled the reader if $MESSAGE contains the message itself, or it has the name of a file that stores the message (the answer is latter).

Indeed. If you look at the next patch (adding support for '-m' and '-F'), you will see that I reuse $MESSAGE to hold the _actual_ message (given by '-m' or '-F').

Quoted 1 line

But I think there is a regression with a missing '$' here.

Yes. Thanks for catching.

Quoted 17 lines
    +	export GIT_INDEX_FILE
    +
            trap '
    -		test -f "$MESSAGE" && rm "$MESSAGE"
    +		test -f "$MSG_FILE" && rm "$MSG_FILE"
    +		test -f "$GIT_INDEX_FILE" && rm "$GIT_INDEX_FILE"
            ' 0

Another improvement is that the variable definitions were moved up before the trap that uses these definitions. This is a good change.

    -	GIT_NOTES_REF= git log -1 $COMMIT | sed "s/^/#/" > "$MESSAGE"
    -
    -	GIT_INDEX_FILE="$MESSAGE".idx
    -	export GIT_INDEX_FILE
    +	git log -1 $COMMIT | sed "s/^/#/" > "$MSG_FILE"
Quoted 3 lines

Dscho's version exports an empty GIT_NOTES_REF, presumably to avoid triggering the notes mechanism, while running this "git log -1", but yours don't. Is there a reason behind this change?

Oh, so _that's_ what he did... I was puzzled by that "stray" 'GIT_NOTES_REF= '...

Yet another screwup on my part.

Quoted 26 lines

The rest are fallouts from s/MESSAGE/MSG_FILE/ renaming.

    @@ -36,16 +37,16 @@ edit)
            else
                    PARENT="-p $CURRENT_HEAD"
                    git read-tree "$GIT_NOTES_REF" || die "Could not read index"
    -		git cat-file blob :$COMMIT >> "$MESSAGE" 2> /dev/null
    +		git cat-file blob :$COMMIT >> "$MSG_FILE" 2> /dev/null
            fi

            core_editor="$(git config core.editor)"
    -	${GIT_EDITOR:-${core_editor:-${VISUAL:-${EDITOR:-vi}}}} "$MESSAGE"
    +	${GIT_EDITOR:-${core_editor:-${VISUAL:-${EDITOR:-vi}}}} "$MSG_FILE"

    -	grep -v ^# < "$MESSAGE" | git stripspace > "$MESSAGE".processed
    -	mv "$MESSAGE".processed "$MESSAGE"
    -	if [ -s "$MESSAGE" ]; then
    -		BLOB=$(git hash-object -w "$MESSAGE") ||
    +	grep -v ^# < "$MSG_FILE" | git stripspace > "$MSG_FILE".processed
    +	mv "$MSG_FILE".processed "$MSG_FILE"
    +	if [ -s "$MSG_FILE" ]; then
    +		BLOB=$(git hash-object -w "$MSG_FILE") ||
      		die "Could not write into object database"
      	git update-index --add --cacheinfo 0644 $BLOB $COMMIT ||
      		die "Could not write index"
Quoted 2 lines

I am a bit reluctant to take this to replace js/notes as-is until I hear answers to these points (and others may have comments too).

Thanks for the review.
I will send an updated series with the above fixes.

...Johan

Signature · 2 lines
Johan Herland, <[email protected]>
www.herland.net
#10 lore raw

PATCH v2 1/5 Introduce commit notes

From: Johannes Schindelin <[email protected]>

Commit notes are blobs which are shown together with the commit message. These blobs are taken from the notes ref, which you can configure by the config variable core.notesRef, which in turn can be overridden by the environment variable GIT_NOTES_REF.

The notes ref is a branch which contains "files" whose names are the names of the corresponding commits (i.e. the SHA-1).

The rationale for putting this information into a ref is this: we want to be able to fetch and possibly union-merge the notes, maybe even look at the date when a note was introduced, and we want to store them efficiently together with the other objects.

[tr: fix core.notesRef documentation]
[tv: fix printing of multi-line notes]

---
 Documentation/config.txt |   13 ++++++++
 Makefile                 |    2 +
 cache.h                  |    4 ++
 commit.c                 |    1 +
 config.c                 |    5 +++
 environment.c            |    1 +
 notes.c                  |   69 ++++++++++++++++++++++++++++++++++++++++++++++
 notes.h                  |    7 ++++
 pretty.c                 |    5 +++
 9 files changed, 107 insertions(+), 0 deletions(-)
 create mode 100644 notes.c
 create mode 100644 notes.h
Documentation/config.txt +13 -0
index 5dcad94..6875ecd 100644
@@ -438,6 +438,19 @@ On some file system/operating system combinations, this is unreliable.
 Set this config setting to 'rename' there; However, This will remove the
 check that makes sure that existing object files will not get overwritten.
 
+core.notesRef::
+	When showing commit messages, also show notes which are stored in
+	the given ref.  This ref is expected to contain files named
+	after the full SHA-1 of the commit they annotate.
++
+If such a file exists in the given ref, the referenced blob is read, and
+appended to the commit message, separated by a "Notes:" line.  If the
+given ref itself does not exist, it is not an error, but means that no
+notes should be printed.
++
+This setting defaults to "refs/notes/commits", and can be overridden by
+the `GIT_NOTES_REF` environment variable.
+
 alias.*::
 	Command aliases for the linkgit:git[1] command wrapper - e.g.
 	after defining "alias.last = cat-file commit HEAD", the invocation
Makefile +2 -0
index 6e21643..59e102c 100644
@@ -406,6 +406,7 @@ LIB_H += ll-merge.h
 LIB_H += log-tree.h
 LIB_H += mailmap.h
 LIB_H += merge-recursive.h
+LIB_H += notes.h
 LIB_H += object.h
 LIB_H += pack.h
 LIB_H += pack-refs.h
@@ -489,6 +490,7 @@ LIB_OBJS += match-trees.o
 LIB_OBJS += merge-file.o
 LIB_OBJS += merge-recursive.o
 LIB_OBJS += name-hash.o
+LIB_OBJS += notes.o
 LIB_OBJS += object.o
 LIB_OBJS += pack-check.o
 LIB_OBJS += pack-refs.o
cache.h +4 -0
index b8503ad..8a830a6 100644
@@ -371,6 +371,8 @@ static inline enum object_type object_type(unsigned int mode)
 #define GITATTRIBUTES_FILE ".gitattributes"
 #define INFOATTRIBUTES_FILE "info/attributes"
 #define ATTRIBUTE_MACRO_PREFIX "[attr]"
+#define GIT_NOTES_REF_ENVIRONMENT "GIT_NOTES_REF"
+#define GIT_NOTES_DEFAULT_REF "refs/notes/commits"
 
 extern int is_bare_repository_cfg;
 extern int is_bare_repository(void);
@@ -561,6 +563,8 @@ enum object_creation_mode {
 
 extern enum object_creation_mode object_creation_mode;
 
+extern char *notes_ref_name;
+
 #define GIT_REPO_VERSION 0
 extern int repository_format_version;
 extern int check_repository_format(void);
commit.c +1 -0
index aa3b35b..cf72143 100644
@@ -5,6 +5,7 @@
 #include "utf8.h"
 #include "diff.h"
 #include "revision.h"
+#include "notes.h"
 
 int save_commit_buffer = 1;
 
config.c +5 -0
index 1682273..d82b580 100644
@@ -469,6 +469,11 @@ static int git_default_core_config(const char *var, const char *value)
 		return 0;
 	}
 
+	if (!strcmp(var, "core.notesref")) {
+		notes_ref_name = xstrdup(value);
+		return 0;
+	}
+
 	if (!strcmp(var, "core.pager"))
 		return git_config_string(&pager_program, var, value);
 
environment.c +1 -0
index 801a005..f445cfd 100644
@@ -50,6 +50,7 @@ enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 
 /* Parallel index stat data preload? */
 int core_preload_index = 0;
+char *notes_ref_name;
 
 /* This is set by setup_git_dir_gently() and/or git_default_config() */
 char *git_work_tree_cfg;
notes.c new file +69 -0

69 lines not on this page · raw message

notes.h new file +7 -0
new file mode 100644
index 0000000..79d21b6
@@ -0,0 +1,7 @@
+#ifndef NOTES_H
+#define NOTES_H
+
+void get_commit_notes(const struct commit *commit, struct strbuf *sb,
+		const char *output_encoding);
+
+#endif
pretty.c +5 -0
index a0ef356..d30a964 100644
@@ -6,6 +6,7 @@
 #include "string-list.h"
 #include "mailmap.h"
 #include "log-tree.h"
+#include "notes.h"
 #include "color.h"
 
 static char *user_format;
@@ -963,5 +964,9 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
 	 */
 	if (fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
 		strbuf_addch(sb, '\n');
+
+	if (fmt != CMIT_FMT_ONELINE)
+		get_commit_notes(commit, sb, encoding);
+
 	free(reencoded);
 }
Signature · 1 line
1.6.3.rc0.1.gf800
#11 lore raw

PATCH v2 2/5 Add a script to edit/inspect notes

In reply to Johan Herland

From: Johannes Schindelin <[email protected]>

The script 'git notes' allows you to edit and show commit notes, by calling either

git notes show <commit>

or

git notes edit <commit>

[tv: fix printing of multi-line notes]
[mg: test and handle empty notes gracefully]
[tr: only clean up message file when editing]
[tr: use GIT_EDITOR and core.editor over VISUAL/EDITOR] [tr: t3301: fix confusing quoting in test for valid notes ref] [tr: t3301: use test_must_fail instead of !]
[tr: refuse to edit notes outside refs/notes/]
[jc: tests: fix "export var=val"]
[cc: Documentation: fix 'linkgit' macro in "git-notes.txt"] [jh: Minor cleanup and bugfixing in git-notes.sh (v2)]

---
 .gitignore                  |    1 +
 Documentation/git-notes.txt |   46 +++++++++++++++++
 Makefile                    |    1 +
 command-list.txt            |    1 +
 git-notes.sh                |   73 +++++++++++++++++++++++++++
 t/t3301-notes.sh            |  114 +++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 236 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/git-notes.txt
 create mode 100755 git-notes.sh
 create mode 100755 t/t3301-notes.sh
.gitignore +1 -0
index 41c0b20..13221e7 100644
@@ -86,6 +86,7 @@ git-mktag
 git-mktree
 git-name-rev
 git-mv
+git-notes
 git-pack-redundant
 git-pack-objects
 git-pack-refs
Documentation/git-notes.txt new file +46 -0

46 lines not on this page · raw message

Makefile +1 -0
index 59e102c..1d91a04 100644
@@ -295,6 +295,7 @@ 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
command-list.txt +1 -0
index fb03a2e..4296941 100644
@@ -74,6 +74,7 @@ git-mktag                               plumbingmanipulators
 git-mktree                              plumbingmanipulators
 git-mv                                  mainporcelain common
 git-name-rev                            plumbinginterrogators
+git-notes                               mainporcelain
 git-pack-objects                        plumbingmanipulators
 git-pack-redundant                      plumbinginterrogators
 git-pack-refs                           ancillarymanipulators
git-notes.sh new file +73 -0

73 lines not on this page · raw message

t/t3301-notes.sh new file +114 -0

114 lines not on this page · raw message

Signature · 1 line
1.6.3.rc0.1.gf800
#12 lore raw

PATCH v2 3/5 Speed up git notes lookup

In reply to Johan Herland

From: Johannes Schindelin <[email protected]>

To avoid looking up each and every commit in the notes ref's tree object, which is very expensive, speed things up by slurping the tree object's contents into a hash_map.

The idea for the hashmap singleton is from David Reiss, initial benchmarking by Jeff King.

Note: the implementation allows for arbitrary entries in the notes tree object, ignoring those that do not reference a valid object. This allows you to annotate arbitrary branches, or objects.

[jc: fixed an obvious error in initialize_hash_map()]

---
 notes.c |  113 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 102 insertions(+), 11 deletions(-)
notes.c +102 -11
index b0cf553..bd73784 100644
@@ -4,16 +4,112 @@
 #include "refs.h"
 #include "utf8.h"
 #include "strbuf.h"
+#include "tree-walk.h"
+
+struct entry {
+	unsigned char commit_sha1[20];
+	unsigned char notes_sha1[20];
+};
+
+struct hash_map {
+	struct entry *entries;
+	off_t count, size;
+};
 
 static int initialized;
+static struct hash_map hash_map;
+
+static int hash_index(struct hash_map *map, const unsigned char *sha1)
+{
+	int i = ((*(unsigned int *)sha1) % map->size);
+
+	for (;;) {
+		unsigned char *current = map->entries[i].commit_sha1;
+
+		if (!hashcmp(sha1, current))
+			return i;
+
+		if (is_null_sha1(current))
+			return -1 - i;
+
+		if (++i == map->size)
+			i = 0;
+	}
+}
+
+static void add_entry(const unsigned char *commit_sha1,
+		const unsigned char *notes_sha1)
+{
+	int index;
+
+	if (hash_map.count + 1 > hash_map.size >> 1) {
+		int i, old_size = hash_map.size;
+		struct entry *old = hash_map.entries;
+
+		hash_map.size = old_size ? old_size << 1 : 64;
+		hash_map.entries = (struct entry *)
+			xcalloc(sizeof(struct entry), hash_map.size);
+
+		for (i = 0; i < old_size; i++)
+			if (!is_null_sha1(old[i].commit_sha1)) {
+				index = -1 - hash_index(&hash_map,
+						old[i].commit_sha1);
+				memcpy(hash_map.entries + index, old + i,
+					sizeof(struct entry));
+			}
+		free(old);
+	}
+
+	index = hash_index(&hash_map, commit_sha1);
+	if (index < 0) {
+		index = -1 - index;
+		hash_map.count++;
+	}
+
+	hashcpy(hash_map.entries[index].commit_sha1, commit_sha1);
+	hashcpy(hash_map.entries[index].notes_sha1, notes_sha1);
+}
+
+static void initialize_hash_map(const char *notes_ref_name)
+{
+	unsigned char sha1[20], commit_sha1[20];
+	unsigned mode;
+	struct tree_desc desc;
+	struct name_entry entry;
+	void *buf;
+
+	if (!notes_ref_name || read_ref(notes_ref_name, commit_sha1) ||
+	    get_tree_entry(commit_sha1, "", sha1, &mode))
+		return;
+
+	buf = fill_tree_descriptor(&desc, sha1);
+	if (!buf)
+		die("Could not read %s for notes-index", sha1_to_hex(sha1));
+
+	while (tree_entry(&desc, &entry))
+		if (!get_sha1(entry.path, commit_sha1))
+			add_entry(commit_sha1, entry.sha1);
+	free(buf);
+}
+
+static unsigned char *lookup_notes(const unsigned char *commit_sha1)
+{
+	int index;
+
+	if (!hash_map.size)
+		return NULL;
+
+	index = hash_index(&hash_map, commit_sha1);
+	if (index < 0)
+		return NULL;
+	return hash_map.entries[index].notes_sha1;
+}
 
 void get_commit_notes(const struct commit *commit, struct strbuf *sb,
 		const char *output_encoding)
 {
 	static const char *utf8 = "utf-8";
-	struct strbuf name = STRBUF_INIT;
-	const char *hex;
-	unsigned char sha1[20];
+	unsigned char *sha1;
 	char *msg, *msg_p;
 	unsigned long linelen, msglen;
 	enum object_type type;
@@ -24,17 +120,12 @@ void get_commit_notes(const struct commit *commit, struct strbuf *sb,
 			notes_ref_name = getenv(GIT_NOTES_REF_ENVIRONMENT);
 		else if (!notes_ref_name)
 			notes_ref_name = GIT_NOTES_DEFAULT_REF;
-		if (notes_ref_name && read_ref(notes_ref_name, sha1))
-			notes_ref_name = NULL;
+		initialize_hash_map(notes_ref_name);
 		initialized = 1;
 	}
 
-	if (!notes_ref_name)
-		return;
-
-	strbuf_addf(&name, "%s:%s", notes_ref_name,
-			sha1_to_hex(commit->object.sha1));
-	if (get_sha1(name.buf, sha1))
+	sha1 = lookup_notes(commit->object.sha1);
+	if (!sha1)
 		return;
 
 	if (!(msg = read_sha1_file(sha1, &type, &msglen)) || !msglen ||
Signature · 1 line
1.6.3.rc0.1.gf800
#13 lore raw

v2 cover RESEND: git notes

In reply to Johan Herland

Hi,

Dscho has asked me to take over the responsibility for the js/notes patch series.

The following is a re-roll and resend of the patch series currently in 'pu', plus my own 2 patches for adding support for '-m' and '-F' to 'git notes edit'.

On advice from Dscho, I have squashed the current bugfix and cleanup patches in js/notes (plus the first of my two patches) into the first 4 "main" patches. As a result the original 15 + 2 patch series is now down to 5 (4 + 1) patches.

In sum, the first 4 patches in this series is equivalent to the original js/notes series plus the first of my 2 patches. The final 5th patch is equivalent to my second patch (adding support for '-m' and '-F' to 'git notes edit').

I have taken the liberty of squashing the various Signed-off-by tags (along with their corresponding patches) into these 5 new patches. I hope this is OK with everybody. If not, I apologize; please tell me, and I will re-send.

Have fun! :)

...Johan

Johan Herland (1):
Teach "-m <msg>" and "-F <file>" to "git notes edit"

Johannes Schindelin (4):
Introduce commit notes
Add a script to edit/inspect notes
Speed up git notes lookup
Add an expensive test for git-notes

 .gitignore                       |    1 +
 Documentation/config.txt         |   13 +++
 Documentation/git-notes.txt      |   60 ++++++++++++++
 Makefile                         |    3 +
 cache.h                          |    4 +
 command-list.txt                 |    1 +
 commit.c                         |    1 +
 config.c                         |    5 +
 environment.c                    |    1 +
 git-notes.sh                     |  121 ++++++++++++++++++++++++++++
 notes.c                          |  160 ++++++++++++++++++++++++++++++++++++++
 notes.h                          |    7 ++
 pretty.c                         |    5 +
 t/t3301-notes.sh                 |  149 +++++++++++++++++++++++++++++++++++
 t/t3302-notes-index-expensive.sh |   98 +++++++++++++++++++++++
 15 files changed, 629 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/git-notes.txt
 create mode 100755 git-notes.sh
 create mode 100644 notes.c
 create mode 100644 notes.h
 create mode 100755 t/t3301-notes.sh
 create mode 100755 t/t3302-notes-index-expensive.sh
#14 lore raw

Hi,

Quoted 2 lines · Johan Herland

Dscho has asked me to take over the responsibility for the js/notes patch series.

How's the enhanced note-tree lookup going?

And do you have updated the patches to attribute people who provided now-squashed-in patches properly?

Ciao,
Dscho

#15 lore raw
Quoted 1 line · Johannes Schindelin

How's the enhanced note-tree lookup going?

Haven't had time to work on it, yet.

Quoted 2 lines

And do you have updated the patches to attribute people who provided now-squashed-in patches properly?

Yes, I have re-rolled the commit message to include the full name (not just the initials) of each contributor. Will resend when time permits. Is there anything more I can do to provide proper attribution?

...Johan

Signature · 2 lines
Johan Herland, <[email protected]>
www.herland.net
#16 lore raw

PATCH v2 4/5 Add an expensive test for git-notes

In reply to Johan Herland

From: Johannes Schindelin <[email protected]>

git-notes have the potential of being pretty expensive, so test with a lot of commits. A lot. So to make things cheaper, you have to opt-in explicitely, by setting the environment variable GIT_NOTES_TIMING_TESTS.

[jc: tests: fix "export var=val"]

---
 t/t3302-notes-index-expensive.sh |   98 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 98 insertions(+), 0 deletions(-)
 create mode 100755 t/t3302-notes-index-expensive.sh
t/t3302-notes-index-expensive.sh new file +98 -0
new file mode 100755
index 0000000..0ef3e95
@@ -0,0 +1,98 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Johannes E. Schindelin
+#
+
+test_description='Test commit notes index (expensive!)'
+
+. ./test-lib.sh
+
+test -z "$GIT_NOTES_TIMING_TESTS" && {
+	say Skipping timing tests
+	test_done
+	exit
+}
+
+create_repo () {
+	number_of_commits=$1
+	nr=0
+	parent=
+	test -d .git || {
+	git init &&
+	tree=$(git write-tree) &&
+	while [ $nr -lt $number_of_commits ]; do
+		test_tick &&
+		commit=$(echo $nr | git commit-tree $tree $parent) ||
+			return
+		parent="-p $commit"
+		nr=$(($nr+1))
+	done &&
+	git update-ref refs/heads/master $commit &&
+	{
+		GIT_INDEX_FILE=.git/temp; export GIT_INDEX_FILE;
+		git rev-list HEAD | cat -n | sed "s/^[ 	][ 	]*/ /g" |
+		while read nr sha1; do
+			blob=$(echo note $nr | git hash-object -w --stdin) &&
+			echo $sha1 | sed "s/^/0644 $blob 0	/"
+		done | git update-index --index-info &&
+		tree=$(git write-tree) &&
+		test_tick &&
+		commit=$(echo notes | git commit-tree $tree) &&
+		git update-ref refs/notes/commits $commit
+	} &&
+	git config core.notesRef refs/notes/commits
+	}
+}
+
+test_notes () {
+	count=$1 &&
+	git config core.notesRef refs/notes/commits &&
+	git log | grep "^    " > output &&
+	i=1 &&
+	while [ $i -le $count ]; do
+		echo "    $(($count-$i))" &&
+		echo "    note $i" &&
+		i=$(($i+1));
+	done > expect &&
+	git diff expect output
+}
+
+cat > time_notes << \EOF
+	mode=$1
+	i=1
+	while [ $i -lt $2 ]; do
+		case $1 in
+		no-notes)
+			GIT_NOTES_REF=non-existing; export GIT_NOTES_REF
+		;;
+		notes)
+			unset GIT_NOTES_REF
+		;;
+		esac
+		git log >/dev/null
+		i=$(($i+1))
+	done
+EOF
+
+time_notes () {
+	for mode in no-notes notes
+	do
+		echo $mode
+		/usr/bin/time sh ../time_notes $mode $1
+	done
+}
+
+for count in 10 100 1000 10000; do
+
+	mkdir $count
+	(cd $count;
+
+	test_expect_success "setup $count" "create_repo $count"
+
+	test_expect_success 'notes work' "test_notes $count"
+
+	test_expect_success 'notes timing' "time_notes 100"
+	)
+done
+
+test_done
Signature · 1 line
1.6.3.rc0.1.gf800
#17 lore raw

PATCH v2 5/5 Teach "-m <msg>" and "-F <file>" to "git notes edit"

In reply to Johan Herland

The "-m" and "-F" options are already the established method (in both git-commit and git-tag) to specify a commit/tag message without invoking the editor. This patch teaches "git notes edit" to respect the same options for specifying a notes message without invoking the editor.

Multiple "-m" and/or "-F" options are concatenated as separate paragraphs.

The patch also updates the "git notes" documentation and adds selftests for the new functionality. Unfortunately, the added selftests include a couple of lines with trailing whitespace (without these the test will fail). This may cause git to warn about "whitespace errors".

---
 Documentation/git-notes.txt |   16 ++++++++++-
 git-notes.sh                |   64 +++++++++++++++++++++++++++++++++++++-----
 t/t3301-notes.sh            |   35 +++++++++++++++++++++++
 3 files changed, 106 insertions(+), 9 deletions(-)
Documentation/git-notes.txt +15 -1
index 7136016..94cceb1 100644
@@ -8,7 +8,7 @@ git-notes - Add/inspect commit notes
 SYNOPSIS
 --------
 [verse]
-'git-notes' (edit | show) [commit]
+'git-notes' (edit [-F <file> | -m <msg>] | show) [commit]
 
 DESCRIPTION
 -----------
@@ -33,6 +33,20 @@ show::
 	Show the notes for a given commit (defaults to HEAD).
 
 
+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.
+
+-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
 ------
 Written by Johannes Schindelin <[email protected]>
git-notes.sh +56 -8

91 lines not on this page · raw message

t/t3301-notes.sh +35 -0
index 73e53be..9eaa338 100755
@@ -110,5 +110,40 @@ 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)' '
+	: > a4 &&
+	git add a4 &&
+	test_tick &&
+	git commit -m 4th &&
+	echo "xyzzy" > note5 &&
+	git notes edit -m spam -F note5 -m "foo
+bar
+baz"
+'
+
+cat > expect-m-and-F << EOF
+commit 15023535574ded8b1a89052b32673f84cf9582b8
+Author: A U Thor <[email protected]>
+Date:   Thu Apr 7 15:16:13 2005 -0700
+
+    4th
+
+Notes:
+    spam
+    
+    xyzzy
+    
+    foo
+    bar
+    baz
+EOF
+
+printf "\n" >> expect-m-and-F
+cat expect-multiline >> expect-m-and-F
+
+test_expect_success 'show -m and -F notes' '
+	git log -3 > output &&
+	test_cmp expect-m-and-F output
+'
 
 test_done
Signature · 1 line
1.6.3.rc0.1.gf800
#18 lore raw

PATCH 5/5 Teach "-m <msg>" and "-F <file>" to "git notes edit"

In reply to Johan Herland

The "-m" and "-F" options are already the established method (in both git-commit and git-tag) to specify a commit/tag message without invoking the editor. This patch teaches "git notes edit" to respect the same options for specifying a notes message without invoking the editor.

Multiple "-m" and/or "-F" options are concatenated as separate paragraphs.

The patch also updates the "git notes" documentation and adds selftests for the new functionality. Unfortunately, the added selftests include a couple of lines with trailing whitespace (without these the test will fail). This may cause git to warn about "whitespace errors".

---
 Documentation/git-notes.txt |   16 ++++++++++-
 git-notes.sh                |   64 +++++++++++++++++++++++++++++++++++++-----
 t/t3301-notes.sh            |   35 +++++++++++++++++++++++
 3 files changed, 106 insertions(+), 9 deletions(-)
Documentation/git-notes.txt +15 -1
index 7136016..94cceb1 100644
@@ -8,7 +8,7 @@ git-notes - Add/inspect commit notes
 SYNOPSIS
 --------
 [verse]
-'git-notes' (edit | show) [commit]
+'git-notes' (edit [-F <file> | -m <msg>] | show) [commit]
 
 DESCRIPTION
 -----------
@@ -33,6 +33,20 @@ show::
 	Show the notes for a given commit (defaults to HEAD).
 
 
+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.
+
+-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
 ------
 Written by Johannes Schindelin <[email protected]>
git-notes.sh +56 -8

91 lines not on this page · raw message

t/t3301-notes.sh +35 -0
index 73e53be..9eaa338 100755
@@ -110,5 +110,40 @@ 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)' '
+	: > a4 &&
+	git add a4 &&
+	test_tick &&
+	git commit -m 4th &&
+	echo "xyzzy" > note5 &&
+	git notes edit -m spam -F note5 -m "foo
+bar
+baz"
+'
+
+cat > expect-m-and-F << EOF
+commit 15023535574ded8b1a89052b32673f84cf9582b8
+Author: A U Thor <[email protected]>
+Date:   Thu Apr 7 15:16:13 2005 -0700
+
+    4th
+
+Notes:
+    spam
+    
+    xyzzy
+    
+    foo
+    bar
+    baz
+EOF
+
+printf "\n" >> expect-m-and-F
+cat expect-multiline >> expect-m-and-F
+
+test_expect_success 'show -m and -F notes' '
+	git log -3 > output &&
+	test_cmp expect-m-and-F output
+'
 
 test_done
Signature · 1 line
1.6.3.rc0.1.gf800