git-clean won't read global ignore

git-clean won't read global ignore

[shunichi fuji]

hi, i setup git to use with eclipse through global excludesfile config.
git-status report just ignore files, but git-clean deleted ignore files.

----
$ git-config -l
core.excludesfile=/home/pal/.gitignore

$ cat /home/pal/.gitignore
# ignore for eclipse
.project
.cproject

 $ git-status
# On branch master
nothing to commit (working directory clean)

$ git-clean
Removing .project

Re: git-clean won't read global ignore

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 719 bytes --]

On Tue, Nov 13, 2007 at 09:49:00PM +0000, shunichi fuji wrote:
> hi, i setup git to use with eclipse through global excludesfile config.
> git-status report just ignore files, but git-clean deleted ignore files.
> 
> ----
> $ git-config -l
> core.excludesfile=/home/pal/.gitignore
> 
> $ cat /home/pal/.gitignore
> # ignore for eclipse
> ..project
> ..cproject
> 
>  $ git-status
> # On branch master
> nothing to commit (working directory clean)
> 
> $ git-clean
> Removing .project

.project is not ..project right ?

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: git-clean won't read global ignore

[Junio C Hamano]

I think the problem is core.excludesfile is too new to be
noticed by anything other than git-add and git-status.

 * git-add and git-status know about it because they call
   add_excludes_from_file() directly with their own notion of
   which standard set of ignore files to use.  This is just a
   stupid duplication of code that need to be updated every time
   the definition of the standard set of ignore files is
   changed.

 * git-ls-files does not notice any of the "ignore" files by
   default, as it predates the standardized set of ignore
   files.  The calling scripts established the convention to use
   .git/info/exclude, .gitignore in each file, and later
   core.excludesfile.

 * git-read-tree takes --exclude-per-directory=<gitignore>,
   not because the flexibility was needed.  Again, this was
   because the option predates the standardization of the ignore
   files.

 * git-merge-recursive uses hardcoded per-directory .gitignore
   and nothing else.  git-clean (scripted version) does not
   honor core.* because its call to underlying ls-files does not
   know about it.  git-clean in C (parked in 'pu') doesn't either.

We probably could change git-ls-files to use the standard set
when no excludes are specified from the command line, or
something like that, but this will be a change in semantics that
would affect the scripts in a subtle way.  I am somewhat
reluctant to make such a change.

On the other hand, I think it makes perfect sense to fix
git-read-tree, git-merge-recursive and git-clean to follow the
same rule as other commands.  I do not think of a valid use case
to give an exclude-per-directory that is nonstandard to
read-tree command, outside a "negative" test in the t1004 test
script.

To untangle this mess, I think the first step would be something
like this (this is against 'maint', as I was in the middle of
something else that is based on 'maint' when I started reading
this thread).

The next step would be to teach read-tree, merge-recursive and
clean (in C) to use setup_standard_excludes().

---

 builtin-add.c |   22 ++--------------------
 cache.h       |    1 +
 config.c      |    7 +++++++
 dir.c         |   12 ++++++++++++
 dir.h         |    1 +
 environment.c |    1 +
 wt-status.c   |   15 +--------------
 7 files changed, 25 insertions(+), 34 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 373f87f..850e1c2 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -17,7 +17,6 @@ static const char builtin_add_usage[] =
 "git-add [-n] [-v] [-f] [--interactive | -i] [-u] [--refresh] [--] <filepattern>...";
 
 static int take_worktree_changes;
-static const char *excludes_file;
 
 static void prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)
 {
@@ -57,12 +56,7 @@ static void fill_directory(struct dir_struct *dir, const char **pathspec,
 	memset(dir, 0, sizeof(*dir));
 	if (!ignored_too) {
 		dir->collect_ignored = 1;
-		dir->exclude_per_dir = ".gitignore";
-		path = git_path("info/exclude");
-		if (!access(path, R_OK))
-			add_excludes_from_file(dir, path);
-		if (excludes_file != NULL && !access(excludes_file, R_OK))
-			add_excludes_from_file(dir, excludes_file);
+		setup_standard_excludes(dir);
 	}
 
 	/*
@@ -144,18 +138,6 @@ static void refresh(int verbose, const char **pathspec)
         free(seen);
 }
 
-static int git_add_config(const char *var, const char *value)
-{
-	if (!strcmp(var, "core.excludesfile")) {
-		if (!value)
-			die("core.excludesfile without value");
-		excludes_file = xstrdup(value);
-		return 0;
-	}
-
-	return git_default_config(var, value);
-}
-
 static struct lock_file lock_file;
 
 static const char ignore_error[] =
@@ -183,7 +165,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		exit(1);
 	}
 
-	git_config(git_add_config);
+	git_config(git_default_config);
 
 	newfd = hold_locked_index(&lock_file, 1);
 
diff --git a/cache.h b/cache.h
index fc195bc..ecd809d 100644
--- a/cache.h
+++ b/cache.h
@@ -571,6 +571,7 @@ extern int pager_in_use;
 extern int pager_use_color;
 
 extern char *editor_program;
+extern char *excludes_file;
 
 /* base85 */
 int decode_85(char *dst, const char *line, int linelen);
diff --git a/config.c b/config.c
index dc3148d..56e99fc 100644
--- a/config.c
+++ b/config.c
@@ -431,6 +431,13 @@ int git_default_config(const char *var, const char *value)
 		return 0;
 	}
 
+	if (!strcmp(var, "core.excludesfile")) {
+		if (!value)
+			die("core.excludesfile without value");
+		excludes_file = xstrdup(value);
+		return 0;
+	}
+
 	/* Add other config variables here and to Documentation/config.txt. */
 	return 0;
 }
diff --git a/dir.c b/dir.c
index f843c4d..73a39ed 100644
--- a/dir.c
+++ b/dir.c
@@ -709,3 +709,15 @@ int is_inside_dir(const char *dir)
 	char buffer[PATH_MAX];
 	return get_relative_cwd(buffer, sizeof(buffer), dir) != NULL;
 }
+
+void setup_standard_excludes(struct dir_struct *dir)
+{
+	const char *path;
+
+	dir->exclude_per_dir = ".gitignore";
+	path = git_path("info/exclude");
+	if (!access(path, R_OK))
+		add_excludes_from_file(dir, path);
+	if (excludes_file && !access(excludes_file, R_OK))
+		add_excludes_from_file(dir, excludes_file);
+}
diff --git a/dir.h b/dir.h
index f55a87b..a5f4237 100644
--- a/dir.h
+++ b/dir.h
@@ -63,5 +63,6 @@ extern struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathna
 
 extern char *get_relative_cwd(char *buffer, int size, const char *dir);
 extern int is_inside_dir(const char *dir);
+extern void setup_standard_excludes(struct dir_struct *dir);
 
 #endif
diff --git a/environment.c b/environment.c
index b5a6c69..1dab72e 100644
--- a/environment.c
+++ b/environment.c
@@ -34,6 +34,7 @@ char *pager_program;
 int pager_in_use;
 int pager_use_color = 1;
 char *editor_program;
+char *excludes_file;
 int auto_crlf = 0;	/* 1: both ways, -1: only when adding git objects */
 
 /* This is set by setup_git_dir_gently() and/or git_default_config() */
diff --git a/wt-status.c b/wt-status.c
index 10ce6ee..58dd716 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -22,7 +22,6 @@ static const char use_add_rm_msg[] =
 "use \"git add/rm <file>...\" to update what will be committed";
 static const char use_add_to_include_msg[] =
 "use \"git add <file>...\" to include in what will be committed";
