From e7c9b3d97bd7312d6ef076ad433a2d74b6a61552 Mon Sep 17 00:00:00 2001 From: Derek Cormier Date: Wed, 22 Jun 2022 22:27:48 -0700 Subject: [PATCH] fix(builtin): remove unnecessary loader script --- .bazelrc | 4 ++-- e2e/BUILD.bazel | 9 ++++++++ e2e/esm_no_linker/.bazelignore | 2 ++ e2e/esm_no_linker/.bazelrc | 1 + e2e/esm_no_linker/BUILD | 10 +++++++++ e2e/esm_no_linker/WORKSPACE | 22 +++++++++++++++++++ e2e/esm_no_linker/main.mjs | 2 ++ e2e/esm_no_linker/package.json | 6 ++++++ e2e/esm_no_linker/yarn.lock | 8 +++++++ internal/node/launcher.sh | 35 +++++++++++++++--------------- internal/node/loader.cjs | 39 ---------------------------------- internal/node/node.bzl | 21 ------------------ 12 files changed, 79 insertions(+), 80 deletions(-) create mode 100644 e2e/esm_no_linker/.bazelignore create mode 100644 e2e/esm_no_linker/.bazelrc create mode 100644 e2e/esm_no_linker/BUILD create mode 100644 e2e/esm_no_linker/WORKSPACE create mode 100644 e2e/esm_no_linker/main.mjs create mode 100644 e2e/esm_no_linker/package.json create mode 100644 e2e/esm_no_linker/yarn.lock delete mode 100644 internal/node/loader.cjs diff --git a/.bazelrc b/.bazelrc index a4635fa8a1..c41d81ac55 100644 --- a/.bazelrc +++ b/.bazelrc @@ -5,8 +5,8 @@ import %workspace%/common.bazelrc # This lets us glob() up all the files inside the examples to make them inputs to tests # To update these lines, just run `yarn bazel:update-deleted-packages` # (Note, we cannot use common --deleted_packages because the bazel version command doesn't support it) -build --deleted_packages=e2e/bazel_managed_deps,e2e/bazel_run_chdir,e2e/bazel_run_chdir/subfolder,e2e/concatjs_devserver,e2e/concatjs_devserver/genrule,e2e/concatjs_devserver/subpackage,e2e/coverage,e2e/fine_grained_symlinks,e2e/jasmine,e2e/node_loader_no_preserve_symlinks,e2e/node_loader_preserve_symlinks,e2e/nodejs_host,e2e/nodejs_image,e2e/nodejs_image/foolib,e2e/packages,e2e/symlinked_node_modules_npm,e2e/symlinked_node_modules_yarn,e2e/typescript,e2e/webapp,examples/angular,examples/angular/e2e,examples/angular/src,examples/angular/src/app,examples/angular/src/app/hello-world,examples/angular/src/app/home,examples/angular/src/app/todos,examples/angular/src/app/todos/reducers,examples/angular/src/assets,examples/angular/src/lib/shorten,examples/angular/src/shared/material,examples/angular/tools,examples/angular_bazel_architect,examples/angular_bazel_architect/projects/frontend-lib,examples/app,examples/app/styles,examples/app/test,examples/closure,examples/create-react-app,examples/cypress,examples/esbuild,examples/esbuild/src,examples/from_source,examples/jest,examples/jest/ts,examples/jest/ts/src,examples/jest/ts/test,examples/kotlin,examples/nestjs,examples/nestjs/src,examples/parcel,examples/protobufjs,examples/react_webpack,examples/toolchain,examples/vendored_node_and_yarn,examples/vendored_node_and_yarn/toolchains,examples/vue,examples/vue/src,examples/vue/src/components/HelloWorld,examples/web_testing,examples/webapp,examples/worker -query --deleted_packages=e2e/bazel_managed_deps,e2e/bazel_run_chdir,e2e/bazel_run_chdir/subfolder,e2e/concatjs_devserver,e2e/concatjs_devserver/genrule,e2e/concatjs_devserver/subpackage,e2e/coverage,e2e/fine_grained_symlinks,e2e/jasmine,e2e/node_loader_no_preserve_symlinks,e2e/node_loader_preserve_symlinks,e2e/nodejs_host,e2e/nodejs_image,e2e/nodejs_image/foolib,e2e/packages,e2e/symlinked_node_modules_npm,e2e/symlinked_node_modules_yarn,e2e/typescript,e2e/webapp,examples/angular,examples/angular/e2e,examples/angular/src,examples/angular/src/app,examples/angular/src/app/hello-world,examples/angular/src/app/home,examples/angular/src/app/todos,examples/angular/src/app/todos/reducers,examples/angular/src/assets,examples/angular/src/lib/shorten,examples/angular/src/shared/material,examples/angular/tools,examples/angular_bazel_architect,examples/angular_bazel_architect/projects/frontend-lib,examples/app,examples/app/styles,examples/app/test,examples/closure,examples/create-react-app,examples/cypress,examples/esbuild,examples/esbuild/src,examples/from_source,examples/jest,examples/jest/ts,examples/jest/ts/src,examples/jest/ts/test,examples/kotlin,examples/nestjs,examples/nestjs/src,examples/parcel,examples/protobufjs,examples/react_webpack,examples/toolchain,examples/vendored_node_and_yarn,examples/vendored_node_and_yarn/toolchains,examples/vue,examples/vue/src,examples/vue/src/components/HelloWorld,examples/web_testing,examples/webapp,examples/worker +build --deleted_packages=e2e/bazel_managed_deps,e2e/bazel_run_chdir,e2e/bazel_run_chdir/subfolder,e2e/concatjs_devserver,e2e/concatjs_devserver/genrule,e2e/concatjs_devserver/subpackage,e2e/coverage,e2e/esm_no_linker,e2e/fine_grained_symlinks,e2e/jasmine,e2e/node_loader_preserve_symlinks,e2e/nodejs_host,e2e/nodejs_image,e2e/nodejs_image/foolib,e2e/packages,e2e/symlinked_node_modules_npm,e2e/symlinked_node_modules_yarn,e2e/typescript,e2e/webapp,examples/angular,examples/angular/e2e,examples/angular/src,examples/angular/src/app,examples/angular/src/app/hello-world,examples/angular/src/app/home,examples/angular/src/app/todos,examples/angular/src/app/todos/reducers,examples/angular/src/assets,examples/angular/src/lib/shorten,examples/angular/src/shared/material,examples/angular/tools,examples/angular_bazel_architect,examples/angular_bazel_architect/projects/frontend-lib,examples/app,examples/app/styles,examples/app/test,examples/closure,examples/create-react-app,examples/cypress,examples/esbuild,examples/esbuild/src,examples/from_source,examples/jest,examples/jest/ts,examples/jest/ts/src,examples/jest/ts/test,examples/kotlin,examples/nestjs,examples/nestjs/src,examples/parcel,examples/protobufjs,examples/react_webpack,examples/toolchain,examples/vendored_node_and_yarn,examples/vendored_node_and_yarn/toolchains,examples/vue,examples/vue/src,examples/vue/src/components/HelloWorld,examples/web_testing,examples/webapp,examples/worker +query --deleted_packages=e2e/bazel_managed_deps,e2e/bazel_run_chdir,e2e/bazel_run_chdir/subfolder,e2e/concatjs_devserver,e2e/concatjs_devserver/genrule,e2e/concatjs_devserver/subpackage,e2e/coverage,e2e/esm_no_linker,e2e/fine_grained_symlinks,e2e/jasmine,e2e/node_loader_preserve_symlinks,e2e/nodejs_host,e2e/nodejs_image,e2e/nodejs_image/foolib,e2e/packages,e2e/symlinked_node_modules_npm,e2e/symlinked_node_modules_yarn,e2e/typescript,e2e/webapp,examples/angular,examples/angular/e2e,examples/angular/src,examples/angular/src/app,examples/angular/src/app/hello-world,examples/angular/src/app/home,examples/angular/src/app/todos,examples/angular/src/app/todos/reducers,examples/angular/src/assets,examples/angular/src/lib/shorten,examples/angular/src/shared/material,examples/angular/tools,examples/angular_bazel_architect,examples/angular_bazel_architect/projects/frontend-lib,examples/app,examples/app/styles,examples/app/test,examples/closure,examples/create-react-app,examples/cypress,examples/esbuild,examples/esbuild/src,examples/from_source,examples/jest,examples/jest/ts,examples/jest/ts/src,examples/jest/ts/test,examples/kotlin,examples/nestjs,examples/nestjs/src,examples/parcel,examples/protobufjs,examples/react_webpack,examples/toolchain,examples/vendored_node_and_yarn,examples/vendored_node_and_yarn/toolchains,examples/vue,examples/vue/src,examples/vue/src/components/HelloWorld,examples/web_testing,examples/webapp,examples/worker # Mock versioning command to test the --stamp behavior build --workspace_status_command="echo BUILD_SCM_VERSION 1.2.3" diff --git a/e2e/BUILD.bazel b/e2e/BUILD.bazel index e698ec8715..b8cde486d3 100644 --- a/e2e/BUILD.bazel +++ b/e2e/BUILD.bazel @@ -178,3 +178,12 @@ e2e_integration_test( # TODO: figure out why this fails on Windows tags = ["no-bazelci-windows"], ) + +e2e_integration_test( + name = "e2e_esm_no_linker", + bazel_commands = [ + "run //:binary", + ], + # TODO: figure out why this fails on Windows + tags = ["no-bazelci-windows"], +) diff --git a/e2e/esm_no_linker/.bazelignore b/e2e/esm_no_linker/.bazelignore new file mode 100644 index 0000000000..de4d1f007d --- /dev/null +++ b/e2e/esm_no_linker/.bazelignore @@ -0,0 +1,2 @@ +dist +node_modules diff --git a/e2e/esm_no_linker/.bazelrc b/e2e/esm_no_linker/.bazelrc new file mode 100644 index 0000000000..3431057af6 --- /dev/null +++ b/e2e/esm_no_linker/.bazelrc @@ -0,0 +1 @@ +import %workspace%/../../common.bazelrc diff --git a/e2e/esm_no_linker/BUILD b/e2e/esm_no_linker/BUILD new file mode 100644 index 0000000000..c92432b6e9 --- /dev/null +++ b/e2e/esm_no_linker/BUILD @@ -0,0 +1,10 @@ +load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary") + +nodejs_binary( + name = "binary", + data = [ + "@npm//typescript", + ], + entry_point = ":main.mjs", + templated_args = ["--nobazel_run_linker"], +) diff --git a/e2e/esm_no_linker/WORKSPACE b/e2e/esm_no_linker/WORKSPACE new file mode 100644 index 0000000000..846d5f8ff9 --- /dev/null +++ b/e2e/esm_no_linker/WORKSPACE @@ -0,0 +1,22 @@ +workspace( + name = "e2e_esm_no_linker", +) + +local_repository( + name = "build_bazel_rules_nodejs", + path = "../..", +) + +load("@build_bazel_rules_nodejs//:repositories.bzl", "build_bazel_rules_nodejs_dependencies") + +build_bazel_rules_nodejs_dependencies() + +load("@build_bazel_rules_nodejs//:index.bzl", "node_repositories", "yarn_install") + +node_repositories() + +yarn_install( + name = "npm", + package_json = "//:package.json", + yarn_lock = "//:yarn.lock", +) diff --git a/e2e/esm_no_linker/main.mjs b/e2e/esm_no_linker/main.mjs new file mode 100644 index 0000000000..82bb8f2dab --- /dev/null +++ b/e2e/esm_no_linker/main.mjs @@ -0,0 +1,2 @@ +import ts from 'typescript'; +console.log('a node script with a dep on typescript', ts.version); diff --git a/e2e/esm_no_linker/package.json b/e2e/esm_no_linker/package.json new file mode 100644 index 0000000000..d25d592be3 --- /dev/null +++ b/e2e/esm_no_linker/package.json @@ -0,0 +1,6 @@ +{ + "private": true, + "dependencies": { + "typescript": "4.7.4" + } +} diff --git a/e2e/esm_no_linker/yarn.lock b/e2e/esm_no_linker/yarn.lock new file mode 100644 index 0000000000..7c600ec061 --- /dev/null +++ b/e2e/esm_no_linker/yarn.lock @@ -0,0 +1,8 @@ +# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. +# yarn lockfile v1 + + +typescript@4.7.4: + version "4.7.4" + resolved "https://registry.yarnpkg.com/typescript/-/typescript-4.7.4.tgz#1a88596d1cf47d59507a1bcdfb5b9dfe4d488235" + integrity sha512-C0WQT0gezHuw6AdY1M2jxUO83Rjf0HP7Sk1DtXj6j1EwkQNZrHAg2XPWlq62oqEhYvONq5pkC2Y9oPljWToLmQ== diff --git a/internal/node/launcher.sh b/internal/node/launcher.sh index 9d1e8df4e5..db31cb45ce 100644 --- a/internal/node/launcher.sh +++ b/internal/node/launcher.sh @@ -331,26 +331,24 @@ if [ "$PATCH_REQUIRE" = true ]; then * ) require_patch_script="${PWD}/${require_patch_script}" ;; esac LAUNCHER_NODE_OPTIONS+=( "--require" "$require_patch_script" ) - # Change the entry point to be the loader.cjs script so we run code before node - MAIN=$(rlocation "TEMPLATED_loader_script") -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 [[ ! -e "$MAIN" ]]; then - if [ "$FROM_EXECROOT" = true ]; then - MAIN="$EXECROOT/"TEMPLATED_entry_point_execroot_path - else - MAIN=TEMPLATED_entry_point_manifest_path - fi - fi - # Always set up source-map-support using our vendored copy, just like the require_patch_script - register_source_map_support=$(rlocation build_bazel_rules_nodejs/third_party/github.com/source-map-support/register.js) - LAUNCHER_NODE_OPTIONS+=( "--require" "${register_source_map_support}" ) - if [[ -n "TEMPLATED_entry_point_main" ]]; then - MAIN="${MAIN}/"TEMPLATED_entry_point_main +fi + +# 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 [[ ! -e "$MAIN" ]]; then + if [ "$FROM_EXECROOT" = true ]; then + MAIN="$EXECROOT/"TEMPLATED_entry_point_execroot_path + else + MAIN=TEMPLATED_entry_point_manifest_path fi fi +# Always set up source-map-support using our vendored copy, just like the require_patch_script +register_source_map_support=$(rlocation build_bazel_rules_nodejs/third_party/github.com/source-map-support/register.js) +LAUNCHER_NODE_OPTIONS+=( "--require" "${register_source_map_support}" ) +if [[ -n "TEMPLATED_entry_point_main" ]]; then + MAIN="${MAIN}/"TEMPLATED_entry_point_main +fi if [ "${SILENT_ON_SUCCESS:-}" = true ]; then if [[ -z "${STDOUT_CAPTURE}" ]]; then @@ -484,3 +482,4 @@ if [[ -n "${EXIT_CODE_CAPTURE}" ]]; then else exit ${RESULT} fi + diff --git a/internal/node/loader.cjs b/internal/node/loader.cjs deleted file mode 100644 index dd7bc0b4b8..0000000000 --- a/internal/node/loader.cjs +++ /dev/null @@ -1,39 +0,0 @@ -/** - * @license - * Copyright 2017 The Bazel Authors. All rights reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * - * You may obtain a copy of the License at - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -/** - * @fileoverview NodeJS module loader for bazel. - */ -'use strict'; - -// Ensure that node is added to the path for any subprocess calls -process.env.PATH = [require('path').dirname(process.execPath), process.env.PATH].join( - /^win/i.test(process.platform) ? ';' : ':'); - -if (require.main === module) { - // Set the actual entry point in the arguments list. - // argv[0] == node, argv[1] == entry point. - // NB: 'TEMPLATED_entry_point_path' & 'TEMPLATED_entry_point' below are replaced during the build process. - var entryPointPath = 'TEMPLATED_entry_point_path'; - var entryPointMain = 'TEMPLATED_entry_point_main'; - var mainScript = process.argv[1] = entryPointMain ? `${entryPointPath}/${entryPointMain}` : entryPointPath; - try { - module.constructor._load(mainScript, this, /*isMain=*/true); - } catch (e) { - console.error(e.stack || e); - process.exit(1); - } -} diff --git a/internal/node/node.bzl b/internal/node/node.bzl index 4f328e5d35..b64638ba08 100644 --- a/internal/node/node.bzl +++ b/internal/node/node.bzl @@ -117,21 +117,6 @@ def _get_entry_point_file(ctx): return ctx.attr.entry_point[DirectoryFilePathInfo].directory fail("entry_point must either be a file, or provide DirectoryFilePathInfo") -def _write_loader_script(ctx): - substitutions = {} - substitutions["TEMPLATED_entry_point_path"] = _ts_to_js(_to_manifest_path(ctx, _get_entry_point_file(ctx))) - if DirectoryFilePathInfo in ctx.attr.entry_point: - substitutions["TEMPLATED_entry_point_main"] = ctx.attr.entry_point[DirectoryFilePathInfo].path - else: - substitutions["TEMPLATED_entry_point_main"] = "" - - ctx.actions.expand_template( - template = ctx.file._loader_template, - output = ctx.outputs.loader_script, - substitutions = substitutions, - is_executable = True, - ) - # Avoid using non-normalized paths (workspace/../other_workspace/path) def _to_manifest_path(ctx, file): if file.short_path.startswith("../"): @@ -202,8 +187,6 @@ def _nodejs_binary_impl(ctx, data = [], runfiles = [], expanded_args = []): node_modules_root = "build_bazel_rules_nodejs/node_modules" _write_require_patch_script(ctx, data, node_modules_root) - _write_loader_script(ctx) - # Provide the target name as an environment variable avaiable to all actions for the # runfiles helpers to use. env_vars = "export BAZEL_TARGET=%s\n" % ctx.label @@ -270,7 +253,6 @@ fi runfiles = runfiles[:] runfiles.extend(node_tool_files) runfiles.extend(ctx.files._bash_runfile_helper) - runfiles.append(ctx.outputs.loader_script) runfiles.append(ctx.outputs.require_patch_script) # First replace any instances of "$(rlocation " with "$$(rlocation " to preserve @@ -325,7 +307,6 @@ if (process.cwd() !== __dirname) { "TEMPLATED_expected_exit_code": str(expected_exit_code), "TEMPLATED_lcov_merger_script": _to_manifest_path(ctx, ctx.file._lcov_merger_script), "TEMPLATED_link_modules_script": _to_manifest_path(ctx, ctx.file._link_modules_script), - "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_require_patch_script": _to_manifest_path(ctx, ctx.outputs.require_patch_script), @@ -372,7 +353,6 @@ if (process.cwd() !== __dirname) { runfiles = ctx.runfiles( transitive_files = depset(runfiles), files = node_tool_files + [ - ctx.outputs.loader_script, ctx.outputs.require_patch_script, ] + ctx.files._source_map_support_files + @@ -636,7 +616,6 @@ Predefined genrule variables are not supported in this context. _NODEJS_EXECUTABLE_OUTPUTS = { "launcher_sh": "%{name}.sh", - "loader_script": "%{name}_loader.cjs", "require_patch_script": "%{name}_require_patch.cjs", }