Re: [PATCH 0/5] RESEND: git notes

10 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 0/5] RESEND: git notes

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:46:47

Johan Herland [off-list ref] writes:
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).

Re: [PATCH 0/5] RESEND: git notes

From: Johan Herland <hidden>
Date: 2016-06-15 22:46:47

On Saturday 16 May 2009, Junio C Hamano wrote:
Johan Herland [off-list ref] writes:
quoted
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'.
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').
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).
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').
But I think there is a regression with a missing '$' here.
Yes. Thanks for catching.
    +	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?
Oh, so _that's_ what he did... I was puzzled by that "stray"
'GIT_NOTES_REF= '...

Yet another screwup on my part.
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).
Thanks for the review.
I will send an updated series with the above fixes.


...Johan

-- 
Johan Herland, [off-list ref]
www.herland.net

[PATCHv2 0/5] RESEND: git notes

From: Johan Herland <hidden>
Date: 2016-06-15 22:46:47

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

[PATCHv2 1/5] Introduce commit notes

From: Johan Herland <hidden>
Date: 2016-06-15 22:46:47

From: Johannes Schindelin <redacted>

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]

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Thomas Rast <redacted>
Signed-off-by: Tor Arne Vestbø <redacted>
Signed-off-by: Johan Herland <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
 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
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5dcad94..6875ecd 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -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
diff --git a/Makefile b/Makefile
index 6e21643..59e102c 100644
--- a/Makefile
+++ b/Makefile
@@ -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
diff --git a/cache.h b/cache.h
index b8503ad..8a830a6 100644
--- a/cache.h
+++ b/cache.h
@@ -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);
diff --git a/commit.c b/commit.c
index aa3b35b..cf72143 100644
--- a/commit.c
+++ b/commit.c
@@ -5,6 +5,7 @@
 #include "utf8.h"
 #include "diff.h"
 #include "revision.h"
+#include "notes.h"
 
 int save_commit_buffer = 1;
 
diff --git a/config.c b/config.c
index 1682273..d82b580 100644
--- a/config.c
+++ b/config.c
@@ -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);
 
diff --git a/environment.c b/environment.c
index 801a005..f445cfd 100644
--- a/environment.c
+++ b/environment.c
@@ -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;
diff --git a/notes.c b/notes.c
new file mode 100644
index 0000000..b0cf553
--- /dev/null
+++ b/notes.c
@@ -0,0 +1,69 @@
+#include "cache.h"
+#include "commit.h"
+#include "notes.h"
+#include "refs.h"
+#include "utf8.h"
+#include "strbuf.h"
+
+static int initialized;
+
+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];
+	char *msg, *msg_p;
+	unsigned long linelen, msglen;
+	enum object_type type;
+
+	if (!initialized) {
+		const char *env = getenv(GIT_NOTES_REF_ENVIRONMENT);
+		if (env)
+			notes_ref_name = getenv(GIT_NOTES_REF_ENVIRONMENT);
+		else if (!notes_ref_name)
+			notes_ref_name = GIT_NOTES_DEFAULT_REF;
+		if (notes_ref_name && read_ref(notes_ref_name, sha1))
+			notes_ref_name = NULL;
+		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))
+		return;
+
+	if (!(msg = read_sha1_file(sha1, &type, &msglen)) || !msglen ||
+			type != OBJ_BLOB)
+		return;
+
+	if (output_encoding && *output_encoding &&
+			strcmp(utf8, output_encoding)) {
+		char *reencoded = reencode_string(msg, output_encoding, utf8);
+		if (reencoded) {
+			free(msg);
+			msg = reencoded;
+			msglen = strlen(msg);
+		}
+	}
+
+	/* we will end the annotation by a newline anyway */
+	if (msglen && msg[msglen - 1] == '\n')
+		msglen--;
+
+	strbuf_addstr(sb, "\nNotes:\n");
+
+	for (msg_p = msg; msg_p < msg + msglen; msg_p += linelen + 1) {
+		linelen = strchrnul(msg_p, '\n') - msg_p;
+
+		strbuf_addstr(sb, "    ");
+		strbuf_add(sb, msg_p, linelen);
+		strbuf_addch(sb, '\n');
+	}
+
+	free(msg);
+}
diff --git a/notes.h b/notes.h
new file mode 100644
index 0000000..79d21b6
--- /dev/null
+++ b/notes.h
@@ -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
diff --git a/pretty.c b/pretty.c
index a0ef356..d30a964 100644
--- a/pretty.c
+++ b/pretty.c
@@ -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);
 }
-- 
1.6.3.rc0.1.gf800

[PATCHv2 2/5] Add a script to edit/inspect notes

From: Johan Herland <hidden>
Date: 2016-06-15 22:46:47

