Git development
 help / color / mirror / Atom feed
* [PATCH v3] Detailed diagnosis when parsing an object name fails.
From: Matthieu Moy @ 2009-12-07 10:10 UTC (permalink / raw)
  To: git, gitster; +Cc: Matthieu Moy
In-Reply-To: <vpqws0zcd1c.fsf@bauges.imag.fr>

The previous error message was the same in many situations (unknown
revision or path not in the working tree). We try to help the user as
much as possible to understand the error, especially with the
sha1:filename notation. In this case, we say whether the sha1 or the
filename is problematic, and diagnose the confusion between
relative-to-root and relative-to-$PWD confusion precisely.

The 7 new error messages are tested.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 cache.h                        |    6 ++-
 setup.c                        |   15 +++++-
 sha1_name.c                    |  115 ++++++++++++++++++++++++++++++++++++++--
 t/t1506-rev-parse-diagnosis.sh |   69 ++++++++++++++++++++++++
 4 files changed, 198 insertions(+), 7 deletions(-)
 create mode 100755 t/t1506-rev-parse-diagnosis.sh

diff --git a/cache.h b/cache.h
index 0e69384..c122bfa 100644
--- a/cache.h
+++ b/cache.h
@@ -708,7 +708,11 @@ static inline unsigned int hexval(unsigned char c)
 #define DEFAULT_ABBREV 7
 
 extern int get_sha1(const char *str, unsigned char *sha1);
-extern int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode);
+extern int get_sha1_with_mode_1(const char *str, unsigned char *sha1, unsigned *mode, int gently, const char *prefix);
+static inline int get_sha1_with_mode(const char *str, unsigned char *sha1, unsigned *mode)
+{
+	return get_sha1_with_mode_1(str, sha1, mode, 1, NULL);
+}
 extern int get_sha1_hex(const char *hex, unsigned char *sha1);
 extern char *sha1_to_hex(const unsigned char *sha1);	/* static buffer result! */
 extern int read_ref(const char *filename, unsigned char *sha1);
diff --git a/setup.c b/setup.c
index f67250b..5792eb7 100644
--- a/setup.c
+++ b/setup.c
@@ -74,6 +74,18 @@ int check_filename(const char *prefix, const char *arg)
 	die_errno("failed to stat '%s'", arg);
 }
 
+static void NORETURN die_verify_filename(const char *prefix, const char *arg)
+{
+	unsigned char sha1[20];
+	unsigned mode;
+	/* try a detailed diagnostic ... */
+	get_sha1_with_mode_1(arg, sha1, &mode, 0, prefix);
+	/* ... or fall back the most general message. */
+	die("ambiguous argument '%s': unknown revision or path not in the working tree.\n"
+	    "Use '--' to separate paths from revisions", arg);
+
+}
+
 /*
  * Verify a filename that we got as an argument for a pathspec
  * entry. Note that a filename that begins with "-" never verifies
@@ -87,8 +99,7 @@ void verify_filename(const char *prefix, const char *arg)
 		die("bad flag '%s' used after filename", arg);
 	if (check_filename(prefix, arg))
 		return;
-	die("ambiguous argument '%s': unknown revision or path not in the working tree.\n"
-	    "Use '--' to separate paths from revisions", arg);
+	die_verify_filename(prefix, arg);
 }
 
 /*
diff --git a/sha1_name.c b/sha1_name.c
index 44bb62d..ca8f9db 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -804,7 +804,96 @@ int get_sha1(const char *name, unsigned char *sha1)
 	return get_sha1_with_mode(name, sha1, &unused);
 }
 
-int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
+/* Must be called only when object_name:filename doesn't exist. */
+static void diagnose_invalid_sha1_path(const char *prefix,
+				       const char *filename,
+				       const unsigned char *tree_sha1,
+				       const char *object_name)
+{
+	struct stat st;
+	unsigned char sha1[20];
+	unsigned mode;
+
+	if (!prefix)
+		prefix = "";
+
+	if (!lstat(filename, &st))
+		die("Path '%s' exists on disk, but not in '%s'.",
+		    filename, object_name);
+	if (errno == ENOENT || errno == ENOTDIR) {
+		char *fullname = xmalloc(strlen(filename)
+					     + strlen(prefix) + 1);
+		strcpy(fullname, prefix);
+		strcat(fullname, filename);
+
+		if (!get_tree_entry(tree_sha1, fullname,
+				    sha1, &mode)) {
+			die("Path '%s' exists, but not '%s'.\n"
+			    "Did you mean '%s:%s'?",
+			    fullname,
+			    filename,
+			    object_name,
+			    fullname);
+		}
+		die("Path '%s' does not exist in '%s'",
+		    filename, object_name);
+	}
+}
+
+/* Must be called only when :stage:filename doesn't exist. */
+static void diagnose_invalid_index_path(int stage,
+					const char *prefix,
+					const char *filename)
+{
+	struct stat st;
+	struct cache_entry *ce;
+	int pos;
+	unsigned namelen = strlen(filename);
+	unsigned fullnamelen;
+	char *fullname;
+
+	if (!prefix)
+		prefix = "";
+
+	/* Wrong stage number? */
+	pos = cache_name_pos(filename, namelen);
+	if (pos < 0)
+		pos = -pos - 1;
+	ce = active_cache[pos];
+	if (ce_namelen(ce) == namelen &&
+	    !memcmp(ce->name, filename, namelen))
+		die("Path '%s' is in the index, but not at stage %d.\n"
+		    "Did you mean ':%d:%s'?",
+		    filename, stage,
+		    ce_stage(ce), filename);
+
+	/* Confusion between relative and absolute filenames? */
+	fullnamelen = namelen + strlen(prefix);
+	fullname = xmalloc(fullnamelen + 1);
+	strcpy(fullname, prefix);
+	strcat(fullname, filename);
+	pos = cache_name_pos(fullname, fullnamelen);
+	if (pos < 0)
+		pos = -pos - 1;
+	ce = active_cache[pos];
+	if (ce_namelen(ce) == fullnamelen &&
+	    !memcmp(ce->name, fullname, fullnamelen))
+		die("Path '%s' is in the index, but not '%s'.\n"
+		    "Did you mean ':%d:%s'?",
+		    fullname, filename,
+		    ce_stage(ce), fullname);
+
+	if (!lstat(filename, &st))
+		die("Path '%s' exists on disk, but not in the index.", filename);
+	if (errno == ENOENT || errno == ENOTDIR)
+		die("Path '%s' does not exist (neither on disk nor in the index).",
+		    filename);
+
+	free(fullname);
+}
+
+
+int get_sha1_with_mode_1(const char *name, unsigned char *sha1, unsigned *mode, int gently, const char *prefix)
 {
 	int ret, bracket_depth;
 	int namelen = strlen(name);
@@ -850,6 +939,8 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
 			}
 			pos++;
 		}
