Merge branch 'jc/notes-batch-removal'

* jc/notes-batch-removal:
  show: --ignore-missing
  notes remove: --stdin reads from the standard input
  notes remove: --ignore-missing
  notes remove: allow removing more than one

Author: gitster

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 [--ignore-missing] [--stdin] [<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::
@@ -154,6 +155,15 @@ OPTIONS
 	'GIT_NOTES_REF' and the "core.notesRef" configuration.  The ref
 	is taken to be in `refs/notes/` if it is not qualified.
 
+--ignore-missing::
+	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

Documentation/git-rev-list.txt

@@ -29,6 +29,7 @@ SYNOPSIS
 	     [ \--tags[=<pattern>] ]
 	     [ \--remotes[=<pattern>] ]
 	     [ \--glob=<glob-pattern> ]
+	     [ \--ignore-missing ]
 	     [ \--stdin ]
 	     [ \--quiet ]
 	     [ \--topo-order ]

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::

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,60 @@ static int merge(int argc, const char **argv, const char *prefix)
 	return result < 0; /* return non-zero on conflicts */
 }
 
+#define IGNORE_MISSING 1
+
+static int remove_one_note(struct notes_tree *t, const char *name, unsigned flag)
+{
+	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 (flag & IGNORE_MISSING) ? 0 : status;
+}
+
 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, "ignore-missing", &flag,
+			"attempt to remove non-existent note is not an error",
+			IGNORE_MISSING),
+		OPT_BOOLEAN(0, "stdin", &from_stdin,
+			    "read object names from the standard input"),
 		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 && !from_stdin) {
+		retval = remove_one_note(t, "HEAD", flag);
+	} else {
+		while (*argv) {
+			retval |= remove_one_note(t, *argv, flag);
+			argv++;
+		}
 	}
+	if (from_stdin) {
+		struct strbuf sb = STRBUF_INIT;
+		while (strbuf_getwholeline(&sb, stdin, '\n') != EOF) {
+			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'");
 	free_notes(t);
 	return retval;
 }

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=",

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);
@@ -1477,6 +1486,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)

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,

t/t3301-notes.sh

@@ -435,6 +435,81 @@ 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" &&
+
+	# 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 notes list | sed -e "s/ .*//" >actual &&
+	test_cmp after-removal-expect 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 'removing with --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 notes remove --ignore-missing HEAD^^ HEAD^^^ HEAD^ &&
+	git notes list | sed -e "s/ .*//" >actual &&
+	test_cmp after-removal-expect actual
+'
+
+test_expect_success 'removing with --ignore-missing 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 --ignore-missing HEAD^^ HEAD^^^ NO-SUCH-COMMIT &&
+	after=$(git rev-parse --verify refs/notes/commits) &&
+	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" &&
+
+	# 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 notes list | sed -e "s/ .*//" >actual &&
+	test_cmp after-removal-expect 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 --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 --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"' '
 	git notes list > output &&
 	test_cmp expect output