Skip to content

Commit

Permalink
fix(pacmak): invokeBinScript fails when using symlinked cache (#4389)
Browse files Browse the repository at this point in the history
This was previously attempted to fix in #4324 While the above fix resolves issues with dependencies, it causes failures when the binary is shelling out to other node processes. This is due to the intrusive and indiscriminate overloading of NODE_OPTIONS, which will forcibly apply to any child processes as well. While in theory adding the symlink flags should not be an issue, this seems to trigger a bug in node: nodejs/node#41000

tl;dr this all sucks very much and we are now just disabling the runtime cache for binaries.

---

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 Jan 3, 2024
1 parent 456f417 commit 37d9dc7
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 10 deletions.
3 changes: 2 additions & 1 deletion gh-pages/content/overview/runtime-architecture.md
@@ -1,4 +1,5 @@
# Runtime Architecture

## Generated Libraries

When using `jsii-pacmak` to generate libraries in different programming
Expand Down Expand Up @@ -149,7 +150,7 @@ The initialization workflow can be described as:
the child's `STDERR` stream, and forwards the decoded data to it's host
process' `STDERR` and `STDOUT` as needed.
4. The *runtime client library* automatically loads the **Javascript** modules
bundled within the *generated bindings* (and their depedencies, bundled in
bundled within the *generated bindings* (and their dependencies, bundled in
other *generated bindings*) into the `node` process when needed.
5. Calls into the *Generated bindings* are encoded into JSON requests and sent
to the child `node` process, which will execute the corresponding
Expand Down
5 changes: 0 additions & 5 deletions packages/@jsii/kernel/src/kernel.ts
Expand Up @@ -1372,11 +1372,6 @@ export class Kernel {
// 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 ?? [],
Expand Down
19 changes: 15 additions & 4 deletions packages/@jsii/python-runtime/tests/test_invoke_bin.py
Expand Up @@ -35,12 +35,21 @@ def silence_node_deprecation_warnings():
environ[var] = store[var]


@pytest.fixture()
def disable_jsii_runtime_package_cache():
"""Disable the jsii runtime cache because it is problematic with InvokeBinScript."""

environ["JSII_RUNTIME_PACKAGE_CACHE"] = "disabled"


class TestInvokeBinScript:
@pytest.mark.skipif(
platform.system() == "Windows",
reason="jsii-pacmak does not generate windows scripts",
)
def test_invoke_script(self, silence_node_deprecation_warnings) -> None:
def test_invoke_script(
self, silence_node_deprecation_warnings, disable_jsii_runtime_package_cache
) -> None:
script_path = f".env/bin/calc"
result = subprocess.run([script_path], capture_output=True)

Expand All @@ -51,7 +60,9 @@ def test_invoke_script(self, silence_node_deprecation_warnings) -> None:
platform.system() == "Windows",
reason="jsii-pacmak does not generate windows scripts",
)
def test_invoke_script_with_args(self, silence_node_deprecation_warnings) -> None:
def test_invoke_script_with_args(
self, silence_node_deprecation_warnings, disable_jsii_runtime_package_cache
) -> None:
script_path = f".env/bin/calc"
result = subprocess.run([script_path, "arg1", "arg2"], capture_output=True)

Expand All @@ -63,7 +74,7 @@ def test_invoke_script_with_args(self, silence_node_deprecation_warnings) -> Non
reason="jsii-pacmak does not generate windows scripts",
)
def test_invoke_script_with_failure(
self, silence_node_deprecation_warnings
self, silence_node_deprecation_warnings, disable_jsii_runtime_package_cache
) -> None:
script_path = f".env/bin/calc"
result = subprocess.run([script_path, "arg1", "fail"], capture_output=True)
Expand All @@ -77,7 +88,7 @@ def test_invoke_script_with_failure(
reason="jsii-pacmak does not generate windows scripts",
)
def test_invoke_script_with_line_flush(
self, silence_node_deprecation_warnings
self, silence_node_deprecation_warnings, disable_jsii_runtime_package_cache
) -> None:
"""Make sure lines are flushed immediately as they are generated, rather than
buffered to the end
Expand Down
5 changes: 5 additions & 0 deletions packages/jsii-pacmak/lib/targets/python.ts
Expand Up @@ -1833,6 +1833,11 @@ class PythonModule implements PythonType {
code.line();
code.line('import jsii');
code.line('import sys');
code.line('import os');
code.line();
code.openBlock('if "JSII_RUNTIME_PACKAGE_CACHE" not in os.environ');
code.line('os.environ["JSII_RUNTIME_PACKAGE_CACHE"] = "disabled"');
code.closeBlock();
code.line();
emitList(
code,
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 37d9dc7

Please sign in to comment.