[PATCH] gitignore: root most patterns at the top-level directory

[PATCH] gitignore: root most patterns at the top-level directory

[Jeff King]

Our gitignore doesn't use a preceding "/" to root its
patterns in the top of the repository. This means that if
you add a file or directory called "git" (for example)
inside a subdirectory, it will be erroneously ignored.

This patch was done mechanically with "s/^[^*]/\/&/" with
one exception: instead of ignoring gitk-wish, we should
gitk-git/gitk-wish (arguably, this should be done in
gitk-git/.gitignore, but because that is a subtree merge
from elsewhere, this is easier).

Acked-by: Sverre Rabbelier <srabbelier@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---

This bit Sverre while I was looking over his shoulder. I doubt it comes
up very often, but we should probably be modeling good gitignore
behavior. I have to admit it looks a lot uglier, though.

 .gitignore |  354 ++++++++++++++++++++++++++++++------------------------------
 1 files changed, 177 insertions(+), 177 deletions(-)

diff --git a/.gitignore b/.gitignore
index 51a37b1..289c3d0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,184 +1,184 @@
-GIT-BUILD-OPTIONS
-GIT-CFLAGS
-GIT-GUI-VARS
-GIT-VERSION-FILE
-git
-git-add
-git-add--interactive
-git-am
-git-annotate
-git-apply
-git-archimport
-git-archive
-git-bisect
-git-bisect--helper
-git-blame
-git-branch
-git-bundle
-git-cat-file
-git-check-attr
-git-check-ref-format
-git-checkout
-git-checkout-index
-git-cherry
-git-cherry-pick
-git-clean
-git-clone
-git-commit
-git-commit-tree
-git-config
-git-count-objects
-git-cvsexportcommit
-git-cvsimport
-git-cvsserver
-git-daemon
-git-diff
-git-diff-files
-git-diff-index
-git-diff-tree
-git-difftool
-git-difftool--helper
-git-describe
-git-fast-export
-git-fast-import
-git-fetch
-git-fetch--tool
-git-fetch-pack
-git-filter-branch
-git-fmt-merge-msg
-git-for-each-ref
-git-format-patch
-git-fsck
-git-fsck-objects
-git-gc
-git-get-tar-commit-id
-git-grep
-git-hash-object
-git-help
-git-http-fetch
-git-http-push
-git-imap-send
-git-index-pack
-git-init
-git-init-db
-git-instaweb
-git-log
-git-lost-found
-git-ls-files
-git-ls-remote
-git-ls-tree
-git-mailinfo
-git-mailsplit
-git-merge
-git-merge-base
-git-merge-index
-git-merge-file
-git-merge-tree
-git-merge-octopus
-git-merge-one-file
-git-merge-ours
-git-merge-recursive
-git-merge-resolve
-git-merge-subtree
-git-mergetool
-git-mergetool--lib
-git-mktag
-git-mktree
-git-name-rev
-git-mv
-git-pack-redundant
-git-pack-objects
-git-pack-refs
-git-parse-remote
-git-patch-id
-git-peek-remote
-git-prune
-git-prune-packed
-git-pull
-git-push
-git-quiltimport
-git-read-tree
-git-rebase
-git-rebase--interactive
-git-receive-pack
-git-reflog
-git-relink
-git-remote
-git-remote-curl
-git-repack
-git-replace
-git-repo-config
-git-request-pull
-git-rerere
-git-reset
-git-rev-list
-git-rev-parse
-git-revert
-git-rm
-git-send-email
-git-send-pack
-git-sh-setup
-git-shell
-git-shortlog
-git-show
-git-show-branch
-git-show-index
-git-show-ref
-git-stage
-git-stash
-git-status
-git-stripspace
-git-submodule
-git-svn
-git-symbolic-ref
-git-tag
-git-tar-tree
-git-unpack-file
-git-unpack-objects
-git-update-index
-git-update-ref
-git-update-server-info
-git-upload-archive
-git-upload-pack
-git-var
-git-verify-pack
-git-verify-tag
-git-web--browse
-git-whatchanged
-git-write-tree
-git-core-*/?*
-gitk-wish
-gitweb/gitweb.cgi
-test-chmtime
-test-ctype
-test-date
-test-delta
-test-dump-cache-tree
-test-genrandom
-test-match-trees
-test-parse-options
-test-path-utils
-test-sha1
-test-sigchain
-common-cmds.h
+/GIT-BUILD-OPTIONS
+/GIT-CFLAGS
+/GIT-GUI-VARS
+/GIT-VERSION-FILE
+/git
+/git-add
+/git-add--interactive
+/git-am
+/git-annotate
+/git-apply
+/git-archimport
+/git-archive
+/git-bisect
+/git-bisect--helper
+/git-blame
+/git-branch
+/git-bundle
+/git-cat-file
+/git-check-attr
+/git-check-ref-format
+/git-checkout
+/git-checkout-index
+/git-cherry
+/git-cherry-pick
+/git-clean
+/git-clone
+/git-commit
+/git-commit-tree
+/git-config
+/git-count-objects
+/git-cvsexportcommit
+/git-cvsimport
+/git-cvsserver
+/git-daemon
+/git-diff
+/git-diff-files
+/git-diff-index
+/git-diff-tree
+/git-difftool
+/git-difftool--helper
+/git-describe
+/git-fast-export
+/git-fast-import
+/git-fetch
+/git-fetch--tool
+/git-fetch-pack
+/git-filter-branch
+/git-fmt-merge-msg
+/git-for-each-ref
+/git-format-patch
+/git-fsck
+/git-fsck-objects
+/git-gc
+/git-get-tar-commit-id
+/git-grep
+/git-hash-object
+/git-help
+/git-http-fetch
+/git-http-push
+/git-imap-send
+/git-index-pack
+/git-init
+/git-init-db
+/git-instaweb
+/git-log
+/git-lost-found
+/git-ls-files
+/git-ls-remote
+/git-ls-tree
+/git-mailinfo
+/git-mailsplit
+/git-merge
+/git-merge-base
+/git-merge-index
+/git-merge-file
+/git-merge-tree
+/git-merge-octopus
+/git-merge-one-file
+/git-merge-ours
+/git-merge-recursive
+/git-merge-resolve
+/git-merge-subtree
+/git-mergetool
+/git-mergetool--lib
+/git-mktag
+/git-mktree
+/git-name-rev
+/git-mv
+/git-pack-redundant
+/git-pack-objects
+/git-pack-refs
+/git-parse-remote
+/git-patch-id
+/git-peek-remote
+/git-prune
+/git-prune-packed
+/git-pull
+/git-push
+/git-quiltimport
+/git-read-tree
+/git-rebase
+/git-rebase--interactive
+/git-receive-pack
+/git-reflog
+/git-relink
+/git-remote
+/git-remote-curl
+/git-repack
+/git-replace
+/git-repo-config
+/git-request-pull
+/git-rerere
+/git-reset
+/git-rev-list
+/git-rev-parse
+/git-revert
+/git-rm
+/git-send-email
+/git-send-pack
+/git-sh-setup
+/git-shell
+/git-shortlog
+/git-show
+/git-show-branch
+/git-show-index
+/git-show-ref
+/git-stage
+/git-stash
+/git-status
+/git-stripspace
+/git-submodule
+/git-svn
+/git-symbolic-ref
+/git-tag
+/git-tar-tree
+/git-unpack-file
+/git-unpack-objects
+/git-update-index
+/git-update-ref
+/git-update-server-info
+/git-upload-archive
+/git-upload-pack
+/git-var
+/git-verify-pack
+/git-verify-tag
+/git-web--browse
+/git-whatchanged
+/git-write-tree
+/git-core-*/?*
+/gitk-git/gitk-wish
+/gitweb/gitweb.cgi
+/test-chmtime
+/test-ctype
+/test-date
+/test-delta
+/test-dump-cache-tree
+/test-genrandom
+/test-match-trees
+/test-parse-options
+/test-path-utils
+/test-sha1
+/test-sigchain
+/common-cmds.h
 *.tar.gz
 *.dsc
 *.deb
-git.spec
+/git.spec
 *.exe
 *.[aos]
 *.py[co]
-config.mak
-autom4te.cache
-config.cache
-config.log
-config.status
-config.mak.autogen
-config.mak.append
-configure
-tags
-TAGS
-cscope*
+/config.mak
+/autom4te.cache
+/config.cache
+/config.log
+/config.status
+/config.mak.autogen
+/config.mak.append
+/configure
+/tags
+/TAGS
+/cscope*
 *.obj
 *.lib
 *.sln
@@ -188,5 +188,5 @@ cscope*
 *.user
 *.idb
 *.pdb
-Debug/
-Release/
+/Debug/
+/Release/
-- 
1.6.5.1.203.g342e8

Re: [PATCH] gitignore: root most patterns at the top-level directory

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Our gitignore doesn't use a preceding "/" to root its
> patterns in the top of the repository. This means that if
> you add a file or directory called "git" (for example)
> inside a subdirectory, it will be erroneously ignored.
>
> This patch was done mechanically with "s/^[^*]/\/&/" with
> one exception: instead of ignoring gitk-wish, we should
> gitk-git/gitk-wish (arguably, this should be done in
> gitk-git/.gitignore, but because that is a subtree merge
> from elsewhere, this is easier).
>
> Acked-by: Sverre Rabbelier <srabbelier@gmail.com>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>
> This bit Sverre while I was looking over his shoulder. I doubt it comes
> up very often, but we should probably be modeling good gitignore
> behavior. I have to admit it looks a lot uglier, though.