-static const char *excludes_file;
 
 static int parse_status_slot(const char *var, int offset)
 {
@@ -247,22 +246,16 @@ static void wt_status_print_changed(struct wt_status *s)
 static void wt_status_print_untracked(struct wt_status *s)
 {
 	struct dir_struct dir;
-	const char *x;
 	int i;
 	int shown_header = 0;
 
 	memset(&dir, 0, sizeof(dir));
 
-	dir.exclude_per_dir = ".gitignore";
 	if (!s->untracked) {
 		dir.show_other_directories = 1;
 		dir.hide_empty_directories = 1;
 	}
-	x = git_path("info/exclude");
-	if (file_exists(x))
-		add_excludes_from_file(&dir, x);
-	if (excludes_file && file_exists(excludes_file))
-		add_excludes_from_file(&dir, excludes_file);
+	setup_standard_excludes(&dir);
 
 	read_directory(&dir, ".", "", 0, NULL);
 	for(i = 0; i < dir.nr; i++) {
@@ -360,11 +353,5 @@ int git_status_config(const char *k, const char *v)
 		int slot = parse_status_slot(k, 13);
 		color_parse(v, k, wt_status_colors[slot]);
 	}
-	if (!strcmp(k, "core.excludesfile")) {
-		if (!v)
-			die("core.excludesfile without value");
-		excludes_file = xstrdup(v);
-		return 0;
-	}
 	return git_default_config(k, v);
 }

Re: git-clean won't read global ignore

[Andreas Ericsson]

Junio C Hamano wrote:
> (this is against 'maint', as I was in the middle of
> something else that is based on 'maint' when I started reading
> this thread).
> 

That's probably not a bad idea, as it really is a bug, and one
that can cause data-loss at that.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: git-clean won't read global ignore

[Junio C Hamano]

Andreas Ericsson <ae@op5.se> writes:

> Junio C Hamano wrote:
>> (this is against 'maint', as I was in the middle of
>> something else that is based on 'maint' when I started reading
>> this thread).
>>
>
> That's probably not a bad idea, as it really is a bug, and one
> that can cause data-loss at that.

I agree that "git clean" needs to be fixed in 'maint', but the
thing is, setup_standard_excludes() approach would not apply to
anything in 'maint', 'master', nor 'next', and it is more of the
longer term thing to go together with the git-clean in C.

The scripted version needs to be fixed independently in a way
with lessor impact, like the patch Shunichi just posted.

Re: git-clean won't read global ignore

[Johannes Schindelin]

Hi,

On Wed, 14 Nov 2007, Junio C Hamano wrote:

> To untangle this mess, I think the first step would be something like 
> this (this is against 'maint', as I was in the middle of something else 
> that is based on 'maint' when I started reading this thread).
> 
> The next step would be to teach read-tree, merge-recursive and clean (in 
> C) to use setup_standard_excludes().

I like it.

Ciao,
Dscho

Re: git-clean won't read global ignore

[Miles Bader]

Junio C Hamano <gitster@pobox.com> writes:
> I think the problem is core.excludesfile is too new to be
> noticed by anything other than git-add and git-status.

"git-add -i" _doesn't_ seems to notice it though...

-Miles

-- 
[|nurgle|]  ddt- demonic? so quake will have an evil kinda setting? one that
            will  make every christian in the world foamm at the mouth?
[iddt]      nurg, that's the goal

Re: git-clean won't read global ignore

[Miles Bader]

Miles Bader <miles.bader@necel.com> writes:
>> I think the problem is core.excludesfile is too new to be
>> noticed by anything other than git-add and git-status.
>
> "git-add -i" _doesn't_ seems to notice it though...

To clarify:  the "add untracked" sub-menu of "git-add -i"

-Miles
-- 
Do not taunt Happy Fun Ball.

[PATCH] Unify the use of standard set of exclude files

[Junio C Hamano]

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

> We probably could change git-ls-files to use the standard set
> when no excludes are specified from the command line, or
> something like that, but this will be a change in semantics that
> would affect the scripts in a subtle way.  I am somewhat
> reluctant to make such a change.

So here it is.

This teaches "git ls-files" to read the standard set of exclude
files when no exclude patterns nor files is given from the
command line.  We used to error out in such a case.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 builtin-ls-files.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index e0b856f..50dcb89 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -542,11 +542,8 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 		ps_matched = xcalloc(1, num);
 	}
 
-	if (dir.show_ignored && !exc_given) {
-		fprintf(stderr, "%s: --ignored needs some exclude pattern\n",
-			argv[0]);
-		exit(1);
-	}
+	if (dir.show_ignored && !exc_given)
+		setup_standard_excludes(&dir);
 
 	/* With no flags, we default to showing the cached files */
 	if (!(show_stage | show_deleted | show_others | show_unmerged |

Re: [PATCH] Unify the use of standard set of exclude files

[Jeff King]

On Wed, Nov 14, 2007 at 10:38:41PM -0800, Junio C Hamano wrote:

> This teaches "git ls-files" to read the standard set of exclude
> files when no exclude patterns nor files is given from the
> command line.  We used to error out in such a case.

Is that really the case, or is this _just_ when we have asked to include
ignored files in the output? Or maybe I am missing something fundamental
here.

git-add--interactive:list_untracked needs something like this, but I
don't think your patch will work. We need something more like this (also
on maint because your standard exclude patch is):

-- >8 --
git-ls-files: add --exclude-standard

This provides a way for scripts to get at the new standard exclude
function.

---
diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index 9e454f0..2ec0c0d 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 		[-x <pattern>|--exclude=<pattern>]
 		[-X <file>|--exclude-from=<file>]
 		[--exclude-per-directory=<file>]
+		[--exclude-standard]
 		[--error-unmatch] [--with-tree=<tree-ish>]
 		[--full-name] [--abbrev] [--] [<file>]\*
 
@@ -77,6 +78,10 @@ OPTIONS
 	read additional exclude patterns that apply only to the
 	directory and its subdirectories in <file>.
 
+--exclude-standard::
+	Add the standard git exclusions: .git/info/exclude, .gitignore
+	in each directory, and the user's global exclusion file.
+
 --error-unmatch::
 	If any <file> does not appear in the index, treat this as an
 	error (return 1).
diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index 171d449..da97278 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -401,8 +401,8 @@ static void overlay_tree(const char *tree_name, const char *prefix)
 static const char ls_files_usage[] =
 	"git-ls-files [-z] [-t] [-v] (--[cached|deleted|others|stage|unmerged|killed|modified])* "
 	"[ --ignored ] [--exclude=<pattern>] [--exclude-from=<file>] "
-	"[ --exclude-per-directory=<filename> ] [--full-name] [--abbrev] "
-	"[--] [<file>]*";
+	"[ --exclude-per-directory=<filename> ] [--exclude-standard] "
+	"[--full-name] [--abbrev] [--] [<file>]*";
 
 int cmd_ls_files(int argc, const char **argv, const char *prefix)
 {
@@ -510,6 +510,11 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 			dir.exclude_per_dir = arg + 24;
 			continue;
 		}
+		if (!strcmp(arg, "--exclude-standard")) {
+			exc_given = 1;
+			setup_standard_excludes(&dir);
+			continue;
+		}
 		if (!strcmp(arg, "--full-name")) {
 			prefix_offset = 0;
 			continue;
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index ac598f8..0317ad9 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -37,10 +37,7 @@ sub list_untracked {
 		chomp $_;
 		$_;
 	}
-	run_cmd_pipe(qw(git ls-files --others
-			--exclude-per-directory=.gitignore),
-		     "--exclude-from=$GIT_DIR/info/exclude",
-		     '--', @_);
+	run_cmd_pipe(qw(git ls-files --others --exclude-standard --), @_);
 }
 
 my $status_fmt = '%12s %12s %s';

Re: [PATCH] Unify the use of standard set of exclude files

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> git-add--interactive:list_untracked needs something like this, but I
> don't think your patch will work. We need something more like this (also
> on maint because your standard exclude patch is):
>
> -- >8 --
> git-ls-files: add --exclude-standard
>
> This provides a way for scripts to get at the new standard exclude
> function.

