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

refactor: always link #1440

Merged
merged 1 commit into from
Jun 16, 2020
Merged

refactor: always link #1440

merged 1 commit into from
Jun 16, 2020

Conversation

alexeagle
Copy link
Collaborator

discussed in meeting with @soldair - we can decouple enabling the linker/node-patches from disabling the custom monkey-patched _loader.js

We should be able to always enable the linker and node-patches, while keeping the loader.js for existing code that depends on it. require()s that aren't handled by loader.js will fall through to what the linker lays out on disk.

@gregmagolan gregmagolan mentioned this pull request Jan 11, 2020
12 tasks
@alexeagle alexeagle added this to the 2.0 milestone May 12, 2020
@alexeagle alexeagle force-pushed the link_all_2 branch 10 times, most recently from 99b5bac to 5e1b0cd Compare June 16, 2020 06:20
@alexeagle alexeagle marked this pull request as ready for review June 16, 2020 06:31
Copy link
Collaborator

@gregmagolan gregmagolan left a comment

Choose a reason for hiding this comment

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

🌯

We now run the linker always, regardless of the configuration of the patch_module_resolver option.

This creates some new flags to the launcher.sh
- `--bazel_patch_module_resolver`/`--nobazel_patch_module_resolver` turns on/off the monkey-patches for require(). Still defaults to true (behavior unchanged)
- `--nobazel_node_patches` disables the patches that make symlinks stay inside the execroot. Undocumented escape valve
- `--nobazel_run_linker` disables running the linker, since it is now on by default. Undocumented escape valve

As a later step we'll change default for patch_module_resolver to false (could be for 2.0 or 3.0 depending on how breaking it looks)
@alexeagle
Copy link
Collaborator Author

Will file an issue to investigate the 8-char truncated windows paths to see where it comes from.

@alexeagle alexeagle merged commit 7d070ff into bazel-contrib:master Jun 16, 2020
@alexeagle alexeagle deleted the link_all_2 branch June 16, 2020 20:17
alexeagle pushed a commit to alexeagle/rules_nodejs that referenced this pull request Dec 2, 2020
This turns off our monkey-patches for require() for nodejs_binary, nodejs_test, and macros like jasmine_node_test.
Note it was previously disabled for npm_package_bin and generated index.bzl binaries in bazel-contrib#1440

BREAKING CHANGE:
we no longer patch the require() function, instead you should rely on the linker to make node modules resolvable at the standard location
if this breaks you, try opting-in to the require patches with templated_args = ["--bazel_patch_module_resolver"]
alexeagle pushed a commit to alexeagle/rules_nodejs that referenced this pull request Dec 2, 2020
This turns off our monkey-patches for require() for nodejs_binary, nodejs_test, and macros like jasmine_node_test.
Note it was previously disabled for npm_package_bin and generated index.bzl binaries in bazel-contrib#1440

BREAKING CHANGE:
we no longer patch the require() function, instead you should rely on the linker to make node modules resolvable at the standard location
if this breaks you, try opting-in to the require patches with templated_args = ["--bazel_patch_module_resolver"]

Fixes bazel-contrib#2125
alexeagle pushed a commit to alexeagle/rules_nodejs that referenced this pull request Dec 2, 2020
This turns off our monkey-patches for require() for nodejs_binary, nodejs_test, and macros like jasmine_node_test.
Note it was previously disabled for npm_package_bin and generated index.bzl binaries in bazel-contrib#1440

BREAKING CHANGE:
we no longer patch the require() function, instead you should rely on the linker to make node modules resolvable at the standard location
if this breaks you, try opting-in to the require patches with templated_args = ["--bazel_patch_module_resolver"]

Fixes bazel-contrib#2125
alexeagle pushed a commit to alexeagle/rules_nodejs that referenced this pull request Dec 2, 2020
This turns off our monkey-patches for require() for nodejs_binary, nodejs_test, and macros like jasmine_node_test.
Note it was previously disabled for npm_package_bin and generated index.bzl binaries in bazel-contrib#1440

BREAKING CHANGE:
we no longer patch the require() function, instead you should rely on the linker to make node modules resolvable at the standard location
if this breaks you, try opting-in to the require patches with templated_args = ["--bazel_patch_module_resolver"]

Fixes bazel-contrib#2125
alexeagle pushed a commit to alexeagle/rules_nodejs that referenced this pull request Dec 2, 2020
This turns off our monkey-patches for require() for nodejs_binary, nodejs_test, and macros like jasmine_node_test.
Note it was previously disabled for npm_package_bin and generated index.bzl binaries in bazel-contrib#1440

BREAKING CHANGE:
we no longer patch the require() function, instead you should rely on the linker to make node modules resolvable at the standard location
if this breaks you, try opting-in to the require patches with templated_args = ["--bazel_patch_module_resolver"]

Fixes bazel-contrib#2125
alexeagle pushed a commit to alexeagle/rules_nodejs that referenced this pull request Dec 9, 2020
This turns off our monkey-patches for require() for nodejs_binary, nodejs_test, and macros like jasmine_node_test.
Note it was previously disabled for npm_package_bin and generated index.bzl binaries in bazel-contrib#1440

BREAKING CHANGE:
we no longer patch the require() function, instead you should rely on the linker to make node modules resolvable at the standard location
if this breaks you, try opting-in to the require patches with templated_args = ["--bazel_patch_module_resolver"]

Fixes bazel-contrib#2125
alexeagle pushed a commit to alexeagle/rules_nodejs that referenced this pull request Dec 11, 2020
This turns off our monkey-patches for require() for nodejs_binary, nodejs_test, and macros like jasmine_node_test.
Note it was previously disabled for npm_package_bin and generated index.bzl binaries in bazel-contrib#1440

BREAKING CHANGE:
By default, we no longer patch the require() function, instead you should rely on the linker to make node modules resolvable at the standard location
if this breaks you, the quickest fix is to flip the flag back on a nodejs_binary/nodejs_test/npm_package_bin with `templated_args = ["--bazel_patch_module_resolver"]`, see bazel-contrib#2344 as an example.
Another fix is to explicitly use our runfiles helper library, see bazel-contrib#2341 as an example.

Fixes bazel-contrib#2125
alexeagle pushed a commit to alexeagle/rules_nodejs that referenced this pull request Dec 11, 2020
This turns off our monkey-patches for require() for nodejs_binary, nodejs_test, and macros like jasmine_node_test.
Note it was previously disabled for npm_package_bin and generated index.bzl binaries in bazel-contrib#1440

BREAKING CHANGE:
By default, we no longer patch the require() function, instead you should rely on the linker to make node modules resolvable at the standard location
if this breaks you, the quickest fix is to flip the flag back on a nodejs_binary/nodejs_test/npm_package_bin with `templated_args = ["--bazel_patch_module_resolver"]`, see bazel-contrib#2344 as an example.
Another fix is to explicitly use our runfiles helper library, see bazel-contrib#2341 as an example.

Fixes bazel-contrib#2125
alexeagle pushed a commit that referenced this pull request Dec 11, 2020
This turns off our monkey-patches for require() for nodejs_binary, nodejs_test, and macros like jasmine_node_test.
Note it was previously disabled for npm_package_bin and generated index.bzl binaries in #1440

BREAKING CHANGE:
By default, we no longer patch the require() function, instead you should rely on the linker to make node modules resolvable at the standard location
if this breaks you, the quickest fix is to flip the flag back on a nodejs_binary/nodejs_test/npm_package_bin with `templated_args = ["--bazel_patch_module_resolver"]`, see #2344 as an example.
Another fix is to explicitly use our runfiles helper library, see #2341 as an example.

Fixes #2125
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.

4 participants