Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tools,build: .gitignore tweaks #17380

Closed
wants to merge 3 commits into from
Closed

tools,build: .gitignore tweaks #17380

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a84f58d
tools,build: .gitignore tweaks
refack Nov 28, 2017
e7cde42
[fixup] refactor sub patterns to deps
refack Nov 30, 2017
869b204
[fixup] moar tweaks
refack Nov 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 40 additions & 31 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
# Whitelist dotfiles
.*
!deps/**/.*
!test/fixtures/**/.*
!tools/eslint/**/.*
!tools/doc/node_modules/**/.*
!deps/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this would trump #17363

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll check. exclusion rules seem to be tricky (CSS style), but I think these are stronger:

node/.gitignore

Lines 111 to 120 in a84f58d

# npm stuff
node_modules
package-lock.json
!/deps/npm/node_modules**
/deps/npm/node_modules/node-gyp/gyp/**/*.pyc
!/tools/eslint/node_modules**
!/tools/doc/node_modules**
!/test/fixtures/**
# not needed and causes issues for distro packagers
/deps/npm/node_modules/.bin/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it at the very least be at the bottom to avoid other rules combatting it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I saw while testing is that most of the rules below apply for /deps (only conflict is debug/release), so IMO it makes sense to keep it up here.

!.editorconfig
!.eslintignore
!.eslintrc.yaml
!.eslintrc.yml
!.gitattributes
!.github
!.gitignore
Expand All @@ -24,9 +22,8 @@ perf.data.old
tags
.lock-wscript
*.pyc
doc/api.xml
/doc/api.xml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean why was this not an absolute path? This PR makes it one right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made all relative paths absolute, since we have all sorts of path patterns deeper in the tree.

tmp/
test/tmp*/
iojs
iojs_g
node
Expand All @@ -36,15 +33,19 @@ node_g
icu_config.gypi
.eslintcache
node_trace.*.log
coverage/

/coverage/
/out

# various stuff that VC++ produces/uses
Debug/
!**/node_modules/debug/
!deps/v8/src/debug/
!/deps/v8/test/debugger/debug/
!/deps/v8/src/base/debug
!/deps/v8/src/debug
/deps/v8/src/debug/obj
deps/v8/src/inspector/Debug/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we are whitelisting all of deps/ above do we need to do this here?
Also should we be ignoring anything in the deps tree at all? this seems odd to me

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because the debug/ pattern re-ignores all paths that have it in anywhere in the path. So we need to do a checkerboard.

  1. ignore `/debug/
  2. exclude the 3 listed dirs explicitly
  3. but ignore build artifacts within them

Probably should try to upstream renaming those directories from debug to something like debugSrc

RE: the last one I'll double check.

Release/
!deps/v8/tools/release
!deps/npm/node_modules/bluebird/js/release
!doc/blog/**
*.sln
!nodemsi.sln
Expand All @@ -62,7 +63,6 @@ ipch/
*.VC.opendb
.vs/
.vscode/
/deps/v8/src/debug/obj
/*.exe

/config.mk
Expand All @@ -75,46 +75,55 @@ ipch/
/tools/msvs/genfiles/
/test/addons/??_*/
email.md
deps/v8-*
deps/icu
deps/icu*.zip
deps/icu*.tgz
deps/icu-tmp
./node_modules
/deps/v8-*
/deps/icu
/deps/icu*.zip
/deps/icu*.tgz
/deps/icu-tmp
android-toolchain/
.svn/

# generated by gyp on Windows
deps/openssl/openssl.props
deps/openssl/openssl.targets
deps/openssl/openssl.xml
/deps/openssl/openssl.props
/deps/openssl/openssl.targets
/deps/openssl/openssl.xml

# generated by gyp on android
/*.target.mk
/*.host.mk
deps/openssl/openssl.target.mk
deps/zlib/zlib.target.mk

# not needed and causes issues for distro packagers
deps/npm/node_modules/.bin/
/deps/openssl/openssl.target.mk
/deps/zlib/zlib.target.mk

# build/release artifacts
/*.tar.*
/*.pkg
/SHASUMS*.txt*

# test artifacts
tools/faketime
tools/remark-cli/node_modules
tools/remark-preset-lint-node/node_modules
/tools/faketime
icu_config.gypi
*.tap

# Xcode workspaces and project folders
*.xcodeproj
*.xcworkspace

# npm stuff
node_modules
package-lock.json
!/deps/npm/node_modules**
/deps/npm/node_modules/node-gyp/gyp/**/*.pyc
!/tools/eslint/node_modules**
!/tools/doc/node_modules**
!/test/fixtures/**
# not needed and causes issues for distro packagers
/deps/npm/node_modules/.bin/

# libuv book and GitHub template
deps/uv/.github/
deps/uv/docs/code/
deps/uv/docs/src/guide/
/deps/uv/.github/
/deps/uv/docs/code/
/deps/uv/docs/src/guide/

!/deps/v8/test/mjsunit/tools/profviz-test.log
!/deps/v8/test/mjsunit/tools/tickprocessor-test-func-info.log
!/deps/v8/test/mjsunit/tools/tickprocessor-test.log