+		if (!gently)
+			diagnose_invalid_index_path(stage, prefix, cp);
 		return -1;
 	}
 	for (cp = name, bracket_depth = 0; *cp; cp++) {
@@ -862,9 +953,25 @@ int get_sha1_with_mode(const char *name, unsigned char *sha1, unsigned *mode)
 	}
 	if (*cp == ':') {
 		unsigned char tree_sha1[20];
-		if (!get_sha1_1(name, cp-name, tree_sha1))
-			return get_tree_entry(tree_sha1, cp+1, sha1,
-					      mode);
+		char *object_name = NULL;
+		if (!gently) {
+			object_name = xmalloc(cp-name+1);
+			strncpy(object_name, name, cp-name);
+			object_name[cp-name] = '\0';
+		}
+		if (!get_sha1_1(name, cp-name, tree_sha1)) {
+			const char *filename = cp+1;
+			ret = get_tree_entry(tree_sha1, filename, sha1, mode);
+			if (!gently) {
+				diagnose_invalid_sha1_path(prefix, filename,
+							   tree_sha1, object_name);
+				free(object_name);
+			}
+			return ret;
+		} else {
+			if (!gently)
+				die("Invalid object name '%s'.", object_name);
+		}
 	}
 	return ret;
 }
diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh
new file mode 100755
index 0000000..af721f9
--- /dev/null
+++ b/t/t1506-rev-parse-diagnosis.sh
@@ -0,0 +1,69 @@
+#!/bin/sh
+
+test_description='test git rev-parse diagnosis for invalid argument'
+
+exec </dev/null
+
+. ./test-lib.sh
+
+HASH_file=
+
+test_expect_success 'set up basic repo' '
+	echo one > file.txt &&
+	mkdir subdir &&
+	echo two > subdir/file.txt &&
+	echo three > subdir/file2.txt &&
+	git add . &&
+	git commit -m init &&
+	echo four > index-only.txt &&
+	git add index-only.txt &&
+	echo five > disk-only.txt
+'
+
+test_expect_success 'correct file objects' '
+	HASH_file=$(git rev-parse HEAD:file.txt) &&
+	git rev-parse HEAD:subdir/file.txt &&
+	git rev-parse :index-only.txt &&
+	(cd subdir &&
+	 git rev-parse HEAD:subdir/file2.txt &&
+	 test $HASH_file = $(git rev-parse HEAD:file.txt) &&
+	 test $HASH_file = $(git rev-parse :file.txt) &&
+	 test $HASH_file = $(git rev-parse :0:file.txt) )
+'
+
+test_expect_success 'incorrect revision id' '
+	test_must_fail git rev-parse foobar:file.txt 2>error &&
+	grep "Invalid object name '"'"'foobar'"'"'." error &&
+	test_must_fail git rev-parse foobar 2> error &&
+	grep "unknown revision or path not in the working tree." error
+'
+
+test_expect_success 'incorrect file in sha1:path' '
+	test_must_fail git rev-parse HEAD:nothing.txt 2> error &&
+	grep "fatal: Path '"'"'nothing.txt'"'"' does not exist in '"'"'HEAD'"'"'" error &&
+	test_must_fail git rev-parse HEAD:index-only.txt 2> error &&
+	grep "fatal: Path '"'"'index-only.txt'"'"' exists on disk, but not in '"'"'HEAD'"'"'." error &&
+	(cd subdir &&
+	 test_must_fail git rev-parse HEAD:file2.txt 2> error &&
+	 grep "Did you mean '"'"'HEAD:subdir/file2.txt'"'"'?" error )
+'
+
+test_expect_success 'incorrect file in :path and :N:path' '
+	test_must_fail git rev-parse :nothing.txt 2> error &&
+	grep "fatal: Path '"'"'nothing.txt'"'"' does not exist (neither on disk nor in the index)." error &&
+	test_must_fail git rev-parse :1:nothing.txt 2> error &&
+	grep "Path '"'"'nothing.txt'"'"' does not exist (neither on disk nor in the index)." error &&
+	test_must_fail git rev-parse :1:file.txt 2> error &&
+	grep "Did you mean '"'"':0:file.txt'"'"'?" error &&
+	(cd subdir &&
+	 test_must_fail git rev-parse :1:file.txt 2> error &&
+	 grep "Did you mean '"'"':0:file.txt'"'"'?" error &&
+	 test_must_fail git rev-parse :file2.txt 2> error &&
+	 grep "Did you mean '"'"':0:subdir/file2.txt'"'"'?" error &&
+	 test_must_fail git rev-parse :2:file2.txt 2> error &&
+	 grep "Did you mean '"'"':0:subdir/file2.txt'"'"'?" error) &&
+	test_must_fail git rev-parse :disk-only.txt 2> error &&
+	grep "fatal: Path '"'"'disk-only.txt'"'"' exists on disk, but not in the index." error
+'
+
+test_done
-- 
1.6.6.rc0.355.gcb53a.dirty

^ permalink raw reply related

* core.worktree bug
From: Robin Rosenberg @ 2009-12-07 10:15 UTC (permalink / raw)
  To: git


According to git-config(1) 

       core.worktree
           Set the path to the working tree. The value will not be used in combination with repositories found automatically in a
           .git directory (i.e. $GIT_DIR is not set). This can be overridden by the GIT_WORK_TREE environment variable and the
           --work-tree command line option. It can be a absolute path or relative path to the directory specified by --git-dir or
           GIT_DIR. Note: If --git-dir or GIT_DIR are specified but none of --work-tree, GIT_WORK_TREE and core.worktree is
           specified, the current working directory is regarded as the top directory of your working tree.

this setting is not used if GIT_DIR is set. But when I try it out 

$ mkdir r1
$ mkdir r2
$ cd r2
$ git init
Initialized empty Git repository in /home/me/tmp/r2/.git/
$ date >f
$ git add f
$ git commit -m "f"
 f
 1 files changed, 1 insertions(+), 0 deletions(-)
 create mode 100644 f
$ git status
# On branch master
nothing to commit (working directory clean)

=> Nothing interesting here. It comes here:

$ git config core.worktree $(cd ../r1;pwd)
$ git status
# On branch master
# Changed but not updated:
#   (use "git add/rm <file>..." to update what will be committed)
#   (use "git checkout -- <file>..." to discard changes in working directory)
#
#       deleted:    f
#
no changes added to commit (use "git add" and/or "git commit -a")

=> Seems the config is actually honored even though GIT_DIR is not set.

Bisect tells me 4f38f6b5bafb1f7f85c7b54d0bb0a0e977cd947c broke it. My main point is that I am
implementing this in JGit so I want the same behaviour. Question: Should we try to fix this
in git so it matches the documentation or fix the documentation to match behaviour.

The breakage appeared over a year ago and no one has complained.

-- robin

My bisection script:

#!/bin/bash

test_description="CEILING"

. ./test-lib.sh

test_expect_success \
    "ceiling" \
    "
     rm -rf r1 r2 &&
     mkdir r1 r2 &&
     (cd r2 &&
     git init &&
     date >f &&
     git add f &&
echo ADD &&
     git commit -m f &&
echo COMMIT &&
     git diff --exit-code f && 
echo DIFF &&
     git config core.worktree \$(cd ../r1/..;pwd) &&
echo CONFIG &&
     (unset GIT_DIR;git diff --exit-code -- f) &&
echo DIFF
     )
     "

test_don

^ permalink raw reply

* [PATCH v2] cvsserver: make the output of 'update' more compatible with cvs.
From: Sergei Organov @ 2009-12-07 11:11 UTC (permalink / raw)
  To: git; +Cc: gitster
In-Reply-To: <87zl5z4y0w.fsf@osv.gnss.ru>

Native cvs update outputs the string "cvs update: Updating <DIR>" for
every directory it processes (to stderr) unless -q or -Q is given on
comman-line. This is used, e.g., by emacs pcl-cvs to split files by
directory. This commit implements this feature in cvsserver.

Signed-off-by: Sergei Organov <osv@javad.com>
---
 git-cvsserver.perl |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 6dc45f5..f4c75bb 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -981,6 +981,8 @@ sub req_update
 
     #$log->debug("update state : " . Dumper($state));
 
+    my $last_dirname = "///";
+    
     # foreach file specified on the command line ...
     foreach my $filename ( @{$state->{args}} )
     {
@@ -988,6 +990,20 @@ sub req_update
 
         $log->debug("Processing file $filename");
 
+        unless ( $state->{globaloptions}{-Q} || $state->{globaloptions}{-q} )
+        {
+            my $cur_dirname = dirname($filename);
+            if ( $cur_dirname ne $last_dirname )
+            {
+                $last_dirname = $cur_dirname;
+                if ( $cur_dirname eq "" )
+                {
+                    $cur_dirname = ".";
+                }
+                print "E cvs update: Updating $cur_dirname\n";
+            }
+        }
+
         # if we have a -C we should pretend we never saw modified stuff
         if ( exists ( $state->{opt}{C} ) )
         {
-- 
1.6.6.rc0.67.gc0456.dirty

^ permalink raw reply related

* Re: Speedlimit at "git clone"
From: Baz @ 2009-12-07 11:15 UTC (permalink / raw)
  To: Daniel Stenberg; +Cc: Jeff King, git
In-Reply-To: <alpine.DEB.2.00.0912061912180.19656@tvnag.unkk.fr>

2009/12/6 Daniel Stenberg <daniel@haxx.se>:
> On Sun, 6 Dec 2009, Jeff King wrote:
>
>>>> how can i limit the download speed at "git clone"?
>>>>
>>> no one any idea?
>>
>> Git has no internal support for doing this.
>
> libcurl provides such a feature so if the clone is done over HTTP it would
> be easy to offer, but I figure it would also be weird to only provide it
> over a single particular transfer method...

There is always 'trickle'.
http://monkey.org/~marius/pages/?page=trickle

I think this is the preload-based tool Peff was thinking of. I've not
used it with git, but I have used it successfully limiting bandwidth
for other downloads.

-Baz

>
> --
>
>  / daniel.haxx.se
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: [PATCHv3 0/2] Add a "fixup" command to "rebase --interactive"
From: Johannes Schindelin @ 2009-12-07 11:21 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: git, gitster, git, bgustavsson
In-Reply-To: <cover.1260177312.git.mhagger@alum.mit.edu>

Hi,

On Mon, 7 Dec 2009, Michael Haggerty wrote:

> Junio C Hamano wrote:
> > Michael Haggerty <mhagger@alum.mit.edu> writes:
> > 
> >> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> >> index 0bd3bf7..a7de5ea 100755
> >> --- a/git-rebase--interactive.sh
> >> +++ b/git-rebase--interactive.sh
> >> @@ -302,7 +302,13 @@ nth_string () {
> >>  
> >>  make_squash_message () {
> >>  	if test -f "$SQUASH_MSG"; then
> >> -		COUNT=$(($(sed -n "s/^# This is [^0-9]*\([1-9][0-9]*\).*/\1/p" \
> >> +		# We want to be careful about matching only the commit
> >> +		# message comment lines generated by this function.
> > 
> >> +		# But supposedly some sed versions don't handle "\|"
> >> +		# correctly, so instead of "\(st\|nd\|rd\|th\)", use
> >> +		# the less accurate "[snrt][tdh]" to match the
> >> +		# nth_string endings.
> > 
> > I'd drop this comment; blaming POSIX-compliant sed without GNU extension
> > is simply wrong.
> 
> Fair enough.  I hope you don't mind my leaving a line explaining the
> cryptic "[snrt][tdh]" to save Dscho a couple of seconds next time :-).

Thanks, very much appreciated here.

My ACK for the patches is still valid.

^ permalink raw reply

* Re: [PATCHv2 2/2] Add a command "fixup" to rebase --interactive
From: Sverre Rabbelier @ 2009-12-07 11:26 UTC (permalink / raw)
  To: Michael Haggerty; +Cc: git, gitster, git, Johannes.Schindelin, bgustavsson
In-Reply-To: <ced6765cff6225a05f196a6896ab577850979ab1.1260099005.git.mhagger@alum.mit.edu>

Heya,

On Mon, Dec 7, 2009 at 05:22, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> The command is like "squash", except that it discards the commit message
> of the corresponding commit.

No no, wait, wasn't "fixup" supposed to let you just edit the commit
message of the commit you're fixing up? :(

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* [RFC/PATCHv10 00/11] git notes
From: Johan Herland @ 2009-12-07 11:27 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce

Hi,

Here is the 10th iteration of the git-notes series. Changes in this
iteration are as follows:

Changes to existing patches:

- Rebased onto 488bdf2... (Fix crasher on encountering SHA1-like non-note
  in notes tree) (There are no conflicts resolved by this, but a test in
  this series crashes without the crash fix in 488bdf2...).

- Yet another rewrite of the fast-import patch. After trying to implement
  Shawn's suggestions, I found that hacking the mode bits of note entries
  could not work, since they are stripped by a mktree()/load_tree()
  round-trip. The new versions treats _all_ entries with note-compatible
  paths (40 hex chars, not including directory separators) as note entries
  (to be subjected to fanout adjustments) in branches where there are note
  activity (one or more 'N' commands).
  Branches without note activity are not touched, of course.

- Otherwise, Shawn's suggestions to the previous iteration have been
  incorporated.

- Extended t9301 tests to verify that non-notes residing in a notes tree
  are not clobbered by the fast-import patch.

- Fix t9301 test #12 to not "cheat" (by 'deleteall' followed by a few
  additions). Instead, remove notes one-by-one to verify correct
  fanout consolidation.

- Minor cleanups here and there


If Shawn is OK with the fast-import patch, I believe that at least
patches #1 - #3 (and possibly #4 - #5) are ready for 'next'.

Patches #6 - #11 drastically extend the notes API. Since there are
currently no users of that API, and it has not been discussed much
on the list (although these patches have already been present in a
few iterations), I would still consider them RFC quality.


TODO:
- Builtin-ify git-notes shell script to take advantage of notes API
- Garbage collect notes whose referenced object is unreachable (gc_notes())
- Handle note objects that are not blobs, but trees


Have fun! :)

...Johan


Johan Herland (11):
  fast-import: Proper notes tree manipulation
  Rename t9301 to t9350, to make room for more fast-import tests
  Add more testcases to test fast-import of notes
  Minor style fixes to notes.c
  Notes API: get_commit_notes() -> format_note() + remove the commit restriction
  Notes API: init_notes(): Initialize the notes tree from the given notes ref
  Notes API: add_note(): Add note objects to the internal notes tree structure
  Notes API: get_note(): Return the note annotating the given object
  Notes API: for_each_note(): Traverse the entire notes tree with a callback
  Notes API: Allow multiple concurrent notes trees with new struct notes_tree
  Refactor notes concatenation into a flexible interface for combining notes

 fast-import.c                                    |  134 +++++-
 notes.c                                          |  345 +++++++++----
 notes.h                                          |  114 ++++-
 pretty.c                                         |    9 +-
 t/t9300-fast-import.sh                           |  156 +++++-
 t/t9301-fast-import-notes.sh                     |  623 ++++++++++++++++++++++
 t/{t9301-fast-export.sh => t9350-fast-export.sh} |    0
 7 files changed, 1259 insertions(+), 122 deletions(-)
 create mode 100755 t/t9301-fast-import-notes.sh
 rename t/{t9301-fast-export.sh => t9350-fast-export.sh} (100%)

^ permalink raw reply

* [RFC/PATCHv10 02/11] Rename t9301 to t9350, to make room for more fast-import tests
From: Johan Herland @ 2009-12-07 11:27 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1260185254-1523-1-git-send-email-johan@herland.net>

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

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

^ permalink raw reply

* [RFC/PATCHv10 01/11] fast-import: Proper notes tree manipulation
From: Johan Herland @ 2009-12-07 11:27 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1260185254-1523-1-git-send-email-johan@herland.net>

This patch teaches 'git fast-import' to automatically organize note objects
in a fast-import stream into an appropriate fanout structure. The notes API
in notes.h is NOT used to accomplish this, because trying to keep the
fast-import and notes data structures in sync would yield a significantly
larger patch with higher complexity.

Note objects are added with the 'N' command, and accounted for with a
per-branch counter, which is used to trigger fanout restructuring when
needed. Note that when restructuring the branch tree, _any_ entry whose
path consists of 40 hex chars (not including directory separators) will
be recognized as a note object. It is therefore not advisable to
manipulate note entries with M/D/R/C commands.

Since note objects are stored in the same tree structure as other objects,
the unloading and reloading of a fast-import branches handle note objects
transparently.

This patch has been improved by the following contributions:
- Shawn O. Pearce: Several style- and logic-related improvements

Cc: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Johan Herland <johan@herland.net>
---

As stated in the cover letter, I simply cannot store note information in
the tree_entry mode bits. So, I chose this somewhat more simple and crude
approach, which I still think solves the problems quite nicely.


Have fun! :)

...Johan


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

diff --git a/fast-import.c b/fast-import.c
index dd3c99d..f04cd17 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -245,6 +245,7 @@ struct branch
 	const char *name;
 	struct tree_entry branch_tree;
 	uintmax_t last_commit;
+	uintmax_t num_notes;
 	unsigned active : 1;
 	unsigned pack_id : PACK_ID_BITS;
 	unsigned char sha1[20];
@@ -693,6 +694,7 @@ static struct branch *new_branch(const char *name)
 	b->table_next_branch = branch_table[hc];
 	b->branch_tree.versions[0].mode = S_IFDIR;
 	b->branch_tree.versions[1].mode = S_IFDIR;
+	b->num_notes = 0;
 	b->active = 0;
 	b->pack_id = MAX_PACK_ID;
 	branch_table[hc] = b;
@@ -1860,6 +1862,109 @@ static void load_branch(struct branch *b)
 	}
 }

+static unsigned char convert_num_notes_to_fanout(uintmax_t num_notes)
+{
+	unsigned char fanout = 0;
+	while ((num_notes >>= 8))
+		fanout++;
+	return fanout;
+}
+
+static void construct_path_with_fanout(const char *hex_sha1,
+		unsigned char fanout, char *path)
+{
+	unsigned int i = 0, j = 0;
+	if (fanout >= 20)
+		die("Too large fanout (%u)", fanout);
+	while (fanout) {
+		path[i++] = hex_sha1[j++];
+		path[i++] = hex_sha1[j++];
+		path[i++] = '/';
+		fanout--;
+	}
+	memcpy(path + i, hex_sha1 + j, 40 - j);
+	path[i + 40 - j] = '\0';
+}
+
+static uintmax_t do_change_note_fanout(
+		struct tree_entry *orig_root, struct tree_entry *root,
+		char *hex_sha1, unsigned int hex_sha1_len,
+		char *fullpath, unsigned int fullpath_len,
+		unsigned char fanout)
+{
+	struct tree_content *t = root->tree;
+	struct tree_entry *e, leaf;
+	unsigned int i, tmp_hex_sha1_len, tmp_fullpath_len;
+	uintmax_t num_notes = 0;
+	unsigned char sha1[20];
+	char realpath[60];
+
+	for (i = 0; t && i < t->entry_count; i++) {
+		e = t->entries[i];
+		tmp_hex_sha1_len = hex_sha1_len + e->name->str_len;
+		tmp_fullpath_len = fullpath_len;
+
+		/*
+		 * We're interested in EITHER existing note entries (entries
+		 * with exactly 40 hex chars in path, not including directory
+		 * separators), OR directory entries that may contain note
+		 * entries (with < 40 hex chars in path).
+		 * Also, each path component in a note entry must be a multiple
+		 * of 2 chars.
+		 */
+		if (!e->versions[1].mode ||
+		    tmp_hex_sha1_len > 40 ||
+		    e->name->str_len % 2)
+			continue;
+
+		/* This _may_ be a note entry, or a subdir containing notes */
+		memcpy(hex_sha1 + hex_sha1_len, e->name->str_dat,
+		       e->name->str_len);
+		if (tmp_fullpath_len)
+			fullpath[tmp_fullpath_len++] = '/';
+		memcpy(fullpath + tmp_fullpath_len, e->name->str_dat,
+		       e->name->str_len);
+		tmp_fullpath_len += e->name->str_len;
+		fullpath[tmp_fullpath_len] = '\0';
+
+		if (tmp_hex_sha1_len == 40 && !get_sha1_hex(hex_sha1, sha1)) {
+			/* This is a note entry */
+			construct_path_with_fanout(hex_sha1, fanout, realpath);
+			if (!strcmp(fullpath, realpath)) {
+				/* Note entry is in correct location */
+				num_notes++;
+				continue;
+			}
+
+			/* Rename fullpath to realpath */
+			if (!tree_content_remove(orig_root, fullpath, &leaf))
+				die("Failed to remove path %s", fullpath);
+			tree_content_set(orig_root, realpath,
+				leaf.versions[1].sha1,
+				leaf.versions[1].mode,
+				leaf.tree);
+		} else if (S_ISDIR(e->versions[1].mode)) {
+			/* This is a subdir that may contain note entries */
+			if (!e->tree)
+				load_tree(e);
+			num_notes += do_change_note_fanout(orig_root, e,
+				hex_sha1, tmp_hex_sha1_len,
+				fullpath, tmp_fullpath_len, fanout);
+		}
+
+		/* The above may have reallocated the current tree_content */
+		t = root->tree;
+	}
+	return num_notes;
+}
+
+static uintmax_t change_note_fanout(struct tree_entry *root,
+		unsigned char fanout)
+{
+	char hex_sha1[40], path[60];
+	return do_change_note_fanout(root, root, hex_sha1, 0, path, 0, fanout);
+}
+
 static void file_change_m(struct branch *b)
 {
 	const char *p = command_buf.buf + 2;
@@ -2010,14 +2115,16 @@ static void file_change_cr(struct branch *b, int rename)
 		leaf.tree);
 }

-static void note_change_n(struct branch *b)
+static void note_change_n(struct branch *b, unsigned char old_fanout)
 {
 	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];
+	char path[60];
 	uint16_t inline_data = 0;
+	unsigned char new_fanout;

 	/* <dataref> or 'inline' */
 	if (*p == ':') {
@@ -2071,7 +2178,7 @@ static void note_change_n(struct branch *b)
 		if (oe->type != OBJ_BLOB)
 			die("Not a blob (actually a %s): %s",
 				typename(oe->type), command_buf.buf);
-	} else {
+	} else if (!is_null_sha1(sha1)) {
 		enum object_type type = sha1_object_info(sha1, NULL);
 		if (type < 0)
 			die("Blob not found: %s", command_buf.buf);
@@ -2080,8 +2187,17 @@ static void note_change_n(struct branch *b)
 			    typename(type), command_buf.buf);
 	}

-	tree_content_set(&b->branch_tree, sha1_to_hex(commit_sha1), sha1,
-		S_IFREG | 0644, NULL);
+	construct_path_with_fanout(sha1_to_hex(commit_sha1), old_fanout, path);
+	if (tree_content_remove(&b->branch_tree, path, NULL))
+		b->num_notes--;
+
+	if (is_null_sha1(sha1))
+		return; /* nothing to insert */
+
+	b->num_notes++;
+	new_fanout = convert_num_notes_to_fanout(b->num_notes);
+	construct_path_with_fanout(sha1_to_hex(commit_sha1), new_fanout, path);
+	tree_content_set(&b->branch_tree, path, sha1, S_IFREG | 0644, NULL);
 }

 static void file_change_deleteall(struct branch *b)
