Quantcast

git clean --exclude broken?

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: git clean --exclude broken?

Junio C Hamano
Todd Rinaldo <[hidden email]> 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?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: git clean --exclude broken?

Todd Rinaldo

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

> Todd Rinaldo <[hidden email]> 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.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: git clean --exclude broken?

Junio C Hamano
In reply to this post by Junio C Hamano
Junio C Hamano <[hidden email]> writes:

> Todd Rinaldo <[hidden email]> 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.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re* git clean --exclude broken?

Junio C Hamano
Junio C Hamano <[hidden email]> 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 <[hidden email]>
---
 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);
 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Re* git clean --exclude broken?

Junio C Hamano
Michael Schubert <[hidden email]> writes:

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

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Re* git clean --exclude broken?

Thomas Rast
In reply to this post by Junio C Hamano
Junio C Hamano wrote:

> Junio C Hamano <[hidden email]> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Re* git clean --exclude broken?

Pete Wyckoff
In reply to this post by Junio C Hamano
[hidden email] wrote on Thu, 25 Aug 2011 11:29 -0700:

> Junio C Hamano <[hidden email]> 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 <[hidden email]>
> ---
>  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 [hidden email]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Re* git clean --exclude broken?

Junio C Hamano
Pete Wyckoff <[hidden email]> 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)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Re* git clean --exclude broken?

Pete Wyckoff
[hidden email] wrote on Sat, 27 Aug 2011 23:27 -0700:

> Pete Wyckoff <[hidden email]> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Loading...