rules_nodejs icon indicating copy to clipboard operation
rules_nodejs copied to clipboard

Wrong nodejs path on windows

Open flemminglundahl opened this issue 3 years ago • 4 comments

🐞 bug report

Affected Rule

rules_nodejs

Is this a regression?

Don't know. This is the first time we use nodejs like this.

Description

build_bazel_rules_nodejs throws an error on windows when running _yarn_install_impl.

🔬 Minimal Reproduction

Initialitze nodejs as done below.


http_archive(
    name = "vendored_node_windows_amd64",
    build_file_content = """exports_files(["node.exe"])""",
    sha256 = "2883e83ac3b1e1cb9a9bf65554043640849b39e86761e7c7ac50b664f42f20ff",
    strip_prefix = "node-v14.18.0-win-x64",
    urls = ["https://nodejs.org/dist/v14.18.0/node-v14.18.0-win-x64.zip"],
)


yarn_install(
    name = "npm",
    exports_directories_only = False,
    node_repository = "vendored_node",
    package_json = "//:package.json",
    yarn = "@vendored_yarn_1_22_0//:bin/yarn.js",
    yarn_lock = "//:yarn.lock",
)

🔥 Exception or Error

image

🌍 Your Environment

Operating System:

  
Windows 10
  

Output of bazel version:

  
5.0.0
  

Rules_nodejs version:

  
5.3.1
  

Anything else relevant? Patch has been created as a workaround: 0001-fix-wrong-nodejs-path-on-windows.zip

I intend to create a PR with my patch changes.

flemminglundahl avatar Apr 08 '22 13:04 flemminglundahl

I'm running into the same problem. I'm curious to know why the Node executable is expected to be bin/node.cmd when every Node package for Windows I've seen has the executable as node.exe (in the root directory). I assume it's an error, but I wonder if it's working as-is for anyone and if making the correction will break their code.

@flemminglundahl , thanks for the patch! Any plans to submit the PR soon? If not, do you mind if I submit it? I'd like to start using rules_nodejs as soon as possible, but this issue is blocking me.

davschne avatar May 16 '22 21:05 davschne

Feel free to submit a PR yourself @davschne. I've simply not had time for it yet.. Even though the patch works, I don't think that this fixes the root cause. I simply followed the stacktrace.

flemminglundahl avatar May 17 '22 09:05 flemminglundahl

Cool; will do. Patch seemed to work for me. Is there more to the issue?

davschne avatar May 17 '22 16:05 davschne

No. Not as far as I know

flemminglundahl avatar May 17 '22 16:05 flemminglundahl

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

github-actions[bot] avatar Nov 17 '22 02:11 github-actions[bot]

This issue was automatically closed because it went 30 days without any activity since it was labeled "Can Close?"

github-actions[bot] avatar Dec 18 '22 02:12 github-actions[bot]