[PATCH 0/3] "git notes remove" updates

[PATCH 0/3] "git notes remove" updates

[Junio C Hamano]

I wanted to do a bit better than

	for sha1 in ... list of old commit objects
	do
		git notes --ref refs/notes/amlog remove $sha1
	done

to remove old entries in my "where did this commit come from" database.

Junio C Hamano (3):
  notes remove: allow removing more than one
  notes remove: --missing-ok
  notes remove: --stdin reads from the standard input

 Documentation/git-notes.txt |   16 ++++++++--
 builtin/notes.c             |   65 ++++++++++++++++++++++++++-------------
 t/t3301-notes.sh            |   71 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 127 insertions(+), 25 deletions(-)

-- 
1.7.5.1.414.gb4910

[PATCH 1/3] notes remove: allow removing more than one

[Junio C Hamano]

While "xargs -n1 git notes rm" is certainly a possible way to remove notes
from many objects, this would create one notes "commit" per removal, which
is not quite suitable for seasonal housekeeping.

Allow taking more than one on the command line, and record their removal
as a single atomic event if everthing goes well.

Even though the old code insisted that "git notes rm" must be given only
one object (or zero, in which case it would default to HEAD), this
condition was not tested. Add tests to handle the new case where we feed
multiple objects, and also make sure if there is a bad input, no change
is recorded.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-notes.txt |    7 +++--
 builtin/notes.c             |   47 ++++++++++++++++++++++--------------------
 t/t3301-notes.sh            |   19 +++++++++++++++++
 3 files changed, 48 insertions(+), 25 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 913ecd8..e2e1c4c 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -17,7 +17,7 @@ SYNOPSIS
 'git notes' merge [-v | -q] [-s <strategy> ] <notes_ref>
 'git notes' merge --commit [-v | -q]
 'git notes' merge --abort [-v | -q]
-'git notes' remove [<object>]
+'git notes' remove [<object>...]
 'git notes' prune [-n | -v]
 'git notes' get-ref
 
@@ -106,8 +106,9 @@ When done, the user can either finalize the merge with
 'git notes merge --abort'.
 
 remove::
-	Remove the notes for a given object (defaults to HEAD).
-	This is equivalent to specifying an empty note message to
+	Remove the notes for given objects (defaults to HEAD). When
+	giving zero or one object from the command line, this is
+	equivalent to specifying an empty note message to
 	the `edit` subcommand.
 
 prune::
diff --git a/builtin/notes.c b/builtin/notes.c
index 1fb1f73..30cee0f 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -29,7 +29,7 @@ static const char * const git_notes_usage[] = {
 	"git notes [--ref <notes_ref>] merge [-v | -q] [-s <strategy> ] <notes_ref>",
 	"git notes merge --commit [-v | -q]",
 	"git notes merge --abort [-v | -q]",
-	"git notes [--ref <notes_ref>] remove [<object>]",
+	"git notes [--ref <notes_ref>] remove [<object>...]",
 	"git notes [--ref <notes_ref>] prune [-n | -v]",
 	"git notes [--ref <notes_ref>] get-ref",
 	NULL
@@ -953,40 +953,43 @@ static int merge(int argc, const char **argv, const char *prefix)
 	return result < 0; /* return non-zero on conflicts */
 }
 
+static int remove_one_note(struct notes_tree *t, const char *name)
+{
+	int status;
+	unsigned char sha1[20];
+	if (get_sha1(name, sha1))
+		return error(_("Failed to resolve '%s' as a valid ref."), name);
+	status = remove_note(t, sha1);
+	if (status)
+		fprintf(stderr, _("Object %s has no note\n"), name);
+	else
+		fprintf(stderr, _("Removing note for object %s\n"), name);
+	return status;
+}
+
 static int remove_cmd(int argc, const char **argv, const char *prefix)
 {
 	struct option options[] = {
 		OPT_END()
 	};
-	const char *object_ref;
 	struct notes_tree *t;
-	unsigned char object[20];
-	int retval;
+	int retval = 0;
 
 	argc = parse_options(argc, argv, prefix, options,
 			     git_notes_remove_usage, 0);
 
-	if (1 < argc) {
-		error(_("too many parameters"));
-		usage_with_options(git_notes_remove_usage, options);
-	}
-
-	object_ref = argc ? argv[0] : "HEAD";
-
-	if (get_sha1(object_ref, object))
-		die(_("Failed to resolve '%s' as a valid ref."), object_ref);
-
 	t = init_notes_check("remove");
 
-	retval = remove_note(t, object);
-	if (retval)
-		fprintf(stderr, _("Object %s has no note\n"), sha1_to_hex(object));
-	else {
-		fprintf(stderr, _("Removing note for object %s\n"),
-			sha1_to_hex(object));
-
-		commit_notes(t, "Notes removed by 'git notes remove'");
+	if (!argc) {
+		retval = remove_one_note(t, "HEAD");
+	} else {
+		while (*argv) {
+			retval |= remove_one_note(t, *argv);
+			argv++;
+		}
 	}
+	if (!retval)
+		commit_notes(t, "Notes removed by 'git notes remove'");
 	free_notes(t);
 	return retval;
 }
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 28e17c8..6a6daa9 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -435,6 +435,25 @@ test_expect_success 'removing non-existing note should not create new commit' '
 	test_cmp before_commit after_commit
 '
 
+test_expect_success 'removing more than one' '
+	before=$(git rev-parse --verify refs/notes/commits) &&
+	test_when_finished "git update-ref refs/notes/commits $before" &&
+	git notes remove HEAD^^ HEAD^^^ &&
+	git diff --name-only refs/notes/commits^ refs/notes/commits >actual &&
+	test 2 = $(wc -l <actual) &&
+	git ls-tree -r --name-only refs/notes/commits >actual &&
+	>empty &&
+	test_cmp empty actual
+'
+
+test_expect_success 'removing is atomic' '
+	before=$(git rev-parse --verify refs/notes/commits) &&
+	test_when_finished "git update-ref refs/notes/commits $before" &&
+	test_must_fail git notes remove HEAD^^ HEAD^^^ HEAD^ &&
+	after=$(git rev-parse --verify refs/notes/commits) &&
+	test "$before" = "$after"
+'
+
 test_expect_success 'list notes with "git notes list"' '
 	git notes list > output &&
 	test_cmp expect output
-- 
1.7.5.1.414.gb4910

[PATCH 2/3] notes remove: --missing-ok

[Junio C Hamano]

Depending on the application, it is not necessarily an error for an object
to lack a note, especially if the only thing the caller wants to make sure
is that notes are cleared for an object.  By passing this option from the
command line, the "git notes remove" command considers it a success if the
object did not have any note to begin with.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * It may be that this should have been the default, I suspect.
---
 Documentation/git-notes.txt |    6 +++++-
 builtin/notes.c             |   14 ++++++++++----
 t/t3301-notes.sh            |   19 +++++++++++++++++++
 3 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index e2e1c4c..6b92060 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -17,7 +17,7 @@ SYNOPSIS
 'git notes' merge [-v | -q] [-s <strategy> ] <notes_ref>
 'git notes' merge --commit [-v | -q]
 'git notes' merge --abort [-v | -q]
-'git notes' remove [<object>...]
+'git notes' remove [--missing-ok] [<object>...]
 'git notes' prune [-n | -v]
 'git notes' get-ref
 
@@ -155,6 +155,10 @@ OPTIONS
 	'GIT_NOTES_REF' and the "core.notesRef" configuration.  The ref
 	is taken to be in `refs/notes/` if it is not qualified.
 
+--missing-ok::
+	Do not consider it an error to request removing notes from an
+	object that does not have notes attached to it.
+
 -n::
 --dry-run::
 	Do not remove anything; just report the object names whose notes
diff --git a/builtin/notes.c b/builtin/notes.c
index 30cee0f..ca045f8 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -953,7 +953,9 @@ static int merge(int argc, const char **argv, const char *prefix)
 	return result < 0; /* return non-zero on conflicts */
 }
 
