git clean --exclude broken?

git clean --exclude broken?

[Todd Rinaldo]

I think I have found a new bug in 1.7.5:
# Setup:
mkdir tmp && cd tmp
git init
mkdir foo && touch foo/bar
mkdir bar && touch bar/baz
echo "/foo" > .gitignore
echo "/bar" >> .gitignore
git add .gitignore
git commit -m ignore

# The problem (Why is foo/ removed?)
$>git clean -dXf --exclude=/foo
Removing bar/
Removing foo/

I apologize if this isn't the correct channel to report a bug. Does anyone know if this is a known issue?

Thanks,
Todd Rinaldo

Re: git clean --exclude broken?

[Junio C Hamano]

Todd Rinaldo <toddr@cpanel.net> writes:

> I think I have found a new bug in 1.7.5:

My quick check indicates 1.7.3 behaves the same way, and 1.7.2.5 didn't
have --exclude option, so this does not seem to be anything particularly
new in the 1.7.5 release.

> # The problem (Why is foo/ removed?)
> $>git clean -dXf --exclude=/foo
> Removing bar/
> Removing foo/

Why is this command line giving -X that tells us not to use the ignore
rules, and --exclude option at the same time?

Re: git clean --exclude broken?

[Todd Rinaldo]

On Aug 24, 2011, at 4:23 PM, Junio C Hamano wrote:

> Todd Rinaldo <toddr@cpanel.net> writes:
> 
>> I think I have found a new bug in 1.7.5:
> 
> My quick check indicates 1.7.3 behaves the same way, and 1.7.2.5 didn't
> have --exclude option, so this does not seem to be anything particularly
> new in the 1.7.5 release.
No. I was just clarifying what my binary was for research purposes.

> 
>> # The problem (Why is foo/ removed?)
>> $>git clean -dXf --exclude=/foo
>> Removing bar/
>> Removing foo/
> 
> Why is this command line giving -X that tells us not to use the ignore
> rules, and --exclude option at the same time?
My more complicated use of the command wanted to use the .gitignore rules to cleanup ignored files with the exception of 1 directory. I believe -dxf --exclude is also broken in the same way.

Re: git clean --exclude broken?

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Todd Rinaldo <toddr@cpanel.net> writes:
>
>> I think I have found a new bug in 1.7.5:
>
> My quick check indicates 1.7.3 behaves the same way, and 1.7.2.5 didn't
> have --exclude option, so this does not seem to be anything particularly
> new in the 1.7.5 release.
>
>> # The problem (Why is foo/ removed?)
>> $>git clean -dXf --exclude=/foo
>> Removing bar/
>> Removing foo/
>
> Why is this command line giving -X that tells us not to use the ignore
> rules, and --exclude option at the same time?

The documentation and the implementation of "git clean" is quite confused.
Here is what is said about "-e":

    -e <pattern>::
    --exclude=<pattern>::
            Specify special exceptions to not be cleaned.  Each <pattern> is
            the same form as in $GIT_DIR/info/excludes and this option can be
            given multiple times.

But in reality, it is not about "not be cleaned" at all. A better
description to reflect what the implementation actually does is probably
this:

	In addition to what are found in usual places like .gitignore (per
	directory) and $GIT_DIR/info/exclude, consider these patterns to
	be in the ignore rules.

This mirrors what --exclude parameter to "ls-files" does [*1*].

I can however see a use case where the user wants to say something like
this which is quite different:

	I know "git clean" (or "git clean -x" or "git clean -X") will try
	to remove paths A, B and C. I want it remove them except for this
	particular path C by adding --except=C option to the command line.

And the current documentation does look like it is describing such an
option. But that is not what --exclude option is about.

One solution might be to say "I know the usual rules stored in .gitignore
and the like tell us to that 'foo' is ignored (and to be cleaned), but for
this invocation only, please treat 'foo' is _not_ ignored.", and there
indeed is a way to do so:

    $ git clean -d -X -e \!foo

(the backslash before '!' is to avoid history substitution in some shells).


[Footnote]

*1* This part in builtin/clean.c looks a bit distasteful:

	for (i = 0; i < exclude_list.nr; i++)
		add_exclude(exclude_list.items[i].string, "", 0, dir.exclude_list);

The last parameter should be &dir.exclude_list[EXC_CMDL] because we are
adding exclude patterns from the command line.  It works by accident
only because EXC_CMDL happens to be defined as 0.

Re* git clean --exclude broken?

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> The documentation and the implementation of "git clean" is quite confused.
> ...

So here is a patch to fix the confusion.

It does not add a new "--except=C" I alluded to, but at least it should
be the right first step to make the document clearly describe what the
existing option does.

