* [PATCH 0/5] RESEND: git notes
@ 2009-05-16 1:45 Johan Herland
2009-05-16 1:45 ` [PATCH 1/5] Introduce commit notes Johan Herland
` (6 more replies)
0 siblings, 7 replies; 19+ messages in thread
From: Johan Herland @ 2009-05-16 1:45 UTC (permalink / raw)
To: gitster; +Cc: git, johannes.schindelin, trast, tavestbo, johan, git, chriscool
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/5] Introduce commit notes
2009-05-16 1:45 [PATCH 0/5] RESEND: git notes Johan Herland
@ 2009-05-16 1:45 ` Johan Herland
2009-05-16 1:45 ` [PATCH 2/5] Add a script to edit/inspect notes Johan Herland
` (5 subsequent siblings)
6 siblings, 0 replies; 19+ messages in thread
From: Johan Herland @ 2009-05-16 1:45 UTC (permalink / raw)
To: gitster
Cc: git, johannes.schindelin, trast, tavestbo, johan, git, chriscool,
Johannes Schindelin
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
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 <johannes.schindelin@gmx.de>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Tor Arne Vestbø <tavestbo@trolltech.com>
Signed-off-by: Johan Herland <johan@herland.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
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 2c03162..c4c039b 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 a579310..8601bdd 100644
--- a/Makefile
+++ b/Makefile
@@ -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
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/5] Add a script to edit/inspect notes
2009-05-16 1:45 [PATCH 0/5] RESEND: git notes Johan Herland
2009-05-16 1:45 ` [PATCH 1/5] Introduce commit notes Johan Herland
@ 2009-05-16 1:45 ` Johan Herland
2009-05-16 1:45 ` [PATCH 3/5] Speed up git notes lookup Johan Herland
` (4 subsequent siblings)
6 siblings, 0 replies; 19+ messages in thread
From: Johan Herland @ 2009-05-16 1:45 UTC (permalink / raw)
To: gitster
Cc: git, johannes.schindelin, trast, tavestbo, johan, git, chriscool,
Johannes Schindelin
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
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]
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Tor Arne Vestbø <tavestbo@trolltech.com>
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Johan Herland <johan@herland.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
.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 8601bdd..ab4a2b7 100644
--- a/Makefile
+++ b/Makefile
@@ -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
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..7c3b8b9
--- /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 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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/5] Speed up git notes lookup
2009-05-16 1:45 [PATCH 0/5] RESEND: git notes Johan Herland
2009-05-16 1:45 ` [PATCH 1/5] Introduce commit notes Johan Herland
2009-05-16 1:45 ` [PATCH 2/5] Add a script to edit/inspect notes Johan Herland
@ 2009-05-16 1:45 ` Johan Herland
2009-05-16 1:45 ` [PATCH 4/5] Add an expensive test for git-notes Johan Herland
` (3 subsequent siblings)
6 siblings, 0 replies; 19+ messages in thread
From: Johan Herland @ 2009-05-16 1:45 UTC (permalink / raw)
To: gitster
Cc: git, johannes.schindelin, trast, tavestbo, johan, git, chriscool,
Johannes Schindelin
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
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 <johannes.schindelin@gmx.de>
Signed-off-by: Johan Herland <johan@herland.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 4/5] Add an expensive test for git-notes
2009-05-16 1:45 [PATCH 0/5] RESEND: git notes Johan Herland
` (2 preceding siblings ...)
2009-05-16 1:45 ` [PATCH 3/5] Speed up git notes lookup Johan Herland
@ 2009-05-16 1:45 ` Johan Herland
2009-05-16 1:45 ` [PATCH 5/5] Teach "-m <msg>" and "-F <file>" to "git notes edit" Johan Herland
` (2 subsequent siblings)
6 siblings, 0 replies; 19+ messages in thread
From: Johan Herland @ 2009-05-16 1:45 UTC (permalink / raw)
To: gitster
Cc: git, johannes.schindelin, trast, tavestbo, johan, git, chriscool,
Johannes Schindelin
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
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 <johannes.schindelin@gmx.de>
Signed-off-by: Johan Herland <johan@herland.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 5/5] Teach "-m <msg>" and "-F <file>" to "git notes edit"
2009-05-16 1:45 [PATCH 0/5] RESEND: git notes Johan Herland
` (3 preceding siblings ...)
2009-05-16 1:45 ` [PATCH 4/5] Add an expensive test for git-notes Johan Herland
@ 2009-05-16 1:45 ` Johan Herland
2009-05-16 7:06 ` [PATCH 0/5] RESEND: git notes Junio C Hamano
2009-05-16 15:06 ` [PATCH " git
6 siblings, 0 replies; 19+ messages in thread
From: Johan Herland @ 2009-05-16 1:45 UTC (permalink / raw)
To: gitster; +Cc: git, johannes.schindelin, trast, tavestbo, johan, git, chriscool
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 <johan@herland.net>
---
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 7c3b8b9..0119f1f 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 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 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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 0/5] RESEND: git notes
2009-05-16 1:45 [PATCH 0/5] RESEND: git notes Johan Herland
` (4 preceding siblings ...)
2009-05-16 1:45 ` [PATCH 5/5] Teach "-m <msg>" and "-F <file>" to "git notes edit" Johan Herland
@ 2009-05-16 7:06 ` Junio C Hamano
2009-05-16 11:20 ` Johan Herland
2009-05-16 15:06 ` [PATCH " git
6 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2009-05-16 7:06 UTC (permalink / raw)
To: Johan Herland; +Cc: git, johannes.schindelin, trast, tavestbo, git, chriscool
Johan Herland <johan@herland.net> 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).
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/5] RESEND: git notes
2009-05-16 7:06 ` [PATCH 0/5] RESEND: git notes Junio C Hamano
@ 2009-05-16 11:20 ` Johan Herland
2009-05-16 11:44 ` [PATCHv2 " Johan Herland
0 siblings, 1 reply; 19+ messages in thread
From: Johan Herland @ 2009-05-16 11:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, johannes.schindelin, trast, tavestbo, git, chriscool
On Saturday 16 May 2009, Junio C Hamano wrote:
> Johan Herland <johan@herland.net> writes:
> > 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, <johan@herland.net>
www.herland.net
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCHv2 0/5] RESEND: git notes
2009-05-16 11:20 ` Johan Herland
@ 2009-05-16 11:44 ` Johan Herland
2009-05-16 11:44 ` [PATCHv2 1/5] Introduce commit notes Johan Herland
` (5 more replies)
0 siblings, 6 replies; 19+ messages in thread
From: Johan Herland @ 2009-05-16 11:44 UTC (permalink / raw)
To: gitster; +Cc: git, johannes.schindelin, trast, tavestbo, johan, git, chriscool
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCHv2 1/5] Introduce commit notes
2009-05-16 11:44 ` [PATCHv2 " Johan Herland
@ 2009-05-16 11:44 ` Johan Herland
2009-05-16 11:44 ` [PATCHv2 2/5] Add a script to edit/inspect notes Johan Herland
` (4 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: Johan Herland @ 2009-05-16 11:44 UTC (permalink / raw)
To: gitster
Cc: git, johannes.schindelin, trast, tavestbo, johan, git, chriscool,
Johannes Schindelin
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
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 <johannes.schindelin@gmx.de>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Tor Arne Vestbø <tavestbo@trolltech.com>
Signed-off-by: Johan Herland <johan@herland.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCHv2 2/5] Add a script to edit/inspect notes
2009-05-16 11:44 ` [PATCHv2 " Johan Herland
2009-05-16 11:44 ` [PATCHv2 1/5] Introduce commit notes Johan Herland
@ 2009-05-16 11:44 ` Johan Herland
2009-05-16 11:44 ` [PATCHv2 3/5] Speed up git notes lookup Johan Herland
` (3 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: Johan Herland @ 2009-05-16 11:44 UTC (permalink / raw)
To: gitster
Cc: git, johannes.schindelin, trast, tavestbo, johan, git, chriscool,
Johannes Schindelin
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
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 <johannes.schindelin@gmx.de>
Signed-off-by: Tor Arne Vestbø <tavestbo@trolltech.com>
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Johan Herland <johan@herland.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
.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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCHv2 3/5] Speed up git notes lookup
2009-05-16 11:44 ` [PATCHv2 " Johan Herland
2009-05-16 11:44 ` [PATCHv2 1/5] Introduce commit notes Johan Herland
2009-05-16 11:44 ` [PATCHv2 2/5] Add a script to edit/inspect notes Johan Herland
@ 2009-05-16 11:44 ` Johan Herland
2009-05-16 11:44 ` [PATCHv2 4/5] Add an expensive test for git-notes Johan Herland
` (2 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: Johan Herland @ 2009-05-16 11:44 UTC (permalink / raw)
To: gitster
Cc: git, johannes.schindelin, trast, tavestbo, johan, git, chriscool,
Johannes Schindelin
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
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 <johannes.schindelin@gmx.de>
Signed-off-by: Johan Herland <johan@herland.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCHv2 4/5] Add an expensive test for git-notes
2009-05-16 11:44 ` [PATCHv2 " Johan Herland
` (2 preceding siblings ...)
2009-05-16 11:44 ` [PATCHv2 3/5] Speed up git notes lookup Johan Herland
@ 2009-05-16 11:44 ` Johan Herland
2009-05-16 11:44 ` [PATCHv2 5/5] Teach "-m <msg>" and "-F <file>" to "git notes edit" Johan Herland
2009-05-20 10:25 ` [PATCHv2 0/5] RESEND: git notes Johannes Schindelin
5 siblings, 0 replies; 19+ messages in thread
From: Johan Herland @ 2009-05-16 11:44 UTC (permalink / raw)
To: gitster
Cc: git, johannes.schindelin, trast, tavestbo, johan, git, chriscool,
Johannes Schindelin
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
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 <johannes.schindelin@gmx.de>
Signed-off-by: Johan Herland <johan@herland.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCHv2 5/5] Teach "-m <msg>" and "-F <file>" to "git notes edit"
2009-05-16 11:44 ` [PATCHv2 " Johan Herland
` (3 preceding siblings ...)
2009-05-16 11:44 ` [PATCHv2 4/5] Add an expensive test for git-notes Johan Herland
@ 2009-05-16 11:44 ` Johan Herland
2009-05-20 10:25 ` [PATCHv2 0/5] RESEND: git notes Johannes Schindelin
5 siblings, 0 replies; 19+ messages in thread
From: Johan Herland @ 2009-05-16 11:44 UTC (permalink / raw)
To: gitster; +Cc: git, johannes.schindelin, trast, tavestbo, johan, git, chriscool
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 <johan@herland.net>
---
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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 0/5] RESEND: git notes
2009-05-16 1:45 [PATCH 0/5] RESEND: git notes Johan Herland
` (5 preceding siblings ...)
2009-05-16 7:06 ` [PATCH 0/5] RESEND: git notes Junio C Hamano
@ 2009-05-16 15:06 ` git
2009-05-16 16:01 ` Johan Herland
2009-05-17 16:31 ` Johannes Schindelin
6 siblings, 2 replies; 19+ messages in thread
From: git @ 2009-05-16 15:06 UTC (permalink / raw)
To: Johan Herland
Cc: gitster, git, johannes.schindelin, trast, tavestbo, chriscool
Johan Herland venit, vidit, dixit 16.05.2009 03:45:
> 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.
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
>
>
> 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
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/5] RESEND: git notes
2009-05-16 15:06 ` [PATCH " git
@ 2009-05-16 16:01 ` Johan Herland
2009-05-17 16:31 ` Johannes Schindelin
1 sibling, 0 replies; 19+ messages in thread
From: Johan Herland @ 2009-05-16 16:01 UTC (permalink / raw)
To: git; +Cc: gitster, git, johannes.schindelin, trast, tavestbo, chriscool
On Saturday 16 May 2009, git@drmicha.warpmail.net wrote:
> Johan Herland venit, vidit, dixit 16.05.2009 03:45:
> > 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.
>
> 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).
> 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
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/5] RESEND: git notes
2009-05-16 15:06 ` [PATCH " git
2009-05-16 16:01 ` Johan Herland
@ 2009-05-17 16:31 ` Johannes Schindelin
1 sibling, 0 replies; 19+ messages in thread
From: Johannes Schindelin @ 2009-05-17 16:31 UTC (permalink / raw)
To: git; +Cc: Johan Herland, gitster, git, trast, tavestbo, chriscool
Hi,
On Sat, 16 May 2009, git@drmicha.warpmail.net wrote:
> Johan Herland venit, vidit, dixit 16.05.2009 03:45:
>
> > 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.
> 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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCHv2 0/5] RESEND: git notes
2009-05-16 11:44 ` [PATCHv2 " Johan Herland
` (4 preceding siblings ...)
2009-05-16 11:44 ` [PATCHv2 5/5] Teach "-m <msg>" and "-F <file>" to "git notes edit" Johan Herland
@ 2009-05-20 10:25 ` Johannes Schindelin
2009-05-20 11:54 ` Johan Herland
5 siblings, 1 reply; 19+ messages in thread
From: Johannes Schindelin @ 2009-05-20 10:25 UTC (permalink / raw)
To: Johan Herland; +Cc: gitster, git, trast, tavestbo, git, chriscool
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCHv2 0/5] RESEND: git notes
2009-05-20 10:25 ` [PATCHv2 0/5] RESEND: git notes Johannes Schindelin
@ 2009-05-20 11:54 ` Johan Herland
0 siblings, 0 replies; 19+ messages in thread
From: Johan Herland @ 2009-05-20 11:54 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: gitster, git, trast, tavestbo, git, chriscool
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, <johan@herland.net>
www.herland.net
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2009-05-20 11:56 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-05-16 1:45 [PATCH 0/5] RESEND: git notes Johan Herland
2009-05-16 1:45 ` [PATCH 1/5] Introduce commit notes Johan Herland
2009-05-16 1:45 ` [PATCH 2/5] Add a script to edit/inspect notes Johan Herland
2009-05-16 1:45 ` [PATCH 3/5] Speed up git notes lookup Johan Herland
2009-05-16 1:45 ` [PATCH 4/5] Add an expensive test for git-notes Johan Herland
2009-05-16 1:45 ` [PATCH 5/5] Teach "-m <msg>" and "-F <file>" to "git notes edit" Johan Herland
2009-05-16 7:06 ` [PATCH 0/5] RESEND: git notes Junio C Hamano
2009-05-16 11:20 ` Johan Herland
2009-05-16 11:44 ` [PATCHv2 " Johan Herland
2009-05-16 11:44 ` [PATCHv2 1/5] Introduce commit notes Johan Herland
2009-05-16 11:44 ` [PATCHv2 2/5] Add a script to edit/inspect notes Johan Herland
2009-05-16 11:44 ` [PATCHv2 3/5] Speed up git notes lookup Johan Herland
2009-05-16 11:44 ` [PATCHv2 4/5] Add an expensive test for git-notes Johan Herland
2009-05-16 11:44 ` [PATCHv2 5/5] Teach "-m <msg>" and "-F <file>" to "git notes edit" Johan Herland
2009-05-20 10:25 ` [PATCHv2 0/5] RESEND: git notes Johannes Schindelin
2009-05-20 11:54 ` Johan Herland
2009-05-16 15:06 ` [PATCH " git
2009-05-16 16:01 ` Johan Herland
2009-05-17 16:31 ` Johannes Schindelin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).