Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport #3890 to Java CDK #4022

Closed
2 tasks
osdrv opened this issue Mar 21, 2023 · 3 comments
Closed
2 tasks

Backport #3890 to Java CDK #4022

osdrv opened this issue Mar 21, 2023 · 3 comments
Labels
feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged.

Comments

@osdrv
Copy link

osdrv commented Mar 21, 2023

Describe the feature

This is a request to backport #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 #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

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

1.77.0

Environment details (OS name and version, etc.)

MacOS Monterey (Intel), Linux x86_64 kernel 5.4.228

@osdrv osdrv added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Mar 21, 2023
@RomainMuller
Copy link
Contributor

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
Copy link
Author

osdrv commented Mar 28, 2023

Resolved by #4024. Many thanks @RomainMuller !

@osdrv osdrv closed this as completed Mar 28, 2023
@github-actions
Copy link
Contributor

⚠️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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

No branches or pull requests

2 participants