Skip to content

Commit

Permalink
chore: remove usage of managed_directories (#3466)
Browse files Browse the repository at this point in the history
* chore: remove usage of managed_directories

We discouraged users from using this as of rnj 5.0, and now Bazel is removing the feature.

Fixes #3465

* Also remove symlink_node_modules = True usage
  • Loading branch information
alexeagle authored Jun 9, 2022
1 parent 66699b8 commit f96cfde
Show file tree
Hide file tree
Showing 13 changed files with 28 additions and 156 deletions.
7 changes: 0 additions & 7 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,6 @@

workspace(
name = "build_bazel_rules_nodejs",
managed_directories = {
# cypress_deps must be a managed directory to ensure it is downloaded before cypress_repositories is run.
"@cypress_deps": ["packages/cypress/test/node_modules"],
"@internal_npm_install_test_patches_npm_symlinked": ["internal/npm_install/test/patches_npm_symlinked/node_modules"],
"@internal_npm_install_test_patches_yarn_symlinked": ["internal/npm_install/test/patches_yarn_symlinked/node_modules"],
"@npm": ["node_modules"],
},
)

#
Expand Down
10 changes: 8 additions & 2 deletions docs/Built-ins.md
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,10 @@ Defaults to `True`

<h4 id="npm_install-symlink_node_modules">symlink_node_modules</h4>

(*Boolean*): Turn symlinking of node_modules on
(*Boolean*): Turn symlinking of node_modules on.

**Use of this feature is not recommended, as Bazel is removing `managed_directories`.
See https://github.com/bazelbuild/bazel/issues/15463

When False, the package manager will run in the external repository
created by this rule.
Expand Down Expand Up @@ -1537,7 +1540,10 @@ Defaults to `True`

<h4 id="yarn_install-symlink_node_modules">symlink_node_modules</h4>

(*Boolean*): Turn symlinking of node_modules on
(*Boolean*): Turn symlinking of node_modules on.

**Use of this feature is not recommended, as Bazel is removing `managed_directories`.
See https://github.com/bazelbuild/bazel/issues/15463

When False, the package manager will run in the external repository
created by this rule.
Expand Down
34 changes: 16 additions & 18 deletions docs/dependencies.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,22 @@ npm_install(

### symlink_node_modules and managed_directories

**The `managed_directories` feature is being removed from Bazel.**
See https://github.com/bazelbuild/bazel/issues/15463.
Using `managed_directories` is not recommended.

Furthermore, as of rules_nodejs 5.0, `symlink_node_modules` defaults to `False`. We've found that the benefits of using
`symlink_node_modules`, which allows Bazel to use a `node_modules` directory
that is in the user workspace, do not outweigh the downsides of the repository
rule not defining all of their inputs and of having to re-run the repository rule
if the user's `node_modules` folder is deleted. On persistent CI machines, that
will delete the `node_modules` folder when cleaning the clone between jobs, the
repository rule will run for every job when `symlink_node_modules` is enabled.
With `symlink_node_modules` disabled, the repository rule will only re-run if
its inputs change between jobs.

The remaining documentation in this section is for legacy usage only.

Set `symlink_node_modules` to `True` to configure `npm_install` and/or
`yarn_install` to install `node_modules` inside the user workspace and have
Bazel use the `node_modules` folder in the user workspace for the build via a
Expand Down Expand Up @@ -92,24 +108,6 @@ workspace(
)
```

As of rules_nodejs 5.0, `symlink_node_modules` defaults to `False` and using
`managed_directories` is not recommended. We've found that the benefits of using
`symlink_node_modules`, which allows Bazel to use a `node_modules` directory
that is in the user workspace, do not outweigh the downsides of the repository
rule not defining all of their inputs and of having to re-run the repository rule
if the user's `node_modules` folder is deleted. On persistent CI machines, that
will delete the `node_modules` folder when cleaning the clone between jobs, the
repository rule will run for every job when `symlink_node_modules` is enabled.
With `symlink_node_modules` disabled, the repository rule will only re-run if
its inputs change between jobs.

NB: On older versions of Bazel you may have to add the following flag to your
`.bazelrc` to enable managed directories.

```
common --experimental_allow_incremental_repository_updates
```

### yarn_install vs. npm_install

`yarn_install` is the preferred rule for setting up Bazel-managed dependencies for a number of reasons:
Expand Down
7 changes: 0 additions & 7 deletions e2e/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,6 @@ e2e_integration_test(
"4.0.x",
]]

e2e_integration_test(
name = "e2e_node_loader_no_preserve_symlinks",
# TODO: figure out why this fails on Windows since setting
# symlink_node_modules to False in the test WORKSPACE
tags = ["no-bazelci-windows"],
)

e2e_integration_test(
name = "e2e_node_loader_preserve_symlinks",
# TODO: figure out why this fails on Windows since setting
Expand Down
1 change: 0 additions & 1 deletion e2e/node_loader_no_preserve_symlinks/.bazelignore

This file was deleted.

3 changes: 0 additions & 3 deletions e2e/node_loader_no_preserve_symlinks/.bazelrc

This file was deleted.

13 changes: 0 additions & 13 deletions e2e/node_loader_no_preserve_symlinks/BUILD.bazel

This file was deleted.

32 changes: 0 additions & 32 deletions e2e/node_loader_no_preserve_symlinks/WORKSPACE

This file was deleted.

24 changes: 0 additions & 24 deletions e2e/node_loader_no_preserve_symlinks/node_loader_test.spec.js

This file was deleted.

12 changes: 0 additions & 12 deletions e2e/node_loader_no_preserve_symlinks/package.json

This file was deleted.

32 changes: 0 additions & 32 deletions e2e/node_loader_no_preserve_symlinks/yarn.lock

This file was deleted.

5 changes: 4 additions & 1 deletion internal/npm_install/npm_install.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,10 @@ NB: replaces specified are performed after preinstall_patches so if you are usin
to the source `package.json`.""",
),
"symlink_node_modules": attr.bool(
doc = """Turn symlinking of node_modules on
doc = """Turn symlinking of node_modules on.
**Use of this feature is not recommended, as Bazel is removing `managed_directories`.
See https://github.com/bazelbuild/bazel/issues/15463
When False, the package manager will run in the external repository
created by this rule.
Expand Down
4 changes: 0 additions & 4 deletions npm_deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,6 @@ filegroup(
patch_args = ["-p0"],
patch_tool = "patch",
post_install_patches = ["//internal/npm_install/test/patches_yarn_symlinked:semver+1.0.0.patch"],
symlink_node_modules = True,
yarn_lock = "//internal/npm_install/test/patches_yarn_symlinked:yarn.lock",
quiet = False,
)
Expand All @@ -482,7 +481,6 @@ filegroup(
patch_args = ["-p0"],
patch_tool = "patch",
post_install_patches = ["//internal/npm_install/test/patches_npm_symlinked:semver+1.0.0.patch"],
symlink_node_modules = True,
quiet = False,
)

Expand Down Expand Up @@ -546,8 +544,6 @@ filegroup(
name = "cypress_deps",
package_json = "//packages/cypress/test:package.json",
yarn_lock = "//packages/cypress/test:yarn.lock",
# TODO: get cypress rule working with symlink_node_modules = False
symlink_node_modules = True,
# TODO: get cypress rule working with exports_directories_only = True
exports_directories_only = False,
)
Expand Down

0 comments on commit f96cfde

Please sign in to comment.