1
0
Fork 0
mirror of https://code.forgejo.org/forgejo/runner.git synced 2025-09-15 18:57:01 +00:00

fix!: fallback to sh if bash does not exist

It is a breaking change because it changes how the shell is
determined.

Before, if `jobs.<job_id>.container.image` is set and the shell is not
specified, it defaults to `sh` instead of `bash`.

After, regardless of `jobs.<job_id>.container.image`, if the shell is
not specified, it defaults to `bash` if available, otherwise it
defaults to `sh`.

Rework the shell integration tests:

- Remove container specific tests because the special behavior related
  to shell being set differently when a container image is present is
  removed
- Modify the defaults test case to verify the fallback logic
- Use container images from code.forgejo.org to escape rate limiting
  in the CI
- Add the missing node test
- Use
      container:
        image: code.forgejo.org/oci/node:22-bookworm
  instead of
      container: code.forgejo.org/oci/node:22-bookworm
  because it silently failed to run (with no exit code)
- Prefer `-z "${BASH}"` because `-z ${BASH+x}` reads obscure

Closes forgejo/runner#150
This commit is contained in:
Earl Warren 2025-07-12 07:53:43 +02:00
parent 6e59f129c1
commit c1892b6398
10 changed files with 76 additions and 56 deletions

View file

