aws / jsii

jsii allows code in any language to naturally interact with JavaScript classes. It is the technology that enables the AWS Cloud Development Kit to deliver polyglot libraries from a single codebase!
https://aws.github.io/jsii
Apache License 2.0
2.64k stars 246 forks source link

Backport #3890 to Java CDK #4022

Closed osdrv closed 1 year ago

osdrv commented 1 year ago

Describe the feature

This is a request to backport https://github.com/aws/jsii/pull/3890 to the Java CDK.

Use Case

The motivation is to be able to alter the node executable location if altering PATH env var is not an option. This would preserve a more granular and precise control over the executable in the owning process rather than the executing environment.

Proposed Solution

The proposal is to interpret JSII_NODE env variable the same way https://github.com/aws/jsii/pull/3890 does.

The existence of JSII_RUNTIME raises a questionmark as the area of the inflience of this variable would overlap with JSII_NODE. I assume, the motivation of leaving it in place should be dictated by a common motivation to keep the env variable set backwards compatible.

Other Information

1 additional thing to bear in mind is the current behavior of JSII_RUNTIME:

At the moment, the value of JSII_RUNTIME is passed "as is" to the ProcessBuilder. It would fail if the env var value contains arguments, e.g.: "node jsii-runtime.js". In this case ProcessBuilder would interpret this entire string as a name of the executable and attempt to call it. The executable name should be "node", not "node jsii-runtime.js".

Code refs: https://github.com/aws/jsii/blob/main/packages/%40jsii/java-runtime/project/src/main/java/software/amazon/jsii/JsiiRuntime.java#L288-L289

These 2 lines https://github.com/aws/jsii/blob/main/packages/%40jsii/java-runtime/project/src/main/java/software/amazon/jsii/JsiiRuntime.java#L280-L281 is where the major distinction between the commands is located.

I assume, JSII_RUNTIME could be split by /\s+/ or a any other special separator like \A so the invoker could pass the entire command as single string and get it split into params before it would be fed to the ProcessBuilder.

Acknowledgements

CDK version used

1.77.0

Environment details (OS name and version, etc.)

MacOS Monterey (Intel), Linux x86_64 kernel 5.4.228

RomainMuller commented 1 year ago

I assume, JSII_RUNTIME could be split by /\s+/ or a any other special separator like \A so the invoker could pass the entire command as single string and get it split into params before it would be fed to the ProcessBuilder.

"Just splitting" is going to lead to more bugs... We'd need to actually parse it out the same way a shell would, preserving quoted strings, etc... Maybe what we need to do is actually make a sh -c invocation on Unixes, and whatever the cmd.exe equivalent is on Windows...

osdrv commented 1 year ago

Resolved by https://github.com/aws/jsii/pull/4024. Many thanks @RomainMuller !

github-actions[bot] commented 1 year ago

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.