nodejs_binary generates .sh script with incorrect path to main module in some cases

[gdmfilippov]

Hi,
We are migrating to npm modules in gerrit. I tried to install some node modules to a separate npm workspace. It works, except some specific case.

To reproduce the problem you can use code from this patchset https://gerrit-review.googlesource.com/c/gerrit/+/250399/21 and run the following command:

bazel test --test_env=GERRIT_NOTEDB=ON --spawn_strategy=standalone --genrule_strategy=standalone --test_output errors --test_summary detailed --flaky_test_attempts 3 --test_verbose_timeout_warnings --build_tests_only --test_timeout 3600 //...

It fails with the error - can't find node_modules/rollup/dist/bin/rollup.
The problematic test is Documentation:check_licenses, but if you run
bazel test Documentation:check_licenses
then it works without a problem (and to reproduce the problem again after it you must run bazel clean or bazel clean --expunge).

The problematic nodejs_binary is here: tools/node_tools/BUILD, name="rollup-bin"

I debugged the problem a little bit. It seems, that .sh files generated by nodejs_binary has incorrect path in MAIN (in the launcher.sh template this path is here MAIN="TEMPLATED_script_path").
nodejs_binary set up MAIN to node_modules/rollup/... instead of tools/node_tools/node_modules/..... It seems, that nodejs_binary doesn't add correct path to subfolder when entry_point is inside some npm workspace.
If I change entry_point to any .js files inside tools/node_tools folder, then it works as expected and MAIN path is valid.

[alexeagle]

The rollup_bundle rule assumes a certain label to find the rollup binary:
https://github.com/bazelbuild/rules_nodejs/blob/master/docs/Rollup.md#rollup_bin

if rollup is somewhere else in your repo then you have to pass a value for this attribute (I guess you install a node_modules tree under tools/node_tools with some name, so it's @that_name//rollup/bin:rollup)
note that if you have a bunch of rollup_bundle calls, you might want a macro somewhere in your repo so you could define this value once for all of them

[gdmfilippov]

I tried it some times ago and I've tried it right now with the latest version - the same problem:
Cannot find module '..../bba865e207dc2c5fee9d2cd8722668a4/execroot/gerrit/node_modules/rollup/dist/bin/rollup (i removed path prefix)
I looked in the execroot directory and it seems, that the valid path must be ...gerrit/tools/node_tools/node_modules/...

I run bazel with --subcommands
The failed subcommand is (keep only relevant part here):
(cd .../bba865e207dc2c5fee9d2cd8722668a4/execroot/gerrit ... bazel-out/host/bin/external/tools_npm/rollup/bin/rollup.sh ...)

Note, then the rollup.sh bin script is executed inside execroot/gerrit folder.
Inside rollup.sh there are the following lines:
ALL_ARGS=( $NODE_REPOSITORY_ARGS "$@") for ARG in "${ALL_ARGS[@]:-}"; do case "$ARG" in --bazel_node_modules_manifest=*) MODULES_MANIFEST="${ARG#--bazel_node_modul\ es_manifest=}" ;; --nobazel_patch_module_resolver) MAIN="node_modules/rollup/dist/bin/rollup"

I suppose, that the MAIN has incorrect value. It must be tools/node_tools/...

[alexeagle]

