threads / discuss / 11710

Why does git track directory listed in .gitignore/".git/info/exclude"?

Subject: Why does git track directory listed in .gitignore/".git/info/exclude"?

## tl;dr

20 messages between Jan 23, 2008 and Feb 1, 2008.

replies: 19people: 8as markdown or json

pradeep singh rautela· Jan 23, 2008, 13:54 UTC · lore
Hi All,

I have an source directory, which in turn has some other directories too. I do not want to track one of these directories. So i added the directory name in .gitignore as well as in .git/info/exclude for my repo.

i.e i have added following line to both of them - xen-3.1.0-src/

I copied xen-3.1.0-src from archives in the git repo's base directory.

Now when i do a git-status i get # On branch master # Untracked files: # (use "git add <file>..." to include in what will be committed) # # xen-3.1.0-src/ nothing added to commit but untracked files present (use "git add" to track)

Why is git seeing xen-3.1.0-src directory at all? Is this the expected behaviour?

I thought i should not get this message after adding relevant entries in .gitignore or in info/exclude .

What am i doing wrong here? Is there a way that this can be done without having to witness this message everytime i do a git status?

Please CC me as I am not subscribed to the list.
Thanks,
         ~Pradeep
-- 
--
pradeep singh rautela
http://eagain.wordpress.com
http://emptydomain.googlepages.com
pradeep singh rautela· Jan 23, 2008, 14:04 UTC · re: pradeep singh rautela · lore

Re: Why does git track directory listed in .gitignore/".git/info/exclude"?

Apologies to all.Kindly pardon my novice experiments with git. Some more trial and error method led to find that you have to put a * at the end of the directory too. i.e xen-3.1.0-src/*

But i still would like to ask git gurus here. Isn't it fine to include a directory name as

   $directory_name/
    instead of
   $directory_name/*

? Thoughts?

Thanks,
          --Pradeep
On 23/01/2008, pradeep singh rautela <rautelap@gmail.com> wrote:
Show 40 quoted lines
> Hi All,
>
> I have an source directory, which in turn has some other directories too.
> I do not want to track one of these directories.
> So i added the directory name in .gitignore as well as in
> .git/info/exclude for my repo.
>
> i.e i have added following line to both of them -
> xen-3.1.0-src/
>
> I copied xen-3.1.0-src from archives in the git repo's base directory.
>
> Now when i do a git-status i get
> # On branch master
> # Untracked files:
> #   (use "git add <file>..." to include in what will be committed)
> #
> #       xen-3.1.0-src/
> nothing added to commit but untracked files present (use "git add" to track)
>
> Why is git seeing xen-3.1.0-src directory at all?
> Is this the expected behaviour?
>
> I thought i should not get this message after adding relevant entries
> in .gitignore or in info/exclude .
>
> What am i doing wrong here?
> Is there a way that this can be done without having to witness this
> message everytime i do a git status?
>
> Please CC me as I am not subscribed to the list.
>
> Thanks,
>          ~Pradeep
> --
> --
> pradeep singh rautela
> http://eagain.wordpress.com
> http://emptydomain.googlepages.com
>
-- 
--
pradeep singh rautela
http://eagain.wordpress.com
http://emptydomain.googlepages.com
Linus Torvalds· Jan 23, 2008, 21:17 UTC · re: pradeep singh rautela · lore

Re: Why does git track directory listed in .gitignore/".git/info/exclude"?

On Wed, 23 Jan 2008, pradeep singh rautela wrote:
Show 13 quoted lines
>
> Apologies to all.Kindly pardon my novice experiments with git.
> Some more trial and error method led to find that you have to put a *
> at the end of the directory too.
>
> i.e xen-3.1.0-src/*
> 
> But i still would like to ask git gurus here.
> Isn't it fine to include a directory name as
> 
>    $directory_name/
>     instead of
>    $directory_name/*
Heh.

I think your problem is that "/" itself. By adding it, the exclude information does *not* match the directory entry itself (because the directory entry itself is called just "xen-3.1.0-src" - note no slash!), and since you added it, it also doesn't match any names _under_ that directory exactly.

So what you *should* have done is to just tell git to ignore the directory named "xen-3.1.0-src", and you'd have been ok.

Using "xen-3.1.0-src/*" works too, but it is heavy-handed and unnecessary.
		Linus
pradeep singh rautela· Jan 24, 2008, 10:44 UTC · re: Linus Torvalds · lore

Re: Why does git track directory listed in .gitignore/".git/info/exclude"?

Hi Linus,

On 24/01/2008, Linus Torvalds <torvalds@linux-foundation.org> wrote: [...]

Show 13 quoted lines
> > Isn't it fine to include a directory name as
> >
> >    $directory_name/
> >     instead of
> >    $directory_name/*
>
> Heh.
>
> I think your problem is that "/" itself. By adding it, the exclude
> information does *not* match the directory entry itself (because the
> directory entry itself is called just "xen-3.1.0-src" - note no slash!),
> and since you added it, it also doesn't match any names _under_ that
> directory exactly.

Got that. Thanks a lot for explaining that to me Linus.

Best Regards,
                --Pradeep
Show 8 quoted lines
>
> So what you *should* have done is to just tell git to ignore the directory
> named "xen-3.1.0-src", and you'd have been ok.
>
> Using "xen-3.1.0-src/*" works too, but it is heavy-handed and unnecessary.
>
>                 Linus
>
-- 
--
pradeep singh rautela
http://eagain.wordpress.com
http://emptydomain.googlepages.com
Adam Piatyszek· Jan 30, 2008, 12:35 UTC · re: Linus Torvalds · lore

Re: Why does git track directory listed in .gitignore/".git/info/exclude"?

* Linus Torvalds [23 I 2008 22:17]:
Show 20 quoted lines
> On Wed, 23 Jan 2008, pradeep singh rautela wrote:
>> But i still would like to ask git gurus here.
>> Isn't it fine to include a directory name as
>>
>>    $directory_name/
>>     instead of
>>    $directory_name/*
> 
> Heh.
> 
> I think your problem is that "/" itself. By adding it, the exclude 
> information does *not* match the directory entry itself (because the 
> directory entry itself is called just "xen-3.1.0-src" - note no slash!), 
> and since you added it, it also doesn't match any names _under_ that 
> directory exactly.
> 
> So what you *should* have done is to just tell git to ignore the directory 
> named "xen-3.1.0-src", and you'd have been ok.
> 
> Using "xen-3.1.0-src/*" works too, but it is heavy-handed and unnecessary.
Hi Linus, Pradeep and All,

In my opinion, the exclude matching routine should convert "dir/" to "dir", especially that the "git status" command lists untracked directories with the trailing slash "/", e.g:

   ediap@lespaul ~/git/acm_ofdm $ git status
   # On branch master
   # Untracked files:
   #   (use "git add <file>..." to include in what will be committed)
   #
   #       ldpc13.bm
   #       results/

So, most newbies will try to add "dir/" to .gitignore or .git/info/exclude instead of "dir" in such a case.

Can you seen any drawbacks of such modification?

BR, /Adam

-- 
.:.  Adam Piatyszek (ediap)  .:.....................................:.
.:.  ediap@users.sourceforge.net  .:................................:.
Junio C Hamano· Jan 30, 2008, 20:39 UTC · re: Adam Piatyszek · lore

Re: Why does git track directory listed in .gitignore/".git/info/exclude"?

Adam Piatyszek <ediap@users.sourceforge.net> writes:
Show 16 quoted lines
> In my opinion, the exclude matching routine should convert "dir/" to
> "dir", especially that the "git status" command lists untracked
> directories with the trailing slash "/", e.g:
>
>   ediap@lespaul ~/git/acm_ofdm $ git status
>   # On branch master
>   # Untracked files:
>   #   (use "git add <file>..." to include in what will be committed)
>   #
>   #       ldpc13.bm
>   #       results/
>
> So, most newbies will try to add "dir/" to .gitignore or
> .git/info/exclude instead of "dir" in such a case.
>
> Can you seen any drawbacks of such modification?
I do not see a problem if you are saying:
	when the user has an entry 'dir/' in .gitignore, it
	should match directory 'dir'.

However, there is a subtle problem in a naive implementation of that. IOW,

	when the user has an entry 'dir/' in .gitignore, behave
	as if the entry were 'dir' instead.
is wrong.

When you say "foo", you mean "I want either 'foo' that is a non-directory, or everything under 'foo' if that is a directory". When you say "foo/", you are saying "I do not want 'foo' if it is a non-directory. I want everything under 'foo' if and only if that is a directory". Compare:

	git ls-files -s Makefile/
        git ls-files -s Makefile

The first one is silent, and the latter answers. On the other hand, for a directory, both of these give you the same:

	git ls-files Documentation/
        git ls-files Documentation
Junio C Hamano· Jan 30, 2008, 21:06 UTC · re: Junio C Hamano · lore

Re: Why does git track directory listed in .gitignore/".git/info/exclude"?

Junio C Hamano <gitster@pobox.com> writes:
Show 31 quoted lines
> Adam Piatyszek <ediap@users.sourceforge.net> writes:
>...
>> Can you seen any drawbacks of such modification?
>
> I do not see a problem if you are saying:
>
> 	when the user has an entry 'dir/' in .gitignore, it
> 	should match directory 'dir'.
>
> However, there is a subtle problem in a naive implementation of
> that.  IOW,
>
> 	when the user has an entry 'dir/' in .gitignore, behave
> 	as if the entry were 'dir' instead.
>
> is wrong.
>
> When you say "foo", you mean "I want either 'foo' that is a
> non-directory, or everything under 'foo' if that is a
> directory".  When you say "foo/", you are saying "I do not want
> 'foo' if it is a non-directory.  I want everything under 'foo'
> if and only if that is a directory".  Compare:
>
> 	git ls-files -s Makefile/
>       git ls-files -s Makefile
>
> The first one is silent, and the latter answers.  On the other
> hand, for a directory, both of these give you the same:
>
> 	git ls-files Documentation/
>       git ls-files Documentation

Perhaps "wrong" might have been too strong a word, and I should have said "inconsistent with other parts of the system."

It could be that people may find pathspec "Makefile/" meant exactly the same thing as "Makefile" in ls-files and other commands. If that is the case, then we could uniformly strip the trailing slash, both in all of these commands _and_ .gitignore entries.

In any case, their behaviour should be consistent.
Adam Piatyszek· Jan 31, 2008, 07:05 UTC · re: Junio C Hamano · lore

Re: Why does git track directory listed in .gitignore/".git/info/exclude"?

Hi Junio,
* Junio C Hamano [30 I 2008 21:39]:
Show 15 quoted lines
> When you say "foo", you mean "I want either 'foo' that is a
> non-directory, or everything under 'foo' if that is a
> directory".  When you say "foo/", you are saying "I do not want
> 'foo' if it is a non-directory.  I want everything under 'foo'
> if and only if that is a directory".  Compare:
> 
> 	git ls-files -s Makefile/
>         git ls-files -s Makefile
> 
> The first one is silent, and the latter answers.  On the other
> hand, for a directory, both of these give you the same:
> 
> 	git ls-files Documentation/
>         git ls-files Documentation
> 

As you said above both "Documentation/" and "Documentation" match the existing tracked directory named "Documentation". That is how ls-files works and it is the only sane way. The problem is that I expect that directory entries ending with "/" in .gitignore and .git/info/exclude files are treated in a similar way, i.e. they are being _ignored_ with all the stuff in them, in the same way as directory entries without the ending slash. Unfortunately this is not the case. See this example:

ediap@lespaul ~/tmp $ mkdir repo && cd repo
ediap@lespaul ~/tmp/repo $ git init
Initialized empty Git repository in .git/
ediap@lespaul ~/tmp/repo $ touch a.txt
ediap@lespaul ~/tmp/repo $ git add a.txt
ediap@lespaul ~/tmp/repo $ git commit -m "a file"
Created initial commit 1712595: a file
  0 files changed, 0 insertions(+), 0 deletions(-)
  create mode 100644 a.txt
ediap@lespaul ~/tmp/repo $ mkdir d
ediap@lespaul ~/tmp/repo $ touch d/b.txt
ediap@lespaul ~/tmp/repo $ git status
# On branch master
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#       d/
nothing added to commit but untracked files present (use "git add" to track)
ediap@lespaul ~/tmp/repo $ echo "d/" > .gitignore
ediap@lespaul ~/tmp/repo $ git add .gitignore
ediap@lespaul ~/tmp/repo $ git commit -m "ignore"
Created commit 29ebf4d: ignore
  1 files changed, 1 insertions(+), 0 deletions(-)
  create mode 100644 .gitignore
ediap@lespaul ~/tmp/repo $ git status
# On branch master
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#       d/
nothing added to commit but untracked files present (use "git add" to track)
But:
ediap@lespaul ~/tmp/repo $ echo "d" > .gitignore
ediap@lespaul ~/tmp/repo $ git add .gitignore
ediap@lespaul ~/tmp/repo $ git commit --amend -m "ignore"
Created commit 43198d4: ignore
  1 files changed, 1 insertions(+), 0 deletions(-)
  create mode 100644 .gitignore
ediap@lespaul ~/tmp/repo $ git status
# On branch master
nothing to commit (working directory clean)

I hope you now understand what I was trying to express in my previous email. :-)

BR, /Adam

-- 
.:.  Adam Piatyszek (ediap)  .:.....................................:.
.:.  ediap@users.sourceforge.net  .:................................:.
Junio C Hamano· Jan 31, 2008, 08:54 UTC · re: Adam Piatyszek · lore

*Re: Why does git track directory listed in .gitignore/".git/info/exclude"?

Adam Piatyszek <ediap@users.sourceforge.net> writes:
> I hope you now understand what I was trying to express in my previous
> email. :-)
I am afraid that it is _you_ who doesn't understand.

We both know that a pattern "foo" in the ignore list matches directory "foo/" and everything under it. We also both know that such a pattern also matches a regular file or a symbolic link "foo", too.

I already said that it would make sense to make a pattern in the ignore list "foo/" to match directory "foo/" and everything under it, in addition to the above. Currently it doesn't. We agreed that it might be a good idea to allow it as well.

My point was that a naive implementation to make "foo/" match directory "foo/" by pretending as if the user said "foo" is not good, because it would also make such a user-supplied pattern "foo/" to match a regular file "foo".

In other words, you would need to do something like the attached patch, if you wanted to solve this correctly.

Note that this is not tested heavily. It just looks obvious enough, but (1) testing is for wimps, (2) I do not care deeply about having to say "foo" instead of "foo/", and (3) we are in pre-release freeze to fix obvious bugs and regressions.

Wimps ^W People who care deeply enough can apply this to their trees, use it for a week or so to make sure it does not break other things, and then send the patch back with Tested-by: if everything works out Ok ;-).

I also strongly suspect that we would need a similar change to gitattributes side (attr.c) for consistency, but I haven't looked at it.

-- >8 -- [PATCH] Allow "foo/" in ignore list to match directory "foo"

A naive implementation that pretends the user said "foo" would not work well as it would make "foo/" match a non directory "foo" as well. This passes down the type of path being checked to excluded() function to make sure that "foo/" matches directory "foo/" and not regular file "foo".

A downside is that recursive directory walk may need to run lstat(2) more often on systems whose "struct dirent" does not give the type of the entry; earlier it did not have to for an excluded path, but we now need to figure out if a path is a directory before deciding to exclude it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-ls-files.c                 |   19 ++++++++++++++-
 cache.h                            |   12 ++++++++++
 dir.c                              |   42 ++++++++++++++++++++++++++---------
 dir.h                              |    3 +-
 t/t3001-ls-files-others-exclude.sh |   41 +++++++++++++++++++++++++++++++++++
 unpack-trees.c                     |    2 +-
 6 files changed, 104 insertions(+), 15 deletions(-)
diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index 3801cf4..24646ef 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -218,6 +218,19 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce)
 	write_name_quoted(ce->name + offset, stdout, line_terminator);
 }
 
+static int ce_mode_to_dtype(struct cache_entry *ce)
+{
+	unsigned ce_mode = ce->ce_mode;
+	if (S_ISREG(ce_mode))
+		return DT_REG;
+	else if (S_ISDIR(ce_mode) || S_ISGITLINK(ce_mode))
+		return DT_DIR;
+	else if (S_ISLNK(ce_mode))
+		return DT_LNK;
+	else
+		return DT_UNKNOWN;
+}
+
 static void show_files(struct dir_struct *dir, const char *prefix)
 {
 	int i;
@@ -238,7 +251,8 @@ static void show_files(struct dir_struct *dir, const char *prefix)
 	if (show_cached | show_stage) {
 		for (i = 0; i < active_nr; i++) {
 			struct cache_entry *ce = active_cache[i];
-			if (excluded(dir, ce->name) != dir->show_ignored)
+			if (excluded(dir, ce->name, ce_mode_to_dtype(ce)) !=
+			    dir->show_ignored)
 				continue;
 			if (show_unmerged && !ce_stage(ce))
 				continue;
@@ -252,7 +266,8 @@ static void show_files(struct dir_struct *dir, const char *prefix)
 			struct cache_entry *ce = active_cache[i];
 			struct stat st;
 			int err;
-			if (excluded(dir, ce->name) != dir->show_ignored)
+			if (excluded(dir, ce->name, ce_mode_to_dtype(ce)) !=
+			    dir->show_ignored)
 				continue;
 			err = lstat(ce->name, &st);
 			if (show_deleted && err)
diff --git a/cache.h b/cache.h
index 549f4bb..c282021 100644
--- a/cache.h
+++ b/cache.h
@@ -141,6 +141,18 @@ static inline unsigned int ce_mode_from_stat(struct cache_entry *ce, unsigned in
 	}
 	return create_ce_mode(mode);
 }
+static inline int ce_to_dtype(const struct cache_entry *ce)
+{
+	unsigned ce_mode = ce->ce_mode;
+	if (S_ISREG(ce_mode))
+		return DT_REG;
+	else if (S_ISDIR(ce_mode) || S_ISGITLINK(ce_mode))
+		return DT_DIR;
+	else if (S_ISLNK(ce_mode))
+		return DT_LNK;
+	else
+		return DT_UNKNOWN;
+}
 #define canon_mode(mode) \
 	(S_ISREG(mode) ? (S_IFREG | ce_permissions(mode)) : \
 	S_ISLNK(mode) ? S_IFLNK : S_ISDIR(mode) ? S_IFDIR : S_IFGITLINK)
diff --git a/dir.c b/dir.c
index 3e345c2..354c8f0 100644
--- a/dir.c
+++ b/dir.c
@@ -126,18 +126,34 @@ static int no_wildcard(const char *string)
 void add_exclude(const char *string, const char *base,
 		 int baselen, struct exclude_list *which)
 {
-	struct exclude *x = xmalloc(sizeof (*x));
+	struct exclude *x; 
+	size_t len;
+	int to_exclude = 1;
+	int flags = 0;
 
-	x->to_exclude = 1;
 	if (*string == '!') {
-		x->to_exclude = 0;
+		to_exclude = 0;
 		string++;
 	}
-	x->pattern = string;
+	len = strlen(string);
+	if (len && string[len - 1] == '/') {
+		char *s;
+		x = xmalloc(sizeof(*x) + len);
+		s = (char*)(x+1);
+		memcpy(s, string, len - 1);
+		s[len - 1] = '\0';
+		string = s;
+		x->pattern = s;
+		flags = EXC_FLAG_MUSTBEDIR;
+	} else {
+		x = xmalloc(sizeof(*x));
+		x->pattern = string;
+	}
+	x->to_exclude = to_exclude;
 	x->patternlen = strlen(string);
 	x->base = base;
 	x->baselen = baselen;
-	x->flags = 0;
+	x->flags = flags;
 	if (!strchr(string, '/'))
 		x->flags |= EXC_FLAG_NODIR;
 	if (no_wildcard(string))
@@ -261,7 +277,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
  * Return 1 for exclude, 0 for include and -1 for undecided.
  */
 static int excluded_1(const char *pathname,
-		      int pathlen, const char *basename,
+		      int pathlen, const char *basename, int dtype,
 		      struct exclude_list *el)
 {
 	int i;
@@ -272,6 +288,10 @@ static int excluded_1(const char *pathname,
 			const char *exclude = x->pattern;
 			int to_exclude = x->to_exclude;
 
+			if ((x->flags & EXC_FLAG_MUSTBEDIR) &&
+			    (dtype != DT_DIR))
+				continue;
+
 			if (x->flags & EXC_FLAG_NODIR) {
 				/* match basename */
 				if (x->flags & EXC_FLAG_NOWILDCARD) {
@@ -314,7 +334,7 @@ static int excluded_1(const char *pathname,
 	return -1; /* undecided */
 }
 
-int excluded(struct dir_struct *dir, const char *pathname)
+int excluded(struct dir_struct *dir, const char *pathname, int dtype)
 {
 	int pathlen = strlen(pathname);
 	int st;
@@ -323,7 +343,8 @@ int excluded(struct dir_struct *dir, const char *pathname)
 
 	prep_exclude(dir, pathname, basename-pathname);
 	for (st = EXC_CMDL; st <= EXC_FILE; st++) {
-		switch (excluded_1(pathname, pathlen, basename, &dir->exclude_list[st])) {
+		switch (excluded_1(pathname, pathlen, basename,
+				   dtype, &dir->exclude_list[st])) {
 		case 0:
 			return 0;
 		case 1:
@@ -560,7 +581,8 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 			if (simplify_away(fullname, baselen + len, simplify))
 				continue;
 
-			exclude = excluded(dir, fullname);
+			dtype = get_dtype(de, fullname);
+			exclude = excluded(dir, fullname, dtype);
 			if (exclude && dir->collect_ignored
 			    && in_pathspec(fullname, baselen + len, simplify))
 				dir_add_ignored(dir, fullname, baselen + len);
@@ -572,8 +594,6 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 			if (exclude && !dir->show_ignored)
 				continue;
 
-			dtype = get_dtype(de, fullname);
-
 			/*
 			 * Do we want to see just the ignored files?
 			 * We still need to recurse into directories,
diff --git a/dir.h b/dir.h
index d8814dc..10d72b5 100644
--- a/dir.h
+++ b/dir.h
@@ -9,6 +9,7 @@ struct dir_entry {
 #define EXC_FLAG_NODIR 1
 #define EXC_FLAG_NOWILDCARD 2
 #define EXC_FLAG_ENDSWITH 4
+#define EXC_FLAG_MUSTBEDIR 8
 
 struct exclude_list {
 	int nr;
@@ -67,7 +68,7 @@ extern int match_pathspec(const char **pathspec, const char *name, int namelen,
 
 extern int read_directory(struct dir_struct *, const char *path, const char *base, int baselen, const char **pathspec);
 
-extern int excluded(struct dir_struct *, const char *);
+extern int excluded(struct dir_struct *, const char *, int);
 extern void add_excludes_from_file(struct dir_struct *, const char *fname);
 extern void add_exclude(const char *string, const char *base,
 			int baselen, struct exclude_list *which);
diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
index e25b255..b4297ba 100755
--- a/t/t3001-ls-files-others-exclude.sh
+++ b/t/t3001-ls-files-others-exclude.sh
@@ -99,4 +99,45 @@ EOF
 test_expect_success 'git-status honours core.excludesfile' \
 	'diff -u expect output'
 
+test_expect_success 'trailing slash in exclude allows directory match(1)' '
+
+	git ls-files --others --exclude=one/ >output &&
+	if grep "^one/" output
+	then
+		echo Ooops
+		false
+	else
+		: happy
+	fi
+
+'
+
+test_expect_success 'trailing slash in exclude allows directory match (2)' '
+
+	git ls-files --others --exclude=one/two/ >output &&
+	if grep "^one/two/" output
+	then
+		echo Ooops
+		false
+	else
+		: happy
+	fi
+
+'
+
+test_expect_success 'trailing slash in exclude forces directory match (1)' '
+
+	>two
+	git ls-files --others --exclude=two/ >output &&
+	grep "^two" output
+
+'
+
+test_expect_success 'trailing slash in exclude forces directory match (2)' '
+
+	git ls-files --others --exclude=one/a.1/ >output &&
+	grep "^one/a.1" output
+
+'
+
 test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index aa2513e..11af263 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -523,7 +523,7 @@ static void verify_absent(struct cache_entry *ce, const char *action,
 	if (!lstat(ce->name, &st)) {
 		int cnt;
 
-		if (o->dir && excluded(o->dir, ce->name))
+		if (o->dir && excluded(o->dir, ce->name, ce_to_dtype(ce)))
 			/*
 			 * ce->name is explicitly excluded, so it is Ok to
 			 * overwrite it.
Junio C Hamano· Jan 31, 2008, 09:17 UTC · re: Junio C Hamano · lore

[PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo"

A pattern "foo/" in the exclude list did not match directory "foo", but a pattern "foo" did. This attempts to extend the exclude mechanism so that it would while not matching a regular file or a symbolic link "foo". In order to differentiate a directory and non directory, this passes down the type of path being checked to excluded() function.

A downside is that the recursive directory walk may need to run lstat(2) more often on systems whose "struct dirent" do not give the type of the entry; earlier it did not have to do so for an excluded path, but we now need to figure out if a path is a directory before deciding to exclude it. This is especially bad because an idea similar to the earlier CE_UPTODATE optimization to reduce number of lstat(2) calls would by definition not apply to the codepaths involved, as (1) directories will not be registered in the index, and (2) excluded paths will not be in the index anyway.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * This replaces the earlier patch, which depended on Linus's
   in-core index patch that makes ce->ce_mode the host endian.
   It also comes with a documentation update, and applies to
   master.
 Documentation/gitignore.txt        |    6 +++++
 builtin-ls-files.c                 |    6 +++-
 cache.h                            |   12 ++++++++++
 dir.c                              |   42 ++++++++++++++++++++++++++---------
 dir.h                              |    3 +-
 t/t3001-ls-files-others-exclude.sh |   41 +++++++++++++++++++++++++++++++++++
 unpack-trees.c                     |    2 +-
 7 files changed, 97 insertions(+), 15 deletions(-)
diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index 08373f5..0290bdb 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -57,6 +57,12 @@ Patterns have the following format:
    included again.  If a negated pattern matches, this will
    override lower precedence patterns sources.
 
+ - If the pattern ends with a slash, it is removed for the
+   purpose of the following description, but it would find match
+   only with a directory.  In other words, `foo/` will match a
+   directory `foo` and paths underneath it, but will not match a
+   regular file or a symbolic link `foo`.
+
  - If the pattern does not contain a slash '/', git treats it as
    a shell glob pattern and checks for a match against the
    pathname without leading directories.
diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index 3801cf4..3089978 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -238,7 +238,8 @@ static void show_files(struct dir_struct *dir, const char *prefix)
 	if (show_cached | show_stage) {
 		for (i = 0; i < active_nr; i++) {
 			struct cache_entry *ce = active_cache[i];
-			if (excluded(dir, ce->name) != dir->show_ignored)
+			if (excluded(dir, ce->name, ce_to_dtype(ce)) !=
+			    dir->show_ignored)
 				continue;
 			if (show_unmerged && !ce_stage(ce))
 				continue;
@@ -252,7 +253,8 @@ static void show_files(struct dir_struct *dir, const char *prefix)
 			struct cache_entry *ce = active_cache[i];
 			struct stat st;
 			int err;
-			if (excluded(dir, ce->name) != dir->show_ignored)
+			if (excluded(dir, ce->name, ce_to_dtype(ce)) !=
+			    dir->show_ignored)
 				continue;
 			err = lstat(ce->name, &st);
 			if (show_deleted && err)
diff --git a/cache.h b/cache.h
index 549f4bb..5529830 100644
--- a/cache.h
+++ b/cache.h
@@ -141,6 +141,18 @@ static inline unsigned int ce_mode_from_stat(struct cache_entry *ce, unsigned in
 	}
 	return create_ce_mode(mode);
 }
+static inline int ce_to_dtype(const struct cache_entry *ce)
+{
+	unsigned ce_mode = ntohl(ce->ce_mode);
+	if (S_ISREG(ce_mode))
+		return DT_REG;
+	else if (S_ISDIR(ce_mode) || S_ISGITLINK(ce_mode))
+		return DT_DIR;
+	else if (S_ISLNK(ce_mode))
+		return DT_LNK;
+	else
+		return DT_UNKNOWN;
+}
 #define canon_mode(mode) \
 	(S_ISREG(mode) ? (S_IFREG | ce_permissions(mode)) : \
 	S_ISLNK(mode) ? S_IFLNK : S_ISDIR(mode) ? S_IFDIR : S_IFGITLINK)
diff --git a/dir.c b/dir.c
index 3e345c2..a4f8c25 100644
--- a/dir.c
+++ b/dir.c
@@ -126,18 +126,34 @@ static int no_wildcard(const char *string)
 void add_exclude(const char *string, const char *base,
 		 int baselen, struct exclude_list *which)
 {
-	struct exclude *x = xmalloc(sizeof (*x));
+	struct exclude *x;
+	size_t len;
+	int to_exclude = 1;
+	int flags = 0;
 
-	x->to_exclude = 1;
 	if (*string == '!') {
-		x->to_exclude = 0;
+		to_exclude = 0;
 		string++;
 	}
-	x->pattern = string;
+	len = strlen(string);
+	if (len && string[len - 1] == '/') {
+		char *s;
+		x = xmalloc(sizeof(*x) + len);
+		s = (char*)(x+1);
+		memcpy(s, string, len - 1);
+		s[len - 1] = '\0';
+		string = s;
+		x->pattern = s;
+		flags = EXC_FLAG_MUSTBEDIR;
+	} else {
+		x = xmalloc(sizeof(*x));
+		x->pattern = string;
+	}
+	x->to_exclude = to_exclude;
 	x->patternlen = strlen(string);
 	x->base = base;
 	x->baselen = baselen;
-	x->flags = 0;
+	x->flags = flags;
 	if (!strchr(string, '/'))
 		x->flags |= EXC_FLAG_NODIR;
 	if (no_wildcard(string))
@@ -261,7 +277,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)
  * Return 1 for exclude, 0 for include and -1 for undecided.
  */
 static int excluded_1(const char *pathname,
-		      int pathlen, const char *basename,
+		      int pathlen, const char *basename, int dtype,
 		      struct exclude_list *el)
 {
 	int i;
@@ -272,6 +288,10 @@ static int excluded_1(const char *pathname,
 			const char *exclude = x->pattern;
 			int to_exclude = x->to_exclude;
 
+			if ((x->flags & EXC_FLAG_MUSTBEDIR) &&
+			    (dtype != DT_DIR))
+				continue;
+
 			if (x->flags & EXC_FLAG_NODIR) {
 				/* match basename */
 				if (x->flags & EXC_FLAG_NOWILDCARD) {
@@ -314,7 +334,7 @@ static int excluded_1(const char *pathname,
 	return -1; /* undecided */
 }
 
-int excluded(struct dir_struct *dir, const char *pathname)
+int excluded(struct dir_struct *dir, const char *pathname, int dtype)
 {
 	int pathlen = strlen(pathname);
 	int st;
@@ -323,7 +343,8 @@ int excluded(struct dir_struct *dir, const char *pathname)
 
 	prep_exclude(dir, pathname, basename-pathname);
 	for (st = EXC_CMDL; st <= EXC_FILE; st++) {
-		switch (excluded_1(pathname, pathlen, basename, &dir->exclude_list[st])) {
+		switch (excluded_1(pathname, pathlen, basename,
+				   dtype, &dir->exclude_list[st])) {
 		case 0:
 			return 0;
 		case 1:
@@ -560,7 +581,8 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 			if (simplify_away(fullname, baselen + len, simplify))
 				continue;
 
-			exclude = excluded(dir, fullname);
+			dtype = get_dtype(de, fullname);
+			exclude = excluded(dir, fullname, dtype);
 			if (exclude && dir->collect_ignored
 			    && in_pathspec(fullname, baselen + len, simplify))
 				dir_add_ignored(dir, fullname, baselen + len);
@@ -572,8 +594,6 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 			if (exclude && !dir->show_ignored)
 				continue;
 
-			dtype = get_dtype(de, fullname);
-
 			/*
 			 * Do we want to see just the ignored files?
 			 * We still need to recurse into directories,
diff --git a/dir.h b/dir.h
index d8814dc..10d72b5 100644
--- a/dir.h
+++ b/dir.h
@@ -9,6 +9,7 @@ struct dir_entry {
 #define EXC_FLAG_NODIR 1
 #define EXC_FLAG_NOWILDCARD 2
 #define EXC_FLAG_ENDSWITH 4
+#define EXC_FLAG_MUSTBEDIR 8
 
 struct exclude_list {
 	int nr;
@@ -67,7 +68,7 @@ extern int match_pathspec(const char **pathspec, const char *name, int namelen,
 
 extern int read_directory(struct dir_struct *, const char *path, const char *base, int baselen, const char **pathspec);
 
-extern int excluded(struct dir_struct *, const char *);
+extern int excluded(struct dir_struct *, const char *, int);
 extern void add_excludes_from_file(struct dir_struct *, const char *fname);
 extern void add_exclude(const char *string, const char *base,
 			int baselen, struct exclude_list *which);
diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
index e25b255..b4297ba 100755
--- a/t/t3001-ls-files-others-exclude.sh
+++ b/t/t3001-ls-files-others-exclude.sh
@@ -99,4 +99,45 @@ EOF
 test_expect_success 'git-status honours core.excludesfile' \
 	'diff -u expect output'
 
+test_expect_success 'trailing slash in exclude allows directory match(1)' '
+
+	git ls-files --others --exclude=one/ >output &&
+	if grep "^one/" output
+	then
+		echo Ooops
+		false
+	else
+		: happy
+	fi
+
+'
+
+test_expect_success 'trailing slash in exclude allows directory match (2)' '
+
+	git ls-files --others --exclude=one/two/ >output &&
+	if grep "^one/two/" output
+	then
+		echo Ooops
+		false
+	else
+		: happy
+	fi
+
+'
+
+test_expect_success 'trailing slash in exclude forces directory match (1)' '
+
+	>two
+	git ls-files --others --exclude=two/ >output &&
+	grep "^two" output
+
+'
+
+test_expect_success 'trailing slash in exclude forces directory match (2)' '
+
+	git ls-files --others --exclude=one/a.1/ >output &&
+	grep "^one/a.1" output
+
+'
+
 test_done
diff --git a/unpack-trees.c b/unpack-trees.c
index aa2513e..11af263 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -523,7 +523,7 @@ static void verify_absent(struct cache_entry *ce, const char *action,
 	if (!lstat(ce->name, &st)) {
 		int cnt;
 
-		if (o->dir && excluded(o->dir, ce->name))
+		if (o->dir && excluded(o->dir, ce->name, ce_to_dtype(ce)))
 			/*
 			 * ce->name is explicitly excluded, so it is Ok to
 			 * overwrite it.
-- 
1.5.4.rc5.16.gc0279
Jeff King· Jan 31, 2008, 09:41 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo"

On Thu, Jan 31, 2008 at 01:17:48AM -0800, Junio C Hamano wrote:
Show 5 quoted lines
> A downside is that the recursive directory walk may need to run
> lstat(2) more often on systems whose "struct dirent" do not give
> the type of the entry; earlier it did not have to do so for an
> excluded path, but we now need to figure out if a path is a
> directory before deciding to exclude it.  This is especially bad

You can at least lazily do the stat so that only users of foo/ need to pay the penalty. Something like this (completely untested):

diff --git a/dir.c b/dir.c
index a4f8c25..9487908 100644
--- a/dir.c
+++ b/dir.c
@@ -17,6 +17,7 @@ struct path_simplify {
 static int read_directory_recursive(struct dir_struct *dir,
 	const char *path, const char *base, int baselen,
 	int check_only, const struct path_simplify *simplify);
+static int get_dtype(struct dirent *de, const char *path, int try_stat);
 
 int common_prefix(const char **pathspec)
 {
@@ -288,9 +289,12 @@ static int excluded_1(const char *pathname,
 			const char *exclude = x->pattern;
 			int to_exclude = x->to_exclude;
 
-			if ((x->flags & EXC_FLAG_MUSTBEDIR) &&
-			    (dtype != DT_DIR))
-				continue;
+			if (x->flags & EXC_FLAG_MUSTBEDIR) {
+				if (dtype == DT_UNKNOWN)
+					dtype = get_dtype(NULL, pathname, 1);
+				if (dtype != DT_DIR)
+					continue;
+			}
 
 			if (x->flags & EXC_FLAG_NODIR) {
 				/* match basename */
@@ -527,13 +531,15 @@ static int in_pathspec(const char *path, int len, const struct path_simplify *si
 	return 0;
 }
 
-static int get_dtype(struct dirent *de, const char *path)
+static int get_dtype(struct dirent *de, const char *path, int try_stat)
 {
-	int dtype = DTYPE(de);
+	int dtype = de ? DTYPE(de) : DT_UNKNOWN;
 	struct stat st;
 
 	if (dtype != DT_UNKNOWN)
 		return dtype;
+	if (!try_stat)
+		return DT_UNKNOWN;
 	if (lstat(path, &st))
 		return dtype;
 	if (S_ISREG(st.st_mode))
@@ -581,7 +587,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
 			if (simplify_away(fullname, baselen + len, simplify))
 				continue;
 
-			dtype = get_dtype(de, fullname);
+			dtype = get_dtype(de, fullname, 0);
 			exclude = excluded(dir, fullname, dtype);
 			if (exclude && dir->collect_ignored
 			    && in_pathspec(fullname, baselen + len, simplify))
Junio C Hamano· Jan 31, 2008, 10:35 UTC · re: Jeff King · lore

Re: [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo"

Jeff King <peff@peff.net> writes:
> You can at least lazily do the stat so that only users of foo/ need to
> pay the penalty. Something like this (completely untested):

Without "foo/", you do not have to pay the price, so I think that is a sane optimization, but at the same time it would make it worse if "foo/" is actually used. excluded_1() is called for the same pathname from a loop to check for a match and you would end up running lstat(2) three times (once each for EXC_CMDL, EXC_DIRS and EXC_FILE).

But maybe people who want "foo/" deserve it.  I dunno.
In any case, if you do this...
Show 9 quoted lines
> @@ -581,7 +587,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
>  			if (simplify_away(fullname, baselen + len, simplify))
>  				continue;
>  
> -			dtype = get_dtype(de, fullname);
> +			dtype = get_dtype(de, fullname, 0);
>  			exclude = excluded(dir, fullname, dtype);
>  			if (exclude && dir->collect_ignored
>  			    && in_pathspec(fullname, baselen + len, simplify))

... I think you would need to get the real dtype again in later part of this function after exclude() decides it should not ignore it, before the "switch (dtype)" really uses it, on systems with NO_D_TYPE_IN_DIRENT.

Jeff King· Jan 31, 2008, 10:42 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo"

On Thu, Jan 31, 2008 at 02:35:33AM -0800, Junio C Hamano wrote:
Show 8 quoted lines
> Without "foo/", you do not have to pay the price, so I think
> that is a sane optimization, but at the same time it would make
> it worse if "foo/" is actually used.  excluded_1() is called for
> the same pathname from a loop to check for a match and you would
> end up running lstat(2) three times (once each for EXC_CMDL,
> EXC_DIRS and EXC_FILE).
> 
> But maybe people who want "foo/" deserve it.  I dunno.
Ah, I didn't look at it that closely.

To do the laziness right, I think you would need to pass a pointer to the dtype around, and just fill it in the first time it is needed.

-Peff
Johannes Schindelin· Jan 31, 2008, 11:38 UTC · re: Jeff King · lore

Re: [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo"

Hi,
On Thu, 31 Jan 2008, Jeff King wrote:
Show 14 quoted lines
> On Thu, Jan 31, 2008 at 02:35:33AM -0800, Junio C Hamano wrote:
> 
> > Without "foo/", you do not have to pay the price, so I think that is a 
> > sane optimization, but at the same time it would make it worse if 
> > "foo/" is actually used.  excluded_1() is called for the same pathname 
> > from a loop to check for a match and you would end up running lstat(2) 
> > three times (once each for EXC_CMDL, EXC_DIRS and EXC_FILE).
> > 
> > But maybe people who want "foo/" deserve it.  I dunno.
> 
> Ah, I didn't look at it that closely.
> 
> To do the laziness right, I think you would need to pass a pointer to 
> the dtype around, and just fill it in the first time it is needed.

Just to add my two eurocents: I think the patch is complicated enough that we could go the other way round: while parsing the ignore entries, we can plainly state that entries with a trailing slash are ignored:

-- snipsnap -- [PATCH] Warn if an ignore/exclude entry ends in a slash

Git does not like ignore entries ending in a slash; they will be ignored. So just be honest and warn the user about it.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
---
 dir.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/dir.c b/dir.c
index 1b9cc7a..c3e9a0d 100644
--- a/dir.c
+++ b/dir.c
@@ -135,6 +135,11 @@ void add_exclude(const char *string, const char *base,
 	}
 	x->pattern = string;
 	x->patternlen = strlen(string);
+	if (x->patternlen && x->pattern[x->patternlen - 1] == '/') {
+		warning("Ignoring ignore entry because of trailing slash: %s",
+			string);
+		return;
+	}
 	x->base = base;
 	x->baselen = baselen;
 	x->flags = 0;
pradeep singh rautela· Jan 31, 2008, 11:56 UTC · re: Johannes Schindelin · lore

Re: [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo"

On 31/01/2008, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: [snip]

Show 29 quoted lines
>
> Just to add my two eurocents: I think the patch is complicated enough that
> we could go the other way round: while parsing the ignore entries, we can
> plainly state that entries with a trailing slash are ignored:
>
> -- snipsnap --
> [PATCH] Warn if an ignore/exclude entry ends in a slash
>
> Git does not like ignore entries ending in a slash; they will be ignored.
> So just be honest and warn the user about it.
>
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
>
> ---
>
>  dir.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index 1b9cc7a..c3e9a0d 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -135,6 +135,11 @@ void add_exclude(const char *string, const char *base,
>         }
>         x->pattern = string;
>         x->patternlen = strlen(string);
> +       if (x->patternlen && x->pattern[x->patternlen - 1] == '/') {
> +               warning("Ignoring ignore entry because of trailing slash: %s",
> +                       string);
How about something like,
                  warning("Ignoring ignore entry because of trailing
slash: %s\n Remove the trailing slash from the directory name to
ignore it", string);

May be this will help absolute git newbies. Please ignore this if it sounds like a "too trivial, everyone should know this" case.

Thanks,
           --Pradeep
Show 6 quoted lines
> +               return;
> +       }
>         x->base = base;
>         x->baselen = baselen;
>         x->flags = 0;
>
-- 
Pradeep Singh Rautela
http://eagain.wordpress.com
http://emptydomain.googlepages.com
Junio C Hamano· Jan 31, 2008, 21:51 UTC · re: pradeep singh rautela · lore

Re: [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo"

"pradeep singh rautela" <rautelap@gmail.com> writes:
Show 5 quoted lines
> How about something like,
>                   warning("Ignoring ignore entry because of trailing
> slash: %s\n Remove the trailing slash from the directory name to
> ignore it", string);
> May be this will help absolute git newbies.
I am afraid that this is leading us in the wrong direction.

What would be the first reaction if somebody sees such a message?

    The message implies that the user said "foo/" which would be
    ignored and the right substitution is "foo".  If that is the
    right substitution, why doesn't the stupid "git" program do
    that for the user automatically?!?!?!?!
See?

"Remove the trailing" suggestion assumes that we would want "foo/" and "foo" to mean the same thing.

Maybe we do, but we usually match both directory "foo/" and regular file "foo" when you say "foo", and we match only directory "foo/" when you say "foo/", as you saw in the ls-files example.

While I am not 100% convinced that we want to keep the distinction between these two forms, I am far from thinking that the existing distinction in other parts of the system is useless and should be removed.

Maybe we would want to drop this distinction in the gitignore entries, and the apparent inconsistency may not hurt in reality. If that is what we would want, that is fine, but then we shouldn't give a warning with a stupid piece of advice, but instead just do it ourselves.

Like this on top of 'master' (i.e. discarding all the previous patches), perhaps...

-- >8 -- [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo"

A pattern "foo/" in the exclude list did not match directory "foo", but a pattern "foo" did. This just strips the trailing slash from such input.

This makes the behaviour slightly inconsistent with that of pathspecs, where "foo/" only matches directory "foo" and not regular file "foo" and make "foo/" in the ignore list match regular file "foo" happily. This may hopefully does not matter in practice.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/gitignore.txt        |    3 +++
 dir.c                              |   22 ++++++++++++++++++----
 t/t3001-ls-files-others-exclude.sh |   26 ++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index 08373f5..081a4df 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -57,6 +57,9 @@ Patterns have the following format:
    included again.  If a negated pattern matches, this will
    override lower precedence patterns sources.
 
+ - If the pattern ends with a slash, it is removed for the
+   purpose of the following description.
+
  - If the pattern does not contain a slash '/', git treats it as
    a shell glob pattern and checks for a match against the
    pathname without leading directories.
diff --git a/dir.c b/dir.c
index 3e345c2..fe51829 100644
--- a/dir.c
+++ b/dir.c
@@ -126,14 +126,28 @@ static int no_wildcard(const char *string)
 void add_exclude(const char *string, const char *base,
 		 int baselen, struct exclude_list *which)
 {
-	struct exclude *x = xmalloc(sizeof (*x));
+	struct exclude *x;
+	size_t len;
+	int to_exclude = 1;
 
-	x->to_exclude = 1;
 	if (*string == '!') {
-		x->to_exclude = 0;
+		to_exclude = 0;
 		string++;
 	}
-	x->pattern = string;
+	len = strlen(string);
+	if (len && string[len - 1] == '/') {
+		char *s;
+		x = xmalloc(sizeof(*x) + len);
+		s = (char*)(x+1);
+		memcpy(s, string, len - 1);
+		s[len - 1] = '\0';
+		string = s;
+		x->pattern = s;
+	} else {
+		x = xmalloc(sizeof(*x));
+		x->pattern = string;
+	}
+	x->to_exclude = to_exclude;
 	x->patternlen = strlen(string);
 	x->base = base;
 	x->baselen = baselen;
diff --git a/t/t3001-ls-files-others-exclude.sh b/t/t3001-ls-files-others-exclude.sh
index e25b255..5bc4885 100755
--- a/t/t3001-ls-files-others-exclude.sh
+++ b/t/t3001-ls-files-others-exclude.sh
@@ -99,4 +99,30 @@ EOF
 test_expect_success 'git-status honours core.excludesfile' \
 	'diff -u expect output'
 
+test_expect_success 'trailing slash in exclude allows directory match(1)' '
+
+	git ls-files --others --exclude=one/ >output &&
+	if grep "^one/" output
+	then
+		echo Ooops
+		false
+	else
+		: happy
+	fi
+
+'
+
+test_expect_success 'trailing slash in exclude allows directory match (2)' '
+
+	git ls-files --others --exclude=one/two/ >output &&
+	if grep "^one/two/" output
+	then
+		echo Ooops
+		false
+	else
+		: happy
+	fi
+
+'
+
 test_done
-- 
1.5.4.rc5.16.gc0279
Adam Piatyszek· Jan 31, 2008, 22:53 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo"

* Junio C Hamano [31 I 2008 22:51]:
Show 13 quoted lines
> [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo"
> 
> A pattern "foo/" in the exclude list did not match directory
> "foo", but a pattern "foo" did.  This just strips the trailing
> slash from such input.
> 
> This makes the behaviour slightly inconsistent with that of
> pathspecs, where "foo/" only matches directory "foo" and not
> regular file "foo" and make "foo/" in the ignore list match
> regular file "foo" happily.  This may hopefully does not matter
> in practice.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is more or less what I suggested originally. ;-)

Anyway, even if this new behaviour is not consistent with that of pathspecs, it is not worse than the current behaviour of git. I.e. now you have to use "foo" to ignore the "foo" directory and its contents, but it does not protect you from masking the file "foo" in the same repository.

However, it is not possible to have both the "foo" directory and "foo" file in the same directory level of a repository at the same time. So, the problem with this patch might be only when one replaces the ignored directory "foo" with a file using the same name and forgets to remove the "foo/" entry from .gitignore or .git/info/exclude. But exactly the same situation can occur for the current implementation.

So, I tend to agree that your latest patch is a sensible solution for 99.9% of cases.

BR, /Adam

-- 
.:.  Adam Piatyszek (ediap)  .:.....................................:.
.:.  ediap@users.sourceforge.net  .:................................:.
Andreas Ericsson· Feb 1, 2008, 08:56 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo"

Junio C Hamano wrote:
Show 71 quoted lines
> "pradeep singh rautela" <rautelap@gmail.com> writes:
> 
>> How about something like,
>>                   warning("Ignoring ignore entry because of trailing
>> slash: %s\n Remove the trailing slash from the directory name to
>> ignore it", string);
>> May be this will help absolute git newbies.
> 
> I am afraid that this is leading us in the wrong direction.
> 
> What would be the first reaction if somebody sees such a
> message?
> 
>     The message implies that the user said "foo/" which would be
>     ignored and the right substitution is "foo".  If that is the
>     right substitution, why doesn't the stupid "git" program do
>     that for the user automatically?!?!?!?!
> 
> See?
> 
> "Remove the trailing" suggestion assumes that we would want "foo/"
> and "foo" to mean the same thing.
> 
> Maybe we do, but we usually match both directory "foo/" and
> regular file "foo" when you say "foo", and we match only
> directory "foo/" when you say "foo/", as you saw in the ls-files
> example.
> 
> While I am not 100% convinced that we want to keep the
> distinction between these two forms, I am far from thinking that
> the existing distinction in other parts of the system is useless
> and should be removed.
> 
> Maybe we would want to drop this distinction in the gitignore
> entries, and the apparent inconsistency may not hurt in reality.
> If that is what we would want, that is fine, but then we
> shouldn't give a warning with a stupid piece of advice, but
> instead just do it ourselves.
> 
> Like this on top of 'master' (i.e. discarding all the previous
> patches), perhaps...
> 
> -- >8 --
> [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo"
> 
> A pattern "foo/" in the exclude list did not match directory
> "foo", but a pattern "foo" did.  This just strips the trailing
> slash from such input.
> 
> This makes the behaviour slightly inconsistent with that of
> pathspecs, where "foo/" only matches directory "foo" and not
> regular file "foo" and make "foo/" in the ignore list match
> regular file "foo" happily.  This may hopefully does not matter
> in practice.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/gitignore.txt        |    3 +++
>  dir.c                              |   22 ++++++++++++++++++----
>  t/t3001-ls-files-others-exclude.sh |   26 ++++++++++++++++++++++++++
>  3 files changed, 47 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
> index 08373f5..081a4df 100644
> --- a/Documentation/gitignore.txt
> +++ b/Documentation/gitignore.txt
> @@ -57,6 +57,9 @@ Patterns have the following format:
>     included again.  If a negated pattern matches, this will
>     override lower precedence patterns sources.
>  
> + - If the pattern ends with a slash,
that slash
Show 6 quoted lines
> is removed for the
> +   purpose of the following description.
> +
>   - If the pattern does not contain a slash '/', git treats it as
>     a shell glob pattern and checks for a match against the
>     pathname without leading directories.
Otherwise it sounds as if the entire pattern is removed.
-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
Adam Piatyszek· Jan 31, 2008, 12:29 UTC · re: Johannes Schindelin · lore

Re: [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo"

* Johannes Schindelin [31 I 2008 12:38]:
Show 11 quoted lines
> Just to add my two eurocents: I think the patch is complicated enough that 
> we could go the other way round: while parsing the ignore entries, we can 
> plainly state that entries with a trailing slash are ignored:
> 
> -- snipsnap --
> [PATCH] Warn if an ignore/exclude entry ends in a slash
> 
> Git does not like ignore entries ending in a slash; they will be ignored.
> So just be honest and warn the user about it.
> 
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
I agree that this is a reasonable remedy for this issue. So:
Acked-by: Adam Piątyszek <ediap@users.sourceforge.net>

BTW, the warning message is a bit "hidden" between the "Changed" and "Untracked" parts of a status message, e.g.:

===== >8 ===== # On branch master # Changed but not updated: # (use "git add <file>..." to update what will be committed) # # modified: src/Makefile # modified: src/ofdm.cpp # warning: Ignoring ignore entry because of trailing slash: results/ # Untracked files: # (use "git add <file>..." to include in what will be committed) # # results/ no changes added to commit (use "git add" and/or "git commit -a") ===== >8 =====

Is it possible to make warnings displayed in red or yellow colour on terminals that support colours?

BR, /Adam

-- 
.:.  Adam Piatyszek (ediap)  .:.....................................:.
.:.  ediap@users.sourceforge.net  .:................................:.
Wayne Davison· Jan 23, 2008, 21:11 UTC · re: pradeep singh rautela · lore

Re: Why does git track directory listed in .gitignore/".git/info/exclude"?

On Wed, Jan 23, 2008 at 07:24:44PM +0530, pradeep singh rautela wrote:
> i.e i have added following line to both of them -
> xen-3.1.0-src/

That doesn't match the directory due to the trailing slash. If you remove that slash, it will match the dir, and then ignore anything you place inside.

..wayne..

← back to recent threads