-static int remove_one_note(struct notes_tree *t, const char *name)
+#define MISSING_OK 1
+
+static int remove_one_note(struct notes_tree *t, const char *name, unsigned flag)
 {
 	int status;
 	unsigned char sha1[20];
@@ -964,12 +966,16 @@ static int remove_one_note(struct notes_tree *t, const char *name)
 		fprintf(stderr, _("Object %s has no note\n"), name);
 	else
 		fprintf(stderr, _("Removing note for object %s\n"), name);
-	return status;
+	return (flag & MISSING_OK) ? 0 : status;
 }
 
 static int remove_cmd(int argc, const char **argv, const char *prefix)
 {
+	unsigned flag = 0;
 	struct option options[] = {
+		OPT_BIT(0, "missing-ok", &flag,
+			"attempt to remove non-existent note is not an error",
+			MISSING_OK),
 		OPT_END()
 	};
 	struct notes_tree *t;
@@ -981,10 +987,10 @@ static int remove_cmd(int argc, const char **argv, const char *prefix)
 	t = init_notes_check("remove");
 
 	if (!argc) {
-		retval = remove_one_note(t, "HEAD");
+		retval = remove_one_note(t, "HEAD", flag);
 	} else {
 		while (*argv) {
-			retval |= remove_one_note(t, *argv);
+			retval |= remove_one_note(t, *argv, flag);
 			argv++;
 		}
 	}
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 6a6daa9..bdd4036 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -454,6 +454,25 @@ test_expect_success 'removing is atomic' '
 	test "$before" = "$after"
 '
 
+test_expect_success 'removing with --missing-ok' '
+	before=$(git rev-parse --verify refs/notes/commits) &&
+	test_when_finished "git update-ref refs/notes/commits $before" &&
+	git notes remove --missing-ok HEAD^^ HEAD^^^ HEAD^ &&
+	git diff --name-only refs/notes/commits^ refs/notes/commits >actual &&
+	test 2 = $(wc -l <actual) &&
+	git ls-tree -r --name-only refs/notes/commits >actual &&
+	>empty &&
+	test_cmp empty actual
+'
+
+test_expect_success 'removing with --missing-ok but bogus ref' '
+	before=$(git rev-parse --verify refs/notes/commits) &&
+	test_when_finished "git update-ref refs/notes/commits $before" &&
+	test_must_fail git notes remove --missing-ok HEAD^^ HEAD^^^ NO-SUCH-COMMIT &&
+	after=$(git rev-parse --verify refs/notes/commits) &&
+	test "$before" = "$after"
+'
+
 test_expect_success 'list notes with "git notes list"' '
 	git notes list > output &&
 	test_cmp expect output
-- 
1.7.5.1.414.gb4910

[PATCH 3/3] notes remove: --stdin reads from the standard input

[Junio C Hamano]

Teach the command to read object names to remove from the standard
input, in addition to the object names given from the command line.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * The logical conclusion of the series.
---
 Documentation/git-notes.txt |    7 ++++++-
 builtin/notes.c             |   14 +++++++++++++-
 t/t3301-notes.sh            |   33 +++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index 6b92060..42e4823 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt
@@ -17,7 +17,7 @@ SYNOPSIS
 'git notes' merge [-v | -q] [-s <strategy> ] <notes_ref>
 'git notes' merge --commit [-v | -q]
 'git notes' merge --abort [-v | -q]
-'git notes' remove [--missing-ok] [<object>...]
+'git notes' remove [--missing-ok] [--stdin] [<object>...]
 'git notes' prune [-n | -v]
 'git notes' get-ref
 
@@ -159,6 +159,11 @@ OPTIONS
 	Do not consider it an error to request removing notes from an
 	object that does not have notes attached to it.
 
+--stdin::
+	Also read the object names to remove notes from from the standard
+	input (there is no reason you cannot combine this with object
+	names from the command line).
+
 -n::
 --dry-run::
 	Do not remove anything; just report the object names whose notes
diff --git a/builtin/notes.c b/builtin/notes.c
index ca045f8..23954e0 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -972,10 +972,13 @@ static int remove_one_note(struct notes_tree *t, const char *name, unsigned flag
 static int remove_cmd(int argc, const char **argv, const char *prefix)
 {
 	unsigned flag = 0;
+	int from_stdin = 0;
 	struct option options[] = {
 		OPT_BIT(0, "missing-ok", &flag,
 			"attempt to remove non-existent note is not an error",
 			MISSING_OK),
+		OPT_BOOLEAN(0, "stdin", &from_stdin,
+			    "read object names from the standard input"),
 		OPT_END()
 	};
 	struct notes_tree *t;
@@ -986,7 +989,7 @@ static int remove_cmd(int argc, const char **argv, const char *prefix)
 
 	t = init_notes_check("remove");
 
-	if (!argc) {
+	if (!argc && !from_stdin) {
 		retval = remove_one_note(t, "HEAD", flag);
 	} else {
 		while (*argv) {
@@ -994,6 +997,15 @@ static int remove_cmd(int argc, const char **argv, const char *prefix)
 			argv++;
 		}
 	}
+	if (from_stdin) {
+		struct strbuf sb = STRBUF_INIT;
+		while (strbuf_getwholeline(&sb, stdin, '\n') != EOF) {
+			int len = sb.len;
+			if (len && sb.buf[len - 1] == '\n')
+				sb.buf[--len] = '\0';
+			retval |= remove_one_note(t, sb.buf, flag);
+		}
+	}
 	if (!retval)
 		commit_notes(t, "Notes removed by 'git notes remove'");
 	free_notes(t);
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index bdd4036..2c52f21 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -473,6 +473,39 @@ test_expect_success 'removing with --missing-ok but bogus ref' '
 	test "$before" = "$after"
 '
 
+test_expect_success 'remove reads from --stdin' '
+	before=$(git rev-parse --verify refs/notes/commits) &&
+	test_when_finished "git update-ref refs/notes/commits $before" &&
+	git rev-parse HEAD^^ HEAD^^^ >input &&
+	git notes remove --stdin <input &&
+	git diff --name-only refs/notes/commits^ refs/notes/commits >actual &&
+	test 2 = $(wc -l <actual) &&
+	git ls-tree -r --name-only refs/notes/commits >actual &&
+	>empty &&
+	test_cmp empty actual
+'
+
+test_expect_success 'remove --stdin is also atomic' '
+	before=$(git rev-parse --verify refs/notes/commits) &&
+	test_when_finished "git update-ref refs/notes/commits $before" &&
+	git rev-parse HEAD^^ HEAD^^^ HEAD^ >input &&
+	test_must_fail git notes remove --stdin <input &&
+	after=$(git rev-parse --verify refs/notes/commits) &&
+	test "$before" = "$after"
+'
+
+test_expect_success 'removing with --stdin --missing-ok' '
+	before=$(git rev-parse --verify refs/notes/commits) &&
+	test_when_finished "git update-ref refs/notes/commits $before" &&
+	git rev-parse HEAD^^ HEAD^^^ HEAD^ >input &&
+	git notes remove --missing-ok --stdin <input &&
+	git diff --name-only refs/notes/commits^ refs/notes/commits >actual &&
+	test 2 = $(wc -l <actual) &&
+	git ls-tree -r --name-only refs/notes/commits >actual &&
+	>empty &&
+	test_cmp empty actual
+'
+
 test_expect_success 'list notes with "git notes list"' '
 	git notes list > output &&
 	test_cmp expect output
-- 
1.7.5.1.414.gb4910

Re: [PATCH 2/3] notes remove: --missing-ok

[Junio C Hamano]

Just FYI, I'll rename this to --ignore-missing to match other commands in
the version I am going to queue in 'pu'.

[PATCH 4/3] show: --ignore-missing

[Junio C Hamano]

Instead of barfing, simply ignore bad object names seen in the
input. This is useful when reading from "git notes list" output
that may refer to objects that have already been garbage collected.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This would let me do something like

	git notes list |
        sed -e 's/. *//' | # strip note-blob
	git show --format='%ci %H ...' --ignore-missing --stdin |
        ... perhaps sort by commit date, filter, etc... |
        git notes remove --stdin --ignore-missing

   Perhaps "git notes list --target-only" can be added to remove the
   need for the second "sed" in the pipeline.

 Documentation/git-rev-list.txt     |    1 +
 Documentation/rev-list-options.txt |    4 ++++
 builtin/rev-parse.c                |    1 +
 revision.c                         |   15 +++++++++++++--
 revision.h                         |    3 ++-
 5 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 415f4f0..38fafca 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -29,6 +29,7 @@ SYNOPSIS
 	     [ \--tags[=<pattern>] ]
 	     [ \--remotes[=<pattern>] ]
 	     [ \--glob=<glob-pattern> ]
+	     [ \--ignore-missing ]
 	     [ \--stdin ]
 	     [ \--quiet ]
 	     [ \--topo-order ]
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 52bae31..7e7ba68 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -139,6 +139,10 @@ parents) and `--max-parents=-1` (negative numbers denote no upper limit).
 	is automatically prepended if missing. If pattern lacks '?', '*',
 	or '[', '/*' at the end is implied.
 
+--ignore-missing::
+
+	Upon seeing an invalid object name in the input, pretend as if
+	the bad input was not given.
 
 ifndef::git-rev-list[]
 --bisect::
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index adb1cae..4c19f84 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -44,6 +44,7 @@ static int is_rev_argument(const char *arg)
 		"--branches=",
 		"--branches",
 		"--header",
+		"--ignore-missing",
 		"--max-age=",
 		"--max-count=",
 		"--min-age=",
diff --git a/revision.c b/revision.c
index a7cf79b..5cb3a75 100644
--- a/revision.c
+++ b/revision.c
@@ -133,6 +133,8 @@ void mark_parents_uninteresting(struct commit *commit)
 
 static void add_pending_object_with_mode(struct rev_info *revs, struct object *obj, const char *name, unsigned mode)
 {
+	if (!obj)
+		return;
 	if (revs->no_walk && (obj->flags & UNINTERESTING))
 		revs->no_walk = 0;
 	if (revs->reflog_info && obj->type == OBJ_COMMIT) {
@@ -174,8 +176,11 @@ static struct object *get_reference(struct rev_info *revs, const char *name, con
 	struct object *object;
 
 	object = parse_object(sha1);
-	if (!object)
+	if (!object) {
+		if (revs->ignore_missing)
+			return object;
 		die("bad object %s", name);
+	}
 	object->flags |= flags;
 	return object;
 }
@@ -906,6 +911,8 @@ static int add_parents_only(struct rev_info *revs, const char *arg, int flags)
 		return 0;
 	while (1) {
 		it = get_reference(revs, arg, sha1, 0);
+		if (!it && revs->ignore_missing)
+			return 0;
 		if (it->type != OBJ_TAG)
 			break;
 		if (!((struct tag*)it)->tagged)
@@ -1044,6 +1051,8 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 			a = lookup_commit_reference(from_sha1);
 			b = lookup_commit_reference(sha1);
 			if (!a || !b) {
+				if (revs->ignore_missing)
+					return 0;
 				die(symmetric ?
 				    "Invalid symmetric difference expression %s...%s" :
 				    "Invalid revision range %s..%s",
@@ -1090,7 +1099,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 		arg++;
 	}
 	if (get_sha1_with_mode(arg, sha1, &mode))
-		return -1;
+		return revs->ignore_missing ? 0 : -1;
 	if (!cant_be_filename)
 		verify_non_filename(revs->prefix, arg);
 	object = get_reference(revs, arg, sha1, flags ^ local_flags);
@@ -1475,6 +1484,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	} else if (!strcmp(arg, "--children")) {
 		revs->children.name = "children";
 		revs->limited = 1;
+	} else if (!strcmp(arg, "--ignore-missing")) {
+		revs->ignore_missing = 1;
 	} else {
 		int opts = diff_opt_parse(&revs->diffopt, argv, argc);
 		if (!opts)
diff --git a/revision.h b/revision.h
index bca9947..93f338d 100644
--- a/revision.h
+++ b/revision.h
@@ -36,7 +36,8 @@ struct rev_info {
 	const char *prefix;
 	const char *def;
 	struct pathspec prune_data;
-	unsigned int early_output;
+	unsigned int	early_output:1,
+			ignore_missing:1;
 
 	/* Traversal flags */
 	unsigned int	dense:1,
-- 
1.7.5.1.414.gb4910

Re: [PATCH 1/3] notes remove: allow removing more than one

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 19.05.2011 02:14:
> While "xargs -n1 git notes rm" is certainly a possible way to remove notes
> from many objects, this would create one notes "commit" per removal, which
> is not quite suitable for seasonal housekeeping.
> 
> Allow taking more than one on the command line, and record their removal
> as a single atomic event if everthing goes well.
> 
> Even though the old code insisted that "git notes rm" must be given only
> one object (or zero, in which case it would default to HEAD), this
> condition was not tested. Add tests to handle the new case where we feed
> multiple objects, and also make sure if there is a bad input, no change
> is recorded.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/git-notes.txt |    7 +++--
>  builtin/notes.c             |   47 ++++++++++++++++++++++--------------------
>  t/t3301-notes.sh            |   19 +++++++++++++++++
>  3 files changed, 48 insertions(+), 25 deletions(-)
> 
> diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
> index 913ecd8..e2e1c4c 100644
> --- a/Documentation/git-notes.txt
> +++ b/Documentation/git-notes.txt
> @@ -17,7 +17,7 @@ SYNOPSIS
>  'git notes' merge [-v | -q] [-s <strategy> ] <notes_ref>
>  'git notes' merge --commit [-v | -q]
>  'git notes' merge --abort [-v | -q]
> -'git notes' remove [<object>]
> +'git notes' remove [<object>...]
>  'git notes' prune [-n | -v]
>  'git notes' get-ref
>  
> @@ -106,8 +106,9 @@ When done, the user can either finalize the merge with
>  'git notes merge --abort'.
>  
>  remove::
> -	Remove the notes for a given object (defaults to HEAD).
> -	This is equivalent to specifying an empty note message to
> +	Remove the notes for given objects (defaults to HEAD). When
> +	giving zero or one object from the command line, this is
> +	equivalent to specifying an empty note message to
>  	the `edit` subcommand.
>  
>  prune::
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 1fb1f73..30cee0f 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -29,7 +29,7 @@ static const char * const git_notes_usage[] = {
>  	"git notes [--ref <notes_ref>] merge [-v | -q] [-s <strategy> ] <notes_ref>",
>  	"git notes merge --commit [-v | -q]",
>  	"git notes merge --abort [-v | -q]",
> -	"git notes [--ref <notes_ref>] remove [<object>]",
> +	"git notes [--ref <notes_ref>] remove [<object>...]",
>  	"git notes [--ref <notes_ref>] prune [-n | -v]",
>  	"git notes [--ref <notes_ref>] get-ref",
>  	NULL
> @@ -953,40 +953,43 @@ static int merge(int argc, const char **argv, const char *prefix)
>  	return result < 0; /* return non-zero on conflicts */
>  }
>  
> +static int remove_one_note(struct notes_tree *t, const char *name)
> +{
> +	int status;
> +	unsigned char sha1[20];
> +	if (get_sha1(name, sha1))
> +		return error(_("Failed to resolve '%s' as a valid ref."), name);
> +	status = remove_note(t, sha1);
> +	if (status)
> +		fprintf(stderr, _("Object %s has no note\n"), name);
> +	else
> +		fprintf(stderr, _("Removing note for object %s\n"), name);
> +	return status;
> +}
> +
>  static int remove_cmd(int argc, const char **argv, const char *prefix)
>  {
>  	struct option options[] = {
>  		OPT_END()
>  	};
> -	const char *object_ref;
>  	struct notes_tree *t;
> -	unsigned char object[20];
> -	int retval;
> +	int retval = 0;
>  
>  	argc = parse_options(argc, argv, prefix, options,
>  			     git_notes_remove_usage, 0);
>  
> -	if (1 < argc) {
> -		error(_("too many parameters"));
> -		usage_with_options(git_notes_remove_usage, options);
> -	}
> -
> -	object_ref = argc ? argv[0] : "HEAD";
> -
> -	if (get_sha1(object_ref, object))
> -		die(_("Failed to resolve '%s' as a valid ref."), object_ref);
> -
>  	t = init_notes_check("remove");
>  
> -	retval = remove_note(t, object);
> -	if (retval)
> -		fprintf(stderr, _("Object %s has no note\n"), sha1_to_hex(object));
> -	else {
> -		fprintf(stderr, _("Removing note for object %s\n"),
> -			sha1_to_hex(object));
> -
> -		commit_notes(t, "Notes removed by 'git notes remove'");
> +	if (!argc) {
> +		retval = remove_one_note(t, "HEAD");
> +	} else {
> +		while (*argv) {
> +			retval |= remove_one_note(t, *argv);
> +			argv++;
> +		}
>  	}
> +	if (!retval)
> +		commit_notes(t, "Notes removed by 'git notes remove'");
>  	free_notes(t);
>  	return retval;
>  }
> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
> index 28e17c8..6a6daa9 100755
> --- a/t/t3301-notes.sh
> +++ b/t/t3301-notes.sh
> @@ -435,6 +435,25 @@ test_expect_success 'removing non-existing note should not create new commit' '
>  	test_cmp before_commit after_commit
>  '
>  
> +test_expect_success 'removing more than one' '
> +	before=$(git rev-parse --verify refs/notes/commits) &&
> +	test_when_finished "git update-ref refs/notes/commits $before" &&
> +	git notes remove HEAD^^ HEAD^^^ &&
> +	git diff --name-only refs/notes/commits^ refs/notes/commits >actual &&
> +	test 2 = $(wc -l <actual) &&
> +	git ls-tree -r --name-only refs/notes/commits >actual &&
> +	>empty &&
> +	test_cmp empty actual
> +'

You're not really testing that this removes the correct notes. Actually,
you're not even testing that this removes only 2 notes when there are
more than 2, are you? I might be a bit fussy, though. I like the feature
in general:

 git notes remove $(git rev-list <fancy-rev-spec>)

rocks :)

Although "notes remove" could grok a rev-spec and dwim --do-walk, come
to think of it. Would be even more gittish.

> +
> +test_expect_success 'removing is atomic' '
> +	before=$(git rev-parse --verify refs/notes/commits) &&
> +	test_when_finished "git update-ref refs/notes/commits $before" &&
> +	test_must_fail git notes remove HEAD^^ HEAD^^^ HEAD^ &&
> +	after=$(git rev-parse --verify refs/notes/commits) &&
> +	test "$before" = "$after"
> +'
> +
>  test_expect_success 'list notes with "git notes list"' '
>  	git notes list > output &&
>  	test_cmp expect output

Re: [PATCH 3/3] notes remove: --stdin reads from the standard input

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 19.05.2011 02:14:
> Teach the command to read object names to remove from the standard
> input, in addition to the object names given from the command line.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 
>  * The logical conclusion of the series.

So that makes 4/3 to be... ;)

I'm torn between asking for a rev-list interface and being happy with
--stdin. I guess we have other places where we allow lists of objects in
argv and stdin (instead of rev-list), and since this targets all kinds
of objects, it's more natural as is (than with with the rev-list interface).

> ---
>  Documentation/git-notes.txt |    7 ++++++-
>  builtin/notes.c             |   14 +++++++++++++-
>  t/t3301-notes.sh            |   33 +++++++++++++++++++++++++++++++++
>  3 files changed, 52 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
> index 6b92060..42e4823 100644
> --- a/Documentation/git-notes.txt
> +++ b/Documentation/git-notes.txt
> @@ -17,7 +17,7 @@ SYNOPSIS
>  'git notes' merge [-v | -q] [-s <strategy> ] <notes_ref>
>  'git notes' merge --commit [-v | -q]
>  'git notes' merge --abort [-v | -q]
> -'git notes' remove [--missing-ok] [<object>...]
> +'git notes' remove [--missing-ok] [--stdin] [<object>...]
>  'git notes' prune [-n | -v]
>  'git notes' get-ref
>  
> @@ -159,6 +159,11 @@ OPTIONS
>  	Do not consider it an error to request removing notes from an
>  	object that does not have notes attached to it.
>  
> +--stdin::
> +	Also read the object names to remove notes from from the standard
> +	input (there is no reason you cannot combine this with object
> +	names from the command line).
> +
>  -n::
>  --dry-run::
>  	Do not remove anything; just report the object names whose notes
> diff --git a/builtin/notes.c b/builtin/notes.c
> index ca045f8..23954e0 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -972,10 +972,13 @@ static int remove_one_note(struct notes_tree *t, const char *name, unsigned flag
>  static int remove_cmd(int argc, const char **argv, const char *prefix)
>  {
>  	unsigned flag = 0;
> +	int from_stdin = 0;
>  	struct option options[] = {
>  		OPT_BIT(0, "missing-ok", &flag,
>  			"attempt to remove non-existent note is not an error",
>  			MISSING_OK),
> +		OPT_BOOLEAN(0, "stdin", &from_stdin,
> +			    "read object names from the standard input"),
>  		OPT_END()
>  	};
>  	struct notes_tree *t;
> @@ -986,7 +989,7 @@ static int remove_cmd(int argc, const char **argv, const char *prefix)
>  
>  	t = init_notes_check("remove");
>  
> -	if (!argc) {
> +	if (!argc && !from_stdin) {
>  		retval = remove_one_note(t, "HEAD", flag);
>  	} else {
>  		while (*argv) {
> @@ -994,6 +997,15 @@ static int remove_cmd(int argc, const char **argv, const char *prefix)
>  			argv++;
>  		}
>  	}
> +	if (from_stdin) {
> +		struct strbuf sb = STRBUF_INIT;
> +		while (strbuf_getwholeline(&sb, stdin, '\n') != EOF) {
> +			int len = sb.len;
> +			if (len && sb.buf[len - 1] == '\n')
> +				sb.buf[--len] = '\0';
> +			retval |= remove_one_note(t, sb.buf, flag);
> +		}
> +	}
>  	if (!retval)
>  		commit_notes(t, "Notes removed by 'git notes remove'");
>  	free_notes(t);
> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
> index bdd4036..2c52f21 100755
> --- a/t/t3301-notes.sh
> +++ b/t/t3301-notes.sh
> @@ -473,6 +473,39 @@ test_expect_success 'removing with --missing-ok but bogus ref' '
>  	test "$before" = "$after"
>  '
>  
> +test_expect_success 'remove reads from --stdin' '
> +	before=$(git rev-parse --verify refs/notes/commits) &&
> +	test_when_finished "git update-ref refs/notes/commits $before" &&
> +	git rev-parse HEAD^^ HEAD^^^ >input &&
> +	git notes remove --stdin <input &&
> +	git diff --name-only refs/notes/commits^ refs/notes/commits >actual &&
> +	test 2 = $(wc -l <actual) &&
> +	git ls-tree -r --name-only refs/notes/commits >actual &&
> +	>empty &&
> +	test_cmp empty actual
> +'

Same comment as for 1/3 applies here.

> +
> +test_expect_success 'remove --stdin is also atomic' '
> +	before=$(git rev-parse --verify refs/notes/commits) &&
> +	test_when_finished "git update-ref refs/notes/commits $before" &&
> +	git rev-parse HEAD^^ HEAD^^^ HEAD^ >input &&
> +	test_must_fail git notes remove --stdin <input &&
> +	after=$(git rev-parse --verify refs/notes/commits) &&
> +	test "$before" = "$after"
> +'
> +
> +test_expect_success 'removing with --stdin --missing-ok' '
> +	before=$(git rev-parse --verify refs/notes/commits) &&
> +	test_when_finished "git update-ref refs/notes/commits $before" &&
> +	git rev-parse HEAD^^ HEAD^^^ HEAD^ >input &&
> +	git notes remove --missing-ok --stdin <input &&
> +	git diff --name-only refs/notes/commits^ refs/notes/commits >actual &&
> +	test 2 = $(wc -l <actual) &&
> +	git ls-tree -r --name-only refs/notes/commits >actual &&
> +	>empty &&
> +	test_cmp empty actual
> +'
> +
>  test_expect_success 'list notes with "git notes list"' '
>  	git notes list > output &&
>  	test_cmp expect output

Re: [PATCH 1/3] notes remove: allow removing more than one

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

[jc: cull the part you did not comment on, thanks]

>> +test_expect_success 'removing more than one' '
>> +	before=$(git rev-parse --verify refs/notes/commits) &&
>> +	test_when_finished "git update-ref refs/notes/commits $before" &&
>> +	git notes remove HEAD^^ HEAD^^^ &&
>> +	git diff --name-only refs/notes/commits^ refs/notes/commits >actual &&
>> +	test 2 = $(wc -l <actual) &&
>> +	git ls-tree -r --name-only refs/notes/commits >actual &&
>> +	>empty &&
>> +	test_cmp empty actual
>> +'
>
> You're not really testing that this removes the correct notes. Actually,
> you're not even testing that this removes only 2 notes when there are
> more than 2, are you?

In the test vector, there are only two notes, and I am testing removal of
multiple.  What do you want me to do?  Test removing one and make sure the
other one survives ;-)? 

Patches on top of it is very welcome, but I wouldn't bother.

Re: [PATCH 0/3] "git notes remove" updates

[Johan Herland]

On Thursday 19 May 2011, Junio C Hamano wrote:
> I wanted to do a bit better than
> 
> 	for sha1 in ... list of old commit objects
> 	do
> 		git notes --ref refs/notes/amlog remove $sha1
> 	done
> 
> to remove old entries in my "where did this commit come from" database.
> 
> Junio C Hamano (3):
>   notes remove: allow removing more than one
>   notes remove: --missing-ok
>   notes remove: --stdin reads from the standard input


After a cursory reading the series looks good to me. Thanks! :)


...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

Re: [PATCH 1/3] notes remove: allow removing more than one

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 19.05.2011 08:50:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
> [jc: cull the part you did not comment on, thanks]
> 
>>> +test_expect_success 'removing more than one' '
>>> +	before=$(git rev-parse --verify refs/notes/commits) &&
>>> +	test_when_finished "git update-ref refs/notes/commits $before" &&
>>> +	git notes remove HEAD^^ HEAD^^^ &&
>>> +	git diff --name-only refs/notes/commits^ refs/notes/commits >actual &&
>>> +	test 2 = $(wc -l <actual) &&
>>> +	git ls-tree -r --name-only refs/notes/commits >actual &&
>>> +	>empty &&
>>> +	test_cmp empty actual
>>> +'
>>
>> You're not really testing that this removes the correct notes. Actually,
>> you're not even testing that this removes only 2 notes when there are
>> more than 2, are you?
> 
> In the test vector, there are only two notes, and I am testing removal of
> multiple.  What do you want me to do?  Test removing one and make sure the
> other one survives ;-)? 

The test has two notes because it was created when remove would remove
one note at a time only, and the test made sure it did not remove the
other one (!).

If you want a meaningful test which tests that "git notes remove A B"
removes exactly 2 notes (not more) you need to add another note first.
Otherwise it could be that your new code simply nukes the notes tree. (I
know it does not, but only by looking at the source; this test does not
check it.)

Checking that it actually removes *those* 2 notes would give extra
credit but may not be necessary (since we know it removes the correct
one in single mode).

> Patches on top of it is very welcome, but I wouldn't bother.

Is that the kind of response you accept from submitters to your (valid)
requests that they provide negative tests in addition to positive ones?

I don't mind helping out with a test patch (later this UTC day) but I'm
concerned about the attitude someone may read into this and may want to
follow as an example. In terms of code-with-test habits, I would claim
that git.git is really an exemplary standard to follow (and to keep).

Cheers,
Michael

Re: [PATCH 3/3] notes remove: --stdin reads from the standard input

[Jeff King]

On Wed, May 18, 2011 at 05:14:21PM -0700, Junio C Hamano wrote:

> +	if (from_stdin) {
> +		struct strbuf sb = STRBUF_INIT;
> +		while (strbuf_getwholeline(&sb, stdin, '\n') != EOF) {
> +			int len = sb.len;
> +			if (len && sb.buf[len - 1] == '\n')
> +				sb.buf[--len] = '\0';
> +			retval |= remove_one_note(t, sb.buf, flag);
> +		}
> +	}

Wouldn't strbuf_rtrim (or even strbuf_trim) be useful here?

Manipulating only a copy of the len parameter while changing the strbuf
itself raised warning flags, though it is correct as-is because you
never actually use sb.len.

Using strbuf_trim is shorter, easier to read, and will helpfully eat any
extraneous whitespace, too.

-Peff

Re: [PATCH 1/3] notes remove: allow removing more than one

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> The test has two notes because it was created when remove would remove
> one note at a time only, and the test made sure it did not remove the
> other one (!).

Fair enough.  Here is an incremental update to be squashed in.

I notice that "notes list <commit>" and "notes list | grep <commit>" give
quite different results, and "notes list <commit1> <commit2>..." does not
even work at all. Probably the "notes" interface was done with interactive
use in mind without realizing the need for batch operations.

 t/t3301-notes.sh |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 6a6daa9..6278fe8 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -438,12 +438,13 @@ test_expect_success 'removing non-existing note should not create new commit' '
 test_expect_success 'removing more than one' '
 	before=$(git rev-parse --verify refs/notes/commits) &&
 	test_when_finished "git update-ref refs/notes/commits $before" &&
+
+	# We have only two -- add another and make sure it stays
+	git notes add -m "extra" &&
+	git notes list HEAD >after-removal-expect &&
 	git notes remove HEAD^^ HEAD^^^ &&
-	git diff --name-only refs/notes/commits^ refs/notes/commits >actual &&
-	test 2 = $(wc -l <actual) &&
-	git ls-tree -r --name-only refs/notes/commits >actual &&
-	>empty &&
-	test_cmp empty actual
+	git notes list | sed -e "s/ .*//" >actual &&
+	test_cmp after-removal-expect actual
 '
 
 test_expect_success 'removing is atomic' '

Re: [PATCH 3/3] notes remove: --stdin reads from the standard input

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Wed, May 18, 2011 at 05:14:21PM -0700, Junio C Hamano wrote:
>
>> +	if (from_stdin) {
>> +		struct strbuf sb = STRBUF_INIT;
>> +		while (strbuf_getwholeline(&sb, stdin, '\n') != EOF) {
>> +			int len = sb.len;
>> +			if (len && sb.buf[len - 1] == '\n')
>> +				sb.buf[--len] = '\0';
>> +			retval |= remove_one_note(t, sb.buf, flag);
>> +		}
>> +	}
>
> Wouldn't strbuf_rtrim (or even strbuf_trim) be useful here?

Thanks for noticing.

I just mimicked what was done to the result of strbuf_getwholeline() in
other places (I think from revision.c but I am not sure).

An incremental correction, relative to what I had in 'pu' overnight, looks
like this.

 builtin/notes.c  |    5 ++---
 t/t3301-notes.sh |   26 ++++++++++++++------------
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 164d605..f8e437d 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -1000,11 +1000,10 @@ static int remove_cmd(int argc, const char **argv, const char *prefix)
 	if (from_stdin) {
 		struct strbuf sb = STRBUF_INIT;
 		while (strbuf_getwholeline(&sb, stdin, '\n') != EOF) {
-			int len = sb.len;
-			if (len && sb.buf[len - 1] == '\n')
-				sb.buf[--len] = '\0';
+			strbuf_rtrim(&sb);
 			retval |= remove_one_note(t, sb.buf, flag);
 		}
+		strbuf_release(&sb);
 	}
 	if (!retval)
 		commit_notes(t, "Notes removed by 'git notes remove'");
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index e1b5619..16de05a 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -478,13 +478,14 @@ test_expect_success 'removing with --ignore-missing but bogus ref' '
 test_expect_success 'remove reads from --stdin' '
 	before=$(git rev-parse --verify refs/notes/commits) &&
 	test_when_finished "git update-ref refs/notes/commits $before" &&
+
+	# We have only two -- add another and make sure it stays
+	git notes add -m "extra" &&
+	git notes list HEAD >after-removal-expect &&
 	git rev-parse HEAD^^ HEAD^^^ >input &&
 	git notes remove --stdin <input &&
-	git diff --name-only refs/notes/commits^ refs/notes/commits >actual &&
-	test 2 = $(wc -l <actual) &&
-	git ls-tree -r --name-only refs/notes/commits >actual &&
-	>empty &&
-	test_cmp empty actual
+	git notes list | sed -e "s/ .*//" >actual &&
+	test_cmp after-removal-expect actual
 '
 
 test_expect_success 'remove --stdin is also atomic' '
@@ -496,16 +497,17 @@ test_expect_success 'remove --stdin is also atomic' '
 	test "$before" = "$after"
 '
 
-test_expect_success 'removing with --stdin --missing-ok' '
+test_expect_success 'removing with --stdin --ignore-missing' '
 	before=$(git rev-parse --verify refs/notes/commits) &&
 	test_when_finished "git update-ref refs/notes/commits $before" &&
+
+	# We have only two -- add another and make sure it stays
+	git notes add -m "extra" &&
+	git notes list HEAD >after-removal-expect &&
 	git rev-parse HEAD^^ HEAD^^^ HEAD^ >input &&
-	git notes remove --missing-ok --stdin <input &&
-	git diff --name-only refs/notes/commits^ refs/notes/commits >actual &&
-	test 2 = $(wc -l <actual) &&
-	git ls-tree -r --name-only refs/notes/commits >actual &&
-	>empty &&
-	test_cmp empty actual
+	git notes remove --ignore-missing --stdin <input &&
+	git notes list | sed -e "s/ .*//" >actual &&
+	test_cmp after-removal-expect actual
 '
 
 test_expect_success 'list notes with "git notes list"' '

Re: [PATCH 3/3] notes remove: --stdin reads from the standard input

[Jeff King]

On Thu, May 19, 2011 at 10:55:38AM -0700, Junio C Hamano wrote:

> >> +	if (from_stdin) {
> >> +		struct strbuf sb = STRBUF_INIT;
> >> +		while (strbuf_getwholeline(&sb, stdin, '\n') != EOF) {
> >> +			int len = sb.len;
> >> +			if (len && sb.buf[len - 1] == '\n')
> >> +				sb.buf[--len] = '\0';
> >> +			retval |= remove_one_note(t, sb.buf, flag);
> >> +		}
> >> +	}
> >
> > Wouldn't strbuf_rtrim (or even strbuf_trim) be useful here?
> 
> Thanks for noticing.
> 
> I just mimicked what was done to the result of strbuf_getwholeline() in
> other places (I think from revision.c but I am not sure).

I am tempted to say that other callsites should be "upgraded" to
strbuf_trim. Is there any reason we should not be liberal in accepting
something like:

  echo "  refs/heads/foo  " | git rev-list --stdin

?

I guess for pathspecs we would want to be more literal, though, so maybe
it is a stupid idea.

> An incremental correction, relative to what I had in 'pu' overnight, looks
> like this.

Thanks. BTW, I was too busy picking this one nit to mention that the
general direction of the series is a nice improvement. :)

-Peff

[PATCH 1/2] fixup! notes remove: --ignore-missing

[Michael J Gruber]

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 t/t3301-notes.sh |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 09cf6b2..1edb7bf 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -438,12 +438,13 @@ test_expect_success 'removing non-existing note should not create new commit' '
 test_expect_success 'removing more than one' '
 	before=$(git rev-parse --verify refs/notes/commits) &&
 	test_when_finished "git update-ref refs/notes/commits $before" &&
+	MSG=4 git notes add HEAD^ &&
 	git notes remove HEAD^^ HEAD^^^ &&
 	git diff --name-only refs/notes/commits^ refs/notes/commits >actual &&
 	test 2 = $(wc -l <actual) &&
 	git ls-tree -r --name-only refs/notes/commits >actual &&
-	>empty &&
-	test_cmp empty actual
+	git rev-parse HEAD^ >expect1 &&
+	test_cmp expect1 actual
 '
 
 test_expect_success 'removing is atomic' '
-- 
1.7.5.1.558.gc8bec

[PATCH 2/2] fixup! notes remove: --stdin reads from the standard input

[Michael J Gruber]

Cheers :)

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 t/t3301-notes.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 1edb7bf..8462173 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -495,11 +495,11 @@ test_expect_success 'remove --stdin is also atomic' '
 	test "$before" = "$after"
 '
 
-test_expect_success 'removing with --stdin --missing-ok' '
+test_expect_success 'removing with --stdin --ignore-missing' '
 	before=$(git rev-parse --verify refs/notes/commits) &&
 	test_when_finished "git update-ref refs/notes/commits $before" &&
 	git rev-parse HEAD^^ HEAD^^^ HEAD^ >input &&
-	git notes remove --missing-ok --stdin <input &&
+	git notes remove --ignore-missing --stdin <input &&
 	git diff --name-only refs/notes/commits^ refs/notes/commits >actual &&
 	test 2 = $(wc -l <actual) &&
 	git ls-tree -r --name-only refs/notes/commits >actual &&
-- 
1.7.5.1.558.gc8bec