runYarn fails on Windows 10

[enriched]

I have been able to reproduce this on two Windows 10 systems so far attempting to build the same project. The project works on a mac.

When running the build the following error occurs:

error An unexpected error occurred: "https://registry.yarnpkg.com/babel-runtime/-/babel-runtime-6.26.0.tgz: ENOENT: no such file or directory, utime 'C:\\users\\admin\\_bazel_admin\\wjqdalcp\\execroot\\services_core\\node_modules_cache\\v1\\npm-babel-runtime-6.26.0-965c7058668e82b55d7bfe04ff2337bc8b5647fe\\core-js\\reflect\\get-metadata-keys.js'".
child_process.js:644
    throw err;
    ^

Error: Command failed: yarn --cwd bazel-out/x64_windows-fastbuild/bin/external/bazel_javascript/internal/packages_installed_dir --ignore-scripts --cache-folder ./node_modules_cache --frozen-lockfile
    at checkExecSyncError (child_process.js:601:13)
    at Object.execSync (child_process.js:641:13)
    at runYarn (C:\users\admin\_bazel_admin\wjqdalcp\external\bazel_javascript\internal\common\run_yarn.js:7:17)
    at Object.<anonymous> (C:\users\admin\_bazel_admin\wjqdalcp\external\bazel_javascript\internal\npm_packages\install.js:30:1)
    at Module._compile (module.js:635:30)
    at Object.Module._extensions..js (module.js:646:10)
    at Module.load (module.js:554:32)
    at tryModuleLoad (module.js:497:12)
    at Function.Module._load (module.js:489:3)
    at Function.Module.runMain (module.js:676:10)

I checked manually and C:\\users\\admin\\_bazel_admin\\wjqdalcp\\execroot\\services_core\\node_modules_cache\\v1\\npm-babel-runtime-6.26.0-965c7058668e82b55d7bfe04ff2337bc8b5647fe\\core-js\\reflect\\get-metadata-keys.js does exist.

Versions

Windows 10 Pro: 10.0.17134 Build 17134
bazel: 0.16.1 (current chocolatey package)
bazel-javascript: 0.0.23
build_bazel_rules_nodejs:  0.10.0
path nodejs: 10.9.0 (current chocolatey package)
path yarn: 1.9.4

I am going to keep trying to figure out how to fix this, but any pointers are appreciated.

[fwouts]

Thanks for reporting. I haven't been able to test on Windows before.

A few ideas:

• The --cache-folder argument is actually a hack that was required to get Yarn running correctly with Bazel. See the code here. Try removing it and see what happens?
• Could there be issues with symbolic links behaving differently on Windows?

[enriched]

Do you think that this should be using @nodejs://:yarn rather than the one installed on the path? Or is the one on the path being used so that it uses the same version that the developer currently is?

[fwouts]

It should be using `@nodejs://:yarn`, the priority is consistency across developers of a team. I didn't realise it wasn't the case!

[enriched]

I just tried it out and I think that I was wrong. It seems to be using @nodejs//:yarn even though it isn't getting passed its location. I think that I don't fully understand if bazel modifies the PATH of actions created through actions.run or if @nodejs//:node modifies its the PATH somehow.

[fwouts]

It's indeed not obvious. The key is that use_default_shell_env is not set, so the user's default PATH is not used. I believe PATH is set by @nodejs//:node, check out this code in particular.

See also the documentation at https://docs.bazel.build/versions/master/skylark/lib/actions.html#run

[enriched]

I was able to get past the caching issue by resolving the cache path (see this commit).

But now I am running into this error (still on windows):

server_lib_transpilation_src failed (Exit 255): node.cmd failed: error executing command
  cd C:/users/admin/_bazel_admin/wjqdalcp/execroot/services_core
  SET NODE_PATH=bazel-out/x64_windows-fastbuild/bin/external/bazel_javascript/internal/packages_installed_dir/node_modules
  external/nodejs/bin/node.cmd external/bazel_javascript/internal/ts_library/create_full_src.js //services/products/server:server_lib bazel-out/x64_windows-fastbuild/bin/packages_installed_dir tsconfig.json  services/products/server/BUILD.bazel|services/products/server/createDb.ts|services/products/server/products.ts|services/products/server/server.ts bazel-out/x64_windows-fastbuild/bin/services/products/server/server_lib_transpilation_src
'services' is not recognized as an internal or external command,
operable program or batch file.

[fwouts]

I think Node and/or Windows' shell might be confused by the //services/products/server:server_lib you have in the second argument to node.cmd? Maybe double quotes would fix it?

[enriched]

Is that supposed to be expanded somehow before going to create_full_src.js?

[fwouts]

No, it's put into a targetLabel variable that is only used for error reporting when compiling, e.g. the following in js_library/create_full_src.js:

// Copy source code.
for (const src of srcs) {
  if (!src) {
    continue;
  }
  if (!fs.existsSync(src)) {
    console.error(`
Missing file ${src} required by ${targetLabel}.
`);
    process.exit(1);
  }
  const destinationFilePath = path.join(destinationDir, src);
  safeSymlink(src, destinationFilePath);
}

[enriched]

I think that this is a problem with the sources that are joined with | on windows if I run:

echo Hi|there

I get a similar error:

'there' is not recognized as an internal or external command,
operable program or batch file.

[fwouts]

Nice find. Let's use another separator?

[enriched]

I'm working on wrapping the arguments with quotes. If I do that windows will leave it alone.

The scripts need to be changed a little though, because in windows the shell doesn't consume the quotes, it passes them through.

[enriched]

@fwouts So I probably pulled more threads than I should have on this one. But in researching actions.run it seems like we should have a param file. So to facilitate that I started creating a JsContext provider to wrap a rule context similar to how rules_go does it. Do you think this is a good way to go?

Also, apologies if some of this stuff is a little incomplete. Still very much a WIP and your feedback is appreciated.

[fwouts]

What do you mean by a "param file"?

Either way, the JsContext provider seems like a good idea! It sounds like it would be useful on its own as a first PR to clean up the existing code?

[enriched]

The Args.use_param_file() option allows args objects created by ctx.actions.args() to overflow into a param file if they will be larger than the systems params limits. I think they are also more performant because they allow you to add depsets as parameters.

And yeah, I should probably break this work into a set of smaller PRs.