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

fix(builtin): fix for nodejs_binary entry point in bazel-out logic #1739

Conversation

gregmagolan
Copy link
Collaborator

@gregmagolan gregmagolan commented Mar 26, 2020

Pre-factor out of #1715

If entry point is a generated file in bazel-out nodejs_binary fails with

Error: Cannot find module '/private/var/tmp/_bazel/.../foobar.sh.runfiles/wksp/bazel-out/plat-form/bin/path/to/entry_point.js'

gregmagolan added a commit to gregmagolan/rules_nodejs that referenced this pull request Mar 26, 2020
@gregmagolan gregmagolan force-pushed the fix_windows_nodejs_binary_entry_point branch from ed6cbe3 to fa0b8fb Compare March 26, 2020 04:17
@gregmagolan gregmagolan changed the title fix(builtin): windows fix for nodejs_binary entry point logic fix(builtin): fix for nodejs_binary entry point logic Mar 26, 2020
@gregmagolan gregmagolan force-pushed the fix_windows_nodejs_binary_entry_point branch from fa0b8fb to ea5058f Compare March 26, 2020 04:20
If entry point is a generated file in bazel-out nodejs_binary fails with
```
Error: Cannot find module '/private/var/tmp/_bazel/.../foobar.sh.runfiles/wksp/bazel-out/plat-form/bin/path/to/entry_point.js'
```
@gregmagolan gregmagolan force-pushed the fix_windows_nodejs_binary_entry_point branch from ea5058f to 6899453 Compare March 26, 2020 04:55
@gregmagolan gregmagolan changed the title fix(builtin): fix for nodejs_binary entry point logic fix(builtin): fix for nodejs_binary entry point in bazel-out logic Mar 26, 2020
@@ -191,7 +191,10 @@ for ARG in "${ALL_ARGS[@]:-}"; do
case "$ARG" in
--bazel_node_modules_manifest=*) MODULES_MANIFEST="${ARG#--bazel_node_modules_manifest=}" ;;
--nobazel_patch_module_resolver)
MAIN="TEMPLATED_script_path"
declare MAIN=$(rlocation "TEMPLATED_entry_point_manifest_path")
Copy link
Collaborator

Choose a reason for hiding this comment

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

How did we never see this bug before?? We've had entry_point pointing to bazel-out for a while haven't we? or is this the first time we are trying to do that without the patched module resolver?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I know. I was surprised as well. We always had main.ts as the entry point of which was renamed to main.js by the rule & the parched resolver always found it.

@gregmagolan gregmagolan merged commit a6e29c2 into bazel-contrib:master Mar 26, 2020
alexeagle added a commit to jbedard/rules_nodejs that referenced this pull request Mar 27, 2020
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