Revert "Revert "js_run_devserver symlinks scoped node_modules to baze…

[alexeagle]

…l-out instead of runfiles (unscoped node_modules are already linked to bazel-out) (#1210)" (#1230)"

Roll-forward #1210 with fixes.
This reverts commit 06ad186.

[alexeagle]

FYI @gregjacobs in case you have time to help green this up so it can land again. Sorry for the churn!

[gregjacobs]

Hey @alexeagle, @gregmagolan.

Alex, thanks for starting this PR.

So this is interesting. I basically have the tool attribute of js_run_devserver pointing to a npm module (Jasmine). Here's the BUILD file (https://github.com/aspect-build/rules_js/pull/1233/files#diff-de3068ddcbc3e316d7dee1ba058141b53d7cf97b4565d67d7ce444578966aae5), and the relevant snippet:

load("@npm//js/private/test/js_run_devserver:jasmine/package_json.bzl", jasmine_bin = "bin")

#... 

js_run_devserver_test(
    name = "node_modules_symlink_to_execroot_test",
    args = ["node_modules_symlink_to_execroot.test.mjs"],
    chdir = "js/private/test/js_run_devserver",
    data = [
        # ...
    ],
    tool = ":jasmine",
)

jasmine_bin.jasmine_binary(   # <-- Seems to be the issue? See below
    name = "jasmine",
)

If I'm reading the below error message correctly, it seems that Jasmine itself isn't being uploaded for RBE? Is this perhaps because I'm linking to Jasmine from the "test" package itself rather than the root of the repo? Here's the error message:

Starting js_run_devserver //js/private/test/js_run_devserver:node_modules_symlink_to_execroot_test
Syncing...
3 files synced in 4 ms
Running '/mnt/engflow/worker/work/1/exec/bazel-out/k8-fastbuild/bin/js/private/test/js_run_devserver/node_modules_symlink_to_execroot_test.sh.runfiles/_main/js/private/test/js_run_devserver/jasmine.sh node_modules_symlink_to_execroot.test.mjs' in /tmp/js_run_devserver-gBvi0h/_main/js/private/test/js_run_devserver


FATAL: aspect_rules_js[js_binary]: the entry_point '/mnt/engflow/worker/work/1/exec/bazel-out/k8-fastbuild/bin/node_modules/.aspect_rules_js/jasmine@5.1.0/node_modules/jasmine/./bin/jasmine.js' not found
child tool process exited with code 1

Log link: https://github.com/aspect-build/rules_js/actions/runs/5969264193/job/16194870847?pr=1233

Thinking that this might be a bug in how the files are synced for RBE itself?

[gregmagolan]

I think this test just needs to be tagged no-remote-exec. js_run_devserver is meant to be bazel run locally so may make assumptions for local execution which don't make sense to test on RBE.

[gregjacobs]

@gregmagolan Was actually thinking the same about the use case for RBE in this context, so that does make sense :) Would you be able to add that tag real quick on this PR? Otherwise I'll have to open another one from my fork

[gregjacobs]

Hey @alexeagle, thanks for updating the PR! Looks like all RBE tests failed now though, seemingly with the same error message:

ERROR: The Build Event Protocol upload failed: All 4 retry attempts failed. UNAVAILABLE: finishConnect(..) failed: Connection timed out UNAVAILABLE: finishConnect(..) failed: Connection timed out

Is your BES service down perhaps? Maybe the jobs just need to be retried?

[alexeagle]

Yeah I did one retry. Maybe Engflow has an outage or maybe they tore down our cluster. Will retry again tomorrow.

[alexeagle]

It's still down. I'm going to see if we can quickly use a free tier BuildBuddy for this testing instead.

[gregjacobs]

@alexeagle Nice, looks like it passed now! Can we merge?