gitignore filter only looks at the top of work tree

[wpt-issue-mover]

Originally posted as w3c/wpt-tools#169 by @gsnedders on 21 Feb 2017, 22:38 UTC:

However, we have a bunch more gitignore files in the tree:

./.gitignore
./annotation-model/annotations/.gitignore
./annotation-model/annotations/bodiesTargets/.gitignore
./annotation-model/collections/.gitignore
./annotation-model/definitions/.gitignore
./annotation-protocol/server/.gitignore
./annotation-vocab/tools/.gitignore
./cors/resources/.gitignore
./docs/.gitignore
./webaudio/.gitignore

[wpt-issue-mover]

Originally posted as w3c/wpt-tools#169 (comment) by @jgraham on 22 Feb 2017, 15:52 UTC:

I am strongly in favour of fixing this by not having multiple gitignore files in the tree. It saves a considerable amount of work compared to checking for, and maybe reading, a .gitignore file in each subdirectory.

[wpt-issue-mover]

Originally posted as w3c/wpt-tools#169 (comment) by @gsnedders on 22 Feb 2017, 17:40 UTC:

Really we probably want to pay attention to things that the specific user has ignored too, especially because IDE files often get ignored in $XDG_CONFIG_HOME/git/ignore/$HOME/.config/git/ignore. Meh.

[wpt-issue-mover]

Originally posted as w3c/wpt-tools#169 (comment) by @jgraham on 22 Feb 2017, 17:48 UTC:

I mean a short static list of locations to check is fine. An O(N) list of locations that have to be examined as we walk the directory tree is less fine.

[wpt-issue-mover]

Originally posted as w3c/wpt-tools#169 (comment) by @gsnedders on 02 Mar 2017, 16:29 UTC:

Those in annotation-model seem to be empty (i.e., zero byte files).

[wpt-issue-mover]

Originally posted as w3c/wpt-tools#169 (comment) by @gsnedders on 02 Mar 2017, 18:25 UTC:

oh wow this is diabolical in my csswg-test working tree because I have thousands of files ignored in $GIT_DIR/info/excludes :(

[gsnedders]

Now:

$ find . -name '.gitignore' -print0 | xargs -0 wc -l | sort
       0 ./annotation-model/annotations/.gitignore
       0 ./annotation-model/annotations/bodiesTargets/.gitignore
       0 ./annotation-model/collections/.gitignore
       0 ./annotation-model/definitions/.gitignore
       0 ./annotation-protocol/server/.gitignore
       0 ./annotation-vocab/tools/.gitignore
       0 ./tools/third_party/attrs/changelog.d/.gitignore
       0 ./webaudio/.gitignore
       1 ./conformance-checkers/.gitignore
       1 ./cors/resources/.gitignore
       1 ./css/css-rhythm/tools/generators/.gitignore
       1 ./css/css-writing-modes/tools/generators/.gitignore
       1 ./tools/wptrunner/.pytest_cache/.gitignore
       2 ./tools/webdriver/.gitignore
       3 ./resources/.gitignore
       3 ./tools/third_party/pytest/doc/en/_themes/.gitignore
       3 ./tools/third_party/pytest/testing/freeze/.gitignore
       4 ./resources/webidl2/.gitignore
       5 ./docs/.gitignore
       8 ./css/tools/apiclient/.gitignore
       8 ./tools/wptrunner/.gitignore
       9 ./tools/third_party/atomicwrites/.gitignore
      11 ./tools/third_party/attrs/.gitignore
      11 ./tools/third_party/h2/.gitignore
      12 ./tools/.gitignore
      12 ./tools/third_party/py/.gitignore
      15 ./css/.gitignore
      32 ./.gitignore
      34 ./tools/third_party/more-itertools/.gitignore
      40 ./tools/third_party/pytest/.gitignore
      40 ./tools/wptserve/.gitignore
      58 ./tools/third_party/pluggy/.gitignore
      82 ./html/tools/.virtualenv/html5lib/.gitignore
      85 ./tools/third_party/html5lib/.gitignore
     482 total

[jgraham]

tools/ is irrelevant since that's hard skipped in the manifest code. So this is 30-40 lines of .gitignore rules outside the root.

[gsnedders]

As I posted in web-platform-tests/rfcs#19:

Can't we conspire to add the CSS build output to the ignore rules in the root?

I don't think there's any stopping us from merging all the non-root non-third-party .gitignore files, but it still won't behave as git does (as it won't obey .git/info/excludes or core.excludesFile, nor will we handle tracked files correctly), which still isn't ideal.

It's also potentially surprising in the case of Chromium or Servo where it won't obey top-level .gitignore files (of course, in mozilla-central it's even harder, given we'd need some way to process .hgignore).

I'm also in general against adding further constraints to how we do things in WPT which aren't generally needed. Really the only reason we reimplement this all ourselves is for projects that use Mercurial (mozilla-central) and SVN (WebKit). I'd also somewhat like to move to using Dulwich's implementation rather than maintain our own. :/

[jgraham]

Yes, I agree, but those are real constraints that we have to work with, even as we may wish it wasn't so.

I created #15952 to clean up some of the existing mess and unsurprisingly it turns out that this is mostly cruft; files in subdirectories ignoring things that are already ignored in the root file, or adding yet more editor cruft that certainly ought to be in the root. Based on that I honestly don't believe that allowing extra rules outside third party code is a feature for this project.

[Hexcles]

I stumbled upon another side effect of this.

If you ever run things like building docs/_build and then run wpt lint --all, you'd get tons of errors like this:

ERROR:lint:docs/_build/doctrees/index.doctree: docs/_build/doctrees/index.doctree matches an ignore filter in .gitignore - please add a .gitignore exception (IGNORED PATH)

This is because

wpt/tools/lint/lint.py

Lines 87 to 89 in a7b88cb

	def all_filesystem_paths(repo_root, subdir=None):
	# type: (str, Optional[str]) -> Iterable[str]
	path_filter = PathFilter(repo_root, extras=[str(".git/")])

would include paths that are ignored by .gitignore in subdirectories.

I'm tentatively promoting this to "roadmap".

I think we should either unify everything into the top-level .gitignore and add a lint, or figure out a way to support them -- I think the main challenge here is still that Mozilla Central is not a git repo?

[stephenmcgruer]

I'm tentatively promoting this to "roadmap".

Do you intend to work on this in the coming months? If not, I would suggest that it's still roadmap because we aren't actively working on it :)

[stephenmcgruer]

It seems like we're still not interested in working on this (based on lack of activity), so I'm downgrading it back to backlog.