I like this, along with the patch to add--interactive.

Will forge your signature ;-).

Re: [PATCH] Unify the use of standard set of exclude files

[Jeff King]

On Thu, Nov 15, 2007 at 01:03:05AM -0800, Junio C Hamano wrote:

> Will forge your signature ;-).

Oops, thank you. :)

-Peff

Re: [PATCH] Unify the use of standard set of exclude files

[Junio C Hamano]

By the way, I think the way in which excluded() is called inside
ls-files for --cached, --staged, --deleted and --modified is
totally bogus, and as a result, ls-files does not honor
per-directory exclude files properly.

This is because dir.c:excluded() needs to be called after
setting up the exclude_list stack properly, just like how
dir.c:read_directory() and unpack-trees.c:unpack_trees_rec()
do.  The directory traversal should look like this:

 - Call push_exclude_per_directory() upon entering a directory.
   This reads the per directory exclude file (.gitignore) from
   the directory, and push it into the existing stack.  This
   way, the patterns from the file is set up to override the
   existing patterns from the .gitignore files of higher level
   directories.

 - Ask excluded() if your paths in that directory matches, and
   do whatever you want to happen.

 - Call pop_exclude_per_directory() when leaving the directory,
   to free the patterns read from the .gitignore file there.

Because the codepaths in question just iterate over the cache
entries without telling the per-directory exclude file stack
which set of .gitignore files should apply to the inquiry using
push/pop mechanism, I _think_ excluded(dir, "a/b") calls in the
codepaths do not honor .gitignore nor a/.gitignore file when
checking if "a/b" is to be ignored.

The push/pop mechanism was designed to be used in read_directory()
and it was a good match to the code structure to make recursive
calls to read_directory_recursive().  Because the paths are
sorted in the pathname order, we can make appropriate calls to
push/pop while iterating over the cache in these codepaths to
fix this issue.

As a longer term fix, I think it makes more sense to make
excluded() automatically push/pop the per directory exclude file
stack, just like the gitattributes mechanism maintains the
attribute stack to match and cache the last checked path.

Re: git-clean won't read global ignore

[Matthieu Moy]

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

> We probably could change git-ls-files to use the standard set
> when no excludes are specified from the command line, or
> something like that, but this will be a change in semantics that
> would affect the scripts in a subtle way.  I am somewhat
> reluctant to make such a change.

+1 for your introduction of setup_standard_excludes().

And woh, I was writting a mail to say that adding a --exclude-standard
would be cool, but it has been implemented even before I said it would
be cool.

Nothing to add, except "Thanks Junio, Thanks Jeff!" :-).

At last, I'll be able to write

$ git ls-files -o --exclude-standard >> .gitignore
$ $EDITOR .gitignore

-- 
Matthieu

Re: git-clean won't read global ignore

[Junio C Hamano]

Matthieu Moy <Matthieu.Moy@imag.fr> writes:

> At last, I'll be able to write
>
> $ git ls-files -o --exclude-standard >> .gitignore
> $ $EDITOR .gitignore

I think this is a good cookbook material to put somewhere in the
docs.

Re: git-clean won't read global ignore

[Matthieu Moy]

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

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> At last, I'll be able to write
>>
>> $ git ls-files -o --exclude-standard >> .gitignore
>> $ $EDITOR .gitignore
>
> I think this is a good cookbook material to put somewhere in the
> docs.

Here it is:

http://git.or.cz/gitwiki/GitTips?action=diff&rev2=217&rev1=216

== How to ignore files which are "Untracked" now? ==

{{{
$ git ls-files -o --exclude-standard >> .gitignore
$ $EDITOR .gitignore
}}}

(note : `--exclude-standard` is not yet in a released version of git
as of november 2007, you'll have to use `--exclude-from=.gitignore
--exclude-from=.git/info/exclude ...` if you don't have it).

-- 
Matthieu