-
Notifications
You must be signed in to change notification settings - Fork 523
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
yarn_install & npm_install with managed directories can't handle deleted or manually regenerated node_modules folder #802
Comments
/cc @alexeagle |
Note: if this issue is a blocker for you, you can easily opt out of managed directories: https://github.com/bazelbuild/rules_nodejs/wiki#opting-out and wait for the fix to come in Bazel 0.26.1 |
Underlying Bazel issue here: bazelbuild/bazel#8487 |
…SPACE and bazel schematics This is needed to work-around issue: yarn_install & npm_install with managed directories can't handle deleted or manually regenerated node_modules folder [bazel-contrib/rules_nodejs#802] Underlying issue has been fixed in Bazel bazelbuild/bazel#8487 but hasn't landed in a release yet
…SPACE and bazel schematics This is needed to work-around issue: yarn_install & npm_install with managed directories can't handle deleted or manually regenerated node_modules folder [bazel-contrib/rules_nodejs#802] Underlying issue has been fixed in Bazel bazelbuild/bazel#8487 but hasn't landed in a release yet
…SPACE and bazel schematics This is needed to work-around issue: yarn_install & npm_install with managed directories can't handle deleted or manually regenerated node_modules folder [bazel-contrib/rules_nodejs#802] Underlying issue has been fixed in Bazel bazelbuild/bazel#8487 but hasn't landed in a release yet
…SPACE and bazel schematics (#30627) This is needed to work-around issue: yarn_install & npm_install with managed directories can't handle deleted or manually regenerated node_modules folder [bazel-contrib/rules_nodejs#802] Underlying issue has been fixed in Bazel bazelbuild/bazel#8487 but hasn't landed in a release yet PR Close #30627
Fix for this has landed in Bazel bazelbuild/bazel@6efc5b7 but did not make it into the 0.26.1 release. It may make it 0.27 which is currently in rc. If not, 0.28 is scheduled for July so should be 0.28.rc0. |
* Entry_point attribute of nodejs_binary & rollup_bundle is now a label * Nodejs rules 0.30.1 has new feature to symlink node_modules with yarn_install and bazel 0.26.0 includes new managed_directories feature which enables this * Symlinking of node_modules for yarn_install temporarily disabled (except for integration/bazel) until the fix for bazelbuild/bazel#8487 makes it into a future bazel release. This is needed to work-around issue: yarn_install & npm_install with managed directories can't handle deleted or manually regenerated node_modules folder [bazel-contrib/rules_nodejs#802]. Underlying issue has been fixed in Bazel bazelbuild/bazel#8487 but hasn't landed in a release yet
* entry_point attribute of nodejs_binary & rollup_bundle is now a label * nodejs rules 0.30.1 has new feature to symlink node_modules with yarn_install and bazel 0.26.0 includes new managed_directories feature which enables this * Symlinking of node_modules for yarn_install temporarily disabled (except for integration/bazel) until the fix for bazelbuild/bazel#8487 makes it into a future bazel release. This is needed to work-around issue: yarn_install & npm_install with managed directories can't handle deleted or manually regenerated node_modules folder [bazel-contrib/rules_nodejs#802]. Underlying issue has been fixed in Bazel bazelbuild/bazel#8487 but hasn't landed in a release yet
* entry_point attribute of nodejs_binary & rollup_bundle is now a label * nodejs rules 0.30.1 has new feature to symlink node_modules with yarn_install and bazel 0.26.0 includes new managed_directories feature which enables this * Symlinking of node_modules for yarn_install temporarily disabled (except for integration/bazel) until the fix for bazelbuild/bazel#8487 makes it into a future bazel release. This is needed to work-around issue: yarn_install & npm_install with managed directories can't handle deleted or manually regenerated node_modules folder [bazel-contrib/rules_nodejs#802]. Underlying issue has been fixed in Bazel bazelbuild/bazel#8487 but hasn't landed in a release yet PR Close #30965
…SPACE and bazel schematics (angular#30627) This is needed to work-around issue: yarn_install & npm_install with managed directories can't handle deleted or manually regenerated node_modules folder [bazel-contrib/rules_nodejs#802] Underlying issue has been fixed in Bazel bazelbuild/bazel#8487 but hasn't landed in a release yet PR Close angular#30627
So some good news and bad news: Bazel 0.27.0rc5, which is now mirrored in the npm package The case where you
The first of these two options is going to be faster so that your build remains incremental. |
This will fix the bazel-contrib#802 case of deleting and manually re-creating node_modules by no longer writing any files under node_modules. A few keys caveats: 1. The downside of this is that the root @npm BUILD file grows huge again as all files under node_modules need to be added to exports_files([...]). If Bazel had a method to export all files without explicitly listing them such as exports_files_all() that would solve this problem. 2. npm package that ships with BUILD files would now cause issues with labels used by yarn_install & npm_install generated targets. With this change, packages such as @bazel/typescript which install bazel workspaces now ship with _BUILD.bazel files. The bazel workspaces installer is already looking for files of this name since the current repo rule renames BUILD files for the same reason. To solve (2), I propose publishing a tool that could be added as a postinstall step to rename all BUILD files under node_modules and prefix them with _. This should not be a common case as most npm packages don't ship with BUILD files but there are a few that do. For example, rxjs < 6.5.0 had build files for a few releases to support building it from source.
This will fix the bazel-contrib#802 case of deleting and manually re-creating node_modules by no longer writing any files under node_modules. A few keys caveats: 1. The downside of this is that the root @npm BUILD file grows huge again as all files under node_modules need to be added to exports_files([...]). If Bazel had a method to export all files without explicitly listing them such as exports_files_all() that would solve this problem. 2. npm package that ships with BUILD files would now cause issues with labels used by yarn_install & npm_install generated targets. With this change, packages such as @bazel/typescript which install bazel workspaces now ship with _BUILD.bazel files. The bazel workspaces installer is already looking for files of this name since the current repo rule renames BUILD files for the same reason. To solve (2), I propose publishing a tool that could be added as a postinstall step to rename all BUILD files under node_modules and prefix them with _. This should not be a common case as most npm packages don't ship with BUILD files but there are a few that do. For example, rxjs < 6.5.0 had build files for a few releases to support building it from source.
This will fix the bazel-contrib#802 case of deleting and manually re-creating node_modules by no longer writing any files under node_modules. A few keys caveats: 1. The downside of this is that the root @npm BUILD file grows huge again as all files under node_modules need to be added to exports_files([...]). If Bazel had a method to export all files without explicitly listing them such as exports_files_all() that would solve this problem. 2. npm package that ships with BUILD files would now cause issues with labels used by yarn_install & npm_install generated targets. With this change, packages such as @bazel/typescript which install bazel workspaces now ship with _BUILD.bazel files. The bazel workspaces installer is already looking for files of this name since the current repo rule renames BUILD files for the same reason. To solve (2), I propose publishing a tool that could be added as a postinstall step to rename all BUILD files under node_modules and prefix them with _. This should not be a common case as most npm packages don't ship with BUILD files but there are a few that do. For example, rxjs < 6.5.0 had build files for a few releases to support building it from source.
This will fix the bazel-contrib#802 case of deleting and manually re-creating node_modules by no longer writing any files under node_modules. A few keys caveats: 1. The downside of this is that the root @npm BUILD file grows huge again as all files under node_modules need to be added to exports_files([...]). If Bazel had a method to export all files without explicitly listing them such as exports_files_all() that would solve this problem. 2. npm package that ships with BUILD files would now cause issues with labels used by yarn_install & npm_install generated targets. With this change, packages such as @bazel/typescript which install bazel workspaces now ship with _BUILD.bazel files. The bazel workspaces installer is already looking for files of this name since the current repo rule renames BUILD files for the same reason. To solve (2), I propose publishing a tool that could be added as a postinstall step to rename all BUILD files under node_modules and prefix them with _. This should not be a common case as most npm packages don't ship with BUILD files but there are a few that do. For example, rxjs < 6.5.0 had build files for a few releases to support building it from source.
@npm//node_modules/foobar:foobar.js labels changed to @npm//:node_modules/foobar/foobar.js with fix for bazel-contrib/rules_nodejs#802 also updates to rules_rass commit compatible with rules_nodejs 0.32.0
@npm//node_modules/foobar:foobar.js labels changed to @npm//:node_modules/foobar/foobar.js with fix for bazel-contrib/rules_nodejs#802 also updates to rules_rass commit compatible with rules_nodejs 0.32.0
Brings in ts_library fixes required to get angular/angular building after 0.32.0: typescript: exclude typescript lib declarations in node_module_library transitive_declarations typescript: remove override of @bazel/tsetse (+1 squashed commit) @npm//node_modules/foobar:foobar.js labels changed to @npm//:node_modules/foobar/foobar.js with fix for bazel-contrib/rules_nodejs#802 also updates to rules_rass commit compatible with rules_nodejs 0.32.0
Brings in ts_library fixes required to get angular/angular building after 0.32.0: typescript: exclude typescript lib declarations in node_module_library transitive_declarations typescript: remove override of @bazel/tsetse (+1 squashed commit) @npm//node_modules/foobar:foobar.js labels changed to @npm//:node_modules/foobar/foobar.js with fix for bazel-contrib/rules_nodejs#802 also updates to rules_rass commit compatible with rules_nodejs 0.32.0
Brings in ts_library fixes required to get angular/angular building after 0.32.0: typescript: exclude typescript lib declarations in node_module_library transitive_declarations typescript: remove override of @bazel/tsetse (+1 squashed commit) @npm//node_modules/foobar:foobar.js labels changed to @npm//:node_modules/foobar/foobar.js with fix for bazel-contrib/rules_nodejs#802 also updates to rules_rass commit compatible with rules_nodejs 0.32.0
Brings in ts_library fixes required to get angular/angular building after 0.32.0: typescript: exclude typescript lib declarations in node_module_library transitive_declarations typescript: remove override of @bazel/tsetse (+1 squashed commit) @npm//node_modules/foobar:foobar.js labels changed to @npm//:node_modules/foobar/foobar.js with fix for bazel-contrib/rules_nodejs#802 also updates to rules_rass commit compatible with rules_nodejs 0.32.0
Brings in ts_library fixes required to get angular/angular building after 0.32.0: typescript: exclude typescript lib declarations in node_module_library transitive_declarations typescript: remove override of @bazel/tsetse (+1 squashed commit) @npm//node_modules/foobar:foobar.js labels changed to @npm//:node_modules/foobar/foobar.js with fix for bazel-contrib/rules_nodejs#802 also updates to rules_rass commit compatible with rules_nodejs 0.32.0
Brings in ts_library fixes required to get angular/angular building after 0.32.0: typescript: exclude typescript lib declarations in node_module_library transitive_declarations typescript: remove override of @bazel/tsetse (+1 squashed commit) @npm//node_modules/foobar:foobar.js labels changed to @npm//:node_modules/foobar/foobar.js with fix for bazel-contrib/rules_nodejs#802 also updates to rules_rass commit compatible with rules_nodejs 0.32.0
Brings in ts_library fixes required to get angular/angular building after 0.32.0: typescript: exclude typescript lib declarations in node_module_library transitive_declarations typescript: remove override of @bazel/tsetse (+1 squashed commit) @npm//node_modules/foobar:foobar.js labels changed to @npm//:node_modules/foobar/foobar.js with fix for bazel-contrib/rules_nodejs#802 also updates to rules_rass commit compatible with rules_nodejs 0.32.0
Brings in ts_library fixes required to get angular/angular building after 0.32.0: typescript: exclude typescript lib declarations in node_module_library transitive_declarations typescript: remove override of @bazel/tsetse (+1 squashed commit) @npm//node_modules/foobar:foobar.js labels changed to @npm//:node_modules/foobar/foobar.js with fix for bazel-contrib/rules_nodejs#802 also updates to rules_rass commit compatible with rules_nodejs 0.32.0
Brings in ts_library fixes required to get angular/angular building after 0.32.0: typescript: exclude typescript lib declarations in node_module_library transitive_declarations typescript: remove override of @bazel/tsetse (+1 squashed commit) @npm//node_modules/foobar:foobar.js labels changed to @npm//:node_modules/foobar/foobar.js with fix for bazel-contrib/rules_nodejs#802 also updates to rules_rass commit compatible with rules_nodejs 0.32.0
Brings in ts_library fixes required to get angular/angular building after 0.32.0: typescript: exclude typescript lib declarations in node_module_library transitive_declarations typescript: remove override of @bazel/tsetse (+1 squashed commit) @npm//node_modules/foobar:foobar.js labels changed to @npm//:node_modules/foobar/foobar.js with fix for bazel-contrib/rules_nodejs#802 also updates to rules_rass commit compatible with rules_nodejs 0.32.0
Brings in ts_library fixes required to get angular/angular building after 0.32.0: typescript: exclude typescript lib declarations in node_module_library transitive_declarations typescript: remove override of @bazel/tsetse (+1 squashed commit) @npm//node_modules/foobar:foobar.js labels changed to @npm//:node_modules/foobar/foobar.js with fix for bazel-contrib/rules_nodejs#802 also updates to rules_rass commit compatible with rules_nodejs 0.32.0
Brings in ts_library fixes required to get angular/angular building after 0.32.0: typescript: exclude typescript lib declarations in node_module_library transitive_declarations typescript: remove override of @bazel/tsetse (+1 squashed commit) @npm//node_modules/foobar:foobar.js labels changed to @npm//:node_modules/foobar/foobar.js with fix for bazel-contrib/rules_nodejs#802 also updates to rules_rass commit compatible with rules_nodejs 0.32.0 PR Close angular#31325
Brings in ts_library fixes required to get angular/angular building after 0.32.0: typescript: exclude typescript lib declarations in node_module_library transitive_declarations typescript: remove override of @bazel/tsetse (+1 squashed commit) @npm//node_modules/foobar:foobar.js labels changed to @npm//:node_modules/foobar/foobar.js with fix for bazel-contrib/rules_nodejs#802 also updates to rules_rass commit compatible with rules_nodejs 0.32.0 PR Close #31326
PSA: https://github.com/angular/angular-bazel-example is finally updated to rules_nodejs 0.32.2. This shows working example of using symlinked node_modules with Angular & Bazel and the new managed directories features in Bazel 0.27.0. |
I'm still seeing this issue from time to time with rules_nodejs 0.32.2 and bazel 0.27.0:
The host with this issue has a bad network connection. The host runes |
@ashi009 we had the same problem with |
Yes. Thanks for pointing that out. That's a known issue as there is no way to express to managed_directories in your WORKSPACE file that a symlinked node_modules folder in an external repository is managed. Also, if symlinked that node_modules folder ends up being in an external repository anyway so there is not much gained by symlinking in that case. |
Canceling a build at its loading phase will also trigger this. The first build:
The second build:
My speculation is that |
Hi @ashi009 , thank you for reporting this.
I created an issue in Bazel project to investigate this behavior. |
Can this issue be re-opened? Or should we create a new one? |
New issue: bazelbuild/bazel#8487 |
@npm//node_modules/foobar:foobar.js labels changed to @npm//:node_modules/foobar/foobar.js with fix for bazel-contrib#802 See bazel-contrib#458 PiperOrigin-RevId: 254046674
@npm//node_modules/foobar:foobar.js labels changed to @npm//:node_modules/foobar/foobar.js with fix for bazel-contrib#802 See bazel-contrib#458 PiperOrigin-RevId: 254046674
In nodejs rules 0.30.0, the yarn_install & npm_install rules now use the new managed directories Bazel 0.26.0 feature so the bazel build will use the user's node_modules folder (eliminating the need to install node_modules twice: once locally and once for the bazel build).
Originally, when the managed director feature was in design phase, it handled the case where node_modules was deleted or recreated manually (via yarn or npm) and the repository rule would re-run and everything would work. However, some of this design and scope of the feature changed and the repository rule is no longer run if node_modules is deleted or manually recreated. It only runs after a full
bazel clean --expunge
or when one of its inputs such aspackage.json
oryarn.lock
orpackage-lock.json
changes.This means that if you manually
rm -rf
your node_modules folder the bazel build will no longer work. You'll see an error that looks like this:Running
yarn
ornpm install
at that point still will not fix the build as the repository rule needs to rerun to create the npm fine grained build targets.The fix for this will likely come in Bazel 0.26.1 so this is mainly a tracking issue here.
The text was updated successfully, but these errors were encountered: