Inline scripts executed via bash or sh#2896
Conversation
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
changed the title
bash or sh
shreyas-goenka
requested review from
andrewnester,
anton-107,
denik and
pietern
as code owners
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
shreyas-goenka
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
|
|
||
| func TestShellExecvOpts(t *testing.T) { | ||
| newOpts, err := shellExecvOpts("echo hello", "/a/b/c", []string{"key1=value1", "key2=value2"}) | ||
| opts, err := shellExecvOpts("echo hello", "/a/b/c", []string{"key1=value1", "key2=value2"}) |
There was a problem hiding this comment.
small typo fix, newOpts -> opts
| @@ -117,7 +117,11 @@ func TestExecutorNoShellFound(t *testing.T) { | |||
| } | |||
|
|
|||
| func TestExecutorCleanupsTempFiles(t *testing.T) { | |||
There was a problem hiding this comment.
Only cmd.exe cleans up temp files. Other shells now do not create temp files.
|
@shreyas-goenka Are there functional or behavioral differences between these execution modes that you know of? |
|
@pietern There is only one minor difference I found. Positional arguments (like I did not find any differences in the execution environment itself though. We also have coverage for some common patterns in #2884. Thanks for flagging, I just noticed a mistake I made. Since we stopped setting the |
|
@shreyas-goenka Thanks. Our docs don't mention anything specifically about how the build command is executed, but it does mention the double ampersand ( That can be updated to include line-separation, correct? |
## Changes This PR fixes a regression introduced in #2896 where we stopped passing the `-e` flag to the shell, which makes both sh and bash bail out at the first command with a non-zero exit code. ## Why Adding back `-e` keeps the behaviour consistent with the one before #2896 was introduced. ## Tests Newly added regression tests
|
@shreyas-goenka I mean they can be updated in the docs. They mention |
3 participants
Changes
This PR inlines the script content using the
-cflag in bash and sh instead of passing them in via-e <file>.Why
Based on discussion in #2862 (comment). Getting rid of the temp file means we no longer have to clean up the temporary file created.
Tests
Tests that were added in: #2884