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

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

[pradeep singh rautela]

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

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

[pradeep singh rautela]

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

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

[Wayne Davison]

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

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

[Linus Torvalds]

On Wed, 23 Jan 2008, pradeep singh rautela wrote:
>
> 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

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

[pradeep singh rautela]

Hi Linus,

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

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

[Adam Piatyszek]

* Linus Torvalds [23 I 2008 22:17]:
> 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  .:................................:.

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

[Junio C Hamano]

Adam Piatyszek <ediap@users.sourceforge.net> writes:

> 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

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

[Junio C Hamano]

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

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

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

[Adam Piatyszek]

Hi Junio,

* Junio C Hamano [30 I 2008 21:39]:
> 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  .:................................:.

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

[Junio C Hamano]

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.

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

[Junio C Hamano]

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

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

[Jeff King]

On Thu, Jan 31, 2008 at 01:17:48AM -0800, Junio C Hamano wrote:

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

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

[Junio C Hamano]

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

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

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

[Jeff King]

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.

-Peff

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

[Johannes Schindelin]

Hi,

On Thu, 31 Jan 2008, Jeff King wrote:

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

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

[pradeep singh rautela]

On 31/01/2008, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
[snip]
>
> 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
> +               return;
> +       }
>         x->base = base;
>         x->baselen = baselen;
>         x->flags = 0;
>


-- 
Pradeep Singh Rautela
http://eagain.wordpress.com
http://emptydomain.googlepages.com

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

[Adam Piatyszek]

* Johannes Schindelin [31 I 2008 12:38]:
> 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  .:................................:.

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

[Junio C Hamano]

"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, 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

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

[Adam Piatyszek]

* Junio C Hamano [31 I 2008 22:51]:
> [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  .:................................:.

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

[Andreas Ericsson]

Junio C Hamano wrote:
> "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

> 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