diff --git a/internal/node/launcher.sh b/internal/node/launcher.sh index f9c6c5814a..723486f563 100644 --- a/internal/node/launcher.sh +++ b/internal/node/launcher.sh @@ -165,9 +165,9 @@ readonly lcov_merger_script=$(rlocation "TEMPLATED_lcov_merger_script") source $repository_args ARGS=() -LAUNCHER_NODE_OPTIONS=() +LAUNCHER_NODE_OPTIONS=($NODE_REPOSITORY_ARGS) USER_NODE_OPTIONS=() -ALL_ARGS=(TEMPLATED_args $NODE_REPOSITORY_ARGS "$@") +ALL_ARGS=(TEMPLATED_args "$@") STDOUT_CAPTURE="" STDERR_CAPTURE="" EXIT_CODE_CAPTURE="" @@ -176,7 +176,7 @@ RUN_LINKER=true NODE_PATCHES=true # TODO(alex): change the default to false PATCH_REQUIRE=true -for ARG in "${ALL_ARGS[@]:-}"; do +for ARG in ${ALL_ARGS[@]+"${ALL_ARGS[@]}"}; do case "$ARG" in # Supply custom linker arguments for first-party dependencies --bazel_node_modules_manifest=*) MODULES_MANIFEST="${ARG#--bazel_node_modules_manifest=}" ;; @@ -316,13 +316,13 @@ _int() { set +e if [[ -n "${STDOUT_CAPTURE}" ]] && [[ -n "${STDERR_CAPTURE}" ]]; then - "${node}" "${LAUNCHER_NODE_OPTIONS[@]:-}" "${USER_NODE_OPTIONS[@]:-}" "${MAIN}" ${ARGS[@]+"${ARGS[@]}"} <&0 >$STDOUT_CAPTURE 2>$STDERR_CAPTURE & + "${node}" ${LAUNCHER_NODE_OPTIONS[@]+"${LAUNCHER_NODE_OPTIONS[@]}"} ${USER_NODE_OPTIONS[@]+"${USER_NODE_OPTIONS[@]}"} "${MAIN}" ${ARGS[@]+"${ARGS[@]}"} <&0 >$STDOUT_CAPTURE 2>$STDERR_CAPTURE & elif [[ -n "${STDOUT_CAPTURE}" ]]; then - "${node}" "${LAUNCHER_NODE_OPTIONS[@]:-}" "${USER_NODE_OPTIONS[@]:-}" "${MAIN}" ${ARGS[@]+"${ARGS[@]}"} <&0 >$STDOUT_CAPTURE & + "${node}" ${LAUNCHER_NODE_OPTIONS[@]+"${LAUNCHER_NODE_OPTIONS[@]}"} ${USER_NODE_OPTIONS[@]+"${USER_NODE_OPTIONS[@]}"} "${MAIN}" ${ARGS[@]+"${ARGS[@]}"} <&0 >$STDOUT_CAPTURE & elif [[ -n "${STDERR_CAPTURE}" ]]; then - "${node}" "${LAUNCHER_NODE_OPTIONS[@]:-}" "${USER_NODE_OPTIONS[@]:-}" "${MAIN}" ${ARGS[@]+"${ARGS[@]}"} <&0 2>$STDERR_CAPTURE & + "${node}" ${LAUNCHER_NODE_OPTIONS[@]+"${LAUNCHER_NODE_OPTIONS[@]}"} ${USER_NODE_OPTIONS[@]+"${USER_NODE_OPTIONS[@]}"} "${MAIN}" ${ARGS[@]+"${ARGS[@]}"} <&0 2>$STDERR_CAPTURE & else - "${node}" "${LAUNCHER_NODE_OPTIONS[@]:-}" "${USER_NODE_OPTIONS[@]:-}" "${MAIN}" ${ARGS[@]+"${ARGS[@]}"} <&0 & + "${node}" ${LAUNCHER_NODE_OPTIONS[@]+"${LAUNCHER_NODE_OPTIONS[@]}"} ${USER_NODE_OPTIONS[@]+"${USER_NODE_OPTIONS[@]}"} "${MAIN}" ${ARGS[@]+"${ARGS[@]}"} <&0 & fi readonly child=$! diff --git a/internal/node/node_patches.js b/internal/node/node_patches.js index 2318e6cdf6..ad98f2ecec 100644 --- a/internal/node/node_patches.js +++ b/internal/node/node_patches.js @@ -551,39 +551,41 @@ Object.defineProperty(exports, "__esModule", { value: true }); // but adds support to ensure the registered loader is included in all nested executions of nodejs. -exports.patcher = (requireScriptName, binDir) => { +exports.patcher = (requireScriptName, nodeDir) => { requireScriptName = path.resolve(requireScriptName); - const dir = path.dirname(requireScriptName); + nodeDir = nodeDir || path.join(path.dirname(requireScriptName), '_node_bin'); const file = path.basename(requireScriptName); - const nodeDir = path.join(binDir || dir, '_node_bin'); - if (!process.env.NP_PATCHED_NODEJS) { - // TODO: WINDOWS. - try { - fs$1.mkdirSync(nodeDir, { recursive: true }); - } - catch (e) { - // with node versions that don't have recursive mkdir this may throw an error. - if (e.code !== 'EEXIST') { - throw e; - } + try { + fs$1.mkdirSync(nodeDir, { recursive: true }); + } + catch (e) { + // with node versions that don't have recursive mkdir this may throw an error. + if (e.code !== 'EEXIST') { + throw e; } - if (process.platform == 'win32') { - fs$1.writeFileSync(path.join(nodeDir, 'node.bat'), `@if not defined DEBUG_HELPER @ECHO OFF -set NP_PATCHED_NODEJS=${nodeDir} + } + if (process.platform == 'win32') { + const nodeEntry = path.join(nodeDir, 'node.bat'); + if (!fs$1.existsSync(nodeEntry)) { + fs$1.writeFileSync(nodeEntry, `@if not defined DEBUG_HELPER @ECHO OFF +set NP_SUBPROCESS_NODE_DIR=${nodeDir} set Path=${nodeDir};%Path% -"${process.execPath}" --require "${requireScriptName}" %* - `); +"${process.execPath}" ${process.env.NODE_REPOSITORY_ARGS} --require "${requireScriptName}" %* +`); } - else { - fs$1.writeFileSync(path.join(nodeDir, 'node'), `#!/bin/bash -export NP_PATCHED_NODEJS="${nodeDir}" + } + else { + const nodeEntry = path.join(nodeDir, 'node'); + if (!fs$1.existsSync(nodeEntry)) { + fs$1.writeFileSync(nodeEntry, `#!/bin/bash +export NP_SUBPROCESS_NODE_DIR="${nodeDir}" export PATH="${nodeDir}":\$PATH if [[ ! "\${@}" =~ "${file}" ]]; then - exec ${process.execPath} --require "${requireScriptName}" "$@" + exec ${process.execPath} ${process.env.NODE_REPOSITORY_ARGS} --require "${requireScriptName}" "$@" else - exec ${process.execPath} "$@" + exec ${process.execPath} ${process.env.NODE_REPOSITORY_ARGS} "$@" fi - `, { mode: 0o777 }); +`, { mode: 0o777 }); } } if (!process.env.PATH) { @@ -659,8 +661,7 @@ var src_2 = src.subprocess; * @fileoverview Description of this file. */ -// todo auto detect bazel env vars instead of adding a new one. -const { BAZEL_PATCH_ROOT, BAZEL_PATCH_GUARDS, NP_SUBPROCESS_BIN_DIR, VERBOSE_LOGS } = process.env; +const { BAZEL_PATCH_ROOT, BAZEL_PATCH_GUARDS, NP_SUBPROCESS_NODE_DIR, VERBOSE_LOGS } = process.env; if (BAZEL_PATCH_ROOT) { const guards = BAZEL_PATCH_GUARDS ? BAZEL_PATCH_GUARDS.split(',') : []; if (VERBOSE_LOGS) @@ -671,7 +672,7 @@ if (BAZEL_PATCH_ROOT) { else if (VERBOSE_LOGS) { console.error(`bazel node patches disabled. set environment BAZEL_PATCH_ROOT`); } -src.subprocess(__filename, NP_SUBPROCESS_BIN_DIR); +src.subprocess(__filename, NP_SUBPROCESS_NODE_DIR); var register = { diff --git a/internal/node/node_repositories.bzl b/internal/node/node_repositories.bzl index d7ce68be30..7a700a8298 100644 --- a/internal/node/node_repositories.bzl +++ b/internal/node/node_repositories.bzl @@ -439,13 +439,10 @@ def _prepare_node(repository_ctx): preserve_symlinks_main_support = check_version(repository_ctx.attr.node_version, "10.2.0") if preserve_symlinks_main_support: node_args = "--preserve-symlinks --preserve-symlinks-main" - node_repo_args = "\"--node_options=--preserve-symlinks --node_options=--preserve-symlinks-main\"" else: node_args = "--preserve-symlinks" - node_repo_args = "--node_options=--preserve-symlinks" else: node_args = "" - node_repo_args = "" # The entry points for node for osx/linux and windows if not is_windows: @@ -476,8 +473,8 @@ CALL "%SCRIPT_DIR%\\{node}" {args} %* # Immediately exit if any command fails. set -e # Generated by node_repositories.bzl -export NODE_REPOSITORY_ARGS={args} -""".format(args = node_repo_args), executable = True) +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 diff --git a/internal/node/test/empty_args_fail.js b/internal/node/test/empty_args_fail.js index 6f4b8c0f5a..bd598e3473 100644 --- a/internal/node/test/empty_args_fail.js +++ b/internal/node/test/empty_args_fail.js @@ -6,5 +6,6 @@ var theArgs = process.argv.slice(2); if (theArgs.length != 0) { // Non-zero exit code if the argument list is not empty + console.error(theArgs) process.exit(42) } diff --git a/packages/node-patches/register.ts b/packages/node-patches/register.ts index 876408ad78..534b9c4d65 100644 --- a/packages/node-patches/register.ts +++ b/packages/node-patches/register.ts @@ -18,8 +18,7 @@ * @fileoverview Description of this file. */ const patcher = require('./src'); -// todo auto detect bazel env vars instead of adding a new one. -const {BAZEL_PATCH_ROOT, BAZEL_PATCH_GUARDS, NP_SUBPROCESS_BIN_DIR, VERBOSE_LOGS} = process.env; +const {BAZEL_PATCH_ROOT, BAZEL_PATCH_GUARDS, NP_SUBPROCESS_NODE_DIR, VERBOSE_LOGS} = process.env; if (BAZEL_PATCH_ROOT) { const guards = BAZEL_PATCH_GUARDS ? BAZEL_PATCH_GUARDS.split(',') : []; @@ -32,4 +31,4 @@ if (BAZEL_PATCH_ROOT) { console.error(`bazel node patches disabled. set environment BAZEL_PATCH_ROOT`); } -patcher.subprocess(__filename, NP_SUBPROCESS_BIN_DIR); +patcher.subprocess(__filename, NP_SUBPROCESS_NODE_DIR); diff --git a/packages/node-patches/src/subprocess.ts b/packages/node-patches/src/subprocess.ts index 9715dcbebd..8242f734c2 100644 --- a/packages/node-patches/src/subprocess.ts +++ b/packages/node-patches/src/subprocess.ts @@ -3,39 +3,41 @@ const fs = require('fs'); const path = require('path'); -export const patcher = (requireScriptName: string, binDir?: string) => { +export const patcher = (requireScriptName: string, nodeDir?: string) => { requireScriptName = path.resolve(requireScriptName); - const dir = path.dirname(requireScriptName); + nodeDir = nodeDir || path.join(path.dirname(requireScriptName), '_node_bin'); const file = path.basename(requireScriptName); - const nodeDir = path.join(binDir || dir, '_node_bin'); - if (!process.env.NP_PATCHED_NODEJS) { - // TODO: WINDOWS. - try { - fs.mkdirSync(nodeDir, {recursive: true}); - } catch (e) { - // with node versions that don't have recursive mkdir this may throw an error. - if (e.code !== 'EEXIST') { - throw e; - } + try { + fs.mkdirSync(nodeDir, {recursive: true}); + } catch (e) { + // with node versions that don't have recursive mkdir this may throw an error. + if (e.code !== 'EEXIST') { + throw e; } - if (process.platform == 'win32') { - fs.writeFileSync(path.join(nodeDir, 'node.bat'), `@if not defined DEBUG_HELPER @ECHO OFF -set NP_PATCHED_NODEJS=${nodeDir} + } + if (process.platform == 'win32') { + const nodeEntry = path.join(nodeDir, 'node.bat'); + if (!fs.existsSync(nodeEntry)) { + fs.writeFileSync(nodeEntry, `@if not defined DEBUG_HELPER @ECHO OFF +set NP_SUBPROCESS_NODE_DIR=${nodeDir} set Path=${nodeDir};%Path% -"${process.execPath}" --require "${requireScriptName}" %* - `) - } else { +"${process.execPath}" ${process.env.NODE_REPOSITORY_ARGS} --require "${requireScriptName}" %* +`); + } + } else { + const nodeEntry = path.join(nodeDir, 'node'); + if (!fs.existsSync(nodeEntry)) { fs.writeFileSync( - path.join(nodeDir, 'node'), `#!/bin/bash -export NP_PATCHED_NODEJS="${nodeDir}" + nodeEntry, `#!/bin/bash +export NP_SUBPROCESS_NODE_DIR="${nodeDir}" export PATH="${nodeDir}":\$PATH if [[ ! "\${@}" =~ "${file}" ]]; then - exec ${process.execPath} --require "${requireScriptName}" "$@" + exec ${process.execPath} ${process.env.NODE_REPOSITORY_ARGS} --require "${requireScriptName}" "$@" else - exec ${process.execPath} "$@" + exec ${process.execPath} ${process.env.NODE_REPOSITORY_ARGS} "$@" fi - `, +`, {mode: 0o777}); } } diff --git a/packages/node-patches/test/subprocess/test.ts b/packages/node-patches/test/subprocess/test.ts index 620873a7ae..0335ff3077 100644 --- a/packages/node-patches/test/subprocess/test.ts +++ b/packages/node-patches/test/subprocess/test.ts @@ -1,27 +1,22 @@ import * as assert from 'assert'; import * as cp from 'child_process'; -import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import * as rimraf from 'rimraf'; -import {patcher} from '../../src/subprocess'; - const requireScript = path.resolve(path.join(__dirname, '..', '..', 'register.js')); -const tmp = path.join(os.tmpdir(), 'node-patches-test-tmp'); +const nodeDir = path.join(os.tmpdir(), 'node-patches-test-tmp', '_node_bin'); // tslint:disable-next-line:no-any function assertPatched(prefix: string, arr: any) { - const binDir = path.join(tmp, '_node_bin'); - const [execPath, execArgv, argv, pathEnv] = arr; assert.deepStrictEqual( - path.join(binDir, 'node'), execPath, + path.join(nodeDir, 'node'), execPath, prefix + ' exec path has been rewritten to subprocess proxy'); assert.deepStrictEqual( - path.join(binDir, 'node'), argv[0], + path.join(nodeDir, 'node'), argv[0], prefix + ' argv[0] has been rewritten to subprocess proxy'); let found = false; @@ -38,23 +33,19 @@ function assertPatched(prefix: string, arr: any) { // something. assert.deepStrictEqual( - binDir, pathEnv.split(path.delimiter)[0], + nodeDir, pathEnv.split(path.delimiter)[0], prefix + ' the highest priority directory in the PATH must be the node shim dir.'); } describe('spawning child processes', () => { - before(() => { - fs.mkdirSync(tmp, {recursive: true}); - }); - it('get patched if run as shell script.', () => { - const res = cp.execSync(`NP_SUBPROCESS_BIN_DIR=${tmp} node -r ${ + const res = cp.execSync(`NP_SUBPROCESS_NODE_DIR=${nodeDir} node -r ${ requireScript} -e 'console.log(JSON.stringify([process.execPath,process.execArgv,process.argv,process.env.PATH]))'`); assertPatched('', JSON.parse(res + '')); }); it('overwrites spawn related variables correctly.', () => { - const res = cp.execSync(`NP_SUBPROCESS_BIN_DIR=${tmp} node -r ${requireScript} ${ + const res = cp.execSync(`NP_SUBPROCESS_NODE_DIR=${nodeDir} node -r ${requireScript} ${ path.join(__dirname, 'worker-threads-script.js')}`); const {mainThread, worker} = JSON.parse(res + ''); @@ -64,7 +55,7 @@ describe('spawning child processes', () => { }); it('can spawn node from the shell', () => { - const res = cp.execSync(`NP_SUBPROCESS_BIN_DIR=${tmp} node -r ${requireScript} ${ + const res = cp.execSync(`NP_SUBPROCESS_NODE_DIR=${nodeDir} node -r ${requireScript} ${ path.join(__dirname, 'shell-script.js')}`); // TODO: this is broken if no environment is passed and a new bash is executed // reading only the rc files to build the environment. @@ -77,7 +68,7 @@ describe('spawning child processes', () => { it('can spawn node from spawn', () => { const res = cp.execSync( - `NP_SUBPROCESS_BIN_DIR=${tmp} node -r ${requireScript} ${ + `NP_SUBPROCESS_NODE_DIR=${nodeDir} node -r ${requireScript} ${ path.join(__dirname, 'spawn-script.js')}`, {env: process.env}); @@ -86,6 +77,6 @@ describe('spawning child processes', () => { }); after(() => { - rimraf.sync(tmp); + rimraf.sync(nodeDir); }); });