@@ -2090,6 +2206,7 @@ static void file_change_deleteall(struct branch *b)
 	hashclr(b->branch_tree.versions[0].sha1);
 	hashclr(b->branch_tree.versions[1].sha1);
 	load_tree(&b->branch_tree);
+	b->num_notes = 0;
 }

 static void parse_from_commit(struct branch *b, char *buf, unsigned long size)
@@ -2213,6 +2330,7 @@ static void parse_new_commit(void)
 	char *committer = NULL;
 	struct hash_list *merge_list = NULL;
 	unsigned int merge_count;
+	unsigned char prev_fanout, new_fanout;

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

+	prev_fanout = convert_num_notes_to_fanout(b->num_notes);
+
 	/* file_change* */
 	while (command_buf.len > 0) {
 		if (!prefixcmp(command_buf.buf, "M "))
@@ -2254,7 +2374,7 @@ static void parse_new_commit(void)
 		else if (!prefixcmp(command_buf.buf, "C "))
 			file_change_cr(b, 0);
 		else if (!prefixcmp(command_buf.buf, "N "))
-			note_change_n(b);
+			note_change_n(b, prev_fanout);
 		else if (!strcmp("deleteall", command_buf.buf))
 			file_change_deleteall(b);
 		else {
@@ -2265,6 +2385,10 @@ static void parse_new_commit(void)
 			break;
 	}

+	new_fanout = convert_num_notes_to_fanout(b->num_notes);
+	if (new_fanout != prev_fanout)
+		b->num_notes = change_note_fanout(&b->branch_tree, new_fanout);
+
 	/* build the tree and the commit */
 	store_tree(&b->branch_tree);
 	hashcpy(b->branch_tree.versions[0].sha1,
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index b49815d..bf8c509 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1092,9 +1092,12 @@ test_expect_success 'P: fail on blob mark in gitlink' '
 ### series Q (notes)
 ###

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

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

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

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

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

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

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

 test_done
--
1.6.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv10 03/11] Add more testcases to test fast-import of notes
From: Johan Herland @ 2009-12-07 11:27 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1260185254-1523-1-git-send-email-johan@herland.net>

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

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

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

^ permalink raw reply related

* [RFC/PATCHv10 04/11] Minor style fixes to notes.c
From: Johan Herland @ 2009-12-07 11:27 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1260185254-1523-1-git-send-email-johan@herland.net>

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/notes.c b/notes.c
index 023adce..57d5cdc 100644
--- a/notes.c
+++ b/notes.c
@@ -93,7 +93,7 @@ static void **note_tree_search(struct int_node **tree,
 
 	i = GET_NIBBLE(*n, key_sha1);
 	p = (*tree)->a[i];
-	switch(GET_PTR_TYPE(p)) {
+	switch (GET_PTR_TYPE(p)) {
 	case PTR_TYPE_INTERNAL:
 		*tree = CLR_PTR_TYPE(p);
 		(*n)++;
@@ -195,7 +195,7 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
 
 	assert(GET_PTR_TYPE(entry) == 0); /* no type bits set */
 	l = (struct leaf_node *) CLR_PTR_TYPE(*p);
-	switch(GET_PTR_TYPE(*p)) {
+	switch (GET_PTR_TYPE(*p)) {
 	case PTR_TYPE_NULL:
 		assert(!*p);
 		*p = SET_PTR_TYPE(entry, type);
@@ -257,7 +257,7 @@ 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)) {
+		switch (GET_PTR_TYPE(p)) {
 		case PTR_TYPE_INTERNAL:
 			note_tree_free(CLR_PTR_TYPE(p));
 			/* fall through */
-- 
1.6.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv10 05/11] Notes API: get_commit_notes() -> format_note() + remove the commit restriction
From: Johan Herland @ 2009-12-07 11:27 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1260185254-1523-1-git-send-email-johan@herland.net>

There is really no reason why only commit objects can be annotated. By
changing the struct commit parameter to get_commit_notes() into a sha1 we
gain the ability to annotate any object type. To reflect this in the function
naming as well, we rename get_commit_notes() to format_note().

This patch also fixes comments and variable names throughout notes.c as a
consequence of the removal of the unnecessary 'commit' restriction.

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c  |   33 ++++++++++++++++-----------------
 notes.h  |   11 ++++++++++-
 pretty.c |    8 ++++----
 3 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/notes.c b/notes.c
index 57d5cdc..5590414 100644
--- a/notes.c
+++ b/notes.c
@@ -1,5 +1,4 @@
 #include "cache.h"
-#include "commit.h"
 #include "notes.h"
 #include "refs.h"
 #include "utf8.h"
@@ -25,10 +24,10 @@ struct int_node {
 /*
  * 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
+ * As a note entry, the key is the SHA1 of the referenced object, 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
+ * referenced object, 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.
  */
@@ -211,7 +210,7 @@ static void note_tree_insert(struct int_node *tree, unsigned char n,
 				if (concatenate_notes(l->val_sha1,
 						entry->val_sha1))
 					die("failed to concatenate note %s "
-					    "into note %s for commit %s",
+					    "into note %s for object %s",
 					    sha1_to_hex(entry->val_sha1),
 					    sha1_to_hex(l->val_sha1),
 					    sha1_to_hex(l->key_sha1));
@@ -299,7 +298,7 @@ static int get_sha1_hex_segment(const char *hex, unsigned int hex_len,
 static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 		unsigned int n)
 {
-	unsigned char commit_sha1[20];
+	unsigned char object_sha1[20];
 	unsigned int prefix_len;
 	void *buf;
 	struct tree_desc desc;
@@ -312,23 +311,23 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 
 	prefix_len = subtree->key_sha1[19];
 	assert(prefix_len * 2 >= n);
-	memcpy(commit_sha1, subtree->key_sha1, prefix_len);
+	memcpy(object_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);
+				object_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 object SHA1 is complete (len == 20), assume note object
+		 * If object 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->key_sha1, object_sha1);
 			hashcpy(l->val_sha1, entry.sha1);
 			if (len < 20) {
 				if (!S_ISDIR(entry.mode))
@@ -344,12 +343,12 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 
 static void initialize_notes(const char *notes_ref_name)
 {
-	unsigned char sha1[20], commit_sha1[20];
+	unsigned char sha1[20], object_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))
+	if (!notes_ref_name || read_ref(notes_ref_name, object_sha1) ||
+	    get_tree_entry(object_sha1, "", sha1, &mode))
 		return;
 
 	hashclr(root_tree.key_sha1);
@@ -357,9 +356,9 @@ static void initialize_notes(const char *notes_ref_name)
 	load_subtree(&root_tree, &root_node, 0);
 }
 
-static unsigned char *lookup_notes(const unsigned char *commit_sha1)
+static unsigned char *lookup_notes(const unsigned char *object_sha1)
 {
-	struct leaf_node *found = note_tree_find(&root_node, 0, commit_sha1);
+	struct leaf_node *found = note_tree_find(&root_node, 0, object_sha1);
 	if (found)
 		return found->val_sha1;
 	return NULL;
@@ -372,7 +371,7 @@ void free_notes(void)
 	initialized = 0;
 }
 
-void get_commit_notes(const struct commit *commit, struct strbuf *sb,
+void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 		const char *output_encoding, int flags)
 {
 	static const char utf8[] = "utf-8";
@@ -391,7 +390,7 @@ void get_commit_notes(const struct commit *commit, struct strbuf *sb,
 		initialized = 1;
 	}
 
-	sha1 = lookup_notes(commit->object.sha1);
+	sha1 = lookup_notes(object_sha1);
 	if (!sha1)
 		return;
 
diff --git a/notes.h b/notes.h
index a1421e3..d745ed1 100644
--- a/notes.h
+++ b/notes.h
@@ -4,10 +4,19 @@
 /* Free (and de-initialize) the internal notes tree structure */
 void free_notes(void);
 
+/* Flags controlling how notes are formatted */
 #define NOTES_SHOW_HEADER 1
 #define NOTES_INDENT 2
 
-void get_commit_notes(const struct commit *commit, struct strbuf *sb,
+/*
+ * Fill the given strbuf with the notes associated with the given object.
+ *
+ * If the internal notes structure is not initialized, it will be auto-
+ * initialized to the default value (see documentation for init_notes() above).
+ *
+ * 'flags' is a bitwise combination of the above formatting flags.
+ */
+void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 		const char *output_encoding, int flags);
 
 #endif
diff --git a/pretty.c b/pretty.c
index 8f5bd1a..fe77090 100644
--- a/pretty.c
+++ b/pretty.c
@@ -775,8 +775,8 @@ static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
 		}
 		return 0;	/* unknown %g placeholder */
 	case 'N':
-		get_commit_notes(commit, sb, git_log_output_encoding ?
-			     git_log_output_encoding : git_commit_encoding, 0);
+		format_note(commit->object.sha1, sb, git_log_output_encoding ?
+			    git_log_output_encoding : git_commit_encoding, 0);
 		return 1;
 	}
 
@@ -1095,8 +1095,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,
-				 NOTES_SHOW_HEADER | NOTES_INDENT);
+		format_note(commit->object.sha1, sb, encoding,
+			    NOTES_SHOW_HEADER | NOTES_INDENT);
 
 	free(reencoded);
 }
-- 
1.6.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv10 06/11] Notes API: init_notes(): Initialize the notes tree from the given notes ref
From: Johan Herland @ 2009-12-07 11:27 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1260185254-1523-1-git-send-email-johan@herland.net>

Created by a simple refactoring of initialize_notes().

Also add a new 'flags' parameter, which is a bitwise combination of notes
initialization flags. For now, there is only one flag - NOTES_INIT_EMPTY -
which indicates that the notes tree should not auto-load the contents of
the given (or default) notes ref, but rather should leave the notes tree
initialized to an empty state. This will become useful in the future when
manipulating the notes tree through the notes API.

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |   27 ++++++++++++++++-----------
 notes.h |   20 ++++++++++++++++++++
 2 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/notes.c b/notes.c
index 5590414..e0dfa24 100644
--- a/notes.c
+++ b/notes.c
@@ -341,13 +341,25 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 	free(buf);
 }
 
-static void initialize_notes(const char *notes_ref_name)
+void init_notes(const char *notes_ref, int flags)
 {
 	unsigned char sha1[20], object_sha1[20];
 	unsigned mode;
 	struct leaf_node root_tree;
 
-	if (!notes_ref_name || read_ref(notes_ref_name, object_sha1) ||
+	assert(!initialized);
+	initialized = 1;
+
+	if (!notes_ref) {
+		const char *env = getenv(GIT_NOTES_REF_ENVIRONMENT);
+		if (env)
+			notes_ref = getenv(GIT_NOTES_REF_ENVIRONMENT);
+		else
+			notes_ref = GIT_NOTES_DEFAULT_REF;
+	}
+
+	if (flags & NOTES_INIT_EMPTY || !notes_ref ||
+	    read_ref(notes_ref, object_sha1) ||
 	    get_tree_entry(object_sha1, "", sha1, &mode))
 		return;
 
@@ -380,15 +392,8 @@ void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 	unsigned long linelen, msglen;
 	enum object_type type;
 
-	if (!initialized) {
-		const char *env = getenv(GIT_NOTES_REF_ENVIRONMENT);
-		if (env)
-			notes_ref_name = getenv(GIT_NOTES_REF_ENVIRONMENT);
-		else if (!notes_ref_name)
-			notes_ref_name = GIT_NOTES_DEFAULT_REF;
-		initialize_notes(notes_ref_name);
-		initialized = 1;
-	}
+	if (!initialized)
+		init_notes(NULL, 0);
 
 	sha1 = lookup_notes(object_sha1);
 	if (!sha1)
diff --git a/notes.h b/notes.h
index d745ed1..6b52799 100644
--- a/notes.h
+++ b/notes.h
@@ -1,6 +1,26 @@
 #ifndef NOTES_H
 #define NOTES_H
 
+/*
+ * Flags controlling behaviour of notes tree initialization
+ *
+ * Default behaviour is to initialize the notes tree from the tree object
+ * specified by the given (or default) notes ref.
+ */
+#define NOTES_INIT_EMPTY 1
+
+/*
+ * Initialize internal notes tree structure with the notes tree at the given
+ * ref. If given ref is NULL, the value of the $GIT_NOTES_REF environment
+ * variable is used, and if that is missing, the default notes ref is used
+ * ("refs/notes/commits").
+ *
+ * If you need to re-intialize the internal notes tree structure (e.g. loading
+ * from a different notes ref), please first de-initialize the current notes
+ * tree by calling free_notes().
+ */
+void init_notes(const char *notes_ref, int flags);
+
 /* Free (and de-initialize) the internal notes tree structure */
 void free_notes(void);
 
-- 
1.6.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv10 07/11] Notes API: add_note(): Add note objects to the internal notes tree structure
From: Johan Herland @ 2009-12-07 11:27 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1260185254-1523-1-git-send-email-johan@herland.net>

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |   11 +++++++++++
 notes.h |    4 ++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/notes.c b/notes.c
index e0dfa24..79bfa24 100644
--- a/notes.c
+++ b/notes.c
@@ -368,6 +368,17 @@ void init_notes(const char *notes_ref, int flags)
 	load_subtree(&root_tree, &root_node, 0);
 }
 
+void add_note(const unsigned char *object_sha1, const unsigned char *note_sha1)
+{
+	struct leaf_node *l;
+
+	assert(initialized);
+	l = (struct leaf_node *) xmalloc(sizeof(struct leaf_node));
+	hashcpy(l->key_sha1, object_sha1);
+	hashcpy(l->val_sha1, note_sha1);
+	note_tree_insert(&root_node, 0, l, PTR_TYPE_NOTE);
+}
+
 static unsigned char *lookup_notes(const unsigned char *object_sha1)
 {
 	struct leaf_node *found = note_tree_find(&root_node, 0, object_sha1);
diff --git a/notes.h b/notes.h
index 6b52799..5f22852 100644
--- a/notes.h
+++ b/notes.h
@@ -21,6 +21,10 @@
  */
 void init_notes(const char *notes_ref, int flags);
 
+/* Add the given note object to the internal notes tree structure */
+void add_note(const unsigned char *object_sha1,
+		const unsigned char *note_sha1);
+
 /* Free (and de-initialize) the internal notes tree structure */
 void free_notes(void);
 
-- 
1.6.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv10 09/11] Notes API: for_each_note(): Traverse the entire notes tree with a callback
From: Johan Herland @ 2009-12-07 11:27 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1260185254-1523-1-git-send-email-johan@herland.net>

This includes a first attempt at creating an optimal fanout scheme (which
is calculated on-the-fly, while traversing).

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |  101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 notes.h |   17 ++++++++++
 2 files changed, 118 insertions(+), 0 deletions(-)

diff --git a/notes.c b/notes.c
index 110404a..de5a847 100644
--- a/notes.c
+++ b/notes.c
@@ -341,6 +341,101 @@ static void load_subtree(struct leaf_node *subtree, struct int_node *node,
 	free(buf);
 }
 
+/*
+ * Determine optimal on-disk fanout for this part of the notes tree
+ *
+ * Given a (sub)tree and the level in the internal tree structure, determine
+ * whether or not the given existing fanout should be expanded for this
+ * (sub)tree.
+ *
+ * Values of the 'fanout' variable:
+ * - 0: No fanout (all notes are stored directly in the root notes tree)
+ * - 1: 2/38 fanout
+ * - 2: 2/2/36 fanout
+ * - 3: 2/2/2/34 fanout
+ * etc.
+ */
+static unsigned char determine_fanout(struct int_node *tree, unsigned char n,
+		unsigned char fanout)
+{
+	/*
+	 * The following is a simple heuristic that works well in practice:
+	 * For each even-numbered 16-tree level (remember that each on-disk
+	 * fanout level corresponds to two 16-tree levels), peek at all 16
+	 * entries at that tree level. If any of them are subtree entries, then
+	 * there are likely plenty of notes below this level, so we return an
+	 * incremented fanout immediately. Otherwise, we return an incremented
+	 * fanout only if all of the entries at this level are int_nodes.
+	 */
+	unsigned int i;
+	if ((n % 2) || (n > 2 * fanout))
+		return fanout;
+	for (i = 0; i < 16; i++) {
+		switch (GET_PTR_TYPE(tree->a[i])) {
+		case PTR_TYPE_SUBTREE:
+			return fanout + 1;
+		case PTR_TYPE_INTERNAL:
+			continue;
+		default:
+			return fanout;
+		}
+	}
+	return fanout + 1;
+}
+
+static void construct_path_with_fanout(const unsigned char *sha1,
+		unsigned char fanout, char *path)
+{
+	unsigned int i = 0, j = 0;
+	const char *hex_sha1 = sha1_to_hex(sha1);
+	assert(fanout < 20);
+	while (fanout) {
+		path[i++] = hex_sha1[j++];
+		path[i++] = hex_sha1[j++];
+		path[i++] = '/';
+		fanout--;
+	}
+	strcpy(path + i, hex_sha1 + j);
+}
+
+static int for_each_note_helper(struct int_node *tree, unsigned char n,
+		unsigned char fanout, each_note_fn fn, void *cb_data)
+{
+	unsigned int i;
+	void *p;
+	int ret = 0;
+	struct leaf_node *l;
+	static char path[40 + 19 + 1];  /* hex SHA1 + 19 * '/' + NUL */
+
+	fanout = determine_fanout(tree, n, fanout);
+	for (i = 0; i < 16; i++) {
+redo:
+		p = tree->a[i];
+		switch (GET_PTR_TYPE(p)) {
+		case PTR_TYPE_INTERNAL:
+			/* recurse into int_node */
+			ret = for_each_note_helper(
+				CLR_PTR_TYPE(p), n + 1, fanout, fn, cb_data);
+			break;
+		case PTR_TYPE_SUBTREE:
+			/* unpack subtree and resume traversal */
+			l = (struct leaf_node *) CLR_PTR_TYPE(p);
+			tree->a[i] = NULL;
+			load_subtree(l, tree, n);
+			free(l);
+			goto redo;
+		case PTR_TYPE_NOTE:
+			l = (struct leaf_node *) CLR_PTR_TYPE(p);
+			construct_path_with_fanout(l->key_sha1, fanout, path);
+			ret = fn(l->key_sha1, l->val_sha1, path, cb_data);
+			break;
+		}
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
 void init_notes(const char *notes_ref, int flags)
 {
 	unsigned char sha1[20], object_sha1[20];
@@ -388,6 +483,12 @@ const unsigned char *get_note(const unsigned char *object_sha1)
 	return found ? found->val_sha1 : NULL;
 }
 
+int for_each_note(each_note_fn fn, void *cb_data)
+{
+	assert(initialized);
+	return for_each_note_helper(&root_node, 0, 0, fn, cb_data);
+}
+
 void free_notes(void)
 {
 	note_tree_free(&root_node);
diff --git a/notes.h b/notes.h
index 21a8930..f67bae8 100644
--- a/notes.h
+++ b/notes.h
@@ -28,6 +28,23 @@ void add_note(const unsigned char *object_sha1,
 /* Get the note object SHA1 containing the note data for the given object */
 const unsigned char *get_note(const unsigned char *object_sha1);
 
+/*
+ * Invoke the specified callback function for each note
+ *
+ * If the callback returns nonzero, the note walk is aborted, and the return
+ * value from the callback is returned from for_each_note().
+ *
+ * IMPORTANT: The callback function is NOT allowed to change the notes tree.
+ * In other words, the following functions can NOT be invoked (on the current
+ * notes tree) from within the callback:
+ * - add_note()
+ * - free_notes()
+ */
+typedef int each_note_fn(const unsigned char *object_sha1,
+		const unsigned char *note_sha1, const char *note_tree_path,
+		void *cb_data);
+int for_each_note(each_note_fn fn, void *cb_data);
+
 /* Free (and de-initialize) the internal notes tree structure */
 void free_notes(void);
 
-- 
1.6.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv10 08/11] Notes API: get_note(): Return the note annotating the given object
From: Johan Herland @ 2009-12-07 11:27 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1260185254-1523-1-git-send-email-johan@herland.net>

Created by a simple cleanup and rename of lookup_notes().

Signed-off-by: Johan Herland <johan@herland.net>
---
 notes.c |   15 ++++++++-------
 notes.h |    3 +++
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/notes.c b/notes.c
index 79bfa24..110404a 100644
--- a/notes.c
+++ b/notes.c
@@ -379,12 +379,13 @@ void add_note(const unsigned char *object_sha1, const unsigned char *note_sha1)
 	note_tree_insert(&root_node, 0, l, PTR_TYPE_NOTE);
 }
 
-static unsigned char *lookup_notes(const unsigned char *object_sha1)
+const unsigned char *get_note(const unsigned char *object_sha1)
 {
-	struct leaf_node *found = note_tree_find(&root_node, 0, object_sha1);
-	if (found)
-		return found->val_sha1;
-	return NULL;
+	struct leaf_node *found;
+
+	assert(initialized);
+	found = note_tree_find(&root_node, 0, object_sha1);
+	return found ? found->val_sha1 : NULL;
 }
 
 void free_notes(void)
@@ -398,7 +399,7 @@ void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 		const char *output_encoding, int flags)
 {
 	static const char utf8[] = "utf-8";
-	unsigned char *sha1;
+	const unsigned char *sha1;
 	char *msg, *msg_p;
 	unsigned long linelen, msglen;
 	enum object_type type;
@@ -406,7 +407,7 @@ void format_note(const unsigned char *object_sha1, struct strbuf *sb,
 	if (!initialized)
 		init_notes(NULL, 0);
 
-	sha1 = lookup_notes(object_sha1);
+	sha1 = get_note(object_sha1);
 	if (!sha1)
 		return;
 
diff --git a/notes.h b/notes.h
index 5f22852..21a8930 100644
--- a/notes.h
+++ b/notes.h
@@ -25,6 +25,9 @@ void init_notes(const char *notes_ref, int flags);
 void add_note(const unsigned char *object_sha1,
 		const unsigned char *note_sha1);
 
+/* Get the note object SHA1 containing the note data for the given object */
+const unsigned char *get_note(const unsigned char *object_sha1);
+
 /* Free (and de-initialize) the internal notes tree structure */
 void free_notes(void);
 
-- 
1.6.5.3.433.g11067

^ permalink raw reply related

* [RFC/PATCHv10 10/11] Notes API: Allow multiple concurrent notes trees with new struct notes_tree
From: Johan Herland @ 2009-12-07 11:27 UTC (permalink / raw)
  To: git; +Cc: gitster, johan, spearce
In-Reply-To: <1260185254-1523-1-git-send-email-johan@herland.net>

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

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

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

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

^ permalink raw reply related

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

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

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

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

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

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

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

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

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

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

^ permalink raw reply related

* Re: [PATCHv2 2/2] Add a command "fixup" to rebase --interactive
From: Michael Haggerty @ 2009-12-07 11:41 UTC (permalink / raw)
  To: Sverre Rabbelier; +Cc: git, gitster, git, Johannes.Schindelin, bgustavsson
In-Reply-To: <fabb9a1e0912070326s6cda5c8r442c4816538d0e2a@mail.gmail.com>

Sverre Rabbelier wrote:
> On Mon, Dec 7, 2009 at 05:22, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>> The command is like "squash", except that it discards the commit message
>> of the corresponding commit.
> 
> No no, wait, wasn't "fixup" supposed to let you just edit the commit
> message of the commit you're fixing up? :(

That command is called "reword".

Michael

^ permalink raw reply

* Re: [PATCHv2 2/2] Add a command "fixup" to rebase --interactive
From: Matthieu Moy @ 2009-12-07 11:57 UTC (permalink / raw)
  To: Sverre Rabbelier
  Cc: Michael Haggerty, git, gitster, git, Johannes.Schindelin,
	bgustavsson
In-Reply-To: <fabb9a1e0912070326s6cda5c8r442c4816538d0e2a@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

> Heya,
>
> On Mon, Dec 7, 2009 at 05:22, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>> The command is like "squash", except that it discards the commit message
>> of the corresponding commit.
>
> No no, wait, wasn't "fixup" supposed to let you just edit the commit
> message of the commit you're fixing up? :(

This is "reword", already in Git (6741aa6c399dec3d8f0b2, Wed Oct 7
08:13:23 2009).

--
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: [PATCHv2 2/2] Add a command "fixup" to rebase --interactive
From: Sverre Rabbelier @ 2009-12-07 12:04 UTC (permalink / raw)
  To: Matthieu Moy
  Cc: Michael Haggerty, git, gitster, git, Johannes.Schindelin,
	bgustavsson
In-Reply-To: <vpqtyw3808q.fsf@bauges.imag.fr>

Heya,

On Mon, Dec 7, 2009 at 12:57, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
> This is "reword", already in Git (6741aa6c399dec3d8f0b2, Wed Oct 7
> 08:13:23 2009).

Oh, wow, I've been missing out then :D. Thanks both!

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [RFC PATCH v3 0/8] Remote helpers smart transport extensions
From: Nanako Shiraishi @ 2009-12-07 12:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ilari Liusvaara, git
In-Reply-To: <7v7hsz9qxj.fsf@alter.siamese.dyndns.org>

Quoting Junio C Hamano <gitster@pobox.com>

> I queued to ease the discussion in 'pu'.  I had to fix-up some conflicts
> while doing so.  Please sanity check the result.

I see that you changed many "char* variable" to "char *variable", but
what is the reason for these changes?

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* Re: Re: [RFC PATCH v2 0/2] git-gui: (un)stage a range of changes at once
From: Heiko Voigt @ 2009-12-07 12:54 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Jeff Epler, git
In-Reply-To: <20091205213613.GG5610@spearce.org>

On Sat, Dec 05, 2009 at 01:36:13PM -0800, Shawn O. Pearce wrote:
> Jeff Epler <jepler@unpythonic.net> wrote:
> > I've found another problem, which I'll work on as soon as I find a
> > chance.
> > 
> > When staging multiple "+" lines preceded by a "-" line that must be
> > turned into context, the converted "-" line must come after *all* the
> > "+" lines, not just the first one.
> 
> So the reason this series got stuck was this message, this bug is
> enough to suggest we shouldn't apply it to my tree yet, so I've
> been waiting for an update on the topic.

In an attempt to help this series forward I tried to reproduce this bug
but were unsuccessfull. It seems that a change like this:

@@ -13,7 +13,9 @@ set appvers {@@GITGUI_VERSION@@}
 set copyright [encoding convertfrom utf-8 {
 Copyright © 2006, 2007 Shawn Pearce, et. al.
 
-This program is free software; you can redistribute it and/or modify
+Blabla
+blubblub
+lalala
 it under the terms of the GNU General Public License as published by
 the Free Software Foundation; either version 2 of the License, or
 (at your option) any later version.

and then trying to stage part of the '+' lines is not enough. Jeff could you
clarify or provide an example?

cheers Heiko

^ permalink raw reply

* Re: [RFC PATCH v3 0/8] Remote helpers smart transport extensions
From: Erik Faye-Lund @ 2009-12-07 12:57 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Junio C Hamano, Ilari Liusvaara, git
In-Reply-To: <20091207210608.6117@nanako3.lavabit.com>

On Mon, Dec 7, 2009 at 1:06 PM, Nanako Shiraishi <nanako3@lavabit.com> wrote:
> Quoting Junio C Hamano <gitster@pobox.com>
>
>> I queued to ease the discussion in 'pu'.  I had to fix-up some conflicts
>> while doing so.  Please sanity check the result.
>
> I see that you changed many "char* variable" to "char *variable", but
> what is the reason for these changes?
>

Documentation/CodingGuidelines:

 - When declaring pointers, the star sides with the variable
   name, i.e. "char *string", not "char* string" or
   "char * string".  This makes it easier to understand code
   like "char *string, c;".

-- 
Erik "kusma" Faye-Lund

^ permalink raw reply

* help: bisect single file from repos
From: walter harms @ 2009-12-07 12:59 UTC (permalink / raw)
  To: git

Hi list,
i am new to git (using: git version 1.6.0.2).

I would like to bisect a single file but i have only commit id, no tags.

Background:
I have a copy of the busybox git repos, and i know there is (perhaps) a bug
in ash.c.

how can i do that ?

re,
 wh

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox