threads / rfc / 14488

RFC patchImplement 'git rm --if-missing'

Subject: [PATCH,RFC] Implement 'git rm --if-missing'

## tl;dr

6 messages between Jul 16, 2008 and Jul 16, 2008. Diffs are folded; open one to read it.

replies: 5people: 6as markdown or json

Ciaran McCreesh· Jul 16, 2008, 18:00 UTC · lore

git rm --if-missing will only remove files if they've already been removed from disk.

Signed-off-by: Ciaran McCreesh <ciaran.mccreesh@googlemail.com>
---

There's nothing here that can't be done using git update-index, but git rm is less scary.

Regarding exit status: I'm not sure whether exit status should be based upon whether any files were actually removed, or whether it should be based upon whether or not all of the supplied patterns were matched. I've gone for the latter, so that 'git rm --if-missing -r .' succeeds if there's nothing to remove.

I'm not sure whether 'missing' is the best word. '--if-noent' might be more appropriate, but less familiar to some. Or is this worth a short option?

 Documentation/git-rm.txt |    8 +++++++-
 builtin-rm.c             |    9 ++++++++-
 t/t3600-rm.sh            |   43 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 2 deletions(-)
Show changes to 3 files +58 −2

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

diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 4d0c495..f9335f3 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -7,7 +7,8 @@ git-rm - Remove files from the working tree and from the index
 
 SYNOPSIS
 --------
-'git rm' [-f] [-n] [-r] [--cached] [--ignore-unmatch] [--quiet] [--] <file>...
+'git rm' [-f] [-n] [-r] [--cached] [--ignore-unmatch] [--if-missing]
+	  [--quiet] [--] <file>...
 
 DESCRIPTION
 -----------
@@ -61,6 +62,11 @@ OPTIONS
 --ignore-unmatch::
 	Exit with a zero status even if no files matched.
 
+--if-missing::
+	Only remove files if they have been removed from disk. Exit status
+	is still based upon whether matches succeed, not whether a remove
+	actually took place.
+
 -q::
 --quiet::
 	'git-rm' normally outputs one line (in the form of an "rm" command)
diff --git a/builtin-rm.c b/builtin-rm.c
index 22c9bd1..4b89705 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -125,7 +125,7 @@ static int check_local_mod(unsigned char *head, int index_only)
 static struct lock_file lock_file;
 
 static int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;
-static int ignore_unmatch = 0;
+static int ignore_unmatch = 0, if_missing = 0;
 
 static struct option builtin_rm_options[] = {
 	OPT__DRY_RUN(&show_only),
@@ -135,6 +135,7 @@ static struct option builtin_rm_options[] = {
 	OPT_BOOLEAN('r', NULL,             &recursive,  "allow recursive removal"),
 	OPT_BOOLEAN( 0 , "ignore-unmatch", &ignore_unmatch,
 				"exit with a zero status even if nothing matched"),
+	OPT_BOOLEAN( 0 , "if-missing",     &if_missing, "only remove missing files"),
 	OPT_END(),
 };
 
@@ -168,6 +169,12 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 		struct cache_entry *ce = active_cache[i];
 		if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen))
 			continue;
+		if (if_missing)
+		{
+			struct stat st;
+			if ((lstat(ce->name, &st) == 0) || (errno != ENOENT))
+				continue;
+		}
 		add_list(ce->name);
 	}
 
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index f542f0a..c7c1810 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -143,6 +143,45 @@ test_expect_success '"rm" command suppressed with --quiet' '
 	git commit -m "remove file from rm --quiet test"
 '
 
+test_expect_success 'Test that "rm --if-missing" works' '
+	echo frotz > test-file &&
+	echo frotz > other-file &&
+	git add test-file other-file &&
+	git commit -m "add files from rm --if-missing test" &&
+	rm test-file &&
+	git rm --if-missing test-file other-file &&
+	! git ls-files --error-unmatch test-file &&
+	git ls-files --error-unmatch other-file &&
+	git rm other-file &&
+	git commit -m "remove file from rm --if-missing test"
+'
+
+test_expect_success 'Test that "rm --if-missing -r *" works' '
+	echo frotz > test-file &&
+	mkdir -p frotz &&
+	echo frotz > frotz/other-file &&
+	git add test-file frotz/other-file &&
+	git commit -m "add file from rm --if-missing -r * test" &&
+	rm frotz/other-file
+	git rm --if-missing --ignore-unmatch -r \* &&
+	git ls-files --error-unmatch test-file &&
+	git rm test-file &&
+	git commit -m "remove file from rm --missing -r * test &&
+	! test -d frotz"
+'
+
+test_expect_success 'Test that "rm --if-missing -r *" works even if nothing is removed' '
+	echo frotz > test-file &&
+	mkdir -p frotz &&
+	echo frotz > frotz/other-file &&
+	git add test-file frotz/other-file &&
+	git commit -m "add file from rm --if-missing -r * test" &&
+	git rm --if-missing --ignore-unmatch -r \* &&
+	git rm test-file frotz/other-file &&
+	git commit -m "remove file from rm --missing -r * test &&
+	! test -d frotz"
+'
+
 # Now, failure cases.
 test_expect_success 'Re-add foo and baz' '
 	git add foo baz &&
