* [PATCHv6 05/14] Teach "-m <msg>" and "-F <file>" to "git notes edit"
From: Johan Herland @ 2009-09-12 16:08 UTC (permalink / raw)
To: gitster
Cc: git, johan, Johannes.Schindelin, trast, tavestbo, git, chriscool,
spearce
In-Reply-To: <200909121752.07523.johan@herland.net>
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".
This patch has been improved by the following contributions:
- Thomas Rast: fix trailing whitespace in t3301
Signed-off-by: Johan Herland <johan@herland.net>
---
Documentation/git-notes.txt | 16 ++++++++++-
git-notes.sh | 64 +++++++++++++++++++++++++++++++++++++-----
t/t3301-notes.sh | 36 ++++++++++++++++++++++++
3 files changed, 107 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..1e34f48 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -110,5 +110,41 @@ 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"
+'
+
+whitespace=" "
+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
+$whitespace
+ xyzzy
+$whitespace
+ 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.4.304.g1365c.dirty
^ permalink raw reply related
* [PATCHv6 07/14] t3302-notes-index-expensive: Speed up create_repo()
From: Johan Herland @ 2009-09-12 16:08 UTC (permalink / raw)
To: gitster
Cc: git, johan, Johannes.Schindelin, trast, tavestbo, git, chriscool,
spearce
In-Reply-To: <200909121752.07523.johan@herland.net>
Creating repos with 10/100/1000/10000 commits and notes takes a lot of time.
However, using git-fast-import to do the job is a lot more efficient than
using plumbing commands to do the same.
This patch decreases the overall run-time of this test on my machine from
~3 to ~1 minutes.
Signed-off-by: Johan Herland <johan@herland.net>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
t/t3302-notes-index-expensive.sh | 74 ++++++++++++++++++++++++--------------
1 files changed, 47 insertions(+), 27 deletions(-)
diff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh
index 0ef3e95..ee84fc4 100755
--- a/t/t3302-notes-index-expensive.sh
+++ b/t/t3302-notes-index-expensive.sh
@@ -16,30 +16,50 @@ test -z "$GIT_NOTES_TIMING_TESTS" && {
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) &&
+ (
+ while [ $nr -lt $number_of_commits ]; do
+ nr=$(($nr+1))
+ mark=$(($nr+$nr))
+ notemark=$(($mark+1))
+ test_tick &&
+ cat <<INPUT_END &&
+commit refs/heads/master
+mark :$mark
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+commit #$nr
+COMMIT
+
+M 644 inline file
+data <<EOF
+file in commit #$nr
+EOF
+
+blob
+mark :$notemark
+data <<EOF
+note for commit #$nr
+EOF
+
+INPUT_END
+
+ echo "N :$notemark :$mark" >> note_commit
+ done &&
test_tick &&
- commit=$(echo notes | git commit-tree $tree) &&
- git update-ref refs/notes/commits $commit
- } &&
+ cat <<INPUT_END &&
+commit refs/notes/commits
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+notes
+COMMIT
+
+INPUT_END
+
+ cat note_commit
+ ) |
+ git fast-import --quiet &&
git config core.notesRef refs/notes/commits
}
}
@@ -48,13 +68,13 @@ 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));
+ i=$count &&
+ while [ $i -gt 0 ]; do
+ echo " commit #$i" &&
+ echo " note for commit #$i" &&
+ i=$(($i-1));
done > expect &&
- git diff expect output
+ test_cmp expect output
}
cat > time_notes << \EOF
--
1.6.4.304.g1365c.dirty
^ permalink raw reply related
* [PATCHv6 08/14] Add flags to get_commit_notes() to control the format of the note string
From: Johan Herland @ 2009-09-12 16:08 UTC (permalink / raw)
To: gitster
Cc: git, johan, Johannes.Schindelin, trast, tavestbo, git, chriscool,
spearce
In-Reply-To: <200909121752.07523.johan@herland.net>
This patch adds the following flags to get_commit_notes() for adjusting the
format of the produced note string:
- NOTES_SHOW_HEADER: Print "Notes:" line before the notes contents
- NOTES_INDENT: Indent notes contents by 4 spaces
Suggested-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johan Herland <johan@herland.net>
---
notes.c | 8 +++++---
notes.h | 5 ++++-
pretty.c | 3 ++-
3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/notes.c b/notes.c
index 9172154..84c30c1 100644
--- a/notes.c
+++ b/notes.c
@@ -106,7 +106,7 @@ static unsigned char *lookup_notes(const unsigned char *commit_sha1)
}
void get_commit_notes(const struct commit *commit, struct strbuf *sb,
- const char *output_encoding)
+ const char *output_encoding, int flags)
{
static const char utf8[] = "utf-8";
unsigned char *sha1;
@@ -146,12 +146,14 @@ void get_commit_notes(const struct commit *commit, struct strbuf *sb,
if (msglen && msg[msglen - 1] == '\n')
msglen--;
- strbuf_addstr(sb, "\nNotes:\n");
+ if (flags & NOTES_SHOW_HEADER)
+ 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, " ");
+ if (flags & NOTES_INDENT)
+ strbuf_addstr(sb, " ");
strbuf_add(sb, msg_p, linelen);
strbuf_addch(sb, '\n');
}
diff --git a/notes.h b/notes.h
index 79d21b6..7f3eed4 100644
--- a/notes.h
+++ b/notes.h
@@ -1,7 +1,10 @@
#ifndef NOTES_H
#define NOTES_H
+#define NOTES_SHOW_HEADER 1
+#define NOTES_INDENT 2
+
void get_commit_notes(const struct commit *commit, struct strbuf *sb,
- const char *output_encoding);
+ const char *output_encoding, int flags);
#endif
diff --git a/pretty.c b/pretty.c
index e25db81..01eadd0 100644
--- a/pretty.c
+++ b/pretty.c
@@ -978,7 +978,8 @@ void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
strbuf_addch(sb, '\n');
if (fmt != CMIT_FMT_ONELINE)
- get_commit_notes(commit, sb, encoding);
+ get_commit_notes(commit, sb, encoding,
+ NOTES_SHOW_HEADER | NOTES_INDENT);
free(reencoded);
}
--
1.6.4.304.g1365c.dirty
^ permalink raw reply related
* [PATCHv6 06/14] fast-import: Add support for importing commit notes
From: Johan Herland @ 2009-09-12 16:08 UTC (permalink / raw)
To: gitster
Cc: git, johan, Johannes.Schindelin, trast, tavestbo, git, chriscool,
spearce
In-Reply-To: <200909121752.07523.johan@herland.net>
Introduce a 'notemodify' subcommand of the 'commit' command. This subcommand
is similar to 'filemodify', except that no mode is supplied (all notes have
mode 0644), and the path is set to the hex SHA1 of the given "comittish".
This enables fast import of note objects along with their associated commits,
since the notes can now be named using the mark references of their
corresponding commits.
The patch also includes a test case of the added functionality.
Signed-off-by: Johan Herland <johan@herland.net>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
---
Documentation/git-fast-import.txt | 45 +++++++++--
fast-import.c | 88 +++++++++++++++++++-
t/t9300-fast-import.sh | 166 +++++++++++++++++++++++++++++++++++++
3 files changed, 289 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index f1c94b4..bb198c2 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -325,7 +325,7 @@ change to the project.
data
('from' SP <committish> LF)?
('merge' SP <committish> LF)?
- (filemodify | filedelete | filecopy | filerename | filedeleteall)*
+ (filemodify | filedelete | filecopy | filerename | filedeleteall | notemodify)*
LF?
....
@@ -348,14 +348,13 @@ commit message use a 0 length data. Commit messages are free-form
and are not interpreted by Git. Currently they must be encoded in
UTF-8, as fast-import does not permit other encodings to be specified.
-Zero or more `filemodify`, `filedelete`, `filecopy`, `filerename`
-and `filedeleteall` commands
+Zero or more `filemodify`, `filedelete`, `filecopy`, `filerename`,
+`filedeleteall` and `notemodify` commands
may be included to update the contents of the branch prior to
creating the commit. These commands may be supplied in any order.
However it is recommended that a `filedeleteall` command precede
-all `filemodify`, `filecopy` and `filerename` commands in the same
-commit, as `filedeleteall`
-wipes the branch clean (see below).
+all `filemodify`, `filecopy`, `filerename` and `notemodify` commands in
+the same commit, as `filedeleteall` wipes the branch clean (see below).
The `LF` after the command is optional (it used to be required).
@@ -604,6 +603,40 @@ more memory per active branch (less than 1 MiB for even most large
projects); so frontends that can easily obtain only the affected
paths for a commit are encouraged to do so.
+`notemodify`
+^^^^^^^^^^^^
+Included in a `commit` command to add a new note (annotating a given
+commit) or change the content of an existing note. This command has
+two different means of specifying the content of the note.
+
+External data format::
+ The data content for the note was already supplied by a prior
+ `blob` command. The frontend just needs to connect it to the
+ commit that is to be annotated.
++
+....
+ 'N' SP <dataref> SP <committish> LF
+....
++
+Here `<dataref>` can be either a mark reference (`:<idnum>`)
+set by a prior `blob` command, or a full 40-byte SHA-1 of an
+existing Git blob object.
+
+Inline data format::
+ The data content for the note has not been supplied yet.
+ The frontend wants to supply it as part of this modify
+ command.
++
+....
+ 'N' SP 'inline' SP <committish> LF
+ data
+....
++
+See below for a detailed description of the `data` command.
+
+In both formats `<committish>` is any of the commit specification
+expressions also accepted by `from` (see above).
+
`mark`
~~~~~~
Arranges for fast-import to save a reference to the current object, allowing
diff --git a/fast-import.c b/fast-import.c
index dcfb8fa..1e91358 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -22,8 +22,8 @@ Format of STDIN stream:
('author' sp name sp '<' email '>' sp when lf)?
'committer' sp name sp '<' email '>' sp when lf
commit_msg
- ('from' sp (ref_str | hexsha1 | sha1exp_str | idnum) lf)?
- ('merge' sp (ref_str | hexsha1 | sha1exp_str | idnum) lf)*
+ ('from' sp committish lf)?
+ ('merge' sp committish lf)*
file_change*
lf?;
commit_msg ::= data;
@@ -41,15 +41,18 @@ Format of STDIN stream:
file_obm ::= 'M' sp mode sp (hexsha1 | idnum) sp path_str lf;
file_inm ::= 'M' sp mode sp 'inline' sp path_str lf
data;
+ note_obm ::= 'N' sp (hexsha1 | idnum) sp committish lf;
+ note_inm ::= 'N' sp 'inline' sp committish lf
+ data;
new_tag ::= 'tag' sp tag_str lf
- 'from' sp (ref_str | hexsha1 | sha1exp_str | idnum) lf
+ 'from' sp committish lf
('tagger' sp name sp '<' email '>' sp when lf)?
tag_msg;
tag_msg ::= data;
reset_branch ::= 'reset' sp ref_str lf
- ('from' sp (ref_str | hexsha1 | sha1exp_str | idnum) lf)?
+ ('from' sp committish lf)?
lf?;
checkpoint ::= 'checkpoint' lf
@@ -88,6 +91,7 @@ Format of STDIN stream:
# stream formatting is: \, " and LF. Otherwise these values
# are UTF8.
#
+ committish ::= (ref_str | hexsha1 | sha1exp_str | idnum);
ref_str ::= ref;
sha1exp_str ::= sha1exp;
tag_str ::= tag;
@@ -2053,6 +2057,80 @@ static void file_change_cr(struct branch *b, int rename)
leaf.tree);
}
+static void note_change_n(struct branch *b)
+{
+ const char *p = command_buf.buf + 2;
+ static struct strbuf uq = STRBUF_INIT;
+ struct object_entry *oe = oe;
+ struct branch *s;
+ unsigned char sha1[20], commit_sha1[20];
+ uint16_t inline_data = 0;
+
+ /* <dataref> or 'inline' */
+ if (*p == ':') {
+ char *x;
+ oe = find_mark(strtoumax(p + 1, &x, 10));
+ hashcpy(sha1, oe->sha1);
+ p = x;
+ } else if (!prefixcmp(p, "inline")) {
+ inline_data = 1;
+ p += 6;
+ } else {
+ if (get_sha1_hex(p, sha1))
+ die("Invalid SHA1: %s", command_buf.buf);
+ oe = find_object(sha1);
+ p += 40;
+ }
+ if (*p++ != ' ')
+ die("Missing space after SHA1: %s", command_buf.buf);
+
+ /* <committish> */
+ s = lookup_branch(p);
+ if (s) {
+ hashcpy(commit_sha1, s->sha1);
+ } else if (*p == ':') {
+ uintmax_t commit_mark = strtoumax(p + 1, NULL, 10);
+ struct object_entry *commit_oe = find_mark(commit_mark);
+ if (commit_oe->type != OBJ_COMMIT)
+ die("Mark :%" PRIuMAX " not a commit", commit_mark);
+ hashcpy(commit_sha1, commit_oe->sha1);
+ } else if (!get_sha1(p, commit_sha1)) {
+ unsigned long size;
+ char *buf = read_object_with_reference(commit_sha1,
+ commit_type, &size, commit_sha1);
+ if (!buf || size < 46)
+ die("Not a valid commit: %s", p);
+ free(buf);
+ } else
+ die("Invalid ref name or SHA1 expression: %s", p);
+
+ if (inline_data) {
+ static struct strbuf buf = STRBUF_INIT;
+
+ if (p != uq.buf) {
+ strbuf_addstr(&uq, p);
+ p = uq.buf;
+ }
+ read_next_command();
+ parse_data(&buf);
+ store_object(OBJ_BLOB, &buf, &last_blob, sha1, 0);
+ } else if (oe) {
+ if (oe->type != OBJ_BLOB)
+ die("Not a blob (actually a %s): %s",
+ typename(oe->type), command_buf.buf);
+ } else {
+ enum object_type type = sha1_object_info(sha1, NULL);
+ if (type < 0)
+ die("Blob not found: %s", command_buf.buf);
+ if (type != OBJ_BLOB)
+ die("Not a blob (actually a %s): %s",
+ typename(type), command_buf.buf);
+ }
+
+ tree_content_set(&b->branch_tree, sha1_to_hex(commit_sha1), sha1,
+ S_IFREG | 0644, NULL);
+}
+
static void file_change_deleteall(struct branch *b)
{
release_tree_content_recursive(b->branch_tree.tree);
@@ -2222,6 +2300,8 @@ static void parse_new_commit(void)
file_change_cr(b, 1);
else if (!prefixcmp(command_buf.buf, "C "))
file_change_cr(b, 0);
+ else if (!prefixcmp(command_buf.buf, "N "))
+ note_change_n(b);
else if (!strcmp("deleteall", command_buf.buf))
file_change_deleteall(b);
else {
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index d33fc55..2f5c323 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1089,6 +1089,172 @@ test_expect_success 'P: fail on blob mark in gitlink' '
test_must_fail git fast-import <input'
###
+### series Q (notes)
+###
+
+note1_data="Note for the first commit"
+note2_data="Note for the second commit"
+note3_data="Note for the third commit"
+
+test_tick
+cat >input <<INPUT_END
+blob
+mark :2
+data <<EOF
+$file2_data
+EOF
+
+commit refs/heads/notes-test
+mark :3
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+first (:3)
+COMMIT
+
+M 644 :2 file2
+
+blob
+mark :4
+data $file4_len
+$file4_data
+commit refs/heads/notes-test
+mark :5
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+second (:5)
+COMMIT
+
+M 644 :4 file4
+
+commit refs/heads/notes-test
+mark :6
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+third (:6)
+COMMIT
+
+M 644 inline file5
+data <<EOF
+$file5_data
+EOF
+
+M 755 inline file6
+data <<EOF
+$file6_data
+EOF
+
+blob
+mark :7
+data <<EOF
+$note1_data
+EOF
+
+blob
+mark :8
+data <<EOF
+$note2_data
+EOF
+
+commit refs/notes/foobar
+mark :9
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+notes (:9)
+COMMIT
+
+N :7 :3
+N :8 :5
+N inline :6
+data <<EOF
+$note3_data
+EOF
+
+INPUT_END
+test_expect_success \
+ 'Q: commit notes' \
+ 'git fast-import <input &&
+ git whatchanged notes-test'
+test_expect_success \
+ 'Q: verify pack' \
+ 'for p in .git/objects/pack/*.pack;do git verify-pack $p||exit;done'
+
+commit1=$(git rev-parse notes-test~2)
+commit2=$(git rev-parse notes-test^)
+commit3=$(git rev-parse notes-test)
+
+cat >expect <<EOF
+author $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+
+first (:3)
+EOF
+test_expect_success \
+ 'Q: verify first commit' \
+ 'git cat-file commit notes-test~2 | sed 1d >actual &&
+ test_cmp expect actual'
+
+cat >expect <<EOF
+parent $commit1
+author $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+
+second (:5)
+EOF
+test_expect_success \
+ 'Q: verify second commit' \
+ 'git cat-file commit notes-test^ | sed 1d >actual &&
+ test_cmp expect actual'
+
+cat >expect <<EOF
+parent $commit2
+author $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+
+third (:6)
+EOF
+test_expect_success \
+ 'Q: verify third commit' \
+ 'git cat-file commit notes-test | sed 1d >actual &&
+ test_cmp expect actual'
+
+cat >expect <<EOF
+author $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+
+notes (:9)
+EOF
+test_expect_success \
+ 'Q: verify notes commit' \
+ 'git cat-file commit refs/notes/foobar | sed 1d >actual &&
+ test_cmp expect actual'
+
+cat >expect.unsorted <<EOF
+100644 blob $commit1
+100644 blob $commit2
+100644 blob $commit3
+EOF
+cat expect.unsorted | sort >expect
+test_expect_success \
+ 'Q: verify notes tree' \
+ 'git cat-file -p refs/notes/foobar^{tree} | sed "s/ [0-9a-f]* / /" >actual &&
+ test_cmp expect actual'
+
+echo "$note1_data" >expect
+test_expect_success \
+ 'Q: verify note for first commit' \
+ 'git cat-file blob refs/notes/foobar:$commit1 >actual && test_cmp expect actual'
+
+echo "$note2_data" >expect
+test_expect_success \
+ 'Q: verify note for second commit' \
+ 'git cat-file blob refs/notes/foobar:$commit2 >actual && test_cmp expect actual'
+
+echo "$note3_data" >expect
+test_expect_success \
+ 'Q: verify note for third commit' \
+ 'git cat-file blob refs/notes/foobar:$commit3 >actual && test_cmp expect actual'
+
+###
### series R (feature and option)
###
--
1.6.4.304.g1365c.dirty
^ permalink raw reply related
* [PATCHv6 10/14] Teach notes code to free its internal data structures on request.
From: Johan Herland @ 2009-09-12 16:08 UTC (permalink / raw)
To: gitster
Cc: git, johan, Johannes.Schindelin, trast, tavestbo, git, chriscool,
spearce
In-Reply-To: <200909121752.07523.johan@herland.net>
There's no need to be rude to memory-concious callers...
Signed-off-by: Johan Herland <johan@herland.net>
---
notes.c | 7 +++++++
notes.h | 2 ++
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/notes.c b/notes.c
index 84c30c1..008c3d4 100644
--- a/notes.c
+++ b/notes.c
@@ -160,3 +160,10 @@ void get_commit_notes(const struct commit *commit, struct strbuf *sb,
free(msg);
}
+
+void free_commit_notes()
+{
+ free(hash_map.entries);
+ memset(&hash_map, 0, sizeof(struct hash_map));
+ initialized = 0;
+}
diff --git a/notes.h b/notes.h
index 7f3eed4..41802e5 100644
--- a/notes.h
+++ b/notes.h
@@ -7,4 +7,6 @@
void get_commit_notes(const struct commit *commit, struct strbuf *sb,
const char *output_encoding, int flags);
+void free_commit_notes();
+
#endif
--
1.6.4.304.g1365c.dirty
^ permalink raw reply related
* [PATCHv6 09/14] Add '%N'-format for pretty-printing commit notes
From: Johan Herland @ 2009-09-12 16:08 UTC (permalink / raw)
To: gitster
Cc: git, johan, Johannes.Schindelin, trast, tavestbo, git, chriscool,
spearce
In-Reply-To: <200909121752.07523.johan@herland.net>
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Johan Herland <johan@herland.net>
---
Documentation/pretty-formats.txt | 1 +
pretty.c | 4 ++++
2 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 2a845b1..5fb10b3 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -123,6 +123,7 @@ The placeholders are:
- '%s': subject
- '%f': sanitized subject line, suitable for a filename
- '%b': body
+- '%N': commit notes
- '%Cred': switch color to red
- '%Cgreen': switch color to green
- '%Cblue': switch color to blue
diff --git a/pretty.c b/pretty.c
index 01eadd0..7f350bb 100644
--- a/pretty.c
+++ b/pretty.c
@@ -702,6 +702,10 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
case 'd':
format_decoration(sb, commit);
return 1;
+ case 'N':
+ get_commit_notes(commit, sb, git_log_output_encoding ?
+ git_log_output_encoding : git_commit_encoding, 0);
+ return 1;
}
/* For the rest we have to parse the commit header. */
--
1.6.4.304.g1365c.dirty
^ permalink raw reply related
* [PATCHv6 11/14] Teach the notes lookup code to parse notes trees with various fanout schemes
From: Johan Herland @ 2009-09-12 16:08 UTC (permalink / raw)
To: gitster
Cc: git, johan, Johannes.Schindelin, trast, tavestbo, git, chriscool,
spearce, Johannes Schindelin
In-Reply-To: <200909121752.07523.johan@herland.net>
The semantics used when parsing notes trees (with regards to fanout subtrees)
follow Dscho's proposal fairly closely:
- No concatenation/merging of notes is performed. If there are several notes
objects referencing a given commit, only one of those objects are used.
- If a notes object for a given commit is present in the "root" notes tree,
no subtrees are consulted; the object in the root tree is used directly.
- If there are more than one subtree that prefix-matches the given commit,
only the subtree with the longest matching prefix is consulted. This
means that if the given commit is e.g. "deadbeef", and the notes tree have
subtrees "de" and "dead", then the following paths in the notes tree are
searched: "deadbeef", "dead/beef". Note that "de/adbeef" is NOT searched.
- Fanout directories (subtrees) must references a whole number of bytes
from the SHA1 sum they subdivide. E.g. subtrees "dead" and "de" are
acceptable; "d" and "dea" are not.
- Multiple levels of fanout are allowed. All the above rules apply
recursively. E.g. "de/adbeef" is preferred over "de/adbe/ef", etc.
This patch changes the in-memory datastructure for holding parsed notes:
Instead of holding all note (and subtree) entries in a hash table, a
simple 16-tree structure is used instead. The tree structure consists of
16-arrays as internal nodes, and note/subtree entries as leaf nodes. The
tree is traversed by indexing subsequent nibbles of the search key until
a leaf node is encountered. If a subtree entry is encountered while
searching for a note, the subtree is unpacked into the 16-tree structure,
and the search continues into that subtree.
The new algorithm performs significantly better in the cases where only
a fraction of the notes need to be looked up (this is assumed to be the
common case for notes lookup). The new code even performs marginally
better in the worst case (where _all_ the notes are looked up).
In addition to this, comes the massive performance win associated with
organizing the notes tree according to some fanout scheme. Even a simple
2/38 fanout scheme is dramatically quicker to traverse (going from tens of
seconds to sub-second runtimes).
As for memory usage, the new code is marginally better than the old code in
the worst case, but in the case of looking up only some notes from a notes
tree with proper fanout, the new code uses only a small fraction of the
memory needed to hold the entire notes tree.
However, there is one casualty of this patch. The old notes lookup code was
able to parse notes that were associated with non-SHA1s (e.g. refs). The new
code requires the referenced object to be named by a SHA1 sum. Still, this
is not considered a major setback, since the notes infrastructure was not
originally intended to annotate objects outside the Git object database.
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Johan Herland <johan@herland.net>
---
notes.c | 317 +++++++++++++++++++++++++++++++++++++++++++++++++--------------
1 files changed, 248 insertions(+), 69 deletions(-)
diff --git a/notes.c b/notes.c
index 008c3d4..6926aa6 100644
--- a/notes.c
+++ b/notes.c
@@ -6,103 +6,282 @@
#include "strbuf.h"
#include "tree-walk.h"
-struct entry {
- unsigned char commit_sha1[20];
- unsigned char notes_sha1[20];
+/*
+ * Use a non-balancing simple 16-tree structure with struct int_node as
+ * internal nodes, and struct leaf_node as leaf nodes. Each int_node has a
+ * 16-array of pointers to its children.
+ * The bottom 2 bits of each pointer is used to identify the pointer type
+ * - ptr & 3 == 0 - NULL pointer, assert(ptr == NULL)
+ * - ptr & 3 == 1 - pointer to next internal node - cast to struct int_node *
+ * - ptr & 3 == 2 - pointer to note entry - cast to struct leaf_node *
+ * - ptr & 3 == 3 - pointer to subtree entry - cast to struct leaf_node *
+ *
+ * The root node is a statically allocated struct int_node.
+ */
+struct int_node {
+ void *a[16];
};
-struct hash_map {
- struct entry *entries;
- off_t count, size;
+/*
+ * Leaf nodes come in two variants, note entries and subtree entries,
+ * distinguished by the LSb of the leaf node pointer (see above).
+ * As a note entry, the key is the SHA1 of the referenced commit, and the
+ * value is the SHA1 of the note object.
+ * As a subtree entry, the key is the prefix SHA1 (w/trailing NULs) of the
+ * referenced commit, using the last byte of the key to store the length of
+ * the prefix. The value is the SHA1 of the tree object containing the notes
+ * subtree.
+ */
+struct leaf_node {
+ unsigned char key_sha1[20];
+ unsigned char val_sha1[20];
};
-static int initialized;
-static struct hash_map hash_map;
+#define PTR_TYPE_NULL 0
+#define PTR_TYPE_INTERNAL 1
+#define PTR_TYPE_NOTE 2
+#define PTR_TYPE_SUBTREE 3
-static int hash_index(struct hash_map *map, const unsigned char *sha1)
-{
- int i = ((*(unsigned int *)sha1) % map->size);
+#define GET_PTR_TYPE(ptr) ((uintptr_t) (ptr) & 3)
+#define CLR_PTR_TYPE(ptr) ((void *) ((uintptr_t) (ptr) & ~3))
+#define SET_PTR_TYPE(ptr, type) ((void *) ((uintptr_t) (ptr) | (type)))
- for (;;) {
- unsigned char *current = map->entries[i].commit_sha1;
+#define GET_NIBBLE(n, sha1) (((sha1[n >> 1]) >> ((n & 0x01) << 2)) & 0x0f)
- if (!hashcmp(sha1, current))
- return i;
+#define SUBTREE_SHA1_PREFIXCMP(key_sha1, subtree_sha1) \
+ (memcmp(key_sha1, subtree_sha1, subtree_sha1[19]))
- if (is_null_sha1(current))
- return -1 - i;
+static struct int_node root_node;
- if (++i == map->size)
- i = 0;
+static int initialized;
+
+static void load_subtree(struct leaf_node *subtree, struct int_node *node,
+ unsigned int n);
+
+/*
+ * To find a leaf_node:
+ * 1. Start at the root node, with n = 0
+ * 2. Use the nth nibble of the key as an index into a:
+ * - If a[n] is an int_node, recurse into that node and increment n
+ * - If a leaf_node with matching key, return leaf_node (assert note entry)
+ * - If a matching subtree entry, unpack that subtree entry (and remove it);
+ * restart search at the current level.
+ * - Otherwise, we end up at a NULL pointer, or a non-matching leaf_node.
+ * Backtrack out of the recursion, one level at a time and check a[0]:
+ * - If a[0] at the current level is a matching subtree entry, unpack that
+ * subtree entry (and remove it); restart search at the current level.
+ */
+static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,
+ const unsigned char *key_sha1)
+{
+ struct leaf_node *l;
+ unsigned char i = GET_NIBBLE(n, key_sha1);
+ void *p = tree->a[i];
+
+ switch(GET_PTR_TYPE(p)) {
+ case PTR_TYPE_INTERNAL:
+ l = note_tree_find(CLR_PTR_TYPE(p), n + 1, key_sha1);
+ if (l)
+ return l;
+ break;
+ case PTR_TYPE_NOTE:
+ l = (struct leaf_node *) CLR_PTR_TYPE(p);
+ if (!hashcmp(key_sha1, l->key_sha1))
+ return l; /* return note object matching given key */
+ break;
+ case PTR_TYPE_SUBTREE:
+ l = (struct leaf_node *) CLR_PTR_TYPE(p);
+ if (!SUBTREE_SHA1_PREFIXCMP(key_sha1, l->key_sha1)) {
+ /* unpack tree and resume search */
+ tree->a[i] = NULL;
+ load_subtree(l, tree, n);
+ free(l);
+ return note_tree_find(tree, n, key_sha1);
+ }
+ break;
+ case PTR_TYPE_NULL:
+ default:
+ assert(!p);
+ break;
}
+
+ /*
+ * Did not find key at this (or any lower) level.
+ * Check if there's a matching subtree entry in tree->a[0].
+ * If so, unpack tree and resume search.
+ */
+ p = tree->a[0];
+ if (GET_PTR_TYPE(p) != PTR_TYPE_SUBTREE)
+ return NULL;
+ l = (struct leaf_node *) CLR_PTR_TYPE(p);
+ if (!SUBTREE_SHA1_PREFIXCMP(key_sha1, l->key_sha1)) {
+ /* unpack tree and resume search */
+ tree->a[0] = NULL;
+ load_subtree(l, tree, n);
+ free(l);
+ return note_tree_find(tree, n, key_sha1);
+ }
+ return NULL;
}
-static void add_entry(const unsigned char *commit_sha1,
- const unsigned char *notes_sha1)
+/*
+ * To insert a leaf_node:
+ * 1. Start at the root node, with n = 0
+ * 2. Use the nth nibble of the key as an index into a:
+ * - If a[n] is NULL, store the tweaked pointer directly into a[n]
+ * - If a[n] is an int_node, recurse into that node and increment n
+ * - If a[n] is a leaf_node:
+ * 1. Check if they're equal, and handle that (abort? overwrite?)
+ * 2. Create a new int_node, and store both leaf_nodes there
+ * 3. Store the new int_node into a[n].
+ */
+static int note_tree_insert(struct int_node *tree, unsigned char n,
+ const struct leaf_node *entry, unsigned char type)
{
- 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);
+ struct int_node *new_node;
+ const struct leaf_node *l;
+ int ret;
+ unsigned char i = GET_NIBBLE(n, entry->key_sha1);
+ void *p = tree->a[i];
+ assert(GET_PTR_TYPE(entry) == PTR_TYPE_NULL);
+ switch(GET_PTR_TYPE(p)) {
+ case PTR_TYPE_NULL:
+ assert(!p);
+ tree->a[i] = SET_PTR_TYPE(entry, type);
+ return 0;
+ case PTR_TYPE_INTERNAL:
+ return note_tree_insert(CLR_PTR_TYPE(p), n + 1, entry, type);
+ default:
+ assert(GET_PTR_TYPE(p) == PTR_TYPE_NOTE ||
+ GET_PTR_TYPE(p) == PTR_TYPE_SUBTREE);
+ l = (const struct leaf_node *) CLR_PTR_TYPE(p);
+ if (!hashcmp(entry->key_sha1, l->key_sha1))
+ return -1; /* abort insert on matching key */
+ new_node = (struct int_node *)
+ xcalloc(sizeof(struct int_node), 1);
+ ret = note_tree_insert(new_node, n + 1,
+ CLR_PTR_TYPE(p), GET_PTR_TYPE(p));
+ if (ret) {
+ free(new_node);
+ return -1;
+ }
+ tree->a[i] = SET_PTR_TYPE(new_node, PTR_TYPE_INTERNAL);
+ return note_tree_insert(new_node, n + 1, entry, type);
}
+}
- index = hash_index(&hash_map, commit_sha1);
- if (index < 0) {
- index = -1 - index;
- hash_map.count++;
+/* Free the entire notes data contained in the given tree */
+static void note_tree_free(struct int_node *tree)
+{
+ unsigned int i;
+ for (i = 0; i < 16; i++) {
+ void *p = tree->a[i];
+ switch(GET_PTR_TYPE(p)) {
+ case PTR_TYPE_INTERNAL:
+ note_tree_free(CLR_PTR_TYPE(p));
+ /* fall through */
+ case PTR_TYPE_NOTE:
+ case PTR_TYPE_SUBTREE:
+ free(CLR_PTR_TYPE(p));
+ }
}
+}
- hashcpy(hash_map.entries[index].commit_sha1, commit_sha1);
- hashcpy(hash_map.entries[index].notes_sha1, notes_sha1);
+/*
+ * Convert a partial SHA1 hex string to the corresponding partial SHA1 value.
+ * - hex - Partial SHA1 segment in ASCII hex format
+ * - hex_len - Length of above segment. Must be multiple of 2 between 0 and 40
+ * - sha1 - Partial SHA1 value is written here
+ * - sha1_len - Max #bytes to store in sha1, Must be >= hex_len / 2, and < 20
+ * Returns -1 on error (invalid arguments or invalid SHA1 (not in hex format).
+ * Otherwise, returns number of bytes written to sha1 (i.e. hex_len / 2).
+ * Pads sha1 with NULs up to sha1_len (not included in returned length).
+ */
+static int get_sha1_hex_segment(const char *hex, unsigned int hex_len,
+ unsigned char *sha1, unsigned int sha1_len)
+{
+ unsigned int i, len = hex_len >> 1;
+ if (hex_len % 2 != 0 || len > sha1_len)
+ return -1;
+ for (i = 0; i < len; i++) {
+ unsigned int val = (hexval(hex[0]) << 4) | hexval(hex[1]);
+ if (val & ~0xff)
+ return -1;
+ *sha1++ = val;
+ hex += 2;
+ }
+ for (; i < sha1_len; i++)
+ *sha1++ = 0;
+ return len;
}
-static void initialize_hash_map(const char *notes_ref_name)
+static void load_subtree(struct leaf_node *subtree, struct int_node *node,
+ unsigned int n)
{
- unsigned char sha1[20], commit_sha1[20];
- unsigned mode;
+ unsigned char commit_sha1[20];
+ unsigned int prefix_len;
+ int status;
+ void *buf;
struct tree_desc desc;
struct name_entry entry;
- void *buf;
+
+ buf = fill_tree_descriptor(&desc, subtree->val_sha1);
+ if (!buf)
+ die("Could not read %s for notes-index",
+ sha1_to_hex(subtree->val_sha1));
+
+ prefix_len = subtree->key_sha1[19];
+ assert(prefix_len * 2 >= n);
+ memcpy(commit_sha1, subtree->key_sha1, prefix_len);
+ while (tree_entry(&desc, &entry)) {
+ int len = get_sha1_hex_segment(entry.path, strlen(entry.path),
+ commit_sha1 + prefix_len, 20 - prefix_len);
+ if (len < 0)
+ continue; /* entry.path is not a SHA1 sum. Skip */
+ len += prefix_len;
+
+ /*
+ * If commit SHA1 is complete (len == 20), assume note object
+ * If commit SHA1 is incomplete (len < 20), assume note subtree
+ */
+ if (len <= 20) {
+ unsigned char type = PTR_TYPE_NOTE;
+ struct leaf_node *l = (struct leaf_node *)
+ xcalloc(sizeof(struct leaf_node), 1);
+ hashcpy(l->key_sha1, commit_sha1);
+ hashcpy(l->val_sha1, entry.sha1);
+ if (len < 20) {
+ l->key_sha1[19] = (unsigned char) len;
+ type = PTR_TYPE_SUBTREE;
+ }
+ status = note_tree_insert(node, n, l, type);
+ assert(!status);
+ }
+ }
+ free(buf);
+}
+
+static void initialize_notes(const char *notes_ref_name)
+{
+ unsigned char sha1[20], commit_sha1[20];
+ unsigned mode;
+ struct leaf_node root_tree;
if (!notes_ref_name || read_ref(notes_ref_name, commit_sha1) ||
get_tree_entry(commit_sha1, "", sha1, &mode))
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);
+ hashclr(root_tree.key_sha1);
+ hashcpy(root_tree.val_sha1, sha1);
+ load_subtree(&root_tree, &root_node, 0);
}
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;
+ struct leaf_node *found = note_tree_find(&root_node, 0, commit_sha1);
+ if (found)
+ return found->val_sha1;
+ return NULL;
}
void get_commit_notes(const struct commit *commit, struct strbuf *sb,
@@ -120,7 +299,7 @@ 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;
- initialize_hash_map(notes_ref_name);
+ initialize_notes(notes_ref_name);
initialized = 1;
}
@@ -163,7 +342,7 @@ void get_commit_notes(const struct commit *commit, struct strbuf *sb,
void free_commit_notes()
{
- free(hash_map.entries);
- memset(&hash_map, 0, sizeof(struct hash_map));
+ note_tree_free(&root_node);
+ memset(&root_node, 0, sizeof(struct int_node));
initialized = 0;
}
--
1.6.4.304.g1365c.dirty
^ permalink raw reply related
* [PATCHv6 12/14] Selftests verifying semantics when loading notes trees with various fanouts
From: Johan Herland @ 2009-09-12 16:08 UTC (permalink / raw)
To: gitster
Cc: git, johan, Johannes.Schindelin, trast, tavestbo, git, chriscool,
spearce
In-Reply-To: <200909121752.07523.johan@herland.net>
Add selftests verifying:
- that we are able to parse notes trees with various fanout schemes
- that notes trees with conflicting fanout schemes are parsed as expected
Signed-off-by: Johan Herland <johan@herland.net>
---
t/t3303-notes-subtrees.sh | 137 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 137 insertions(+), 0 deletions(-)
create mode 100755 t/t3303-notes-subtrees.sh
diff --git a/t/t3303-notes-subtrees.sh b/t/t3303-notes-subtrees.sh
new file mode 100755
index 0000000..d24203e
--- /dev/null
+++ b/t/t3303-notes-subtrees.sh
@@ -0,0 +1,137 @@
+#!/bin/sh
+
+test_description='Test commit notes organized in subtrees'
+
+. ./test-lib.sh
+
+number_of_commits=100
+
+start_note_commit () {
+ test_tick &&
+ cat <<INPUT_END
+commit refs/notes/commits
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+notes
+COMMIT
+
+from refs/notes/commits^0
+deleteall
+INPUT_END
+
+}
+
+verify_notes () {
+ git log | grep "^ " > output &&
+ i=$number_of_commits &&
+ while [ $i -gt 0 ]; do
+ echo " commit #$i" &&
+ echo " note for commit #$i" &&
+ i=$(($i-1));
+ done > expect &&
+ test_cmp expect output
+}
+
+test_expect_success "setup: create $number_of_commits commits" '
+
+ (
+ nr=0 &&
+ while [ $nr -lt $number_of_commits ]; do
+ nr=$(($nr+1)) &&
+ test_tick &&
+ cat <<INPUT_END
+commit refs/heads/master
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+commit #$nr
+COMMIT
+
+M 644 inline file
+data <<EOF
+file in commit #$nr
+EOF
+
+INPUT_END
+
+ done &&
+ test_tick &&
+ cat <<INPUT_END
+commit refs/notes/commits
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+no notes
+COMMIT
+
+deleteall
+
+INPUT_END
+
+ ) |
+ git fast-import --quiet &&
+ git config core.notesRef refs/notes/commits
+'
+
+test_sha1_based () {
+ (
+ start_note_commit &&
+ nr=$number_of_commits &&
+ git rev-list refs/heads/master |
+ while read sha1; do
+ note_path=$(echo "$sha1" | sed "$1")
+ cat <<INPUT_END &&
+M 100644 inline $note_path
+data <<EOF
+note for commit #$nr
+EOF
+
+INPUT_END
+
+ nr=$(($nr-1))
+ done
+ ) |
+ git fast-import --quiet
+}
+
+test_expect_success 'test notes in 2/38-fanout' 'test_sha1_based "s|^..|&/|"'
+test_expect_success 'verify notes in 2/38-fanout' 'verify_notes'
+
+test_expect_success 'test notes in 4/36-fanout' 'test_sha1_based "s|^....|&/|"'
+test_expect_success 'verify notes in 4/36-fanout' 'verify_notes'
+
+test_expect_success 'test notes in 2/2/36-fanout' 'test_sha1_based "s|^\(..\)\(..\)|\1/\2/|"'
+test_expect_success 'verify notes in 2/2/36-fanout' 'verify_notes'
+
+test_preferred () {
+ (
+ start_note_commit &&
+ nr=$number_of_commits &&
+ git rev-list refs/heads/master |
+ while read sha1; do
+ preferred_note_path=$(echo "$sha1" | sed "$1")
+ ignored_note_path=$(echo "$sha1" | sed "$2")
+ cat <<INPUT_END &&
+M 100644 inline $ignored_note_path
+data <<EOF
+IGNORED note for commit #$nr
+EOF
+
+M 100644 inline $preferred_note_path
+data <<EOF
+note for commit #$nr
+EOF
+
+INPUT_END
+
+ nr=$(($nr-1))
+ done
+ ) |
+ git fast-import --quiet
+}
+
+test_expect_success 'test notes in 4/36-fanout overriding 2/38-fanout' 'test_preferred "s|^....|&/|" "s|^..|&/|"'
+test_expect_success 'verify notes in 4/36-fanout overriding 2/38-fanout' 'verify_notes'
+
+test_expect_success 'test notes in 2/38-fanout overriding 2/2/36-fanout' 'test_preferred "s|^..|&/|" "s|^\(..\)\(..\)|\1/\2/|"'
+test_expect_success 'verify notes in 2/38-fanout overriding 2/2/36-fanout' 'verify_notes'
+
+test_done
--
1.6.4.304.g1365c.dirty
^ permalink raw reply related
* [PATCHv6 13/14] Allow flexible organization of notes trees, using both commit date and SHA1
From: Johan Herland @ 2009-09-12 16:08 UTC (permalink / raw)
To: gitster
Cc: git, johan, Johannes.Schindelin, trast, tavestbo, git, chriscool,
spearce
In-Reply-To: <200909121752.07523.johan@herland.net>
This is a major expansion of the notes lookup code to allow for variations
in the notes tree organization. The variations allowed include mixing fanout
schemes based on the commit dates of the annotated commits (aka. date-based
fanout) with fanout schemes based on the SHA1 of the annotated commits (aka.
SHA1-based fanout).
Using date-based fanout in the notes tree structure enables considerable
speedup in the notes lookup process, since notes are almost always looked up
sequentially in the (reverse) chronological order of their associated commits.
Furthermore, organizing notes in a way that allow (near) sequential lookup,
enables us to decrease memory consumption both by lazily loading parts of the
notes tree structure on-demand, and freeing parts of the notes structure that
are unlikely to be used again soon.
The new flexible organization of the notes tree changes the rules for valid
note tree entries. The new rules are as follows:
1. Note objects are named by the SHA1 of the commit they annotate, possibly
split across several SHA1-based fanout levels (this is the same as is
implemented earlier in this series).
2. Note entries are located within zero or more date-based fanout levels.
3. Date-based fanout schemes may use the year, month and day values of the
associated commit's timestamp. The values must be prefixed by 'y', 'm'
and 'd' (respectively) in the notes tree.
4. The date-based components can be combined in one fanout level, or split
across multiple fanout levels. Individual components may not be split
across multiple fanout levels.
5. The year/month/date values must be specified in that order, and month or
date values may not occur without the preceding year or month value.
6. All entries of a tree object in the notes tree structure must follow the
same scheme used at that level.
Thus, the following example note entries are all valid locations for a note
annotating commit 123456789abcdef0123456789abcdef0123456789 at 2009-09-01:
- 123456789abcdef0123456789abcdef0123456789
- 12/3456789abcdef0123456789abcdef0123456789
- 1234/56789abcdef0123456789abcdef0123456789
- 12/34/56789abcdef0123456789abcdef0123456789
- 1234/5678/9abcdef0123456789abcdef0123456789
- 1234/56/78/9abcdef0123456789abcdef0123456789
- y2009/123456789abcdef0123456789abcdef0123456789
- y2009/m09/12/3456789abcdef0123456789abcdef0123456789
- y2009/m09/d01/123456789abcdef0123456789abcdef0123456789
- y2009m09/12/34/56789abcdef0123456789abcdef0123456789
- y2009m09/d01/1234/567/89abcdef0123456789abcdef0123456789
- y2009/m09d01/12/34/56/78/9abcdef0123456789abcdef0123456789
- y2009m09d01/123456789abcdef0123456789abcdef0123456789
Conversely, the following example note entries are all invalid:
- 1/23456789abcdef0123456789abcdef0123456789 (violates #1)
- 123/456789abcdef0123456789abcdef0123456789 (violates #1)
- 12/345/6789abcdef0123456789abcdef0123456789 (violates #1)
- y2009123456789abcdef0123456789abcdef0123456789 (violates #2)
- 2009/09/01/123456789abcdef0123456789abcdef0123456789 (violates #3)
- y20/09/m09/12/3456789abcdef0123456789abcdef0123456789 (violates #4)
- y20/09m09/d01/123456789abcdef0123456789abcdef0123456789 (violates #4)
- y2009m/09/12/34/56789abcdef0123456789abcdef0123456789 (violates #4)
- y2009/d01/1234/5678/9abcdef0123456789abcdef0123456789 (violates #5)
- m09/y2009/d01/12/34/56/78/9abcdef0123456789abcdef0123456789 (violates #5)
>From rule #6, we see that the following example notes tree is valid:
- y2009m09/0123456789abcdef0123456789abcdef012345678
- y2009m09/123456789abcdef0123456789abcdef0123456789
- y2008m01/d31/23/456789abcdef0123456789abcdef0123456789a
- y2008m01/d31/34/56789abcdef0123456789abcdef0123456789ab
- y2008m01/d16/4567/89abcdef0123456789abcdef0123456789abc
- y2008m01/d16/5678/9abcdef0123456789abcdef0123456789abcd
Conversely the following structure is invalid (violates rule #6):
- y2009m09/0123456789abcdef0123456789abcdef012345678
- y2009m09/12/3456789abcdef0123456789abcdef0123456789
- y2008m01/d31/23/456789abcdef0123456789abcdef0123456789a
- y2008m01/34/56789abcdef0123456789abcdef0123456789ab
- y2008m01/d16/45/6789abcdef0123456789abcdef0123456789abc
- y2008/m01d16/5678/9abcdef0123456789abcdef0123456789abcd
The flexibility added by this patch adds considerable complexity to the notes
tree parser, but the runtime and memory usage is not significantly affected
(except for the effects introduced by the chosen notes tree structure).
Internally, the 16-tree data structure introduced in earlier patches is still
used to hold the SHA1-based fanout levels and the note entries themselves.
However, this patch adds a hierarchical date-based linked-list structure
around the 16-tree structure that mirrors the fanout scheme used in the
actual notes tree.
Signed-off-by: Johan Herland <johan@herland.net>
---
notes.c | 403 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 364 insertions(+), 39 deletions(-)
diff --git a/notes.c b/notes.c
index 6926aa6..a3e3f83 100644
--- a/notes.c
+++ b/notes.c
@@ -7,6 +7,70 @@
#include "tree-walk.h"
/*
+ * Format of entries in the notes tree structure:
+ *
+ * note-entry ::= (period sep)? sha1-spec
+ * period ::= year sep?
+ * (month sep?
+ * (date sep?)?
+ * )?;
+ * year ::= 'y' yearnum;
+ * month ::= 'm' monthnum;
+ * date ::= 'd' datenum;
+ * yearnum ::= # 4-digit decimal year, from annotated commit's timestamp;
+ * monthnum ::= # 2-digit decimal month, from annotated commit's timestamp;
+ * datenum ::= # 2-digit decimal date, from annotated commit's timestamp;
+ * sha1-spec ::= (hex-fragment sep?){20}
+ * sep ::= '/';
+ * hex-fragment ::= # Fragment of hexsha1 (2 bytes);
+ * hexsha1 ::= # SHA1 of annotated commit in hex format (40 bytes);
+ *
+ * Thus, the following example note entries are all valid:
+ * - 0123456789abcdef0123456789abcdef012345678
+ * - 01/23456789abcdef0123456789abcdef012345678
+ * - 0123/456789abcdef0123456789abcdef012345678
+ * - 01/23/456789abcdef0123456789abcdef012345678
+ * - 0123/4567/89abcdef0123456789abcdef012345678
+ * - 0123/45/67/89abcdef0123456789abcdef012345678
+ * - y2009/0123456789abcdef0123456789abcdef012345678
+ * - y2009/m09/01/23456789abcdef0123456789abcdef012345678
+ * - y2009/m09/d01/0123456789abcdef0123456789abcdef012345678
+ * - y2009m09/01/23/456789abcdef0123456789abcdef012345678
+ * - y2009m09/d01/0123/4567/89abcdef0123456789abcdef012345678
+ * - y2009/m09d01/01/23/45/67/89abcdef0123456789abcdef012345678
+ * - y2009m09d01/0123456789abcdef0123456789abcdef012345678
+ *
+ * and the following example note entries are all invalid:
+ * - 0/123456789abcdef0123456789abcdef012345678
+ * - 012/3456789abcdef0123456789abcdef012345678
+ * - 01/234/56789abcdef0123456789abcdef012345678
+ * - y20090123456789abcdef0123456789abcdef012345678
+ * - y20/09/m09/01/23456789abcdef0123456789abcdef012345678
+ * - y20/09m09/d01/0123456789abcdef0123456789abcdef012345678
+ * - y2009m/09/01/23/456789abcdef0123456789abcdef012345678
+ * - y2009/d01/0123/4567/89abcdef0123456789abcdef012345678
+ * - m09/y2009/d01/01/23/45/67/89abcdef0123456789abcdef012345678
+ *
+ * In addition to the above per-entry rules, we require that _all_ entries at
+ * a given level in the notes tree (levels are separated by '/') follow the
+ * exact same format at that level. Thus the following structure is valid:
+ * - y2009m09/0123456789abcdef0123456789abcdef012345678
+ * - y2009m09/123456789abcdef0123456789abcdef0123456789
+ * - y2008m01/d31/23/456789abcdef0123456789abcdef0123456789a
+ * - y2008m01/d31/34/56789abcdef0123456789abcdef0123456789ab
+ * - y2008m01/d16/4567/89abcdef0123456789abcdef0123456789abc
+ * - y2008m01/d16/5678/9abcdef0123456789abcdef0123456789abcd
+ *
+ * but the following structure is invalid:
+ * - y2009m09/0123456789abcdef0123456789abcdef012345678
+ * - y2009m09/12/3456789abcdef0123456789abcdef0123456789
+ * - y2008m01/d31/23/456789abcdef0123456789abcdef0123456789a
+ * - y2008m01/34/56789abcdef0123456789abcdef0123456789ab
+ * - y2008m01/d16/45/6789abcdef0123456789abcdef0123456789abc
+ * - y2008/m01d16/5678/9abcdef0123456789abcdef0123456789abcd
+ */
+
+/*
* Use a non-balancing simple 16-tree structure with struct int_node as
* internal nodes, and struct leaf_node as leaf nodes. Each int_node has a
* 16-array of pointers to its children.
@@ -17,9 +81,45 @@
* - ptr & 3 == 3 - pointer to subtree entry - cast to struct leaf_node *
*
* The root node is a statically allocated struct int_node.
+ *
+ * In order to allow date-based fanout schemes in addition to the original
+ * SHA1-based fanout schemes, we need to overload this structure, as follows:
+ * If the first pointer in the 16-array is ~0 (i.e. 0xffffffff on 32-bit
+ * systems and 0xffffffffffffffff on 64-bit systems), then the int_node is NOT
+ * to be interpreted as a 16-array of child node pointers. Rather, the int_node
+ * now represents a period-based node with the following properties:
+ * - The node has a pointer to a "child" node of type struct int_node, which is
+ * EITHER a "regular" int_node object representing the root node of a 16-tree
+ * structure holding notes associated with commits with timestamps within
+ * that time period, OR another period-based int_node representing some
+ * subdivision of the time period.
+ * - The node also has a pointer to a "previous" period-based int_node, which
+ * represents the previous time period for which there exist note objects.
+ * - The node has a pointer to a "parent" node, which is the period-based
+ * int_node that has this int_node as one of its children. This is needed
+ * when traversing the date-based int_nodes looking for a period matching the
+ * given commit. For top-level objects, this is set to NULL.
+ * - The node stores the SHA1 sum of the tree object that represents its child
+ * (within the notes tree structure). Thus, we keep a reference to the child
+ * structure that without necessarily allocating the child node (and
+ * underlying structure).
+ * - Finally, the node has a period string, which indicates the time period of
+ * the notes contained within, typically of the form "YYYY", "YYYY-MM" or
+ * "YYYY-MM-DD", depending on the granularity of the corresponding
+ * period-based entries in the notes tree structure.
*/
struct int_node {
- void *a[16];
+ union {
+ void *a[16];
+ struct {
+ void *magic; /* ~0 "enables" this part of the union */
+ struct int_node *child;
+ struct int_node *prev;
+ struct int_node *parent;
+ unsigned char tree_sha1[20];
+ char period[11]; /* Enough to hold "YYYY-MM-DD" */
+ };
+ };
};
/*
@@ -51,12 +151,18 @@ struct leaf_node {
#define SUBTREE_SHA1_PREFIXCMP(key_sha1, subtree_sha1) \
(memcmp(key_sha1, subtree_sha1, subtree_sha1[19]))
+#define SUBTREE_DATE_PREFIXCMP(commit_date, subtree_date) \
+ (prefixcmp(commit_date, subtree_date))
+
static struct int_node root_node;
+static struct int_node *cur_node;
+
static int initialized;
-static void load_subtree(struct leaf_node *subtree, struct int_node *node,
- unsigned int n);
+static void load_subtree(const unsigned char *sha1,
+ const unsigned char *prefix, unsigned int prefix_len,
+ struct int_node *node, struct int_node *parent, int n);
/*
* To find a leaf_node:
@@ -94,7 +200,8 @@ static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,
if (!SUBTREE_SHA1_PREFIXCMP(key_sha1, l->key_sha1)) {
/* unpack tree and resume search */
tree->a[i] = NULL;
- load_subtree(l, tree, n);
+ load_subtree(l->val_sha1, l->key_sha1, l->key_sha1[19],
+ tree, NULL, (int) n);
free(l);
return note_tree_find(tree, n, key_sha1);
}
@@ -117,7 +224,8 @@ static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,
if (!SUBTREE_SHA1_PREFIXCMP(key_sha1, l->key_sha1)) {
/* unpack tree and resume search */
tree->a[0] = NULL;
- load_subtree(l, tree, n);
+ load_subtree(l->val_sha1, l->key_sha1, l->key_sha1[19], tree,
+ NULL, (int) n);
free(l);
return note_tree_find(tree, n, key_sha1);
}
@@ -173,16 +281,28 @@ static int note_tree_insert(struct int_node *tree, unsigned char n,
/* Free the entire notes data contained in the given tree */
static void note_tree_free(struct int_node *tree)
{
- unsigned int i;
- for (i = 0; i < 16; i++) {
- void *p = tree->a[i];
- switch(GET_PTR_TYPE(p)) {
- case PTR_TYPE_INTERNAL:
- note_tree_free(CLR_PTR_TYPE(p));
- /* fall through */
- case PTR_TYPE_NOTE:
- case PTR_TYPE_SUBTREE:
- free(CLR_PTR_TYPE(p));
+ if (tree->magic == (void *) ~0) {
+ if (tree->prev) {
+ note_tree_free(tree->prev);
+ free(tree->prev);
+ }
+ if (tree->child) {
+ note_tree_free(tree->child);
+ free(tree->child);
+ }
+ }
+ else {
+ unsigned int i;
+ for (i = 0; i < 16; i++) {
+ void *p = tree->a[i];
+ switch(GET_PTR_TYPE(p)) {
+ case PTR_TYPE_INTERNAL:
+ note_tree_free(CLR_PTR_TYPE(p));
+ /* fall through */
+ case PTR_TYPE_NOTE:
+ case PTR_TYPE_SUBTREE:
+ free(CLR_PTR_TYPE(p));
+ }
}
}
}
@@ -215,29 +335,139 @@ static int get_sha1_hex_segment(const char *hex, unsigned int hex_len,
return len;
}
-static void load_subtree(struct leaf_node *subtree, struct int_node *node,
- unsigned int n)
+/*
+ * Parse year/month/date strings, and generate the corresponding period string
+ * for the given path entry:
+ * - prefix must follow one of these forms: "", "YYYY", "YYYY-MM"
+ * - path should follow one of these forms: "yYYYY", "yYYYYmMM", "yYYYYmMMdDD",
+ * "mMMdDD", "mMM" or "dDD"
+ * The resulting string (which follows the form "YYYY", "YYYY-MM" or
+ * "YYYY-MM-DD") is returned as a static string. If path is not valid in the
+ * given (prefix) context, NULL is returned.
+ */
+static const char *parse_period(const char *prefix, unsigned int prefix_len,
+ const char *path, unsigned int path_len)
+{
+ static char result[11];
+ char expect_type; /* y/m/d for year/month/day-based fanout */
+ unsigned int expect_len, value;
+ char *endptr, *target = result;
+
+ switch (prefix_len) {
+ case 0:
+ /* No prefix, expect year-based fanout in path */
+ expect_type = 'y';
+ expect_len = 4;
+ break;
+ case 4:
+ /* Year in prefix, expect month-based fanout in path */
+ expect_type = 'm';
+ expect_len = 2;
+ break;
+ case 7:
+ /* "YYYY-MM" in prefix, expect day-based fanout in path */
+ expect_type = 'd';
+ expect_len = 2;
+ break;
+ default:
+ die("Date-based notes tree loading invoked with invalid "
+ "prefix '%.*s'", prefix_len, prefix);
+ }
+
+ if (path[0] != expect_type) {
+ warning("Unexpected entry path in date-based notes tree: '%s' "
+ "(skipping)", path);
+ return NULL;
+ }
+ value = (unsigned int) strtoul(path + 1, &endptr, 10);
+ switch (expect_type) {
+ case 'y':
+ if (value < 1969 || value >= 3000) {
+ warning("Invalid year value in date-based notes tree:"
+ " '%s' (skipping)", path);
+ return NULL;
+ }
+ break;
+ case 'm':
+ if (value < 1 || value > 12) {
+ warning("Invalid month value in date-based notes tree:"
+ " '%s' (skipping)", path);
+ return NULL;
+ }
+ break;
+ case 'd':
+ if (value < 1 || value > 31) {
+ warning("Invalid day value in date-based notes tree:"
+ " '%s' (skipping)", path);
+ return NULL;
+ }
+ break;
+ }
+
+ if (prefix == result) {
+ target = result + prefix_len;
+ prefix = NULL;
+ prefix_len = 0;
+ }
+ prefix_len = snprintf(target, 11, "%.*s%s%0*u", prefix_len, prefix,
+ expect_len == 2 ? "-" : "", expect_len, value);
+ prefix_len += target - result;
+ assert(prefix_len < 11);
+
+ if (*endptr) /* there are more components in this path */
+ return parse_period(result, prefix_len, endptr,
+ path_len - (endptr - path));
+ return result;
+}
+
+static void load_date_subtree(struct tree_desc *tree_desc,
+ const char *prefix, unsigned int prefix_len,
+ struct int_node *node, struct int_node *parent)
+{
+ struct name_entry entry;
+ struct int_node *cur_node = NULL;
+ struct int_node *new_node;
+
+ while (tree_entry(tree_desc, &entry)) {
+ const char *period = parse_period(
+ prefix, prefix_len, entry.path, strlen(entry.path));
+ if (!period)
+ continue;
+ if (tree_desc->size) /* this is not the last tree entry */
+ new_node = (struct int_node *)
+ xmalloc(sizeof(struct int_node));
+ else /* this is the last entry, store directly into node */
+ new_node = node;
+
+ new_node->magic = (void *) ~0;
+ new_node->child = NULL;
+ new_node->prev = cur_node;
+ new_node->parent = parent;
+ hashcpy(new_node->tree_sha1, entry.sha1);
+ strcpy(new_node->period, period);
+ cur_node = new_node;
+ }
+ assert(!cur_node || cur_node == node);
+}
+
+static void load_sha1_subtree(struct tree_desc *tree_desc,
+ const unsigned char *prefix, unsigned int prefix_len,
+ struct int_node *node, unsigned char n)
{
unsigned char commit_sha1[20];
- unsigned int prefix_len;
int status;
- void *buf;
- struct tree_desc desc;
struct name_entry entry;
- buf = fill_tree_descriptor(&desc, subtree->val_sha1);
- if (!buf)
- die("Could not read %s for notes-index",
- sha1_to_hex(subtree->val_sha1));
-
- prefix_len = subtree->key_sha1[19];
assert(prefix_len * 2 >= n);
- memcpy(commit_sha1, subtree->key_sha1, prefix_len);
- while (tree_entry(&desc, &entry)) {
+ memcpy(commit_sha1, prefix, prefix_len);
+ while (tree_entry(tree_desc, &entry)) {
int len = get_sha1_hex_segment(entry.path, strlen(entry.path),
commit_sha1 + prefix_len, 20 - prefix_len);
- if (len < 0)
+ if (len < 0) {
+ warning("Invalid value in notes tree: '%s' (skipping)",
+ entry.path);
continue; /* entry.path is not a SHA1 sum. Skip */
+ }
len += prefix_len;
/*
@@ -258,6 +488,42 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
assert(!status);
}
}
+}
+
+static void load_subtree(const unsigned char *sha1,
+ const unsigned char *prefix, unsigned int prefix_len,
+ struct int_node *node, struct int_node *parent, int n)
+{
+ void *buf;
+ struct tree_desc desc;
+
+ buf = fill_tree_descriptor(&desc, sha1);
+ if (!buf)
+ die("Could not read notes subtree at %s", sha1_to_hex(sha1));
+ /*
+ * After fill_tree_descriptor(), we can peek at the first tree entry
+ * in desc.entry.
+ */
+ switch (desc.entry.path[0]) {
+ case 'd':
+ if (strlen(desc.entry.path) != 3)
+ break;
+ /* fall-through */
+ case 'm':
+ case 'y':
+ /* path cannot be a SHA1 fragment */
+ load_date_subtree(&desc, (const char *) prefix, prefix_len,
+ node, parent);
+ free(buf);
+ return;
+ }
+ if (n < 0) {
+ /* Arriving from a date-based subtree; reset prefix */
+ n = 0;
+ prefix = NULL;
+ prefix_len = 0;
+ }
+ load_sha1_subtree(&desc, prefix, prefix_len, node, n);
free(buf);
}
@@ -265,23 +531,81 @@ static void initialize_notes(const char *notes_ref_name)
{
unsigned char sha1[20], commit_sha1[20];
unsigned mode;
- struct leaf_node root_tree;
if (!notes_ref_name || read_ref(notes_ref_name, commit_sha1) ||
get_tree_entry(commit_sha1, "", sha1, &mode))
return;
- hashclr(root_tree.key_sha1);
- hashcpy(root_tree.val_sha1, sha1);
- load_subtree(&root_tree, &root_node, 0);
+ load_subtree(sha1, NULL, 0, &root_node, NULL, 0);
+ cur_node = &root_node;
}
-static unsigned char *lookup_notes(const unsigned char *commit_sha1)
+static unsigned char *lookup_notes(const struct commit *commit)
{
- struct leaf_node *found = note_tree_find(&root_node, 0, commit_sha1);
- if (found)
- return found->val_sha1;
- return NULL;
+ struct int_node *node = cur_node, *seen_node = cur_node;
+ struct leaf_node *found;
+ const char *short_date;
+
+ if (!node)
+ return NULL;
+
+ /* Convert commit->date to YYYY-MM-DD format */
+ short_date = show_date(commit->date, 0, DATE_SHORT);
+
+ while (node->magic == (void *) ~0) { /* date-based node */
+ int cmp = SUBTREE_DATE_PREFIXCMP(short_date, node->period);
+ if (cmp == 0) {
+ /* Search inside child node */
+ if (!node->child) {
+ /* Must unpack child node first */
+ node->child = (struct int_node *)
+ xcalloc(sizeof(struct int_node), 1);
+ load_subtree(node->tree_sha1,
+ (const unsigned char *) node->period,
+ strlen(node->period), node->child,
+ node, -1);
+ }
+ seen_node = node;
+ node = node->child;
+ }
+ else if (cmp > 0) {
+ /* Search in past node */
+ if (node->prev)
+ node = node->prev;
+ else
+ node = node->parent;
+ }
+ else {
+ /* Search in future node */
+ if (!node->parent) {
+ /* Restart from root_node */
+ seen_node = node;
+ node = &root_node;
+ }
+ else
+ node = node->parent;
+ }
+ if (!node || node == seen_node) {
+ /* We've been here before, give up search */
+ return NULL;
+ }
+ }
+ while (cur_node &&
+ SUBTREE_DATE_PREFIXCMP(cur_node->period, seen_node->period) < 0)
+ {
+ /*
+ * We're about to move cur_node backwards in history. We are
+ * unlikely to need this cur_node in the future, so free() it.
+ */
+ note_tree_free(cur_node->child);
+ cur_node->child = NULL;
+ cur_node = cur_node->parent;
+ }
+ cur_node = seen_node;
+
+ /* Drill down further with SHA1-based lookup */
+ found = note_tree_find(node, 0, commit->object.sha1);
+ return found ? found->val_sha1 : NULL;
}
void get_commit_notes(const struct commit *commit, struct strbuf *sb,
@@ -303,7 +627,7 @@ void get_commit_notes(const struct commit *commit, struct strbuf *sb,
initialized = 1;
}
- sha1 = lookup_notes(commit->object.sha1);
+ sha1 = lookup_notes(commit);
if (!sha1)
return;
@@ -342,6 +666,7 @@ void get_commit_notes(const struct commit *commit, struct strbuf *sb,
void free_commit_notes()
{
+ cur_node = NULL;
note_tree_free(&root_node);
memset(&root_node, 0, sizeof(struct int_node));
initialized = 0;
--
1.6.4.304.g1365c.dirty
^ permalink raw reply related
* [PATCHv6 14/14] Add test cases for various date-based fanouts
From: Johan Herland @ 2009-09-12 16:08 UTC (permalink / raw)
To: gitster
Cc: git, johan, Johannes.Schindelin, trast, tavestbo, git, chriscool,
spearce
In-Reply-To: <200909121752.07523.johan@herland.net>
Signed-off-by: Johan Herland <johan@herland.net>
---
t/t3303-notes-subtrees.sh | 64 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 64 insertions(+), 0 deletions(-)
diff --git a/t/t3303-notes-subtrees.sh b/t/t3303-notes-subtrees.sh
index d24203e..d3cbf6d 100755
--- a/t/t3303-notes-subtrees.sh
+++ b/t/t3303-notes-subtrees.sh
@@ -134,4 +134,68 @@ test_expect_success 'verify notes in 4/36-fanout overriding 2/38-fanout' 'verify
test_expect_success 'test notes in 2/38-fanout overriding 2/2/36-fanout' 'test_preferred "s|^..|&/|" "s|^\(..\)\(..\)|\1/\2/|"'
test_expect_success 'verify notes in 2/38-fanout overriding 2/2/36-fanout' 'verify_notes'
+test_date_based () {
+ (
+ start_note_commit &&
+ nr=$number_of_commits &&
+ git log --format="%H %ct" refs/heads/master |
+ while read sha1 date_t; do
+ date=$(date -u -d "@$date_t" +"$1")
+ note_path="$date/$(echo "$sha1" | sed "$2")"
+ cat <<INPUT_END &&
+M 100644 inline $note_path
+data <<EOF
+note for commit #$nr
+EOF
+
+INPUT_END
+
+ nr=$(($nr-1))
+ done
+ ) |
+ git fast-import --quiet
+}
+
+test_expect_success 'test notes in y/40-fanout' 'test_date_based "y%Y" ""'
+test_expect_success 'verify notes in y/40-fanout' 'verify_notes'
+
+test_expect_success 'test notes in y/2/38-fanout' 'test_date_based "y%Y" "s|^..|&/|"'
+test_expect_success 'verify notes in y/2/38-fanout' 'verify_notes'
+
+test_expect_success 'test notes in ym/40-fanout' 'test_date_based "y%Ym%m" ""'
+test_expect_success 'verify notes in ym/40-fanout' 'verify_notes'
+
+test_expect_success 'test notes in ym/2/38-fanout' 'test_date_based "y%Ym%m" "s|^..|&/|"'
+test_expect_success 'verify notes in ym/2/38-fanout' 'verify_notes'
+
+test_expect_success 'test notes in ymd/40-fanout' 'test_date_based "y%Ym%md%d" ""'
+test_expect_success 'verify notes in ymd/40-fanout' 'verify_notes'
+
+test_expect_success 'test notes in ymd/2/38-fanout' 'test_date_based "y%Ym%md%d" "s|^..|&/|"'
+test_expect_success 'verify notes in ymd/2/38-fanout' 'verify_notes'
+
+test_expect_success 'test notes in y/m/40-fanout' 'test_date_based "y%Y/m%m" ""'
+test_expect_success 'verify notes in y/m/40-fanout' 'verify_notes'
+
+test_expect_success 'test notes in y/m/2/38-fanout' 'test_date_based "y%Y/m%m" "s|^..|&/|"'
+test_expect_success 'verify notes in y/m/2/38-fanout' 'verify_notes'
+
+test_expect_success 'test notes in y/md/40-fanout' 'test_date_based "y%Y/m%md%d" ""'
+test_expect_success 'verify notes in y/md/40-fanout' 'verify_notes'
+
+test_expect_success 'test notes in y/md/2/38-fanout' 'test_date_based "y%Y/m%md%d" "s|^..|&/|"'
+test_expect_success 'verify notes in y/md/2/38-fanout' 'verify_notes'
+
+test_expect_success 'test notes in ym/d/40-fanout' 'test_date_based "y%Ym%m/d%d" ""'
+test_expect_success 'verify notes in ym/d/40-fanout' 'verify_notes'
+
+test_expect_success 'test notes in ym/d/2/38-fanout' 'test_date_based "y%Ym%m/d%d" "s|^..|&/|"'
+test_expect_success 'verify notes in ym/d/2/38-fanout' 'verify_notes'
+
+test_expect_success 'test notes in y/m/d/40-fanout' 'test_date_based "y%Y/m%m/d%d" ""'
+test_expect_success 'verify notes in y/m/d/40-fanout' 'verify_notes'
+
+test_expect_success 'test notes in y/m/d/2/38-fanout' 'test_date_based "y%Y/m%m/d%d" "s|^..|&/|"'
+test_expect_success 'verify notes in y/m/d/2/38-fanout' 'verify_notes'
+
test_done
--
1.6.4.304.g1365c.dirty
^ permalink raw reply related
* [PATCH v3] gitweb: check given hash before trying to create snapshot
From: Mark Rada @ 2009-09-12 16:32 UTC (permalink / raw)
To: git, Jakub Narebski
> [This mail was very strangely wrapped; I fixed this for readability]
Yeah, sorry, format=flowed
> In first test you check that _contents_ contain specific error message,
> but you do not check if HTTP status code matches it (it should, because
> of how die_error works). In second test you check HTTP status. If the
> t/t9501-gitweb-standalone-http-status.sh is to be about status, I guess
> that you should check HTTP status, and not contents of the page (which
> is more likely to change, e.g. due to some prettifying).
>
> In t9501 tests you need, I think, only the HTTP headers part, unless
> you want also to check that the contents matches. There was some sed
> script shown to extract only HTTP headers.
Yes, t9501 also checks for the specific error message, this was originally
how it had to be done in order to show what trap was being executed for
that first block of checks in the snapshot routine. The header does not
contain the specific error string that gitweb outputs though, only the
standard message (i.e. 404 Not Found), so I check the script output for
negative test cases.
>> Also, right now gitweb will not accept tags for hashes. This seems to be
>> because it passes the --verify option to rev-parse, but the output
>> from using and not using the verify option seems to be the same (other
>> than also accepting all tree-ishes). Could you let me know if there is
>> a good reason not to take off the --verify option? Otherwise, I would
>> like to take it off in the next version of this patch.
>
> Errr, what?
>
> $ 5096:[gitweb/web@git]# git rev-parse --verify v1.5.0
> 6db027ffe03210324939b3dd655c4223ca023b45
> $ git rev-parse --verify refs/tags/v1.5.0
> 6db027ffe03210324939b3dd655c4223ca023b45
>
> So it works as intended. The problem must be in some other place.
Let's just say the problem is not in the computer. :(
--
Mark Rada (ferrous26)
marada@uwaterloo.ca
--->8---
Makes things nicer in cases when you hand craft the snapshot URL but
make a typo in defining the hash variable (e.g. netx instead of next);
you will now get an error message instead of a broken tarball.
To maintain backwards compatibility, git_get_head_hash is now a wrapper
for git_get_full_hash, as suggested by Jakub Narebski.
Tests for t9501 are included to demonstrate changed functionality.
Signed-off-by: Mark Rada <marada@uwaterloo.ca>
---
gitweb/gitweb.perl | 13 ++++++++++---
t/t9501-gitweb-standalone-http-status.sh | 29 +++++++++++++++++++++++++++++
2 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 24b2193..e9cac8d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1983,11 +1983,17 @@ sub quote_command {
# get HEAD ref of given project as hash
sub git_get_head_hash {
+ return git_get_full_hash(shift, 'HEAD');
+}
+
+# given a project and tree-ish, returns full hash
+sub git_get_full_hash {
my $project = shift;
+ my $hash = shift;
my $o_git_dir = $git_dir;
my $retval = undef;
$git_dir = "$projectroot/$project";
- if (open my $fd, "-|", git_cmd(), "rev-parse", "--verify", "HEAD") {
+ if (open my $fd, '-|', git_cmd(), 'rev-parse', '--verify', $hash) {
my $head = <$fd>;
close $fd;
if (defined $head && $head =~ /^([0-9a-fA-F]{40})$/) {
@@ -5196,8 +5202,9 @@ sub git_snapshot {
die_error(403, "Unsupported snapshot format");
}
- if (!defined $hash) {
- $hash = git_get_head_hash($project);
+ my $snapshot = git_get_full_hash($project, $hash);
+ if (!$snapshot) {
+ die_error(404, 'Hash id was not valid');
}
my $name = $project;
diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
index d0ff21d..632007e 100644
--- a/t/t9501-gitweb-standalone-http-status.sh
+++ b/t/t9501-gitweb-standalone-http-status.sh
@@ -75,4 +75,33 @@ test_expect_success \
test_debug 'cat gitweb.output'
+# ----------------------------------------------------------------------
+# snapshot hash ids
+
+test_expect_success \
+ 'snapshots: good treeish id' \
+ 'gitweb_run "p=.git;a=snapshot;h=master;sf=tgz" &&
+ grep "Status: 200 OK" gitweb.output'
+test_debug 'cat gitweb.output'
+
+test_expect_success \
+ 'snapshots: bad treeish id' \
+ 'gitweb_run "p=.git;a=snapshot;h=frizzumFrazzum;sf=tgz" &&
+ grep "404 - Hash id was not valid" gitweb.output'
+test_debug 'cat gitweb.output'
+
+test_expect_success \
+ 'snapshots: good object id' \
+ 'ID=`git rev-parse --verify HEAD` &&
+ gitweb_run "p=.git;a=snapshot;h=$ID;sf=tgz" &&
+ grep "Status: 200 OK" gitweb.output'
+test_debug 'cat gitweb.output'
+
+test_expect_success \
+ 'snapshots: bad object id' \
+ 'gitweb_run "p=.git;a=snapshot;h=abcdef01234;sf=tgz" &&
+ grep "404 - Hash id was not valid" gitweb.output'
+test_debug 'cat gitweb.output'
+
+
test_done
--
1.6.4.2
^ permalink raw reply related
* Re: Issue 323 in msysgit: Can't clone over http
From: Junio C Hamano @ 2009-09-12 17:31 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: git, msysgit, Tom Preston-Werner, Jakub Narebski
In-Reply-To: <be6fef0d0909120301r79072d5ase0d4b174753af9b5@mail.gmail.com>
Tay Ray Chuan <rctay89@gmail.com> writes:
> PS. The above was solely based on my reading of the code, no testing done.
That matches the understanding I had from reading the code. I just wanted
a sanity check by another person who looked at and worked on the code more
deeply than I did.
Thanks, so the patch is a go.
^ permalink raw reply
* git push --confirm ?
From: Owen Taylor @ 2009-09-12 17:51 UTC (permalink / raw)
To: git; +Cc: Colin Walters
[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]
People sometimes push things they don't mean to. Depending on the
workflow and environment, that can be anywhere between a trivial
nuisance to an embarrassing and awkward cleanup.
It would seem handy to me to have a --confirm option to git-push
(and 'core.confirm-push' to turn it on be default), that would
have the following behavior:
* An initial --dry-run pass is done but with more verbosity -
for updates of existing references, it would show what commits
were being added or removed in a one-line format.
* The user is prompted if they want to proceed
* If the user agrees, then the push is run without --dry-run
I've attached a mockup of this as a porcelain 'git safe-push'.
(Done not using 'git push --porcelain' because I wanted it to work
with existing released Git versions. I was hoping to be able to do
'git config alias.push safe-push', but no facility for interactive
only aliases...)
I think this wouldn't be too hard to add to 'git push', though
I haven't tried to code it. Yes, it's not atomic without protocol
changes - I think that's OK:
- If the push isn't being forced intermediate ref updates will
be caught as a non-fast-forward in the second pass.
- If the push is being forced, you might overwrite someone else's
push anyways even without --confirm.
Thoughts on whether this makes sense as an addition?
- Owen
[-- Attachment #2: git-safe-push --]
[-- Type: application/x-shellscript, Size: 2584 bytes --]
^ permalink raw reply
* Re: [PATCHv5 00/14] git notes
From: Shawn O. Pearce @ 2009-09-12 18:11 UTC (permalink / raw)
To: Johan Herland
Cc: git, gitster, Johannes.Schindelin, trast, tavestbo, git,
chriscool
In-Reply-To: <200909121750.00733.johan@herland.net>
Johan Herland <johan@herland.net> wrote:
> Shawn, do you have any additional defence for the date-based fanout?
No.
The only defense I have for it is "it sounds like a nice theory
given access patterns", and the note about memory usage you made,
but which I clipped to keep this email shorter. :-)
It was only a theory I tossed out there in a back-seat-driver
sort of way. Your results show my hunch was correct, it may help.
But they also say it may not help enough to justify the complexity,
so I now agree with you that SHA-1 fan out may be good enough.
> Are
> there untested reasonable scenarios that would show the benefits of date-
> based fanout?
I don't think there are, your tests were pretty good at covering
things.
> How does the plan for notes usage in your code-review thingy
> compare to my test scenario?
I think your tests may still have been too low in volume, 115k notes
isn't a lot. Based on the distributions I was looking at before,
I could be seeing a growth of >100k notes/year. Ask me again in
5 years if 115k notes is a lot. :-)
But we all know that SHA-1 distributes data quite well, so the SHA-1
fan-out may just need to change from 2_38 to 2_2_2_34 (or something)
to handle that larger volume.
--
Shawn.
^ permalink raw reply
* Re: diffstat and signoff in git-format-patch
From: Jeff King @ 2009-09-12 18:19 UTC (permalink / raw)
To: Robert Wohlrab; +Cc: git
In-Reply-To: <200909121717.57896.robert.wohlrab@gmx.de>
On Sat, Sep 12, 2009 at 05:17:57PM +0200, Robert Wohlrab wrote:
> > Hi,
> > I am currently trying to use git-format-patch in a script to generate files
> > which must be in a special format. This means that I have to override also
> > changes by users. This is as far as I can see suffix, signoff and some
> > more. Most of the stuff works, but I cannot get away the signoff message
> > as their is nothing like --no-signoff and the -p option which should be
> > disable generation of diffstat doesn't do anything. How can I disable them
> > anyway?
> > git version 1.6.3.3
> And can i disable this "--\n1.6.3.3" at the end of the patch?
format-patch isn't very configurable in its output. Maybe you want to
just do:
git log -1 --pretty=email -p
instead (or if you want to tweak the format even further, try
--pretty=format)?
-Peff
^ permalink raw reply
* Re: diffstat and signoff in git-format-patch
From: Robert Wohlrab @ 2009-09-12 18:29 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20090912181919.GA20561@coredump.intra.peff.net>
> On Sat, Sep 12, 2009 at 05:17:57PM +0200, Robert Wohlrab wrote:
> > > Hi,
> > > I am currently trying to use git-format-patch in a script to generate
> > > files which must be in a special format. This means that I have to
> > > override also changes by users. This is as far as I can see suffix,
> > > signoff and some more. Most of the stuff works, but I cannot get away
> > > the signoff message as their is nothing like --no-signoff and the -p
> > > option which should be disable generation of diffstat doesn't do
> > > anything. How can I disable them anyway?
> > > git version 1.6.3.3
> >
> > And can i disable this "--\n1.6.3.3" at the end of the patch?
>
> format-patch isn't very configurable in its output. Maybe you want to
> just do:
>
> git log -1 --pretty=email -p
>
> instead (or if you want to tweak the format even further, try
> --pretty=format)?
Thanks. I am currently playing with it and i think that I can create what I
want with it.
--
Robert Wohlrab
^ permalink raw reply
* Re: [PATCH] completion: Replace config --list with --get-regexp
From: Shawn O. Pearce @ 2009-09-12 18:31 UTC (permalink / raw)
To: Todd Zullinger; +Cc: Jeff King, james bardin, git, Junio C Hamano
In-Reply-To: <20090911232344.GH2582@inocybe.localdomain>
Todd Zullinger <tmz@pobox.com> wrote:
> James Bardin noted that the completion spewed warnings when no git
> config file is present. This is likely a bug to be fixed in git config,
> but it's also a good excuse to simplify the completion code by using the
> --get-regexp option as Jeff King pointed out.
>
> Signed-off-by: Todd Zullinger <tmz@pobox.com>
Thanks, looks good.
Trivially-acked-by: Shawn O. Pearce <spearce@spearce.org>
> contrib/completion/git-completion.bash | 30 +++++++++---------------------
> 1 files changed, 9 insertions(+), 21 deletions(-)
--
Shawn.
^ permalink raw reply
* Re: obnoxious CLI complaints
From: John Tapsell @ 2009-09-12 18:32 UTC (permalink / raw)
To: Dmitry Potapov; +Cc: Brendan Miller, Jakub Narebski, git
In-Reply-To: <20090912103156.GA30385@dpotapov.dyndns.org>
2009/9/12 Dmitry Potapov <dpotapov@gmail.com>:
> On Wed, Sep 09, 2009 at 05:09:31PM -0700, Brendan Miller wrote:
>> On Wed, Sep 9, 2009 at 2:54 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>> > Brendan Miller <catphive@catphive.net> writes:
>> >>
>> >> This is what I want to do 90% of the time, so it should just have the
>> >> proper defaults, and not make me look at the man page every time I
>> >> want to use it.
>> >
>> > You learn those idioms.
>>
>> I guess. Is that a good thing?
>
> In general, yes, because most of them exist for a good reason.
>
>> Is the goal of interface design to make
>> it difficult so I need to learn a lot of things, or easy so I can
>> remain blissfully ignorant but still do what I want?
>
> Neither. You cannot get what unless you have specified what you want,
> and for that you have to learn how to say that. Having good defaults is
> very important, but the problem with choosing them is that people have
> different preferences about them. For instance, you wanted the default
> prefix for git-archive to be $myproject. For me, a good default would be
> either $tag_name, or $myproject-$tag_name, or empty (as it is now!). So,
> what you propose is *never* a good default for me. Moreover, changing
> any default will cause a lot of pain for other people who use Git now.
> Besides, writing something like --prefix='' is very ugly. So, the
> current default makes perfect sense.
Ah, great logic. You can't find a default that will suit everyone,
therefore don't bother.
>> Yeah, I've been reading them. I'm saying that the docs are a crutch.
>> RTFM is the problem not the solution. It makes the user do more work
>> to avoid fixing usability issues.
>
> A usability issue exists when a person knows how to do that, but it is
> inconvenient or error-prone; or when a learning curve is too steep.
> But when someone cannot use, let's say, a compiler, because he or she
> refuses to read to learn the language, it is not a usability issue.
It's a usability issue when it doesn't just do the right thing in the
majority of cases and lets you specify what you want it to do in the
rest of the cases.
John
^ permalink raw reply
* Re: [PATCHv5 00/14] git notes
From: Johan Herland @ 2009-09-12 18:35 UTC (permalink / raw)
To: Shawn O. Pearce
Cc: git, gitster, Johannes.Schindelin, trast, tavestbo, git,
chriscool
In-Reply-To: <20090912181150.GN1033@spearce.org>
On Saturday 12 September 2009, Shawn O. Pearce wrote:
> Johan Herland <johan@herland.net> wrote:
> > Shawn, do you have any additional defence for the date-based fanout?
>
> No.
>
> The only defense I have for it is "it sounds like a nice theory
> given access patterns", and the note about memory usage you made,
> but which I clipped to keep this email shorter. :-)
>
> It was only a theory I tossed out there in a back-seat-driver
> sort of way. Your results show my hunch was correct, it may help.
> But they also say it may not help enough to justify the complexity,
> so I now agree with you that SHA-1 fan out may be good enough.
Ok, so I guess we can drop the flexible part of notes code. Junio: Feel free
to drop the two last patches from the jh/notes series.
> > How does the plan for notes usage in your code-review thingy
> > compare to my test scenario?
>
> I think your tests may still have been too low in volume, 115k notes
> isn't a lot. Based on the distributions I was looking at before,
> I could be seeing a growth of >100k notes/year. Ask me again in
> 5 years if 115k notes is a lot. :-)
>
> But we all know that SHA-1 distributes data quite well, so the SHA-1
> fan-out may just need to change from 2_38 to 2_2_2_34 (or something)
> to handle that larger volume.
Yes, I expect that the optimal number of entries per tree level is ~256, so
if we add an upper threshold at ~300 (where we start using another fanout
level), and a lower threshold at ~200 (where we consolidate subtrees and put
all into this level), the (still-to-be-written) writing part of the notes
code should automatically adjust the notes tree to the optimal layout.
With those assumptions, and a growth of 100k notes/year, a 2/2/36 fanout
should last you ~150 years, and a 2/2/2/34 fanout should be enough for the
next ~40,000 years... ;)
Have fun! :)
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCHv6 10/14] Teach notes code to free its internal data structures on request.
From: Junio C Hamano @ 2009-09-12 18:40 UTC (permalink / raw)
To: Johan Herland
Cc: gitster, git, Johannes.Schindelin, trast, tavestbo, git,
chriscool, spearce
In-Reply-To: <1252771728-27206-11-git-send-email-johan@herland.net>
Johan Herland <johan@herland.net> writes:
> There's no need to be rude to memory-concious callers...
Will squash this in.
-- >8 --
From: Junio C Hamano <gitster@pobox.com>
Date: Sat, 12 Sep 2009 11:34:24 -0700
Subject: [PATCH] notes.[ch] fixup: avoid old-style declaration
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
notes.c | 2 +-
notes.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/notes.c b/notes.c
index 008c3d4..9ed2c87 100644
--- a/notes.c
+++ b/notes.c
@@ -161,7 +161,7 @@ void get_commit_notes(const struct commit *commit, struct strbuf *sb,
free(msg);
}
-void free_commit_notes()
+void free_commit_notes(void)
{
free(hash_map.entries);
memset(&hash_map, 0, sizeof(struct hash_map));
diff --git a/notes.h b/notes.h
index 41802e5..d1dd1d1 100644
--- a/notes.h
+++ b/notes.h
@@ -7,6 +7,6 @@
void get_commit_notes(const struct commit *commit, struct strbuf *sb,
const char *output_encoding, int flags);
-void free_commit_notes();
+void free_commit_notes(void);
#endif
--
1.6.5.rc0.82.g1c5d9
^ permalink raw reply related
* Re: [PATCHv6 13/14] Allow flexible organization of notes trees, using both commit date and SHA1
From: Junio C Hamano @ 2009-09-12 18:41 UTC (permalink / raw)
To: Johan Herland
Cc: gitster, git, Johannes.Schindelin, trast, tavestbo, git,
chriscool, spearce
In-Reply-To: <1252771728-27206-14-git-send-email-johan@herland.net>
Johan Herland <johan@herland.net> writes:
> This is a major expansion of the notes lookup code to allow for variations
> in the notes tree organization. The variations allowed include mixing fanout
> schemes based on the commit dates of the annotated commits (aka. date-based
> fanout) with fanout schemes based on the SHA1 of the annotated commits (aka.
> SHA1-based fanout).
Will squash this in.
-- >8 --
From: Junio C Hamano <gitster@pobox.com>
Date: Sat, 12 Sep 2009 11:36:42 -0700
Subject: [PATCH] notes.[ch] fixup: avoid unnamed struct members
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
notes.c | 82 +++++++++++++++++++++++++++++++-------------------------------
1 files changed, 41 insertions(+), 41 deletions(-)
diff --git a/notes.c b/notes.c
index c573ffa..9a92853 100644
--- a/notes.c
+++ b/notes.c
@@ -118,8 +118,8 @@ struct int_node {
struct int_node *parent;
unsigned char tree_sha1[20];
char period[11]; /* Enough to hold "YYYY-MM-DD" */
- };
- };
+ } s;
+ } u;
};
/*
@@ -182,7 +182,7 @@ static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,
{
struct leaf_node *l;
unsigned char i = GET_NIBBLE(n, key_sha1);
- void *p = tree->a[i];
+ void *p = tree->u.a[i];
switch(GET_PTR_TYPE(p)) {
case PTR_TYPE_INTERNAL:
@@ -199,7 +199,7 @@ static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,
l = (struct leaf_node *) CLR_PTR_TYPE(p);
if (!SUBTREE_SHA1_PREFIXCMP(key_sha1, l->key_sha1)) {
/* unpack tree and resume search */
- tree->a[i] = NULL;
+ tree->u.a[i] = NULL;
load_subtree(l->val_sha1, l->key_sha1, l->key_sha1[19],
tree, NULL, (int) n);
free(l);
@@ -214,16 +214,16 @@ static struct leaf_node *note_tree_find(struct int_node *tree, unsigned char n,
/*
* Did not find key at this (or any lower) level.
- * Check if there's a matching subtree entry in tree->a[0].
+ * Check if there's a matching subtree entry in tree->u.a[0].
* If so, unpack tree and resume search.
*/
- p = tree->a[0];
+ p = tree->u.a[0];
if (GET_PTR_TYPE(p) != PTR_TYPE_SUBTREE)
return NULL;
l = (struct leaf_node *) CLR_PTR_TYPE(p);
if (!SUBTREE_SHA1_PREFIXCMP(key_sha1, l->key_sha1)) {
/* unpack tree and resume search */
- tree->a[0] = NULL;
+ tree->u.a[0] = NULL;
load_subtree(l->val_sha1, l->key_sha1, l->key_sha1[19], tree,
NULL, (int) n);
free(l);
@@ -250,12 +250,12 @@ static int note_tree_insert(struct int_node *tree, unsigned char n,
const struct leaf_node *l;
int ret;
unsigned char i = GET_NIBBLE(n, entry->key_sha1);
- void *p = tree->a[i];
+ void *p = tree->u.a[i];
assert(GET_PTR_TYPE(entry) == PTR_TYPE_NULL);
switch(GET_PTR_TYPE(p)) {
case PTR_TYPE_NULL:
assert(!p);
- tree->a[i] = SET_PTR_TYPE(entry, type);
+ tree->u.a[i] = SET_PTR_TYPE(entry, type);
return 0;
case PTR_TYPE_INTERNAL:
return note_tree_insert(CLR_PTR_TYPE(p), n + 1, entry, type);
@@ -273,7 +273,7 @@ static int note_tree_insert(struct int_node *tree, unsigned char n,
free(new_node);
return -1;
}
- tree->a[i] = SET_PTR_TYPE(new_node, PTR_TYPE_INTERNAL);
+ tree->u.a[i] = SET_PTR_TYPE(new_node, PTR_TYPE_INTERNAL);
return note_tree_insert(new_node, n + 1, entry, type);
}
}
@@ -281,20 +281,20 @@ static int note_tree_insert(struct int_node *tree, unsigned char n,
/* Free the entire notes data contained in the given tree */
static void note_tree_free(struct int_node *tree)
{
- if (tree->magic == (void *) ~0) {
- if (tree->prev) {
- note_tree_free(tree->prev);
- free(tree->prev);
+ if (tree->u.s.magic == (void *) ~0) {
+ if (tree->u.s.prev) {
+ note_tree_free(tree->u.s.prev);
+ free(tree->u.s.prev);
}
- if (tree->child) {
- note_tree_free(tree->child);
- free(tree->child);
+ if (tree->u.s.magic) {
+ note_tree_free(tree->u.s.magic);
+ free(tree->u.s.magic);
}
}
else {
unsigned int i;
for (i = 0; i < 16; i++) {
- void *p = tree->a[i];
+ void *p = tree->u.a[i];
switch(GET_PTR_TYPE(p)) {
case PTR_TYPE_INTERNAL:
note_tree_free(CLR_PTR_TYPE(p));
@@ -439,12 +439,12 @@ static void load_date_subtree(struct tree_desc *tree_desc,
else /* this is the last entry, store directly into node */
new_node = node;
- new_node->magic = (void *) ~0;
- new_node->child = NULL;
- new_node->prev = cur_node;
- new_node->parent = parent;
- hashcpy(new_node->tree_sha1, entry.sha1);
- strcpy(new_node->period, period);
+ new_node->u.s.magic = (void *) ~0;
+ new_node->u.s.magic = NULL;
+ new_node->u.s.prev = cur_node;
+ new_node->u.s.parent = parent;
+ hashcpy(new_node->u.s.tree_sha1, entry.sha1);
+ strcpy(new_node->u.s.period, period);
cur_node = new_node;
}
assert(!cur_node || cur_node == node);
@@ -552,38 +552,38 @@ static unsigned char *lookup_notes(const struct commit *commit)
/* Convert commit->date to YYYY-MM-DD format */
short_date = show_date(commit->date, 0, DATE_SHORT);
- while (node->magic == (void *) ~0) { /* date-based node */
- int cmp = SUBTREE_DATE_PREFIXCMP(short_date, node->period);
+ while (node->u.s.magic == (void *) ~0) { /* date-based node */
+ int cmp = SUBTREE_DATE_PREFIXCMP(short_date, node->u.s.period);
if (cmp == 0) {
/* Search inside child node */
- if (!node->child) {
+ if (!node->u.s.magic) {
/* Must unpack child node first */
- node->child = (struct int_node *)
+ node->u.s.magic = (struct int_node *)
xcalloc(sizeof(struct int_node), 1);
- load_subtree(node->tree_sha1,
- (const unsigned char *) node->period,
- strlen(node->period), node->child,
+ load_subtree(node->u.s.tree_sha1,
+ (const unsigned char *) node->u.s.period,
+ strlen(node->u.s.period), node->u.s.magic,
node, -1);
}
seen_node = node;
- node = node->child;
+ node = node->u.s.magic;
}
else if (cmp > 0) {
/* Search in past node */
- if (node->prev)
- node = node->prev;
+ if (node->u.s.prev)
+ node = node->u.s.prev;
else
- node = node->parent;
+ node = node->u.s.parent;
}
else {
/* Search in future node */
- if (!node->parent) {
+ if (!node->u.s.parent) {
/* Restart from root_node */
seen_node = node;
node = &root_node;
}
else
- node = node->parent;
+ node = node->u.s.parent;
}
if (!node || node == seen_node) {
/* We've been here before, give up search */
@@ -591,15 +591,15 @@ static unsigned char *lookup_notes(const struct commit *commit)
}
}
while (cur_node &&
- SUBTREE_DATE_PREFIXCMP(cur_node->period, seen_node->period) < 0)
+ SUBTREE_DATE_PREFIXCMP(cur_node->u.s.period, seen_node->u.s.period) < 0)
{
/*
* We're about to move cur_node backwards in history. We are
* unlikely to need this cur_node in the future, so free() it.
*/
- note_tree_free(cur_node->child);
- cur_node->child = NULL;
- cur_node = cur_node->parent;
+ note_tree_free(cur_node->u.s.magic);
+ cur_node->u.s.magic = NULL;
+ cur_node = cur_node->u.s.parent;
}
cur_node = seen_node;
--
1.6.5.rc0.82.g1c5d9
^ permalink raw reply related
* Re: git push --confirm ?
From: Jeff King @ 2009-09-12 18:43 UTC (permalink / raw)
To: Owen Taylor; +Cc: git, Colin Walters
In-Reply-To: <1252777897.2974.24.camel@localhost.localdomain>
On Sat, Sep 12, 2009 at 01:51:37PM -0400, Owen Taylor wrote:
> * An initial --dry-run pass is done but with more verbosity -
> for updates of existing references, it would show what commits
> were being added or removed in a one-line format.
>
> * The user is prompted if they want to proceed
>
> * If the user agrees, then the push is run without --dry-run
>
> [...]
>
> I think this wouldn't be too hard to add to 'git push', though
> I haven't tried to code it. Yes, it's not atomic without protocol
> changes - I think that's OK:
I have never wanted such a feature, so maybe I am a bad person to
comment, but I don't see much advantage from a UI standpoint over what
we have now. Which is "git push --dry-run", check to see if you like it,
and then re-run without --dry-run. If you just want to see more output
in the first --dry-run, then that is easy to do with an alternate
format.
But what _would_ be useful is doing it atomically. You can certainly do
all three of those steps from within one "git push" invocation, and I
think that is enough without any protocol changes. The protocol already
sends for each ref a line like:
<old-sha1> <new-sha1> <ref>
and receive-pack will not proceed with the update unless the <old-sha1>
matches what is about to be changed.
> - If the push isn't being forced intermediate ref updates will
> be caught as a non-fast-forward in the second pass.
>
> - If the push is being forced, you might overwrite someone else's
> push anyways even without --confirm.
Yeah, "--force" is not very fine-grained. I wonder if rather than a
complete --confirm you would rather have something iterative like:
$ git push --interactive
Pushing to server:/path/to/repo.git
* [new branch] topic -> topic
Push this branch [Yn]?
5ad9dce..cfc497a topic -> topic
Push this branch [Yn]?
5ad9dce...cfc497a topic -> topic (non-fast forward)
Force this branch [yN]?
where of course the actual output text and y/n defaults are subject to
debate. You could even have a 'v' option at each prompt to visualize the
differences in gitk so you can easily get more information on what you
might be overwriting in a non-fast-forward scenario.
-Peff
^ permalink raw reply
* Re: [PATCH v7 2/6] fast-import: put marks reading in it's own function
From: Shawn O. Pearce @ 2009-09-12 18:47 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Junio C Hamano, Johannes Schindelin, Git List, Ian Clatworthy,
Matt McClure, Miklos Vajna, Julian Phillips, vcs-fast-import-devs
In-Reply-To: <1252247748-14507-3-git-send-email-srabbelier@gmail.com>
Sverre Rabbelier <srabbelier@gmail.com> wrote:
> All options do nothing but set settings, with the exception of the
> --input-marks option. Delay the reading of the marks file till after
> all options have been parsed.
...
> diff --git a/fast-import.c b/fast-import.c
> index b904f20..812fcf0 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -315,6 +315,7 @@ static struct object_entry_pool *blocks;
> static struct object_entry *object_table[1 << 16];
> static struct mark_set *marks;
> static const char *mark_file;
> +static const char *input_file;
Sorry I didn't notice this earlier in the series, but input_file
is too generic of a name for a global in this program. Its *not*
the input stream, and I fear that it might be confused as such.
Likewise now mark_file is also confusing. I would suggest renaming
these two:
static const char *export_marks_file;
static const char *import_marks_file;
--
Shawn.
^ permalink raw reply
* Re: [PATCH v7 3/6] fast-import: add feature command
From: Shawn O. Pearce @ 2009-09-12 18:51 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Junio C Hamano, Johannes Schindelin, Git List, Ian Clatworthy,
Matt McClure, Miklos Vajna, Julian Phillips, vcs-fast-import-devs
In-Reply-To: <1252247748-14507-4-git-send-email-srabbelier@gmail.com>
Sverre Rabbelier <srabbelier@gmail.com> wrote:
> This allows the fronted to require a specific feature to be supported
> by the frontend, or abort.
>
> Also add support for the first feature, date-format=.
...
> +`feature`
> +~~~~~~~~~
> +Require that fast-import supports the specified feature, or abort if
> +it does not.
> +
> +....
> + 'feature' SP <feature> LF
> +....
> +
> +The <feature> part of the command may be any string matching
> +[a-zA-Z-] and should be understood by a version of fast-import.
> +
Where is the documentation for 'feature date-format=<FORMAT>'?
Also, IIRC the fast-import list agreed that the <feature> name must
match the re ^[a-zA-Z][a-zA-Z-]*$. Saying that here does somewhat
help another fast-import developer to use the same stream format,
but it does not help a user to understand what features they can
ask for in their stream.
> diff --git a/fast-import.c b/fast-import.c
> index 812fcf0..9bf06a4 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -2450,6 +2450,17 @@ static void parse_one_option(const char *option)
> }
> }
>
> +static void parse_feature(void)
> +{
> + char *feature = command_buf.buf + 8;
> +
> + if (!prefixcmp(feature, "date-format=")) {
> + option_date_format(feature + 12);
> + } else {
> + die("This version of fast-import does not support feature %s.", feature);
> + }
> +}
> +
> static int git_pack_config(const char *k, const char *v, void *cb)
> {
> if (!strcmp(k, "pack.depth")) {
> @@ -2526,6 +2537,8 @@ int main(int argc, const char **argv)
> parse_checkpoint();
> else if (!prefixcmp(command_buf.buf, "progress "))
> parse_progress();
> + else if (!prefixcmp(command_buf.buf, "feature "))
> + parse_feature();
So its legal to change the data format in the middle of a stream?
I thought we agreed on fast-import list that "feature" needs to come
before any data commands, and isn't legal once data commands have
appeared in the stream. Thus it should not be possible to request
a change in the date-format once a blob or commit has been stored.
--
Shawn.
^ permalink raw reply
* Re: [PATCH v7 4/6] fast-import: test the new feature command
From: Shawn O. Pearce @ 2009-09-12 18:52 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Junio C Hamano, Johannes Schindelin, Git List, Ian Clatworthy,
Matt McClure, Miklos Vajna, Julian Phillips, vcs-fast-import-devs
In-Reply-To: <1252247748-14507-5-git-send-email-srabbelier@gmail.com>
Sverre Rabbelier <srabbelier@gmail.com> wrote:
> Test that an unknown feature causes fast-import to abort, and that a
> known feature is accepted.
This should be squashed with the prior patch.
> t/t9300-fast-import.sh | 20 ++++++++++++++++++++
> 1 files changed, 20 insertions(+), 0 deletions(-)
--
Shawn.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox