Skip to content

Commit

Permalink
fix(kernel): invokeBinScript fails when using symlinked cache (#4324)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mrgrain committed Nov 13, 2023
1 parent 8784725 commit a2ab316
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 3 deletions.
11 changes: 9 additions & 2 deletions packages/@jsii/kernel/src/kernel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`,
},
Expand Down
2 changes: 1 addition & 1 deletion packages/@jsii/kernel/src/tar-cache/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions packages/jsii-calc/bin/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(', ')}`);
Expand Down

0 comments on commit a2ab316

Please sign in to comment.