From: Johannes Schindelin <redacted>

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)]

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Tor Arne Vestbø <redacted>
Signed-off-by: Michael J Gruber <redacted>
Signed-off-by: Thomas Rast <redacted>
Signed-off-by: Christian Couder <redacted>
Signed-off-by: Johan Herland <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
 .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
diff --git a/.gitignore b/.gitignore
index 41c0b20..13221e7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -86,6 +86,7 @@ git-mktag
 git-mktree
 git-name-rev
 git-mv
+git-notes
 git-pack-redundant
 git-pack-objects
 git-pack-refs
diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
new file mode 100644
index 0000000..7136016
--- /dev/null
+++ b/Documentation/git-notes.txt
@@ -0,0 +1,46 @@
+git-notes(1)
+============
+
+NAME
+----
+git-notes - Add/inspect commit notes
+
+SYNOPSIS
+--------
+[verse]
+'git-notes' (edit | show) [commit]
+
+DESCRIPTION
+-----------
+This command allows you to add notes to commit messages, without
+changing the commit.  To discern these notes from the message stored
+in the commit object, the notes are indented like the message, after
+an unindented line saying "Notes:".
+
+To disable commit notes, you have to set the config variable
+core.notesRef to the empty string.  Alternatively, you can set it
+to a different ref, something like "refs/notes/bugzilla".  This setting
+can be overridden by the environment variable "GIT_NOTES_REF".
+
+
+SUBCOMMANDS
+-----------
+
+edit::
+	Edit the notes for a given commit (defaults to HEAD).
+
+show::
+	Show the notes for a given commit (defaults to HEAD).
+
+
+Author
+------
+Written by Johannes Schindelin <johannes.schindelin@gmx.de>
+
+Documentation
+-------------
+Documentation by Johannes Schindelin
+
+GIT
+---
+Part of the linkgit:git[7] suite
diff --git a/Makefile b/Makefile
index 59e102c..1d91a04 100644
--- a/Makefile
+++ b/Makefile
@@ -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
diff --git a/command-list.txt b/command-list.txt
index fb03a2e..4296941 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -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
diff --git a/git-notes.sh b/git-notes.sh
new file mode 100755
index 0000000..f06c254
--- /dev/null
+++ b/git-notes.sh
@@ -0,0 +1,73 @@
+#!/bin/sh
+
+USAGE="(edit | show) [commit]"
+. git-sh-setup
+
+test -n "$3" && usage
+
+test -z "$1" && usage
+ACTION="$1"; shift
+
+test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="$(git config core.notesref)"
+test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="refs/notes/commits"
+
+COMMIT=$(git rev-parse --verify --default HEAD "$@") ||
+die "Invalid commit: $@"
+
+case "$ACTION" in
+edit)
+	if [ "${GIT_NOTES_REF#refs/notes/}" = "$GIT_NOTES_REF" ]; then
+		die "Refusing to edit notes in $GIT_NOTES_REF (outside of refs/notes/)"
+	fi
+
+	MSG_FILE="$GIT_DIR/new-notes-$COMMIT"
+	GIT_INDEX_FILE="$MSG_FILE.idx"
+	export GIT_INDEX_FILE
+
+	trap '
+		test -f "$MSG_FILE" && rm "$MSG_FILE"
+		test -f "$GIT_INDEX_FILE" && rm "$GIT_INDEX_FILE"
+	' 0
+
+	GIT_NOTES_REF= git log -1 $COMMIT | sed "s/^/#/" > "$MSG_FILE"
+
+	CURRENT_HEAD=$(git show-ref "$GIT_NOTES_REF" | cut -f 1 -d ' ')
+	if [ -z "$CURRENT_HEAD" ]; then
+		PARENT=
+	else
+		PARENT="-p $CURRENT_HEAD"
+		git read-tree "$GIT_NOTES_REF" || die "Could not read index"
+		git cat-file blob :$COMMIT >> "$MSG_FILE" 2> /dev/null
+	fi
+
+	core_editor="$(git config core.editor)"
+	${GIT_EDITOR:-${core_editor:-${VISUAL:-${EDITOR:-vi}}}} "$MSG_FILE"
+
+	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"
+	else
+		test -z "$CURRENT_HEAD" &&
+			die "Will not initialise with empty tree"
+		git update-index --force-remove $COMMIT ||
+			die "Could not update index"
+	fi
+
+	TREE=$(git write-tree) || die "Could not write tree"
+	NEW_HEAD=$(echo Annotate $COMMIT | git commit-tree $TREE $PARENT) ||
+		die "Could not annotate"
+	git update-ref -m "Annotate $COMMIT" \
+		"$GIT_NOTES_REF" $NEW_HEAD $CURRENT_HEAD
+;;
+show)
+	git rev-parse -q --verify "$GIT_NOTES_REF":$COMMIT > /dev/null ||
+		die "No note for commit $COMMIT."
+	git show "$GIT_NOTES_REF":$COMMIT
+;;
+*)
+	usage
+esac
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
new file mode 100755
index 0000000..73e53be
--- /dev/null
+++ b/t/t3301-notes.sh
@@ -0,0 +1,114 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Johannes E. Schindelin
+#
+
+test_description='Test commit notes'
+
+. ./test-lib.sh
+
+cat > fake_editor.sh << \EOF
+echo "$MSG" > "$1"
+echo "$MSG" >& 2
+EOF
+chmod a+x fake_editor.sh
+VISUAL=./fake_editor.sh
+export VISUAL
+
+test_expect_success 'cannot annotate non-existing HEAD' '
+	(MSG=3 && export MSG && test_must_fail git notes edit)
+'
+
+test_expect_success setup '
+	: > a1 &&
+	git add a1 &&
+	test_tick &&
+	git commit -m 1st &&
+	: > a2 &&
+	git add a2 &&
+	test_tick &&
+	git commit -m 2nd
+'
+
+test_expect_success 'need valid notes ref' '
+	(MSG=1 GIT_NOTES_REF=/ && export MSG GIT_NOTES_REF &&
+	 test_must_fail git notes edit) &&
+	(MSG=2 GIT_NOTES_REF=/ && export MSG GIT_NOTES_REF &&
+	 test_must_fail git notes show)
+'
+
+test_expect_success 'refusing to edit in refs/heads/' '
+	(MSG=1 GIT_NOTES_REF=refs/heads/bogus &&
+	 export MSG GIT_NOTES_REF &&
+	 test_must_fail git notes edit)
+'
+
+test_expect_success 'refusing to edit in refs/remotes/' '
+	(MSG=1 GIT_NOTES_REF=refs/remotes/bogus &&
+	 export MSG GIT_NOTES_REF &&
+	 test_must_fail git notes edit)
+'
+
+# 1 indicates caught gracefully by die, 128 means git-show barked
+test_expect_success 'handle empty notes gracefully' '
+	git notes show ; test 1 = $?
+'
+
+test_expect_success 'create notes' '
+	git config core.notesRef refs/notes/commits &&
+	MSG=b1 git notes edit &&
+	test ! -f .git/new-notes &&
+	test 1 = $(git ls-tree refs/notes/commits | wc -l) &&
+	test b1 = $(git notes show) &&
+	git show HEAD^ &&
+	test_must_fail git notes show HEAD^
+'
+
+cat > expect << EOF
+commit 268048bfb8a1fb38e703baceb8ab235421bf80c5
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:14:13 2005 -0700
+
+    2nd
+
+Notes:
+    b1
+EOF
+
+test_expect_success 'show notes' '
+	! (git cat-file commit HEAD | grep b1) &&
+	git log -1 > output &&
+	test_cmp expect output
+'
+test_expect_success 'create multi-line notes (setup)' '
+	: > a3 &&
+	git add a3 &&
+	test_tick &&
+	git commit -m 3rd &&
+	MSG="b3
+c3c3c3c3
+d3d3d3" git notes edit
+'
+
+cat > expect-multiline << EOF
+commit 1584215f1d29c65e99c6c6848626553fdd07fd75
+Author: A U Thor <author@example.com>
+Date:   Thu Apr 7 15:15:13 2005 -0700
+
+    3rd
+
+Notes:
+    b3
+    c3c3c3c3
+    d3d3d3
+EOF
+
+printf "\n" >> expect-multiline
+cat expect >> expect-multiline
+
+test_expect_success 'show multi-line notes' '
+	git log -2 > output &&
+	test_cmp expect-multiline output
+'
+
+test_done
-- 
1.6.3.rc0.1.gf800

