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

DORMANTno replies

8 messages, 4 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:51:16 

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:51:16 

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 <redacted>
---
 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 3/3] notes remove: --stdin reads from the standard input

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:51:16 

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

 * 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

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:51:16 

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

 * 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

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

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:51:16 

Junio C Hamano venit, vidit, dixit 19.05.2011 02:14:
quoted hunk
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 <redacted>
---
 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

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:51:16 

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

 * 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).
quoted hunk
---
 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 0/3] "git notes remove" updates

From: Johan Herland <hidden>
Date: 2016-06-15 22:51:16 

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, [off-list ref]
www.herland.net

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

From: Jeff King <hidden>
Date: 2016-06-15 22:51:16 

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help