@ -12,11 +12,13 @@ import (
"fmt"
"io"
"os"
"path"
"path/filepath"
"regexp"
"runtime"
"strings"
"text/template"
"time"
"github.com/docker/docker/api/types/network"
"github.com/docker/go-connections/nat"
@ -585,6 +587,42 @@ func (rc *RunContext) startJobContainer() common.Executor {
}
}
func (rc *RunContext) sh(ctx context.Context, script string) (stdout, stderr string, err error) {
timeed, cancel := context.WithTimeout(ctx, time.Minute)
defer cancel()
hout := &bytes.Buffer{}
herr := &bytes.Buffer{}
env := map[string]string{}
for k, v := range rc.Env {
env[k] = v
}
base, err := common.RandName(8)
if err != nil {
return "", "", err
}
name := base + ".sh"
oldStdout, oldStderr := rc.JobContainer.ReplaceLogWriter(hout, herr)
err = rc.JobContainer.Copy(rc.JobContainer.GetActPath(), &container.FileEntry{
Name: name,
Mode: 0o644,
Body: script,
}).
Then(rc.execJobContainer([]string{"sh", path.Join(rc.JobContainer.GetActPath(), name)},
env, "", "")).
Finally(func(context.Context) error {
rc.JobContainer.ReplaceLogWriter(oldStdout, oldStderr)
return nil
})(timeed)
if err != nil {
return "", "", err
}
stdout = hout.String()
stderr = herr.String()
return stdout, stderr, nil
}
func (rc *RunContext) execJobContainer(cmd []string, env map[string]string, user, workdir string) common.Executor {
return func(ctx context.Context) error {
return rc.JobContainer.Exec(cmd, env, user, workdir)(ctx)

View file

@ -227,12 +227,12 @@ func TestRunEvent(t *testing.T) {
tables := []TestJobFileInfo{
// Shells
{workdir, "shells/defaults", "push", "", platforms, secrets},
// TODO: figure out why it fails
// {workdir, "shells/custom", "push", "", map[string]string{"ubuntu-latest": "catthehacker/ubuntu:pwsh-latest"}, }, // custom image with pwsh
{workdir, "shells/pwsh", "push", "", map[string]string{"ubuntu-latest": "catthehacker/ubuntu:pwsh-latest"}, secrets}, // custom image with pwsh
{workdir, "shells/custom", "push", "", platforms, secrets},
{workdir, "shells/bash", "push", "", platforms, secrets},
{workdir, "shells/python", "push", "", map[string]string{"ubuntu-latest": "node:16-buster"}, secrets}, // slim doesn't have python
{workdir, "shells/node", "push", "", platforms, secrets},
{workdir, "shells/python", "push", "", platforms, secrets},
{workdir, "shells/sh", "push", "", platforms, secrets},
{workdir, "shells/pwsh", "push", "", platforms, secrets},
// Local action
{workdir, "local-action-docker-url", "push", "", platforms, secrets},

View file

@ -194,9 +194,20 @@ func (sr *stepRun) setupShell(ctx context.Context) {
if err != nil {
step.Shell = shellWithFallback[1]
}
} else if containerImage := rc.containerImage(ctx); containerImage != "" {
// Currently only linux containers are supported, use sh by default like actions/runner
step.Shell = "sh"
} else {
shell_fallback := `
if command -v bash >/dev/null; then
echo -n bash
else
echo -n sh
fi
`
stdout, _, err := rc.sh(ctx, shell_fallback)
if err != nil {
common.Logger(ctx).Error("fail to run %q: %v", shell_fallback, err)
return
}
step.Shell = stdout
}
}
}

View file

@ -14,7 +14,8 @@ jobs:
fi
check-container:
runs-on: ubuntu-latest
container: node:16-buster-slim
container:
image: code.forgejo.org/oci/node:22-bookworm
steps:
- shell: ${{ env.MY_SHELL }}
run: |

View file

@ -3,12 +3,6 @@ jobs:
check:
runs-on: ubuntu-latest
steps:
# prints version and exits, it's not valid (for github) if {0} is not included
- shell: pwsh -v '. {0}'
run: ''
check-container:
runs-on: ubuntu-latest
container: catthehacker/ubuntu:pwsh-latest
steps:
- shell: pwsh -v '. {0}'
run: ''
- shell: cat {0}
run: |
exit 1

View file

@ -1,6 +1,6 @@
on: push
jobs:
check: # GHA uses `bash` as default for runners
check: # use `bash` as default for runners
runs-on: ubuntu-latest
steps:
- run: |
@ -9,12 +9,13 @@ jobs:
else
exit 1
fi
check-container: # GHA uses `sh` as default for containers
fallback: # uses `sh` as fallback default if `bash` is not available
runs-on: ubuntu-latest
container: alpine:latest
container:
image: code.forgejo.org/oci/alpine:latest
steps:
- run: |
if [ -z ${BASH+x} ]; then
if [ -z "${BASH}" ]; then
echo "I'm sh!"
else
exit 1

View file

@ -8,13 +8,6 @@ jobs:
- shell: ${{ env.MY_SHELL }}
run: |
console.log(process.version)
check-container:
runs-on: ubuntu-latest
container: node:16-buster-slim
steps:
- shell: ${{ env.MY_SHELL }}
run: |
console.log(process.version)
check-job-default:
runs-on: ubuntu-latest
defaults:

View file

@ -4,19 +4,16 @@ env:
jobs:
check:
runs-on: ubuntu-latest
steps:
- shell: ${{ env.MY_SHELL }}
run: |
$PSVersionTable
check-container:
runs-on: ubuntu-latest
container: catthehacker/ubuntu:pwsh-latest
container:
image: code.forgejo.org/oci/ci:1
steps:
- shell: ${{ env.MY_SHELL }}
run: |
$PSVersionTable
check-job-default:
runs-on: ubuntu-latest
container:
image: code.forgejo.org/oci/ci:1
defaults:
run:
shell: ${{ env.MY_SHELL }}

View file

@ -4,14 +4,8 @@ env:
jobs:
check:
runs-on: ubuntu-latest
steps:
- shell: ${{ env.MY_SHELL }}
run: |
import platform
print(platform.python_version())
check-container:
runs-on: ubuntu-latest
container: node:16-buster
container:
image: code.forgejo.org/oci/python:slim
steps:
- shell: ${{ env.MY_SHELL }}
run: |
@ -19,6 +13,8 @@ jobs:
print(platform.python_version())
check-job-default:
runs-on: ubuntu-latest
container:
image: code.forgejo.org/oci/python:slim
defaults:
run:
shell: ${{ env.MY_SHELL }}

View file

@ -7,18 +7,7 @@ jobs:
steps:
- shell: ${{ env.MY_SHELL }}
run: |
if [ -z ${BASH+x} ]; then
echo "I'm sh!"
else
exit 1
fi
check-container:
runs-on: ubuntu-latest
container: alpine:latest
steps:
- shell: ${{ env.MY_SHELL }}
run: |
if [ -z ${BASH+x} ]; then
if [ -z "${BASH}" ]; then
echo "I'm sh!"
else
exit 1
@ -30,7 +19,7 @@ jobs:
shell: ${{ env.MY_SHELL }}
steps:
- run: |
if [ -z ${BASH+x} ]; then
if [ -z "${BASH}" ]; then
echo "I'm sh!"
else
exit 1