[PATCHv2 3/5] Speed up git notes lookup

From: Johan Herland <hidden>
Date: 2016-06-15 22:46:47

From: Johannes Schindelin <redacted>

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()]

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Johan Herland <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
 notes.c |  113 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 files changed, 102 insertions(+), 11 deletions(-)
diff --git a/notes.c b/notes.c
index b0cf553..bd73784 100644
--- a/notes.c
+++ b/notes.c
@@ -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 ||
-- 
1.6.3.rc0.1.gf800

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

From: Johan Herland <hidden>
Date: 2016-06-15 22:46:47

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

Signed-off-by: Johan Herland <redacted>
---
 Documentation/git-notes.txt |   16 ++++++++++-
 git-notes.sh                |   64 +++++++++++++++++++++++++++++++++++++-----
 t/t3301-notes.sh            |   35 +++++++++++++++++++++++
 3 files changed, 106 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 7136016..94cceb1 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -8,7 +8,7 @@ git-notes - Add/inspect commit notes
 SYNOPSIS
 --------
 [verse]
-'git-notes' (edit | 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 <johannes.schindelin@gmx.de>
diff --git a/git-notes.sh b/git-notes.sh
index f06c254..e642e47 100755
--- a/git-notes.sh
+++ b/git-notes.sh
@@ -1,16 +1,59 @@
 #!/bin/sh
 
-USAGE="(edit | show) [commit]"
+USAGE="(edit [-F <file> | -m <msg>] | show) [commit]"
 . git-sh-setup
 
-test -n "$3" && usage
-
 test -z "$1" && usage
 ACTION="$1"; shift
 
 test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="$(git config core.notesref)"
 test -z "$GIT_NOTES_REF" && GIT_NOTES_REF="refs/notes/commits"
 
+MESSAGE=
+while test $# != 0
+do
+	case "$1" in
+	-m)
+		test "$ACTION" = "edit" || usage
+		shift
+		if test "$#" = "0"; then
+			die "error: option -m needs an argument"
+		else
+			if [ -z "$MESSAGE" ]; then
+				MESSAGE="$1"
+			else
+				MESSAGE="$MESSAGE
+
+$1"
+			fi
+			shift
+		fi
+		;;
+	-F)
+		test "$ACTION" = "edit" || usage
+		shift
+		if test "$#" = "0"; then
+			die "error: option -F needs an argument"
+		else
+			if [ -z "$MESSAGE" ]; then
+				MESSAGE="$(cat "$1")"
+			else
+				MESSAGE="$MESSAGE
+
+$(cat "$1")"
+			fi
+			shift
+		fi
+		;;
+	-*)
+		usage
+		;;
+	*)
+		break
+		;;
+	esac
+done
+
 COMMIT=$(git rev-parse --verify --default HEAD "$@") ||
 die "Invalid commit: $@"
 