How does .cvsignore and .svnignore work?  Don't they have the same issue,
and perhaps worse as I do not recall seeing a way to anchor a pattern to a
particular directory like we do in their .SCMignore files?  And judging
from the fact that they can get away with the lack of that "feature", this
perhaps is not an issue in real life?

I am actually a bit reluctant to queue this, even though I most likely
will, and then hope that we will think of a better solution later, at
which time this file again needs to change.

For example, it crossed my mind that perhaps we can change the ignore
rules so that a non-globbing pattern is automatically anchored at the
current directly but globbing ones are recursive as before.

If we do so, there is no need to change the current .gitignore entires.
You need to spell a concrete filename as a glob pattern that matches only
one path if you want the recursive behaviour.  E.g. if you have a Makefile
per subdirectory, each of which generates and includes Makefile.depend
file, you would write "Makefile.depen[d]" in the toplevel .gitignore file.

But that is a kind of incompatible change whose necessity is unproven and
has to cook and wait.

Re: [PATCH] gitignore: root most patterns at the top-level directory

[Johannes Sixt]

Junio C Hamano schrieb:
> How does .cvsignore and .svnignore work?  Don't they have the same issue,
> and perhaps worse as I do not recall seeing a way to anchor a pattern to a
> particular directory like we do in their .SCMignore files?  And judging
> from the fact that they can get away with the lack of that "feature", this
> perhaps is not an issue in real life?

.cvsignore and .svnignore do not apply recursively to subdirectories, do they?

> For example, it crossed my mind that perhaps we can change the ignore
> rules so that a non-globbing pattern is automatically anchored at the
> current directly but globbing ones are recursive as before.
> 
> If we do so, there is no need to change the current .gitignore entires.
> You need to spell a concrete filename as a glob pattern that matches only
> one path if you want the recursive behaviour.  E.g. if you have a Makefile
> per subdirectory, each of which generates and includes Makefile.depend
> file, you would write "Makefile.depen[d]" in the toplevel .gitignore file.

In one project that uses autotools, I have "Makefile" and "Makefile.in" in
the top-level .gitignore. I would be forced to use this ugliness instead.

Granted, to write "/git", "/git-add", etc in .gitignore is not exactly
pretty, either, but the reason that it is so extra-ugly in the git code
itself is only because there are so many build products in a single
directory that cannot be caught by a glob pattern. In practice, you
usually have only a hand-full non-glob ignored files per directory; it
doesn't hurt to anchor them using "/frotz" style.

