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.

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

Don’t gitignore /internal/npm_install/test/golden/node_modules #711

Merged

Conversation

gregmagolan
Copy link
Collaborator

@gregmagolan gregmagolan commented Apr 21, 2019

This fixes a few pain points for local development.

  1. golden files like internal/npm_install/test/golden/node_modules/@angular/core/BUILD.bazel.golden are git ignored since they are in a node_modules folder. This PR fixes that by adding the negate pattern !/internal/npm_install/test/golden/node_modules to gitignore.

  2. clean scripts now recursively delete all node_modules folders in the repo since .bazelignore doesn't behave like .gitignore and its node_modules entry only tells bazel to ignore the root /node_modules folder. Each nested folder must be listed explicitly in .bazelignore.

Will file an issue about the .bazelignore behavior to Bazel about this soon as it will cause issues for all users of nodejs rules that have nested node_modules and multiple package.json files (especially once #704 lands).

@gregmagolan gregmagolan force-pushed the golden-files-gitignore-fix branch 3 times, most recently from 1e34db2 to 809899c Compare April 21, 2019 18:04
@alexeagle
Copy link
Collaborator

Why not change the .gitignore file to be rooted at root instead? Making it match .bazelignore semantics?

@gregmagolan
Copy link
Collaborator Author

Why not change the .gitignore file to be rooted at root instead? Making it match .bazelignore semantics?

Because there are a lot of nested node_modules folders in nested WORKSPACES as well as the ones generated by bazel run @nodejs//:yarn:

node_repositories(
    package_json = [
        "//:package.json",
        "@examples_program//:package.json",
        "//internal/npm_install/test:package/package.json",
    ],
)

... and with the user node_modules symlink change every yarn_install & npm_install rule will generate one as well. Even if they are all listed explicitly in both bazelignore and gitignore it would still be an issue when switching between branches if folders got moved around.

I submitted an issue to Bazel re: the bazelignore semantics. bazelbuild/bazel#8106. Ideally the semantics would match.

@gregmagolan
Copy link
Collaborator Author

Oh... I can use ! in .gitignore to accomplish this

@gregmagolan gregmagolan changed the title Move yarn_install/npm_install golden BUILD files under node_modules to a path that is not gitignored Don’t gitignore /internal/npm_install/test/golden/node_modules Apr 22, 2019
Also clean *all* node_modules folders in clean script included nested ones
@alexeagle alexeagle merged commit 584eedc into bazel-contrib:master Apr 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants