diff --git a/internal/node/launcher.sh b/internal/node/launcher.sh index 436546d731..9d1e8df4e5 100644 --- a/internal/node/launcher.sh +++ b/internal/node/launcher.sh @@ -337,7 +337,7 @@ else # Entry point is the user-supplied script MAIN="${PWD}/"TEMPLATED_entry_point_execroot_path # TODO: after we link-all-bins we should not need this extra lookup - if [[ ! -f "$MAIN" ]]; then + if [[ ! -e "$MAIN" ]]; then if [ "$FROM_EXECROOT" = true ]; then MAIN="$EXECROOT/"TEMPLATED_entry_point_execroot_path else diff --git a/internal/node/node.bzl b/internal/node/node.bzl index c84a007be2..72c961102b 100644 --- a/internal/node/node.bzl +++ b/internal/node/node.bzl @@ -139,12 +139,21 @@ def _to_manifest_path(ctx, file): return ctx.workspace_name + "/" + file.short_path def _to_execroot_path(ctx, file): + # Check for special case of external//node_modules or + # bazel-out//bin/external//node_modules. + # TODO: This assumes we are linking this to the root node_modules which is not always the case + # since the linker was updated to support linking to sub-directories since this special case was + # added parts = file.path.split("/") - if parts[0] == "external": - if parts[2] == "node_modules": - # external/npm/node_modules -> node_modules/foo - # the linker will make sure we can resolve node_modules from npm - return "/".join(parts[2:]) + if file.is_directory and not file.is_source: + # Strip the output root to handle the case of a TreeArtifact exported by js_library with + # exports directories only. Since a TreeArtifact is generated by an action, it resides + # inside bazel-out directory. + parts = parts[3:] + if len(parts) > 3 and parts[0] == "external" and parts[2] == "node_modules": + # Transform external/npm/node_modules/foo to node_modules/foo. + # The linker will make sure we can resolve node_modules from npm. + return "/".join(parts[2:]) return file.path diff --git a/internal/node/test/instanceof_test_execroot/BUILD.bazel b/internal/node/test/instanceof_test_execroot/BUILD.bazel new file mode 100644 index 0000000000..b23912e26d --- /dev/null +++ b/internal/node/test/instanceof_test_execroot/BUILD.bazel @@ -0,0 +1,19 @@ +load("@npm//pkg_with_bin:index.bzl", "instanceof_execroot_test") +load( + "@npm_directory_artifacts//pkg_with_bin:index.bzl", + instanceof_execroot_test_exports_directories = "instanceof_execroot_test", +) + +instanceof_execroot_test( + name = "test", + data = [ + "//internal/node/test/instanceof_test_execroot/lib", + ], +) + +instanceof_execroot_test_exports_directories( + name = "test_exports_directories_only", + data = [ + "//internal/node/test/instanceof_test_execroot/lib", + ], +) diff --git a/internal/node/test/instanceof_test_execroot/index.bzl b/internal/node/test/instanceof_test_execroot/index.bzl new file mode 100644 index 0000000000..e69de29bb2 diff --git a/internal/node/test/instanceof_test_execroot/lib/BUILD.bazel b/internal/node/test/instanceof_test_execroot/lib/BUILD.bazel new file mode 100644 index 0000000000..518e76efbc --- /dev/null +++ b/internal/node/test/instanceof_test_execroot/lib/BUILD.bazel @@ -0,0 +1,16 @@ +load("//:index.bzl", "js_library") + +package(default_visibility = ["//internal/node/test/instanceof_test_execroot:__subpackages__"]) + +js_library( + name = "lib", + # required by tools/npm_packages/pkg_with_bin as third-party dep + package_name = "instanceof_test_execroot_lib", + srcs = [ + "index.js", + "package.json", + ], + deps = [ + "@npm//node_resolve_main", + ], +) diff --git a/internal/node/test/instanceof_test_execroot/lib/index.js b/internal/node/test/instanceof_test_execroot/lib/index.js new file mode 100644 index 0000000000..94ce9184d4 --- /dev/null +++ b/internal/node/test/instanceof_test_execroot/lib/index.js @@ -0,0 +1,20 @@ +// This file will fail when one dependency is resolved from two distinct node_modules +// mostly likely to be one inside runfiles and other from bazel-out +// See: https://github.com/bazelbuild/rules_nodejs/pull/3380 for more. +const assert = require("assert"); + +const resolved_by_lib = require.resolve("node_resolve_main"); +const resolved_by_pkg_with_bin = globalThis["node_resolve_main_resolved_path_by_pkg_with_bin"] +assert.equal( + resolved_by_lib, + resolved_by_pkg_with_bin, + ` +Expected to resolve package "node_resolve_main" from the same node_modules but + +tools/npm_packages/pkg_with_bin resolved it from ${resolved_by_pkg_with_bin} + +internal/node/test/instanceof_test_execroot/lib resolved it from ${resolved_by_lib} + +See: https://github.com/bazelbuild/rules_nodejs/pull/3380 for context. +` +); \ No newline at end of file diff --git a/internal/node/test/instanceof_test_execroot/lib/package.json b/internal/node/test/instanceof_test_execroot/lib/package.json new file mode 100644 index 0000000000..84582c4845 --- /dev/null +++ b/internal/node/test/instanceof_test_execroot/lib/package.json @@ -0,0 +1 @@ +{"name": "instanceof_test_execroot_lib"} \ No newline at end of file diff --git a/npm_deps.bzl b/npm_deps.bzl index ca247d66e8..2e7f8a8a97 100644 --- a/npm_deps.bzl +++ b/npm_deps.bzl @@ -42,6 +42,8 @@ def npm_deps(): "//:tools/npm_packages/node_resolve_nested_main/nested/package.json", "//:tools/npm_packages/testy/index.js", "//:tools/npm_packages/testy/package.json", + "//:tools/npm_packages/pkg_with_bin/main.js", + "//:tools/npm_packages/pkg_with_bin/package.json", ], environment = { "SOME_USER_ENV": "yarn is great!", @@ -107,6 +109,8 @@ js_library( "//:tools/npm_packages/node_resolve_nested_main/nested/package.json", "//:tools/npm_packages/testy/index.js", "//:tools/npm_packages/testy/package.json", + "//:tools/npm_packages/pkg_with_bin/main.js", + "//:tools/npm_packages/pkg_with_bin/package.json", ], environment = { "SOME_USER_ENV": "yarn is great!", diff --git a/package.json b/package.json index d51f642bd7..0d8b80769c 100644 --- a/package.json +++ b/package.json @@ -77,6 +77,7 @@ "node_resolve_main": "file:./tools/npm_packages/node_resolve_main", "node_resolve_main_2": "file:./tools/npm_packages/node_resolve_main_2", "node_resolve_nested_main": "file:./tools/npm_packages/node_resolve_nested_main", + "pkg_with_bin": "file:./tools/npm_packages/pkg_with_bin", "npm": "6.13.7", "patch-package": "^6.2.2", "protobufjs": "6.8.8", diff --git a/tools/npm_packages/pkg_with_bin/main.js b/tools/npm_packages/pkg_with_bin/main.js new file mode 100644 index 0000000000..7eb78154a8 --- /dev/null +++ b/tools/npm_packages/pkg_with_bin/main.js @@ -0,0 +1,3 @@ +globalThis["node_resolve_main_resolved_path_by_pkg_with_bin"] = require.resolve("node_resolve_main"); +require("instanceof_test_execroot_lib"); +// See: internal/node/test/instanceof_test_execroot \ No newline at end of file diff --git a/tools/npm_packages/pkg_with_bin/package.json b/tools/npm_packages/pkg_with_bin/package.json new file mode 100644 index 0000000000..0d1830eac1 --- /dev/null +++ b/tools/npm_packages/pkg_with_bin/package.json @@ -0,0 +1,10 @@ +{ + "version": "0.0.1", + "description": "See: internal/node/test/instanceof_test_execroot", + "bin": { + "instanceof_execroot": "main.js" + }, + "peerDependencies": { + "node_resolve_main": "*" + } +} \ No newline at end of file diff --git a/yarn.lock b/yarn.lock index 6b1db7ac1e..c06c487b42 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8740,6 +8740,9 @@ pkg-dir@^4.2.0: dependencies: find-up "^4.0.0" +"pkg_with_bin@file:./tools/npm_packages/pkg_with_bin": + version "0.0.1" + please-upgrade-node@^3.1.1: version "3.2.0" resolved "https://registry.yarnpkg.com/please-upgrade-node/-/please-upgrade-node-3.2.0.tgz#aeddd3f994c933e4ad98b99d9a556efa0e2fe942"