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
fix: use shell matching platform when running scripts #5034
Conversation
} | ||
|
||
const scriptError = new WorkflowScriptError({ | ||
message: `Script exited with code ${error.exitCode}`, | ||
message: `Script exited with code ${err.detail.code}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, please keep using it! The ChildProcessError will still have details, the difference is that GardenError will simply not be a generic but the details are defined and typed in every single subclass of it that need them for use cases like this.
What this PR does / why we need it:
The
runScript()
function that is used to run script steps in workflows as well as theinitScript
was directly shelling out via/bin/bash -s
which causes the script to be run in WSL on Windows. This can cause confusing behaviour because the commands needed in theinitScript
might not be available in WSL e.g. clis to authenticate to cloud providers.This PR uses the
exec
wrapper and setsshell: true
which causes execa to dynamically pick the right shell for the platform.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: