threads / patch / 7668

patchAdd -q option to "git rm" to suppress output when there aren't errors.

Subject: [PATCH] Add -q option to "git rm" to suppress output when there aren't errors.

## tl;dr

14 messages between Apr 16, 2007 and Apr 16, 2007. Diffs are folded; open one to read it.

replies: 13people: 5as markdown or json

Steven Grimm· Apr 16, 2007, 00:04 UTC · lore

This suppresses the output of "rm" commands, and also exits with a zero exit code when no files match. This allows "git rm" (and more importantly, "git rm -r") to be used as an index filter with cg-admin-rewritehist.

Signed-off-by: Steven Grimm <koreth@midwinter.com>
---
 Documentation/git-rm.txt |    4 ++++
 builtin-rm.c             |   26 ++++++++++++++++++++------
 t/t3600-rm.sh            |   28 ++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 6 deletions(-)
Show changes to 3 files +52 −6

Documentation/git-rm.txt, builtin-rm.c, t/t3600-rm.sh

diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 6feebc0..c354134 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -33,6 +33,10 @@ OPTIONS
         Don't actually remove the file(s), just show if they exist in
         the index.
 
+-q::
+	Don't output the names of the files being removed, and exit
+	with a zero status even if no files matched.
+
 -r::
         Allow recursive removal when a leading directory name is
         given.
diff --git a/builtin-rm.c b/builtin-rm.c
index 8a0738f..3d438de 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -10,7 +10,7 @@
 #include "tree-walk.h"
 
 static const char builtin_rm_usage[] =
-"git-rm [-f] [-n] [-r] [--cached] [--] <file>...";
+"git-rm [-f] [-n] [-q] [-r] [--cached] [--] <file>...";
 
 static struct {
 	int nr, alloc;
@@ -104,7 +104,7 @@ static struct lock_file lock_file;
 int cmd_rm(int argc, const char **argv, const char *prefix)
 {
 	int i, newfd;
-	int show_only = 0, force = 0, index_only = 0, recursive = 0;
+	int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;
 	const char **pathspec;
 	char *seen;
 
@@ -130,6 +130,8 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 			index_only = 1;
 		else if (!strcmp(arg, "-f"))
 			force = 1;
+		else if (!strcmp(arg, "-q"))
+			quiet = 1;
 		else if (!strcmp(arg, "-r"))
 			recursive = 1;
 		else
@@ -153,14 +155,24 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 
 	if (pathspec) {
 		const char *match;
+		int seen_any = 0;
 		for (i = 0; (match = pathspec[i]) != NULL ; i++) {
-			if (!seen[i])
-				die("pathspec '%s' did not match any files",
-				    match);
+			if (!seen[i]) {
+				if (! quiet) {
+					die("pathspec '%s' did not match any files",
+					    match);
+				}
+			}
+			else {
+				seen_any = 1;
+			}
 			if (!recursive && seen[i] == MATCHED_RECURSIVELY)
 				die("not removing '%s' recursively without -r",
 				    *match ? match : ".");
 		}
+
+		if (! seen_any)
+			exit(0);
 	}
 
 	/*
@@ -187,7 +199,9 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	 */
 	for (i = 0; i < list.nr; i++) {
 		const char *path = list.name[i];
-		printf("rm '%s'\n", path);
+		if (!quiet) {
+			printf("rm '%s'\n", path);
+		}
 
 		if (remove_file_from_cache(path))
 			die("git-rm: unable to remove %s", path);
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index e31cf93..bd72feb 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -84,6 +84,30 @@ test_expect_success \
     'When the rm in "git-rm -f" fails, it should not remove the file from the index' \
     'git-ls-files --error-unmatch baz'
 
+test_expect_success 'Remove nonexistent file with -q returns zero exit status' '
+	git rm -q nonexistent
+'
+
+test_expect_success '"rm" command printed' '
+	echo frotz > test-file &&
+	git add test-file &&
+	git commit -m "add file for rm test" &&
+	git rm test-file > rm-output &&
+	test `egrep "^rm " rm-output | wc -l` = 1 &&
+	rm -f test-file rm-output &&
+	git commit -m "remove file from rm test"
+'
+
+test_expect_success '"rm" command suppressed with -q' '
+	echo frotz > test-file &&
+	git add test-file &&
+	git commit -m "add file for rm -q test" &&
+	git rm -q test-file > rm-output &&
+	test `wc -l < rm-output` = 0 &&
+	rm -f test-file rm-output &&
+	git commit -m "remove file from rm test"
+'
+
 # Now, failure cases.
 test_expect_success 'Re-add foo and baz' '
 	git add foo baz &&
@@ -154,4 +178,8 @@ test_expect_success 'Recursive with -r -f' '
 	! test -d frotz
 '
 
+test_expect_failure 'Remove nonexistent file returns nonzero exit status' '
+	git rm nonexistent
+'
+
 test_done
-- 
1.5.1.1.99.g0ea98
Junio C Hamano· Apr 16, 2007, 00:13 UTC · re: Steven Grimm · lore

Re: [PATCH] Add -q option to "git rm" to suppress output when there aren't errors.

Steven Grimm <koreth@midwinter.com> writes:
> +-q::
> +	Don't output the names of the files being removed, and exit
> +	with a zero status even if no files matched.
> +

Suppressing output is understandable and probably is a useful thing to do, but I do not see a justification to tie that quietness to making the status unuable...

Steven Grimm· Apr 16, 2007, 00:17 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add -q option to "git rm" to suppress output when there aren't errors.

Junio C Hamano wrote:
> Suppressing output is understandable and probably is a useful
> thing to do, but I do not see a justification to tie that
> quietness to making the status unuable...
>   

The status is unusable as is, actually, for the particular use case of cg-admin-rewritehist. If you try to use git-rm as an index filter, cg-admin-rewritehist will stop running as soon as you hit a revision that doesn't contain the file you're looking to filter out. (If the file doesn't exist in the first revision in your repo, that means it will do no work at all.)

My justification was that the "-f" option to the normal UNIX "rm" command does the same thing: it suppresses output and causes the command to exit with a zero status code even if no files match. But "git rm -f" was already taken.

If you like I'll be happy to split it into two separate options.
-Steve
Junio C Hamano· Apr 16, 2007, 01:14 UTC · re: Steven Grimm · lore

Re: [PATCH] Add -q option to "git rm" to suppress output when there aren't errors.

Steven Grimm <koreth@midwinter.com> writes:
Show 12 quoted lines
> Junio C Hamano wrote:
>
>> Suppressing output is understandable and probably is a useful
>> thing to do, but I do not see a justification to tie that
>> quietness to making the status unuable...
>
> The status is unusable as is, actually, for the particular use case of
> cg-admin-rewritehist. If you try to use git-rm as an index filter,
> cg-admin-rewritehist will stop running as soon as you hit a revision
> that doesn't contain the file you're looking to filter out. (If the
> file doesn't exist in the first revision in your repo, that means it
> will do no work at all.)

Probably "git-grep foo" wouldn't be suitable as the index filter for admin-rewritehist, either, nor "git-fsck", nor many other things.

What does it have to do with anything?

Saying "git rm --quiet foo" from the command line, wishing to supress the output, is very understandable.

Saying "git rm --ignore-unmatch foo bar baz", wishing to remove bar (which exists) even when foo does not exist, is also very understandable.

I think they are pretty much independent options.
Steven Grimm· Apr 16, 2007, 07:46 UTC · re: Junio C Hamano · lore

[PATCH 1/2] Add --quiet option to suppress output of "rm" commands for removed files.

Signed-off-by: Steven Grimm <koreth@midwinter.com>
---

Splitting this into two different options (and two patches) per Junio's comments.

 Documentation/git-rm.txt |    4 ++++
 builtin-rm.c             |   10 +++++++---
 t/t3600-rm.sh            |   24 ++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 3 deletions(-)
Show changes to 3 files +35 −3

Documentation/git-rm.txt, builtin-rm.c, t/t3600-rm.sh

diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 6feebc0..b051ccb 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -47,6 +47,10 @@ OPTIONS
 	the paths only from the index, leaving working tree
 	files.
 
+\--quiet::
+	git-rm normally outputs one line (in the form of an "rm" command)
+	for each file removed. This option suppresses that output.
+
 
 DISCUSSION
 ----------
diff --git a/builtin-rm.c b/builtin-rm.c
index 8a0738f..7eb9a42 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -10,7 +10,7 @@
 #include "tree-walk.h"
 
 static const char builtin_rm_usage[] =
-"git-rm [-f] [-n] [-r] [--cached] [--] <file>...";
+"git-rm [-f] [-n] [-r] [--cached] [--quiet] [--] <file>...";
 
 static struct {
 	int nr, alloc;
@@ -104,7 +104,7 @@ static struct lock_file lock_file;
 int cmd_rm(int argc, const char **argv, const char *prefix)
 {
 	int i, newfd;
-	int show_only = 0, force = 0, index_only = 0, recursive = 0;
+	int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;
 	const char **pathspec;
 	char *seen;
 
@@ -132,6 +132,8 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 			force = 1;
 		else if (!strcmp(arg, "-r"))
 			recursive = 1;
+		else if (!strcmp(arg, "--quiet"))
+			quiet = 1;
 		else
 			usage(builtin_rm_usage);
 	}
@@ -187,7 +189,9 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	 */
 	for (i = 0; i < list.nr; i++) {
 		const char *path = list.name[i];
-		printf("rm '%s'\n", path);
+		if (!quiet) {
+			printf("rm '%s'\n", path);
+		}
 
 		if (remove_file_from_cache(path))
 			die("git-rm: unable to remove %s", path);
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index e31cf93..da9da92 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -84,6 +84,26 @@ test_expect_success \
     'When the rm in "git-rm -f" fails, it should not remove the file from the index' \
     'git-ls-files --error-unmatch baz'
 
+test_expect_success '"rm" command printed' '
+	echo frotz > test-file &&
+	git add test-file &&
+	git commit -m "add file for rm test" &&
+	git rm test-file > rm-output &&
+	test `egrep "^rm " rm-output | wc -l` = 1 &&
+	rm -f test-file rm-output &&
+	git commit -m "remove file from rm test"
+'
+
+test_expect_success '"rm" command suppressed with --quiet' '
+	echo frotz > test-file &&
+	git add test-file &&
+	git commit -m "add file for rm --quiet test" &&
+	git rm --quiet test-file > rm-output &&
+	test `wc -l < rm-output` = 0 &&
+	rm -f test-file rm-output &&
+	git commit -m "remove file from rm --quiet test"
+'
+
 # Now, failure cases.
 test_expect_success 'Re-add foo and baz' '
 	git add foo baz &&
@@ -154,4 +174,8 @@ test_expect_success 'Recursive with -r -f' '
 	! test -d frotz
 '
 
+test_expect_failure 'Remove nonexistent file returns nonzero exit status' '
+	git rm nonexistent
+'
+
 test_done
-- 
1.5.1.1.99.g0ea98
Steven Grimm· Apr 16, 2007, 07:53 UTC · re: Steven Grimm · lore

[PATCH 2/2] Add --ignore-notfound option to exit with zero status when no files are removed.

Signed-off-by: Steven Grimm <koreth@midwinter.com>
---

This allows "git rm -r --ignore-notfound" to be used as an index filter with cg-admin-rewritehist. The documentation for that command recommends using git-update-index --remove to filter files out of a tree's history, but that doesn't support recursive deletion like git-rm does, making it less convenient to filter directories from history.

 Documentation/git-rm.txt |    3 +++
 builtin-rm.c             |   21 +++++++++++++++++----
 t/t3600-rm.sh            |    4 ++++
 3 files changed, 24 insertions(+), 4 deletions(-)
Show changes to 3 files +24 −4

Documentation/git-rm.txt, builtin-rm.c, t/t3600-rm.sh

diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index b051ccb..9ffb515 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -47,6 +47,9 @@ OPTIONS
 	the paths only from the index, leaving working tree
 	files.
 
+\--ignore-notfound::
+	Exit with a zero status even if no files matched.
+
 \--quiet::
 	git-rm normally outputs one line (in the form of an "rm" command)
 	for each file removed. This option suppresses that output.
diff --git a/builtin-rm.c b/builtin-rm.c
index 7eb9a42..71166fb 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -10,7 +10,7 @@
 #include "tree-walk.h"
 
 static const char builtin_rm_usage[] =
-"git-rm [-f] [-n] [-r] [--cached] [--quiet] [--] <file>...";
+"git-rm [-f] [-n] [-r] [--cached] [--ignore-notfound] [--quiet] [--] <file>...";
 
 static struct {
 	int nr, alloc;
@@ -105,6 +105,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 {
 	int i, newfd;
 	int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;
+	int ignore_notfound = 0;
 	const char **pathspec;
 	char *seen;
 
@@ -134,6 +135,8 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 			recursive = 1;
 		else if (!strcmp(arg, "--quiet"))
 			quiet = 1;
+		else if (!strcmp(arg, "--ignore-notfound"))
+			ignore_notfound = 1;
 		else
 			usage(builtin_rm_usage);
 	}
@@ -155,14 +158,24 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 
 	if (pathspec) {
 		const char *match;
+		int seen_any = 0;
 		for (i = 0; (match = pathspec[i]) != NULL ; i++) {
-			if (!seen[i])
-				die("pathspec '%s' did not match any files",
-				    match);
+			if (!seen[i]) {
+				if (!ignore_notfound) {
+					die("pathspec '%s' did not match any files",
+					    match);
+				}
+			}
+			else {
+				seen_any = 1;
+			}
 			if (!recursive && seen[i] == MATCHED_RECURSIVELY)
 				die("not removing '%s' recursively without -r",
 				    *match ? match : ".");
 		}
+
+		if (! seen_any)
+			exit(0);
 	}
 
 	/*
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index da9da92..665b8b0 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -84,6 +84,10 @@ test_expect_success \
     'When the rm in "git-rm -f" fails, it should not remove the file from the index' \
     'git-ls-files --error-unmatch baz'
 
+test_expect_success 'Remove nonexistent file with --ignore-notfound' '
+	git rm --ignore-notfound nonexistent
+'
+
 test_expect_success '"rm" command printed' '
 	echo frotz > test-file &&
 	git add test-file &&
-- 
1.5.1.1.99.g0ea98
Junio C Hamano· Apr 16, 2007, 07:59 UTC · re: Steven Grimm · lore

Re: [PATCH 2/2] Add --ignore-notfound option to exit with zero status when no files are removed.

Steven Grimm <koreth@midwinter.com> writes:
>  Documentation/git-rm.txt |    3 +++
>  builtin-rm.c             |   21 +++++++++++++++++----
>  t/t3600-rm.sh            |    4 ++++
>  3 files changed, 24 insertions(+), 4 deletions(-)

I find it easier to see what is going on with two independent features separated out, especially with tests. Much nicer.

Show 7 quoted lines
> @@ -47,6 +47,9 @@ OPTIONS
>  	the paths only from the index, leaving working tree
>  	files.
>  
> +\--ignore-notfound::
> +	Exit with a zero status even if no files matched.
> +

ls-files has --error-unmatch so we may want to make the name consistent by saying --ignore-unmatch? I particularly do not care about the minute naming issues _right_ _now_, but we might regret it in the long run.

No need to resend unless you have an actual fix; I'll queue your patch as is in 'pu', wait for a few days for comments on the naming, and amend it as needed.

Thanks.
Steven Grimm· Apr 16, 2007, 08:13 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] Add --ignore-notfound option to exit with zero status when no files are removed.

Junio C Hamano wrote:
Show 5 quoted lines
> ls-files has --error-unmatch so we may want to make the name
> consistent by saying --ignore-unmatch?  I particularly do not
> care about the minute naming issues _right_ _now_, but we might
> regret it in the long run.
>   
Sure, UI consistency is a good thing. Feel free to amend.
-Steve
Jeff King· Apr 16, 2007, 08:50 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] Add --ignore-notfound option to exit with zero status when no files are removed.

On Mon, Apr 16, 2007 at 12:59:35AM -0700, Junio C Hamano wrote:
Show 7 quoted lines
> > +\--ignore-notfound::
> > +	Exit with a zero status even if no files matched.
> > +
> ls-files has --error-unmatch so we may want to make the name
> consistent by saying --ignore-unmatch?  I particularly do not
> care about the minute naming issues _right_ _now_, but we might
> regret it in the long run.
Isn't this quite similar to the '-k' option to git-mv?
-Peff
Junio C Hamano· Apr 16, 2007, 08:53 UTC · re: Jeff King · lore

Re: [PATCH 2/2] Add --ignore-notfound option to exit with zero status when no files are removed.

Jeff King <peff@peff.net> writes:
Show 11 quoted lines
> On Mon, Apr 16, 2007 at 12:59:35AM -0700, Junio C Hamano wrote:
>
>> > +\--ignore-notfound::
>> > +	Exit with a zero status even if no files matched.
>> > +
>> ls-files has --error-unmatch so we may want to make the name
>> consistent by saying --ignore-unmatch?  I particularly do not
>> care about the minute naming issues _right_ _now_, but we might
>> regret it in the long run.
>
> Isn't this quite similar to the '-k' option to git-mv?

Heh, I never use 'git-mv' and 'git-rm' myself, so I did not know about that option, but from the Documentation/git-mv.txt, it sounds similar. What does it stand for, I wonder?

Jeff King· Apr 16, 2007, 09:04 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] Add --ignore-notfound option to exit with zero status when no files are removed.

On Mon, Apr 16, 2007 at 01:53:46AM -0700, Junio C Hamano wrote:
Show 13 quoted lines
> >> > +\--ignore-notfound::
> >> > +	Exit with a zero status even if no files matched.
> >> > +
> >> ls-files has --error-unmatch so we may want to make the name
> >> consistent by saying --ignore-unmatch?  I particularly do not
> >> care about the minute naming issues _right_ _now_, but we might
> >> regret it in the long run.
> >
> > Isn't this quite similar to the '-k' option to git-mv?
> 
> Heh, I never use 'git-mv' and 'git-rm' myself, so I did not know
> about that option, but from the Documentation/git-mv.txt, it
> sounds similar.  What does it stand for, I wonder?
As you are so fond of telling everyone else, use git-blame to find out. :)

Looks like '-k' got added in the move from git-rename to git-mv. Josef, can you shed any light on the choice of '-k'?

-Peff
Josef Weidendorfer· Apr 16, 2007, 18:29 UTC · re: Jeff King · lore

Re: [PATCH 2/2] Add --ignore-notfound option to exit with zero status when no files are removed.

On Monday 16 April 2007, you wrote:
Show 20 quoted lines
> On Mon, Apr 16, 2007 at 01:53:46AM -0700, Junio C Hamano wrote:
> 
> > >> > +\--ignore-notfound::
> > >> > +	Exit with a zero status even if no files matched.
> > >> > +
> > >> ls-files has --error-unmatch so we may want to make the name
> > >> consistent by saying --ignore-unmatch?  I particularly do not
> > >> care about the minute naming issues _right_ _now_, but we might
> > >> regret it in the long run.
> > >
> > > Isn't this quite similar to the '-k' option to git-mv?
> > 
> > Heh, I never use 'git-mv' and 'git-rm' myself, so I did not know
> > about that option, but from the Documentation/git-mv.txt, it
> > sounds similar.  What does it stand for, I wonder?
> 
> As you are so fond of telling everyone else, use git-blame to find out. :)
> 
> Looks like '-k' got added in the move from git-rename to git-mv. Josef,
> can you shed any light on the choice of '-k'?

That is similiar to "-k" option to "make", synonym for "--keep-going". I thought this was quite natural; at least I am used to this option for "make".

Josef
Steven Grimm· Apr 16, 2007, 08:12 UTC · re: Steven Grimm · lore

Re: [PATCH 2/2] Add --ignore-notfound option to exit with zero status when no files are removed.

Steven Grimm wrote:
Show 6 quoted lines
> This allows "git rm -r --ignore-notfound" to be used as an index filter
> with cg-admin-rewritehist. The documentation for that command recommends
> using git-update-index --remove to filter files out of a tree's history,
> but that doesn't support recursive deletion like git-rm does, making it
> less convenient to filter directories from history.
>   

Having said that, in some sense the problem here is arguably with cg-admin-rewritehist bailing out when a filter command exits with a nonzero status; given that git-rm is likely to be a common filter people will want to use, ideally cg-admin-rewritehist should do the right thing with git-rm's normal exit codes. But it appears Cogito is not being actively maintained any more (correct me if I'm wrong; the latest stable release is half a year old at this point, while git has changed substantially since then) so it didn't seem too worthwhile to try to patch things from that side.

I think it makes sense to be able to optionally treat a lack of matches as a non-error condition like "rm -f" does, so I'm comfortable submitting this patch regardless.

On a related note, last time I asked a question that caused someone to point me to cg-admin-rewritehist, Linus said he'd like to see its functionality folded into git proper. I have to agree with that idea; there are some obvious enhancements that program could use, and it'll be easier to do that if it's part of an actively maintained code base.

-Steve
Alex Riesen· Apr 16, 2007, 07:54 UTC · re: Junio C Hamano · lore

Re: [PATCH] Add -q option to "git rm" to suppress output when there aren't errors.

On 4/16/07, Junio C Hamano <junkio@cox.net> wrote:
Show 8 quoted lines
> What does it have to do with anything?
>
> Saying "git rm --quiet foo" from the command line, wishing to
> supress the output, is very understandable.
>
> Saying "git rm --ignore-unmatch foo bar baz", wishing to remove
> bar (which exists) even when foo does not exist, is also very
> understandable.

How about doing what rm(1) does? Something like "rm -f"? It returns 0 even if nothing given in command-line, but any error (like permissions) is reported and the status is not 0.

It is often that it is not the commands output which is annoying, but the _unwanted_ output. No one wants to see "rm 'file'". No one besides people debugging git-rm (you can't even use it for copy and paste of the filenames: it has "rm"'s in the text).

← back to recent threads