in the execroot tools/node_tools/... only has your own sources, not the node_modules (that's symlinked into the workspace by the managed_directories feature)

the path to the MAIN should end up like external/tools_npm/node_modules/rollup/dist/bin/rollup

I pasted your tools/node_tools/BUILD into a minimal repro, and the binary runs for me. I dunno how to clone your gerrit state to try that way.

[gdmfilippov]

How did you run it in a minimal repro? As I wrote in the first message, if I run the rule by the name - it works.
But it doesn't work if I run it with wildcards from the root folder:
bazel test //...
(of course, this can be specific to gerrit, but not sure)

[alexeagle]

https://github.com/alexeagle/repro_1575

alexeagle@alexeagle-glaptop:~/Projects/repro_1575$ bazel test ...
INFO: Invocation ID: 0d8529f4-48d3-4660-9f65-e02d7bfedb43
INFO: Analyzed target //:rollup-bin (0 packages loaded, 0 targets configured).
INFO: Found 1 target and 0 test targets...
Target //:rollup-bin up-to-date:
  dist/bin/rollup-bin.sh
  dist/bin/rollup-bin_loader.js
  dist/bin/rollup-bin_require_patch.js
INFO: Elapsed time: 0.207s, Critical Path: 0.00s
INFO: 0 processes.
INFO: Build completed successfully, 1 total action
INFO: Build completed successfully, 1 total action
alexeagle@alexeagle-glaptop:~/Projects/repro_1575$ bazel run :rollup-bin 
INFO: Invocation ID: 12e8d52c-8404-4d77-9f20-b192180bb2c6
INFO: Analyzed target //:rollup-bin (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:rollup-bin up-to-date:
  dist/bin/rollup-bin.sh
  dist/bin/rollup-bin_loader.js
  dist/bin/rollup-bin_require_patch.js
INFO: Elapsed time: 0.120s, Critical Path: 0.00s
INFO: 0 processes.
INFO: Build completed successfully, 1 total action
INFO: Build completed successfully, 1 total action

 → stdout...
[!] Error: Could not resolve entry module ().
Error: Could not resolve entry module ().
    at error (node_modules/rollup/dist/shared/node-entry.js:5400:30)
    at node_modules/rollup/dist/shared/node-entry.js:12178:20
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async Promise.all (index 0)
    at async Promise.all (index 0)
    at async Promise.all (index 0)

[gdmfilippov]

It seems, you reproduced something else.
This is a small repro:
https://github.com/gdmfilippov/repro_1575 . The problem appears with --spawn-strategy local flag

$ npm run test_ok

> @ test_ok /usr/local/google/home/dmfilippov/abc/repro_1575
> bazel clean && bazel test ...

INFO: Starting clean (this may take a while). Consider using --async if the clean takes more than several minutes.
INFO: Analyzed 4 targets (41 packages loaded, 284 targets configured).
INFO: Found 2 targets and 2 test targets...
INFO: Elapsed time: 0.727s, Critical Path: 0.30s
INFO: 6 processes: 6 linux-sandbox.
INFO: Build completed successfully, 25 total actions
//tests:my_test_1                                                        PASSED in 0.1s
//tests:my_test_2                                                        PASSED in 0.1s

Executed 2 out of 2 tests: 2 tests pass.
INFO: Build completed successfully, 25 total actions

$ npm run test_fail

> @ test_fail /usr/local/google/home/dmfilippov/abc/repro_1575
> bazel clean && bazel test --spawn_strategy local ...

INFO: Starting clean (this may take a while). Consider using --async if the clean takes more than several minutes.
INFO: Analyzed 4 targets (41 packages loaded, 284 targets configured).
INFO: Found 2 targets and 2 test targets...
ERROR: /usr/local/google/home/dmfilippov/abc/repro_1575/tests/BUILD:10:1: Action tests/output_2.txt failed (Exit 1)
internal/modules/cjs/loader.js:797
    throw err;
    ^

Error: Cannot find module '/usr/local/google/home/dmfilippov/.cache/bazel/_bazel_dmfilippov/740f98108c004c897f3ff609afb7fe8d/execroot/gerrit/node_modules/npm_bin/bin/run_me_2'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:794:15)
    at Function.Module._load (internal/modules/cjs/loader.js:687:27)
    at Function.Module.runMain (internal/modules/cjs/loader.js:1025:10)
    at internal/main/run_main_module.js:17:11 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

[github-actions]

This issue has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

[github-actions]

This issue was automatically closed because it went two weeks without a reply since it was labeled "Can Close?"