@@ -217,4 +256,8 @@ test_expect_success 'Remove nonexistent file returns nonzero exit status' '
 	! git rm nonexistent
 '
 
+test_expect_success 'Test that "rm --if-missing nonexistent" fails' '
+	! git rm --if-missing nonexistent
+'
+
 test_done
-- 
1.5.6.3.385.g7c3f1
Petr Baudis· Jul 16, 2008, 18:06 UTC · re: Ciaran McCreesh · lore

Re: [PATCH,RFC] Implement 'git rm --if-missing'

  Hi,
On Wed, Jul 16, 2008 at 07:00:50PM +0100, Ciaran McCreesh wrote:
> git rm --if-missing will only remove files if they've already been removed from
> disk.
> 
> Signed-off-by: Ciaran McCreesh <ciaran.mccreesh@googlemail.com>
  what is the usage scenario? The porcelain options space is a precious
resource, so please explain why do you need this and who is going to use
it (especially with such a long name).
				Petr "Pasky" Baudis
Avery Pennarun· Jul 16, 2008, 18:17 UTC · re: Petr Baudis · lore

Re: [PATCH,RFC] Implement 'git rm --if-missing'

On 7/16/08, Petr Baudis <pasky@suse.cz> wrote:
Show 9 quoted lines
>  On Wed, Jul 16, 2008 at 07:00:50PM +0100, Ciaran McCreesh wrote:
>  > git rm --if-missing will only remove files if they've already been removed from
>  > disk.
>  >
>  > Signed-off-by: Ciaran McCreesh <ciaran.mccreesh@googlemail.com>
>
>   what is the usage scenario? The porcelain options space is a precious
>  resource, so please explain why do you need this and who is going to use
>  it (especially with such a long name).
I see the idea here: right now you can do:
         touch a b c
         git add .

And have it auto-add all the new files, so "git commit" will work. But there is no equivalent for rm, because for obvious reasons,

        rm b c
        git rm .

Doesn't do the same thing. And "git add ." doesn't auto-recognize deletions, which probably also makes sense.

"git commit -a", on the other hand, will automatically commit all deletions for you. But you don't always want to commit *all* your changes just because you want to commit all your deletions.

That said, --if-missing is a bit unwieldy. I don't have a better suggestion though.

Have fun,
Avery
Junio C Hamano· Jul 16, 2008, 18:48 UTC · re: Ciaran McCreesh · lore

Re: [PATCH,RFC] Implement 'git rm --if-missing'

Ciaran McCreesh <ciaran.mccreesh@googlemail.com> writes:
> git rm --if-missing will only remove files if they've already been removed from
> disk.

This probably is a borderline with feaping creaturism. What's the use of it in a real workflow that you need this for?

"git add -u" may be too broad in that it also adds anything modified, but so is --if-missing too broad in that it removes anything removed, and if you are going to limit by giving pathspecs _anyway_, then...

Old timers might just do:
	git diff --name-only --diff-filter=D |
        git update-index --remove --stdin
;-)
Show 10 quoted lines
> diff --git a/builtin-rm.c b/builtin-rm.c
> index 22c9bd1..4b89705 100644
> --- a/builtin-rm.c
> +++ b/builtin-rm.c
> @@ -125,7 +125,7 @@ static int check_local_mod(unsigned char *head, int index_only)
>  static struct lock_file lock_file;
>  
>  static int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;
> -static int ignore_unmatch = 0;
> +static int ignore_unmatch = 0, if_missing = 0;

Not your fault in entirety, but we should drop these " = 0" initializations for static variables in a clean-up patch.

Show 7 quoted lines
>  static struct option builtin_rm_options[] = {
>  	OPT__DRY_RUN(&show_only),
> @@ -135,6 +135,7 @@ static struct option builtin_rm_options[] = {
>  	OPT_BOOLEAN('r', NULL,             &recursive,  "allow recursive removal"),
>  	OPT_BOOLEAN( 0 , "ignore-unmatch", &ignore_unmatch,
>  				"exit with a zero status even if nothing matched"),
> +	OPT_BOOLEAN( 0 , "if-missing",     &if_missing, "only remove missing files"),

Perhaps the command should error out if some of the named files still exist in the working tree?

Show 10 quoted lines
> @@ -168,6 +169,12 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
>  		struct cache_entry *ce = active_cache[i];
>  		if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen))
>  			continue;
> +		if (if_missing)
> +		{
> +			struct stat st;
> +			if ((lstat(ce->name, &st) == 0) || (errno != ENOENT))
> +				continue;
> +		}
 (1) (Style).  Opening brace comes on the same line as "if ()".
 (2) (Design). How should this new option interact with --cached mode of
     operation?
 (3) (Design). Shouldn't "git rm --if-missing" without any pathspec remove
     all missing paths from the index?
 (4) If lstat fails due to I/O error or something, you do not continue and
     add that path you did not get ENOENT for to the kill-list.  Is that
     desirable?
 (5) I wonder if lstat() is enough here.  
     Consider:
	- current commit has "kernel" symlink to "linux-2.6/" directory but
          you want to remove kernel and move directory linux-2.6 to it, so:
          - you run "rm kernel; mv linux-2.6 kernel"
	  - then you run "git rm --if-missing -- kernel"
     What should the command do?
Peter Baumann· Jul 16, 2008, 18:58 UTC · re: Junio C Hamano · lore

Re: [PATCH,RFC] Implement 'git rm --if-missing'

On Wed, Jul 16, 2008 at 11:48:42AM -0700, Junio C Hamano wrote:
Show 19 quoted lines
> Ciaran McCreesh <ciaran.mccreesh@googlemail.com> writes:
> 
> > git rm --if-missing will only remove files if they've already been removed from
> > disk.
> 
> This probably is a borderline with feaping creaturism.  What's the use of
> it in a real workflow that you need this for?
> 
> "git add -u" may be too broad in that it also adds anything modified, but
> so is --if-missing too broad in that it removes anything removed, and if
> you are going to limit by giving pathspecs _anyway_, then...
> 
> Old timers might just do:
> 
> 	git diff --name-only --diff-filter=D |
>         git update-index --remove --stdin
> 
> ;-)
> 

Ah. This comes in handy. I already searched for a command to delete all missing files. After reading through the fine manual of 'git rm', I went to git update-index but didn't come up with a solution to my problem.

But I have to say, an argument to 'git rm' would be preferable than the above plumping.

-Peter
David Christensen· Jul 16, 2008, 19:43 UTC · re: Peter Baumann · lore

Re: [PATCH,RFC] Implement 'git rm --if-missing'

On Jul 16, 2008, at 1:58 PM, Peter Baumann wrote:
Show 34 quoted lines
> On Wed, Jul 16, 2008 at 11:48:42AM -0700, Junio C Hamano wrote:
>> Ciaran McCreesh <ciaran.mccreesh@googlemail.com> writes:
>>
>>> git rm --if-missing will only remove files if they've already been  
>>> removed from
>>> disk.
>>
>> This probably is a borderline with feaping creaturism.  What's the  
>> use of
>> it in a real workflow that you need this for?
>>
>> "git add -u" may be too broad in that it also adds anything  
>> modified, but
>> so is --if-missing too broad in that it removes anything removed,  
>> and if
>> you are going to limit by giving pathspecs _anyway_, then...
>>
>> Old timers might just do:
>>
>> 	git diff --name-only --diff-filter=D |
>>        git update-index --remove --stdin
>>
>> ;-)
>>
>
> Ah. This comes in handy. I already searched for a command to delete  
> all
> missing files. After reading through the fine manual of 'git rm', I  
> went
> to git update-index but didn't come up with a solution to my problem.
>
> But I have to say, an argument to 'git rm' would be preferable than  
> the
> above plumping.
Wouldn't:
git rm $(git ls-files --deleted)
do the trick, or am I missing something?
Regards,

David -- David Christensen End Point Corporation david@endpoint.com

← back to recent threads