From f16e89d25a6296e3c7857cf11d4cf4cc0687bfdd Mon Sep 17 00:00:00 2001 From: Momo Kornher Date: Wed, 3 Jan 2024 16:04:10 +0000 Subject: [PATCH 1/3] fix(pacmak): invokeBinScript fails when using symlinked cache This was previously attempted to fix in https://github.com/aws/jsii/pull/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: https://github.com/nodejs/node/issues/41000 tl;dr this all sucks very much and we are now just disabling the runtime cache for binaries. --- .../content/overview/runtime-architecture.md | 3 +- packages/@jsii/kernel/src/kernel.ts | 5 -- packages/jsii-pacmak/lib/targets/python.ts | 5 ++ .../__snapshots__/target-python.test.js.snap | 67 ++----------------- 4 files changed, 11 insertions(+), 69 deletions(-) diff --git a/gh-pages/content/overview/runtime-architecture.md b/gh-pages/content/overview/runtime-architecture.md index b625de6dc2..3cbbec0f14 100644 --- a/gh-pages/content/overview/runtime-architecture.md +++ b/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 @@ -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 diff --git a/packages/@jsii/kernel/src/kernel.ts b/packages/@jsii/kernel/src/kernel.ts index caab41c6d0..c1c00127dc 100644 --- a/packages/@jsii/kernel/src/kernel.ts +++ b/packages/@jsii/kernel/src/kernel.ts @@ -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 ?? [], diff --git a/packages/jsii-pacmak/lib/targets/python.ts b/packages/jsii-pacmak/lib/targets/python.ts index c0ab22271e..4f66c51e7d 100644 --- a/packages/jsii-pacmak/lib/targets/python.ts +++ b/packages/jsii-pacmak/lib/targets/python.ts @@ -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" in os.environ'); + code.line('os.environ["JSII_RUNTIME_PACKAGE_CACHE"] = "disabled"'); + code.closeBlock(); code.line(); emitList( code, diff --git a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.js.snap b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.js.snap index dac58bf1dd..1f85526934 100644 --- a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.js.snap +++ b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.js.snap @@ -491,69 +491,6 @@ exports[`Generated code for "@scope/jsii-calc-base": /python/src/scope/j `; -exports[`Generated code for "@scope/jsii-calc-base": / 1`] = ` - - ┗━ 📁 python - ┗━ 📁 src - ┗━ 📁 scope - ┗━ 📁 jsii_calc_base - ┗━ 📄 __init__.py.diff -`; - -exports[`Generated code for "@scope/jsii-calc-base": /python/src/scope/jsii_calc_base/__init__.py.diff 1`] = ` ---- python/src/scope/jsii_calc_base/__init__.py --no-runtime-type-checking -+++ python/src/scope/jsii_calc_base/__init__.py --runtime-type-checking -@@ -50,10 +50,14 @@ - ) -> None: - ''' - :param foo: - - :param bar: - - ''' -+ if __debug__: -+ type_hints = typing.get_type_hints(_typecheckingstub__e2d8a566db7d86eb1cb511cb869273dae39a21b3ad7041359aabb7bd283dcfef) -+ check_type(argname="argument foo", value=foo, expected_type=type_hints["foo"]) -+ check_type(argname="argument bar", value=bar, expected_type=type_hints["bar"]) - self._values: typing.Dict[builtins.str, typing.Any] = { - "foo": foo, - "bar": bar, - } - -@@ -117,10 +121,13 @@ - @builtins.classmethod - def consume(cls, *args: typing.Any) -> None: - ''' - :param args: - - ''' -+ if __debug__: -+ type_hints = typing.get_type_hints(_typecheckingstub__66400f6ebe2f9a3fbb550342b894bebf4f5368890843f3917e2b903f62d48b38) -+ check_type(argname="argument args", value=args, expected_type=typing.Tuple[type_hints["args"], ...]) # pyright: ignore [reportGeneralTypeIssues] - return typing.cast(None, jsii.sinvoke(cls, "consume", [*args])) - - - __all__ = [ - "Base", -@@ -128,5 +135,19 @@ - "IBaseInterface", - "StaticConsumer", - ] - - publication.publish() -+ -+def _typecheckingstub__e2d8a566db7d86eb1cb511cb869273dae39a21b3ad7041359aabb7bd283dcfef( -+ *, -+ foo: _scope_jsii_calc_base_of_base_49fa37fe.Very, -+ bar: builtins.str, -+) -> None: -+ """Type checking stubs""" -+ pass -+ -+def _typecheckingstub__66400f6ebe2f9a3fbb550342b894bebf4f5368890843f3917e2b903f62d48b38( -+ *args: typing.Any, -+) -> None: -+ """Type checking stubs""" -+ pass -`; - exports[`Generated code for "@scope/jsii-calc-base-of-base": / 1`] = ` ┗━ 📁 python @@ -11757,6 +11694,10 @@ exports[`Generated code for "jsii-calc": /python/src/jsii_calc/_jsii/bin import jsii import sys +import os + +if "JSII_RUNTIME_PACKAGE_CACHE" in os.environ: + os.environ["JSII_RUNTIME_PACKAGE_CACHE"] = "disabled" __jsii_assembly__ = jsii.JSIIAssembly.load( "jsii-calc", "3.20.120", "jsii_calc", "jsii-calc@3.20.120.jsii.tgz" From 1e7a5958d639ebdb6077bf8df3f6b0a522d36d9d Mon Sep 17 00:00:00 2001 From: Momo Kornher Date: Wed, 3 Jan 2024 16:43:26 +0000 Subject: [PATCH 2/3] fixup generated code --- packages/jsii-pacmak/lib/targets/python.ts | 2 +- .../__snapshots__/target-python.test.js.snap | 65 ++++++++++++++++++- 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/packages/jsii-pacmak/lib/targets/python.ts b/packages/jsii-pacmak/lib/targets/python.ts index 4f66c51e7d..63b245d0fd 100644 --- a/packages/jsii-pacmak/lib/targets/python.ts +++ b/packages/jsii-pacmak/lib/targets/python.ts @@ -1835,7 +1835,7 @@ class PythonModule implements PythonType { code.line('import sys'); code.line('import os'); code.line(); - code.openBlock('if "JSII_RUNTIME_PACKAGE_CACHE" in os.environ'); + code.openBlock('if "JSII_RUNTIME_PACKAGE_CACHE" not in os.environ'); code.line('os.environ["JSII_RUNTIME_PACKAGE_CACHE"] = "disabled"'); code.closeBlock(); code.line(); diff --git a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.js.snap b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.js.snap index 1f85526934..34a2c1428f 100644 --- a/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.js.snap +++ b/packages/jsii-pacmak/test/generated-code/__snapshots__/target-python.test.js.snap @@ -491,6 +491,69 @@ exports[`Generated code for "@scope/jsii-calc-base": /python/src/scope/j `; +exports[`Generated code for "@scope/jsii-calc-base": / 1`] = ` + + ┗━ 📁 python + ┗━ 📁 src + ┗━ 📁 scope + ┗━ 📁 jsii_calc_base + ┗━ 📄 __init__.py.diff +`; + +exports[`Generated code for "@scope/jsii-calc-base": /python/src/scope/jsii_calc_base/__init__.py.diff 1`] = ` +--- python/src/scope/jsii_calc_base/__init__.py --no-runtime-type-checking ++++ python/src/scope/jsii_calc_base/__init__.py --runtime-type-checking +@@ -50,10 +50,14 @@ + ) -> None: + ''' + :param foo: - + :param bar: - + ''' ++ if __debug__: ++ type_hints = typing.get_type_hints(_typecheckingstub__e2d8a566db7d86eb1cb511cb869273dae39a21b3ad7041359aabb7bd283dcfef) ++ check_type(argname="argument foo", value=foo, expected_type=type_hints["foo"]) ++ check_type(argname="argument bar", value=bar, expected_type=type_hints["bar"]) + self._values: typing.Dict[builtins.str, typing.Any] = { + "foo": foo, + "bar": bar, + } + +@@ -117,10 +121,13 @@ + @builtins.classmethod + def consume(cls, *args: typing.Any) -> None: + ''' + :param args: - + ''' ++ if __debug__: ++ type_hints = typing.get_type_hints(_typecheckingstub__66400f6ebe2f9a3fbb550342b894bebf4f5368890843f3917e2b903f62d48b38) ++ check_type(argname="argument args", value=args, expected_type=typing.Tuple[type_hints["args"], ...]) # pyright: ignore [reportGeneralTypeIssues] + return typing.cast(None, jsii.sinvoke(cls, "consume", [*args])) + + + __all__ = [ + "Base", +@@ -128,5 +135,19 @@ + "IBaseInterface", + "StaticConsumer", + ] + + publication.publish() ++ ++def _typecheckingstub__e2d8a566db7d86eb1cb511cb869273dae39a21b3ad7041359aabb7bd283dcfef( ++ *, ++ foo: _scope_jsii_calc_base_of_base_49fa37fe.Very, ++ bar: builtins.str, ++) -> None: ++ """Type checking stubs""" ++ pass ++ ++def _typecheckingstub__66400f6ebe2f9a3fbb550342b894bebf4f5368890843f3917e2b903f62d48b38( ++ *args: typing.Any, ++) -> None: ++ """Type checking stubs""" ++ pass +`; + exports[`Generated code for "@scope/jsii-calc-base-of-base": / 1`] = ` ┗━ 📁 python @@ -11696,7 +11759,7 @@ import jsii import sys import os -if "JSII_RUNTIME_PACKAGE_CACHE" in os.environ: +if "JSII_RUNTIME_PACKAGE_CACHE" not in os.environ: os.environ["JSII_RUNTIME_PACKAGE_CACHE"] = "disabled" __jsii_assembly__ = jsii.JSIIAssembly.load( From 2b35ce208b3f3916c02a3c0f2cd29b6e16566e00 Mon Sep 17 00:00:00 2001 From: Momo Kornher Date: Wed, 3 Jan 2024 16:49:10 +0000 Subject: [PATCH 3/3] fix tests --- .../python-runtime/tests/test_invoke_bin.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/@jsii/python-runtime/tests/test_invoke_bin.py b/packages/@jsii/python-runtime/tests/test_invoke_bin.py index f4e91fbee5..00e21ef91d 100644 --- a/packages/@jsii/python-runtime/tests/test_invoke_bin.py +++ b/packages/@jsii/python-runtime/tests/test_invoke_bin.py @@ -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) @@ -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) @@ -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) @@ -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