> But that is a kind of incompatible change whose necessity is unproven and
> has to cook and wait.

I would be concerned by this change.

-- Hannes

Re: [PATCH] gitignore: root most patterns at the top-level directory

[Jeff King]

On Tue, Oct 27, 2009 at 11:03:28PM -0700, Junio C Hamano wrote:

> How does .cvsignore and .svnignore work?  Don't they have the same issue,
> and perhaps worse as I do not recall seeing a way to anchor a pattern to a
> particular directory like we do in their .SCMignore files?  And judging
> from the fact that they can get away with the lack of that "feature", this
> perhaps is not an issue in real life?

Happily, I did not remember how .cvsignore worked and had to go read the
documentation. :) The answer is that no, it does not have the recursive
feature in the root .cvsignore list at all. But it does apply the
repo-wide CVSROOT/cvsignore, the user's ~/.cvsignore, and the
environment's $CVSIGNORE to all directories. So it is safe from this
problem (though now that I think on it, I think I was once bitten by
something similar in the CVSROOT/cvsignore).

SVN implements this with "properties" on the directories. They are not
recursive at all. However, it also implements "global-ignores" which
applies everywhere.

So no, they don't have the same issue, because they explicitly split the
"everywhere" and "this directory" cases into two locations. We wouldn't
want to use .git/info/excludes for this, because we do want to support
the project shipping some global excludes itself.

> I am actually a bit reluctant to queue this, even though I most likely
> will, and then hope that we will think of a better solution later, at
> which time this file again needs to change.

I am mixed on it, as well. I did see it bite someone, but I think it's
very rare, and everyone who reads or touches the file will have to deal
with the ugliness every time. If you want to drop the patch, I will not
complain.

> For example, it crossed my mind that perhaps we can change the ignore
> rules so that a non-globbing pattern is automatically anchored at the
> current directly but globbing ones are recursive as before.

That makes some sense to me (and in fact when making the patch, it was
the rule of thumb I used). Though I think you might want to make it
"starts with glob" as the trigger for the rule. We have "git-core-*/?*",
which I would expect to still be anchored at the root.

> If we do so, there is no need to change the current .gitignore entires.
> You need to spell a concrete filename as a glob pattern that matches only
> one path if you want the recursive behaviour.  E.g. if you have a Makefile
> per subdirectory, each of which generates and includes Makefile.depend
> file, you would write "Makefile.depen[d]" in the toplevel .gitignore file.

While clever, that use of '[d]' seems unneccessarily obscure to me. Why
not just give a wildcard for "any subdirectory of me" and do:

  Makefile.depend
  **/Makefile.depend

Since "**" is in common use in other systems, it's pretty clear (to me,
anyway, but then I am the one suggesting the syntax ;) ) what that
means.

-Peff

Re: [PATCH] gitignore: root most patterns at the top-level directory

[James Pickens]

On Fri, Oct 30, 2009 at 11:24 AM, Jeff King <peff@peff.net> wrote:
>> If we do so, there is no need to change the current .gitignore entires.
>> You need to spell a concrete filename as a glob pattern that matches only
>> one path if you want the recursive behaviour.  E.g. if you have a Makefile
>> per subdirectory, each of which generates and includes Makefile.depend
>> file, you would write "Makefile.depen[d]" in the toplevel .gitignore file.
>
> While clever, that use of '[d]' seems unneccessarily obscure to me. Why
> not just give a wildcard for "any subdirectory of me" and do:
>
>  Makefile.depend
>  **/Makefile.depend
>
> Since "**" is in common use in other systems, it's pretty clear (to me,
> anyway, but then I am the one suggesting the syntax ;) ) what that
> means.

+1 to that.  I've often wished for Git to support the ** wildcard, not only in
.gitignore but also in other places like .gitattributes and sparse checkout (if
that feature ever gets completed anyways).  It's on my list of "git features I
would work on if I ever had any free time."

James