-- >8 --
Subject: [PATCH] Documentation: clarify "git clean -e <pattern>"

The current explanation of -e can be misread as allowing the user to say

    I know 'git clean -XYZ' (substitute -XYZ with any option and/or
    parameter) will remove paths A, B, and C, and I want them all removed
    except for paths matching this pattern by adding '-e C' to the same
    command line, i.e. 'git clean -e C -XYZ'.

But that is not what this option does. It augments the set of ignore rules
from the command line, just like the same "-e <pattern>" argument does
with the "ls-files" command (the user could probably pass "-e \!C" to tell
the command to clean everything the command would normally remove, except
for C).

It also fixes small style nit in the parameter to add_exclude() call. The
current code only works because EXC_CMDL happens to be defined as 0.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-clean.txt |    6 +++---
 builtin/clean.c             |    5 ++++-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index 974e04e..a7a18e3 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -47,9 +47,9 @@ OPTIONS
 
 -e <pattern>::
 --exclude=<pattern>::
-	Specify special exceptions to not be cleaned.  Each <pattern> is
-	the same form as in $GIT_DIR/info/excludes and this option can be
-	given multiple times.
+	In addition to what are found in .gitignore (per directory) and
+	$GIT_DIR/info/exclude, also consider these patterns to be in the
+	set of the ignore rules in effect.
 
 -x::
 	Don't use the ignore rules.  This allows removing all untracked
diff --git a/builtin/clean.c b/builtin/clean.c
index 75697f7..3782718 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -76,6 +76,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 
 	if (ignored && ignored_only)
 		die(_("-x and -X cannot be used together"));
+	if (ignored && exclude_list.nr)
+		die(_("adding exclude with -e and ignoring it with -x is crazy"));
 
 	if (!show_only && !force) {
 		if (config_set)
@@ -98,7 +100,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 		setup_standard_excludes(&dir);
 
 	for (i = 0; i < exclude_list.nr; i++)
-		add_exclude(exclude_list.items[i].string, "", 0, dir.exclude_list);
+		add_exclude(exclude_list.items[i].string, "", 0,
+			    &dir.exclude_list[EXC_CMDL]);
 
 	pathspec = get_pathspec(prefix, argv);
 

Re: Re* git clean --exclude broken?

[Michael Schubert]

On 08/25/2011 08:29 PM, Junio C Hamano wrote:
> diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
> index 974e04e..a7a18e3 100644
> --- a/Documentation/git-clean.txt
> +++ b/Documentation/git-clean.txt
> @@ -47,9 +47,9 @@ OPTIONS
>  
>  -e <pattern>::
>  --exclude=<pattern>::
> -	Specify special exceptions to not be cleaned.  Each <pattern> is
> -	the same form as in $GIT_DIR/info/excludes and this option can be
> -	given multiple times.
> +	In addition to what are found in .gitignore (per directory) and
> +	$GIT_DIR/info/exclude, also consider these patterns to be in the
> +	set of the ignore rules in effect.

Nitpick: Shouldn't this be "In addition to what is found in .." or
"In addition to those found in .."?

Re: Re* git clean --exclude broken?

[Junio C Hamano]

Michael Schubert <mschub@elegosoft.com> writes:

> Nitpick: Shouldn't this be "In addition to what is found in .." or
> "In addition to those found in .."?

Thanks.

Re: Re* git clean --exclude broken?

[Thomas Rast]

Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > The documentation and the implementation of "git clean" is quite confused.
> > ...
> 
> So here is a patch to fix the confusion.
> 
> It does not add a new "--except=C" I alluded to, but at least it should
> be the right first step to make the document clearly describe what the
> existing option does.
> 
> -- >8 --
> Subject: [PATCH] Documentation: clarify "git clean -e <pattern>"

It's not exclusively a doc patch, is it?

> +	if (ignored && exclude_list.nr)
> +		die(_("adding exclude with -e and ignoring it with -x is crazy"));

Please also add something like the following patch, so that 'git clean -h' 
does not confuse the user either.

diff --git i/builtin/clean.c w/builtin/clean.c
index 75697f7..33a3df9 100644
--- i/builtin/clean.c
+++ w/builtin/clean.c
@@ -54,7 +54,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN('d', NULL, &remove_directories,
 				"remove whole directories"),
 		{ OPTION_CALLBACK, 'e', "exclude", &exclude_list, "pattern",
-		  "exclude <pattern>", PARSE_OPT_NONEG, exclude_cb },
+		  "add <pattern> to ignore rules", PARSE_OPT_NONEG, exclude_cb },
 		OPT_BOOLEAN('x', NULL, &ignored, "remove ignored files, too"),
 		OPT_BOOLEAN('X', NULL, &ignored_only,
 				"remove only ignored files"),


-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: Re* git clean --exclude broken?

[Pete Wyckoff]

gitster@pobox.com wrote on Thu, 25 Aug 2011 11:29 -0700:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > The documentation and the implementation of "git clean" is quite confused.
> > ...
> 
> So here is a patch to fix the confusion.
> 
> It does not add a new "--except=C" I alluded to, but at least it should
> be the right first step to make the document clearly describe what the
> existing option does.
> 
> -- >8 --
> Subject: [PATCH] Documentation: clarify "git clean -e <pattern>"
> 
> The current explanation of -e can be misread as allowing the user to say
> 
>     I know 'git clean -XYZ' (substitute -XYZ with any option and/or
>     parameter) will remove paths A, B, and C, and I want them all removed
>     except for paths matching this pattern by adding '-e C' to the same
>     command line, i.e. 'git clean -e C -XYZ'.
> 
> But that is not what this option does. It augments the set of ignore rules
> from the command line, just like the same "-e <pattern>" argument does
> with the "ls-files" command (the user could probably pass "-e \!C" to tell
> the command to clean everything the command would normally remove, except
> for C).
> 
> It also fixes small style nit in the parameter to add_exclude() call. The
> current code only works because EXC_CMDL happens to be defined as 0.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/git-clean.txt |    6 +++---
>  builtin/clean.c             |    5 ++++-
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
> index 974e04e..a7a18e3 100644
> --- a/Documentation/git-clean.txt
> +++ b/Documentation/git-clean.txt
> @@ -47,9 +47,9 @@ OPTIONS
>  
>  -e <pattern>::
>  --exclude=<pattern>::
> -	Specify special exceptions to not be cleaned.  Each <pattern> is
> -	the same form as in $GIT_DIR/info/excludes and this option can be
> -	given multiple times.
> +	In addition to what are found in .gitignore (per directory) and
> +	$GIT_DIR/info/exclude, also consider these patterns to be in the
> +	set of the ignore rules in effect.
>  
>  -x::
>  	Don't use the ignore rules.  This allows removing all untracked
> diff --git a/builtin/clean.c b/builtin/clean.c
> index 75697f7..3782718 100644
> --- a/builtin/clean.c
> +++ b/builtin/clean.c
> @@ -76,6 +76,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
>  
>  	if (ignored && ignored_only)
>  		die(_("-x and -X cannot be used together"));
> +	if (ignored && exclude_list.nr)
> +		die(_("adding exclude with -e and ignoring it with -x is crazy"));

This breaks one of my use cases for git clean.

We have "precious" files that are listed in .gitignore so that
they don't show up in "git status" output.  They're not part of
the repository, but special per-user per-workspace configuration
settings that are required to build the code.

There's plenty of other stuff in .gitignore that should be
deleted.  So we invoke:

    git clean -dqfx -e .magic_file -e "Magic*"

It's been discussed on the list a couple of times that a separate
category for files that I want to ignore, but do not want to
have cleaned, would fill this gap.

		-- Pete

>  	if (!show_only && !force) {
>  		if (config_set)
> @@ -98,7 +100,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
>  		setup_standard_excludes(&dir);
>  
>  	for (i = 0; i < exclude_list.nr; i++)
> -		add_exclude(exclude_list.items[i].string, "", 0, dir.exclude_list);
> +		add_exclude(exclude_list.items[i].string, "", 0,
> +			    &dir.exclude_list[EXC_CMDL]);
>  
>  	pathspec = get_pathspec(prefix, argv);
>  
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: Re* git clean --exclude broken?

[Junio C Hamano]

Pete Wyckoff <pw@padd.com> writes:

>> diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
>> index 974e04e..a7a18e3 100644
>> --- a/Documentation/git-clean.txt
>> +++ b/Documentation/git-clean.txt
>> @@ -47,9 +47,9 @@ OPTIONS
>>  
>>  -e <pattern>::
>>  --exclude=<pattern>::
>> -	Specify special exceptions to not be cleaned.  Each <pattern> is
>> -	the same form as in $GIT_DIR/info/excludes and this option can be
>> -	given multiple times.
>> +	In addition to what are found in .gitignore (per directory) and
>> +	$GIT_DIR/info/exclude, also consider these patterns to be in the
>> +	set of the ignore rules in effect.
>>  
>>  -x::
>>  	Don't use the ignore rules.  This allows removing all untracked
>> diff --git a/builtin/clean.c b/builtin/clean.c
>> index 75697f7..3782718 100644
>> --- a/builtin/clean.c
>> +++ b/builtin/clean.c
>> @@ -76,6 +76,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
>>  
>>  	if (ignored && ignored_only)
>>  		die(_("-x and -X cannot be used together"));
>> +	if (ignored && exclude_list.nr)
>> +		die(_("adding exclude with -e and ignoring it with -x is crazy"));
>
> This breaks one of my use cases for git clean.

The description of '-x' needs to be also updated to reflect what it does.

How about this on top?

 Documentation/git-clean.txt |    4 +++-
 builtin/clean.c             |    2 --
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
index b49674f..79fb984 100644
--- a/Documentation/git-clean.txt
+++ b/Documentation/git-clean.txt
@@ -52,7 +52,9 @@ OPTIONS
 	set of the ignore rules in effect.
 
 -x::
-	Don't use the ignore rules.  This allows removing all untracked
+	Don't use the standard ignore rules read from .gitignore (per
+	directory) and $GIT_DIR/info/exclude, but do still use the ignore
+	rules given with `-e` options.  This allows removing all untracked
 	files, including build products.  This can be used (possibly in
 	conjunction with 'git reset') to create a pristine
 	working directory to test a clean build.
