Merge pull request #17 from Canva/christianscott-xplat_cache_hits

allow cache hits across platforms

internal/node/launcher.sh

@@ -171,10 +171,9 @@ if [[ "${BAZEL_NODE_PATCH_REQUIRE}" != /* ]] && [[ ! "${BAZEL_NODE_PATCH_REQUIRE
   export BAZEL_NODE_PATCH_REQUIRE=$(pwd)/${BAZEL_NODE_PATCH_REQUIRE}
 fi
 
-readonly repository_args=$(rlocation "TEMPLATED_repository_args")
 readonly lcov_merger_script=$(rlocation "TEMPLATED_lcov_merger_script")
 
-source $repository_args
+export NODE_REPOSITORY_ARGS="--preserve-symlinks"
 
 ARGS=()
 LAUNCHER_NODE_OPTIONS=($NODE_REPOSITORY_ARGS)

internal/node/node.bzl

@@ -242,7 +242,7 @@ fi
     #
     # Rules such as nodejs_image should use only ctx.toolchains["@build_bazel_rules_nodejs//toolchains/node:toolchain_type"].nodeinfo
     # when building the image as that will reflect the selected --platform.
-    node_tool_files = ctx.files._node[:]
+    node_tool_files = []
     node_tool_files.extend(ctx.toolchains["@build_bazel_rules_nodejs//toolchains/node:toolchain_type"].nodeinfo.tool_files)
 
     node_tool_files.append(ctx.file._link_modules_script)
@@ -259,7 +259,6 @@ fi
     runfiles.extend(ctx.files._bash_runfile_helper)
     runfiles.append(ctx.outputs.loader_script)
     runfiles.append(ctx.outputs.require_patch_script)
-    runfiles.append(ctx.file._repository_args)
 
     # First replace any instances of "$(rlocation " with "$$(rlocation " to preserve
     # legacy uses of "$(rlocation"
@@ -308,10 +307,9 @@ fi
         "TEMPLATED_loader_script": _to_manifest_path(ctx, ctx.outputs.loader_script),
         "TEMPLATED_modules_manifest": _to_manifest_path(ctx, node_modules_manifest),
         "TEMPLATED_node_patches_script": _to_manifest_path(ctx, ctx.file._node_patches_script),
-        "TEMPLATED_repository_args": _to_manifest_path(ctx, ctx.file._repository_args),
         "TEMPLATED_require_patch_script": _to_manifest_path(ctx, ctx.outputs.require_patch_script),
         "TEMPLATED_runfiles_helper_script": _to_manifest_path(ctx, ctx.file._runfile_helpers_main),
-        "TEMPLATED_vendored_node": "" if is_builtin else strip_external(ctx.file._node.path),
+        "TEMPLATED_vendored_node": ctx.toolchains["@build_bazel_rules_nodejs//toolchains/node:toolchain_type"].nodeinfo.target_tool_path,
     }
 
     substitutions["TEMPLATED_entry_point_manifest"] = _ts_to_js(_to_manifest_path(ctx, _get_entry_point_file(ctx)))
@@ -585,10 +583,6 @@ Predefined genrule variables are not supported in this context.
         default = Label("//internal/node:node_patches.js"),
         allow_single_file = True,
     ),
-    "_repository_args": attr.label(
-        default = Label("@nodejs//:bin/node_repo_args.sh"),
-        allow_single_file = True,
-    ),
     "_require_patch_template": attr.label(
         default = Label("//internal/node:require_patch.js"),
         allow_single_file = True,

internal/node/node_repositories.bzl

@@ -193,16 +193,6 @@ and `{filename}` with the matching entry from the `node_repositories` attribute.
             `bazel run @nodejs//:yarn_node_repositories` or `bazel run @nodejs//:npm_node_repositories install`.
             If you use bazel-managed dependencies, you should omit this attribute.""",
     ),
-    "preserve_symlinks": attr.bool(
-        default = True,
-        doc = """Turn on --node_options=--preserve-symlinks for nodejs_binary and nodejs_test rules.
-
-When this option is turned on, node will preserve the symlinked path for resolves instead of the default
-behavior of resolving to the real path. This means that all required files must be in be included in your
-runfiles as it prevents the default behavior of potentially resolving outside of the runfiles. For example,
-all required files need to be included in your node_modules filegroup. This option is desirable as it gives
-a stronger guarantee of hermeticity which is required for remote execution.""",
-    ),
     "vendored_node": attr.label(
         allow_single_file = True,
         doc = """the local path to a pre-installed NodeJS runtime.
@@ -469,11 +459,7 @@ def _prepare_node(repository_ctx):
     npm_script_relative = npm_script if repository_ctx.attr.vendored_node else paths.relativize(npm_script, "bin")
     yarn_script_relative = yarn_script if repository_ctx.attr.vendored_yarn else paths.relativize(yarn_script, "bin")
 
-    if repository_ctx.attr.preserve_symlinks:
-        node_args = "--preserve-symlinks"
-    else:
-        node_args = ""
-
+    node_args = "--preserve-symlinks"
     # The entry points for node for osx/linux and windows
     if not is_windows:
         # Sets PATH and runs the application
@@ -498,14 +484,6 @@ SET PATH=%SCRIPT_DIR%;%PATH%
 CALL "%SCRIPT_DIR%\\{node}" {args} %*
 """.format(node = node_bin_relative, args = node_args))
 
-    # Shell script to set repository arguments for node used by nodejs_binary & nodejs_test launcher
-    repository_ctx.file("bin/node_repo_args.sh", content = """#!/usr/bin/env bash
-# Immediately exit if any command fails.
-set -e
-# Generated by node_repositories.bzl
-export NODE_REPOSITORY_ARGS="{args}"
-""".format(args = node_args), executable = True)
-
     # The entry points for npm for osx/linux and windows
     # Runs npm using appropriate node entry point
     # --scripts-prepend-node-path is set to false since the correct paths
@@ -671,8 +649,7 @@ if %errorlevel% neq 0 exit /b %errorlevel%
 package(default_visibility = ["//visibility:public"])
 exports_files([
   "run_npm.sh.template",
-  "run_npm.bat.template",
-  "bin/node_repo_args.sh",{node_bin_export}{npm_bin_export}{npx_bin_export}{yarn_bin_export}
+  "run_npm.bat.template",{node_bin_export}{npm_bin_export}{npx_bin_export}{yarn_bin_export}
   "{node_entry}",
   "{npm_entry}",
   "{yarn_entry}",
@@ -738,7 +715,6 @@ package(default_visibility = ["//visibility:public"])
 # aliases for exports_files
 alias(name = "run_npm.sh.template", actual = "{node_repository}//:run_npm.sh.template")
 alias(name = "run_npm.bat.template", actual = "{node_repository}//:run_npm.bat.template")
-alias(name = "bin/node_repo_args.sh", actual = "{node_repository}//:bin/node_repo_args.sh")
 # aliases for other aliases
 alias(name = "node_bin", actual = "{node_repository}//:node_bin")
 alias(name = "npm_bin", actual = "{node_repository}//:npm_bin")

internal/npm_install/npm_install.bzl

@@ -738,6 +738,9 @@ def _yarn_install_impl(repository_ctx):
 
     yarn_args = []
 
+    if repository_ctx.attr.ignore_platform:
+        yarn_args.append("--ignore-platform")
+
     # Set frozen lockfile as default install to install the exact version from the yarn.lock
     # file. To perform an yarn install use the vendord yarn binary with:
     # `bazel run @nodejs//:yarn install` or `bazel run @nodejs//:yarn install -- -D <dep-name>`
@@ -905,6 +908,10 @@ to yarn so that the local cache is contained within the external repository.
             mandatory = True,
             allow_single_file = True,
         ),
+        "ignore_platform": attr.bool(
+            default = False,
+            doc = "Use the --ignore-platform flag for yarn install. Skips platform checks when installing packages.",
+        ),
     }),
     environ = PROXY_ENVVARS,
     doc = """Runs yarn install during workspace setup.