From a2ab31609d361ac5ceca6c928584ec59f2d705d3 Mon Sep 17 00:00:00 2001 From: Momo Kornher Date: Mon, 13 Nov 2023 23:43:23 +0000 Subject: [PATCH] fix(kernel): invokeBinScript fails when using symlinked cache (#4324) Invoking bin scripts from jsii packages would fail if the symlinked cache was enabled and the invoked script depended on an other package. The reason for this was that scripts were only invoked with `--preserve-symlinks`, however for "main scripts" (like standalone binaries), we also need to call `--preserve-symlinks-main` on the node process. Note this cannot be tested in the kernel package itself, as it's not possible to invoke the test process with the correct options. --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0 --- packages/@jsii/kernel/src/kernel.ts | 11 +++++++++-- packages/@jsii/kernel/src/tar-cache/index.ts | 2 +- packages/jsii-calc/bin/run.ts | 5 +++++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/@jsii/kernel/src/kernel.ts b/packages/@jsii/kernel/src/kernel.ts index 7acf8a37c1..5b310ec9a5 100644 --- a/packages/@jsii/kernel/src/kernel.ts +++ b/packages/@jsii/kernel/src/kernel.ts @@ -1371,13 +1371,20 @@ export class Kernel { throw new JsiiFault(`Script with name ${req.script} was not defined.`); } + // Make sure the current NODE_OPTIONS are honored if we shell out to node + const nodeOptions = [...process.execArgv]; + + // When we are using the symlinked version of the cache, we need to preserve both symlink settings for binaries + if (nodeOptions.includes('--preserve-symlinks')) { + nodeOptions.push('--preserve-symlinks-main'); + } + return { command: path.join(packageDir, scriptPath), args: req.args ?? [], env: { ...process.env, - // Make sure the current NODE_OPTIONS are honored if we shell out to node - NODE_OPTIONS: process.execArgv.join(' '), + NODE_OPTIONS: nodeOptions.join(' '), // Make sure "this" node is ahead of $PATH just in case PATH: `${path.dirname(process.execPath)}:${process.env.PATH}`, }, diff --git a/packages/@jsii/kernel/src/tar-cache/index.ts b/packages/@jsii/kernel/src/tar-cache/index.ts index 1bc67cdfa9..7d7fbc0358 100644 --- a/packages/@jsii/kernel/src/tar-cache/index.ts +++ b/packages/@jsii/kernel/src/tar-cache/index.ts @@ -15,7 +15,7 @@ export interface ExtractResult { * When `'hit'`, the data was already present in cache and was returned from * cache. * - * When `'miss'`, the data was extracted into the caache and returned from + * When `'miss'`, the data was extracted into the cache and returned from * cache. * * When `undefined`, the cache is not enabled. diff --git a/packages/jsii-calc/bin/run.ts b/packages/jsii-calc/bin/run.ts index bfa844c8e6..647ffebc53 100755 --- a/packages/jsii-calc/bin/run.ts +++ b/packages/jsii-calc/bin/run.ts @@ -2,11 +2,16 @@ /* eslint-disable no-console */ +import * as calcLib from '@scope/jsii-calc-lib'; + const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); const runCommand = async () => { console.info('Hello World!'); + // Make sure this binary depends on an external package to test dependencies with invokeBinScript + new calcLib.Number(1); + const args = process.argv.slice(2); if (args.length > 0) { console.info(` arguments: ${args.join(', ')}`);