diff --git a/builtin/clean.c b/builtin/clean.c
index 7fcbf87..0c7b3d0 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -76,8 +76,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 
 	if (ignored && ignored_only)
 		die(_("-x and -X cannot be used together"));
-	if (ignored && exclude_list.nr)
-		die(_("adding exclude with -e and ignoring it with -x is crazy"));
 
 	if (!show_only && !force) {
 		if (config_set)

Re: Re* git clean --exclude broken?

[Pete Wyckoff]

gitster@pobox.com wrote on Sat, 27 Aug 2011 23:27 -0700:
> Pete Wyckoff <pw@padd.com> writes:
> 
> >> diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
> >> index 974e04e..a7a18e3 100644
> >> --- a/Documentation/git-clean.txt
> >> +++ b/Documentation/git-clean.txt
> >> @@ -47,9 +47,9 @@ OPTIONS
> >>  
> >>  -e <pattern>::
> >>  --exclude=<pattern>::
> >> -	Specify special exceptions to not be cleaned.  Each <pattern> is
> >> -	the same form as in $GIT_DIR/info/excludes and this option can be
> >> -	given multiple times.
> >> +	In addition to what are found in .gitignore (per directory) and
> >> +	$GIT_DIR/info/exclude, also consider these patterns to be in the
> >> +	set of the ignore rules in effect.
> >>  
> >>  -x::
> >>  	Don't use the ignore rules.  This allows removing all untracked
> >> diff --git a/builtin/clean.c b/builtin/clean.c
> >> index 75697f7..3782718 100644
> >> --- a/builtin/clean.c
> >> +++ b/builtin/clean.c
> >> @@ -76,6 +76,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
> >>  
> >>  	if (ignored && ignored_only)
> >>  		die(_("-x and -X cannot be used together"));
> >> +	if (ignored && exclude_list.nr)
> >> +		die(_("adding exclude with -e and ignoring it with -x is crazy"));
> >
> > This breaks one of my use cases for git clean.
> 
> The description of '-x' needs to be also updated to reflect what it does.
> 
> How about this on top?
> 
>  Documentation/git-clean.txt |    4 +++-
>  builtin/clean.c             |    2 --
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
> index b49674f..79fb984 100644
> --- a/Documentation/git-clean.txt
> +++ b/Documentation/git-clean.txt
> @@ -52,7 +52,9 @@ OPTIONS
>  	set of the ignore rules in effect.
>  
>  -x::
> -	Don't use the ignore rules.  This allows removing all untracked
> +	Don't use the standard ignore rules read from .gitignore (per
> +	directory) and $GIT_DIR/info/exclude, but do still use the ignore
> +	rules given with `-e` options.  This allows removing all untracked
>  	files, including build products.  This can be used (possibly in
>  	conjunction with 'git reset') to create a pristine
>  	working directory to test a clean build.
> diff --git a/builtin/clean.c b/builtin/clean.c
> index 7fcbf87..0c7b3d0 100644
> --- a/builtin/clean.c
> +++ b/builtin/clean.c
> @@ -76,8 +76,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
>  
>  	if (ignored && ignored_only)
>  		die(_("-x and -X cannot be used together"));
> -	if (ignored && exclude_list.nr)
> -		die(_("adding exclude with -e and ignoring it with -x is crazy"));
>  
>  	if (!show_only && !force) {
>  		if (config_set)
> 

This works, thanks.  It is a confusing set of options, but we
need them all.  I couldn't think of a better way to describe
how they interact.

		-- Pete