* Why does git track directory listed in .gitignore/".git/info/exclude"? @ 2008-01-23 13:54 pradeep singh rautela 2008-01-23 14:04 ` pradeep singh rautela 2008-01-23 21:11 ` Why does git track directory listed in .gitignore/".git/info/exclude"? Wayne Davison 0 siblings, 2 replies; 20+ messages in thread From: pradeep singh rautela @ 2008-01-23 13:54 UTC (permalink / raw) To: git 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Why does git track directory listed in .gitignore/".git/info/exclude"? 2008-01-23 13:54 Why does git track directory listed in .gitignore/".git/info/exclude"? pradeep singh rautela @ 2008-01-23 14:04 ` pradeep singh rautela 2008-01-23 21:17 ` Linus Torvalds 2008-01-23 21:11 ` Why does git track directory listed in .gitignore/".git/info/exclude"? Wayne Davison 1 sibling, 1 reply; 20+ messages in thread From: pradeep singh rautela @ 2008-01-23 14:04 UTC (permalink / raw) To: git 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Why does git track directory listed in .gitignore/".git/info/exclude"? 2008-01-23 14:04 ` pradeep singh rautela @ 2008-01-23 21:17 ` Linus Torvalds 2008-01-24 10:44 ` pradeep singh rautela 2008-01-30 12:35 ` Adam Piatyszek 0 siblings, 2 replies; 20+ messages in thread From: Linus Torvalds @ 2008-01-23 21:17 UTC (permalink / raw) To: pradeep singh rautela; +Cc: git 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Why does git track directory listed in .gitignore/".git/info/exclude"? 2008-01-23 21:17 ` Linus Torvalds @ 2008-01-24 10:44 ` pradeep singh rautela 2008-01-30 12:35 ` Adam Piatyszek 1 sibling, 0 replies; 20+ messages in thread From: pradeep singh rautela @ 2008-01-24 10:44 UTC (permalink / raw) To: Linus Torvalds; +Cc: git 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Why does git track directory listed in .gitignore/".git/info/exclude"? 2008-01-23 21:17 ` Linus Torvalds 2008-01-24 10:44 ` pradeep singh rautela @ 2008-01-30 12:35 ` Adam Piatyszek 2008-01-30 20:39 ` Junio C Hamano 1 sibling, 1 reply; 20+ messages in thread From: Adam Piatyszek @ 2008-01-30 12:35 UTC (permalink / raw) To: Linus Torvalds; +Cc: pradeep singh rautela, git * 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 .:................................:. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Why does git track directory listed in .gitignore/".git/info/exclude"? 2008-01-30 12:35 ` Adam Piatyszek @ 2008-01-30 20:39 ` Junio C Hamano 2008-01-30 21:06 ` Junio C Hamano 2008-01-31 7:05 ` Adam Piatyszek 0 siblings, 2 replies; 20+ messages in thread From: Junio C Hamano @ 2008-01-30 20:39 UTC (permalink / raw) To: Adam Piatyszek; +Cc: Linus Torvalds, pradeep singh rautela, git 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Why does git track directory listed in .gitignore/".git/info/exclude"? 2008-01-30 20:39 ` Junio C Hamano @ 2008-01-30 21:06 ` Junio C Hamano 2008-01-31 7:05 ` Adam Piatyszek 1 sibling, 0 replies; 20+ messages in thread From: Junio C Hamano @ 2008-01-30 21:06 UTC (permalink / raw) To: Adam Piatyszek; +Cc: Linus Torvalds, pradeep singh rautela, git 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Why does git track directory listed in .gitignore/".git/info/exclude"? 2008-01-30 20:39 ` Junio C Hamano 2008-01-30 21:06 ` Junio C Hamano @ 2008-01-31 7:05 ` Adam Piatyszek 2008-01-31 8:54 ` *Re: " Junio C Hamano 1 sibling, 1 reply; 20+ messages in thread From: Adam Piatyszek @ 2008-01-31 7:05 UTC (permalink / raw) To: Junio C Hamano; +Cc: Linus Torvalds, pradeep singh rautela, git 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 .:................................:. ^ permalink raw reply [flat|nested] 20+ messages in thread
* *Re: Why does git track directory listed in .gitignore/".git/info/exclude"? 2008-01-31 7:05 ` Adam Piatyszek @ 2008-01-31 8:54 ` Junio C Hamano 2008-01-31 9:17 ` [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo" Junio C Hamano 0 siblings, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2008-01-31 8:54 UTC (permalink / raw) To: Adam Piatyszek; +Cc: Linus Torvalds, pradeep singh rautela, git 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. ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo" 2008-01-31 8:54 ` *Re: " Junio C Hamano @ 2008-01-31 9:17 ` Junio C Hamano 2008-01-31 9:41 ` Jeff King 0 siblings, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2008-01-31 9:17 UTC (permalink / raw) To: Adam Piatyszek; +Cc: Linus Torvalds, pradeep singh rautela, git 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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo" 2008-01-31 9:17 ` [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo" Junio C Hamano @ 2008-01-31 9:41 ` Jeff King 2008-01-31 10:35 ` Junio C Hamano 0 siblings, 1 reply; 20+ messages in thread From: Jeff King @ 2008-01-31 9:41 UTC (permalink / raw) To: Junio C Hamano; +Cc: Adam Piatyszek, Linus Torvalds, pradeep singh rautela, git 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)) ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo" 2008-01-31 9:41 ` Jeff King @ 2008-01-31 10:35 ` Junio C Hamano 2008-01-31 10:42 ` Jeff King 0 siblings, 1 reply; 20+ messages in thread From: Junio C Hamano @ 2008-01-31 10:35 UTC (permalink / raw) To: Jeff King; +Cc: Adam Piatyszek, Linus Torvalds, pradeep singh rautela, git 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo" 2008-01-31 10:35 ` Junio C Hamano @ 2008-01-31 10:42 ` Jeff King 2008-01-31 11:38 ` Johannes Schindelin 0 siblings, 1 reply; 20+ messages in thread From: Jeff King @ 2008-01-31 10:42 UTC (permalink / raw) To: Junio C Hamano; +Cc: Adam Piatyszek, Linus Torvalds, pradeep singh rautela, git 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo" 2008-01-31 10:42 ` Jeff King @ 2008-01-31 11:38 ` Johannes Schindelin 2008-01-31 11:56 ` pradeep singh rautela 2008-01-31 12:29 ` Adam Piatyszek 0 siblings, 2 replies; 20+ messages in thread From: Johannes Schindelin @ 2008-01-31 11:38 UTC (permalink / raw) To: Jeff King Cc: Junio C Hamano, Adam Piatyszek, Linus Torvalds, pradeep singh rautela, git 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; ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo" 2008-01-31 11:38 ` Johannes Schindelin @ 2008-01-31 11:56 ` pradeep singh rautela 2008-01-31 21:51 ` Junio C Hamano 2008-01-31 12:29 ` Adam Piatyszek 1 sibling, 1 reply; 20+ messages in thread From: pradeep singh rautela @ 2008-01-31 11:56 UTC (permalink / raw) To: Johannes Schindelin Cc: Jeff King, Junio C Hamano, Adam Piatyszek, Linus Torvalds, git 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo" 2008-01-31 11:56 ` pradeep singh rautela @ 2008-01-31 21:51 ` Junio C Hamano 2008-01-31 22:53 ` Adam Piatyszek 2008-02-01 8:56 ` Andreas Ericsson 0 siblings, 2 replies; 20+ messages in thread From: Junio C Hamano @ 2008-01-31 21:51 UTC (permalink / raw) To: pradeep singh rautela Cc: Johannes Schindelin, Jeff King, Junio C Hamano, Adam Piatyszek, Linus Torvalds, git "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 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo" 2008-01-31 21:51 ` Junio C Hamano @ 2008-01-31 22:53 ` Adam Piatyszek 2008-02-01 8:56 ` Andreas Ericsson 1 sibling, 0 replies; 20+ messages in thread From: Adam Piatyszek @ 2008-01-31 22:53 UTC (permalink / raw) To: Junio C Hamano Cc: pradeep singh rautela, Johannes Schindelin, Jeff King, Linus Torvalds, git * 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 .:................................:. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo" 2008-01-31 21:51 ` Junio C Hamano 2008-01-31 22:53 ` Adam Piatyszek @ 2008-02-01 8:56 ` Andreas Ericsson 1 sibling, 0 replies; 20+ messages in thread From: Andreas Ericsson @ 2008-02-01 8:56 UTC (permalink / raw) To: Junio C Hamano Cc: pradeep singh rautela, Johannes Schindelin, Jeff King, Adam Piatyszek, Linus Torvalds, git 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo" 2008-01-31 11:38 ` Johannes Schindelin 2008-01-31 11:56 ` pradeep singh rautela @ 2008-01-31 12:29 ` Adam Piatyszek 1 sibling, 0 replies; 20+ messages in thread From: Adam Piatyszek @ 2008-01-31 12:29 UTC (permalink / raw) To: Johannes Schindelin Cc: Jeff King, Junio C Hamano, Linus Torvalds, pradeep singh rautela, git * 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 .:................................:. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Why does git track directory listed in .gitignore/".git/info/exclude"? 2008-01-23 13:54 Why does git track directory listed in .gitignore/".git/info/exclude"? pradeep singh rautela 2008-01-23 14:04 ` pradeep singh rautela @ 2008-01-23 21:11 ` Wayne Davison 1 sibling, 0 replies; 20+ messages in thread From: Wayne Davison @ 2008-01-23 21:11 UTC (permalink / raw) To: pradeep singh rautela; +Cc: git 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.. ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2008-02-01 8:58 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-01-23 13:54 Why does git track directory listed in .gitignore/".git/info/exclude"? pradeep singh rautela 2008-01-23 14:04 ` pradeep singh rautela 2008-01-23 21:17 ` Linus Torvalds 2008-01-24 10:44 ` pradeep singh rautela 2008-01-30 12:35 ` Adam Piatyszek 2008-01-30 20:39 ` Junio C Hamano 2008-01-30 21:06 ` Junio C Hamano 2008-01-31 7:05 ` Adam Piatyszek 2008-01-31 8:54 ` *Re: " Junio C Hamano 2008-01-31 9:17 ` [PATCH] gitignore(5): Allow "foo/" in ignore list to match directory "foo" Junio C Hamano 2008-01-31 9:41 ` Jeff King 2008-01-31 10:35 ` Junio C Hamano 2008-01-31 10:42 ` Jeff King 2008-01-31 11:38 ` Johannes Schindelin 2008-01-31 11:56 ` pradeep singh rautela 2008-01-31 21:51 ` Junio C Hamano 2008-01-31 22:53 ` Adam Piatyszek 2008-02-01 8:56 ` Andreas Ericsson 2008-01-31 12:29 ` Adam Piatyszek 2008-01-23 21:11 ` Why does git track directory listed in .gitignore/".git/info/exclude"? Wayne Davison
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).