threads / patch / 21369

patchgitignore: root most patterns at the top-level directory

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

## tl;dr

5 messages between Oct 27, 2009 and Oct 30, 2009. Diffs are folded; open one to read it.

replies: 4people: 4as markdown or json

Jeff King· Oct 27, 2009, 01:10 UTC · lore

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

Show changes to .gitignore +177 −177

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

Junio C Hamano· Oct 28, 2009, 06:03 UTC · re: Jeff King · lore

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

Jeff King <peff@peff.net> writes:

Show 18 quoted lines

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

Johannes Sixt· Oct 28, 2009, 07:21 UTC · re: Junio C Hamano · lore

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

Junio C Hamano schrieb:

Show 5 quoted lines

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

Show 9 quoted lines

> 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

Jeff King· Oct 30, 2009, 18:24 UTC · re: Junio C Hamano · lore

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

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

Show 5 quoted lines

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

Show 5 quoted lines

> 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

James Pickens· Oct 30, 2009, 21:52 UTC · re: Jeff King · lore

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

On Fri, Oct 30, 2009 at 11:24 AM, Jeff King <peff@peff.net> wrote:

Show 15 quoted lines

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