@@ -29,19 +72,24 @@ edit)
 		test -f "$GIT_INDEX_FILE" && rm "$GIT_INDEX_FILE"
 	' 0
 
-	GIT_NOTES_REF= git log -1 $COMMIT | sed "s/^/#/" > "$MSG_FILE"
-
 	CURRENT_HEAD=$(git show-ref "$GIT_NOTES_REF" | cut -f 1 -d ' ')
 	if [ -z "$CURRENT_HEAD" ]; then
 		PARENT=
 	else
 		PARENT="-p $CURRENT_HEAD"
 		git read-tree "$GIT_NOTES_REF" || die "Could not read index"
-		git cat-file blob :$COMMIT >> "$MSG_FILE" 2> /dev/null
 	fi
 
-	core_editor="$(git config core.editor)"
-	${GIT_EDITOR:-${core_editor:-${VISUAL:-${EDITOR:-vi}}}} "$MSG_FILE"
+	if [ -z "$MESSAGE" ]; then
+		GIT_NOTES_REF= git log -1 $COMMIT | sed "s/^/#/" > "$MSG_FILE"
+		if [ ! -z "$CURRENT_HEAD" ]; then
+			git cat-file blob :$COMMIT >> "$MSG_FILE" 2> /dev/null
+		fi
+		core_editor="$(git config core.editor)"
+		${GIT_EDITOR:-${core_editor:-${VISUAL:-${EDITOR:-vi}}}} "$MSG_FILE"
+	else
+		echo "$MESSAGE" > "$MSG_FILE"
+	fi
 
 	grep -v ^# < "$MSG_FILE" | git stripspace > "$MSG_FILE".processed
 	mv "$MSG_FILE".processed "$MSG_FILE"
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 73e53be..9eaa338 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -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 <author@example.com>
+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
-- 
1.6.3.rc0.1.gf800

[PATCHv2 4/5] Add an expensive test for git-notes

From: Johan Herland <hidden>
Date: 2016-06-15 22:46:47

From: Johannes Schindelin <redacted>

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

Signed-off-by: Johannes Schindelin <redacted>
Signed-off-by: Johan Herland <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
 t/t3302-notes-index-expensive.sh |   98 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 98 insertions(+), 0 deletions(-)
 create mode 100755 t/t3302-notes-index-expensive.sh
diff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh
new file mode 100755
index 0000000..0ef3e95
--- /dev/null
+++ b/t/t3302-notes-index-expensive.sh
@@ -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
-- 
1.6.3.rc0.1.gf800

Re: [PATCHv2 0/5] RESEND: git notes

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:46:48

Hi,

On Sat, 16 May 2009, Johan Herland wrote:
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

Re: [PATCHv2 0/5] RESEND: git notes

From: Johan Herland <hidden>
Date: 2016-06-15 22:46:48

On Wednesday 20 May 2009, Johannes Schindelin wrote:
How's the enhanced note-tree lookup going?
Haven't had time to work on it, yet.
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

-- 
Johan Herland, [off-list ref]
www.herland.net
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help