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

chore: remove usage of managed_directories #3466

Merged
merged 2 commits into from
Jun 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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