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

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

[Ciaran McCreesh]

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(-)

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

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

[Petr Baudis]

  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

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

[Avery Pennarun]

On 7/16/08, Petr Baudis <pasky@suse.cz> wrote:
>  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

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

[Junio C Hamano]

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

;-)

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

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

> @@ -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?

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

[Peter Baumann]

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.

-Peter

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

[David Christensen]

On Jul 16, 2008, at 1:58 PM, Peter Baumann wrote:

> 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