[BUG] Forgejo Actions is missing fallback shell support #150

Open
opened 2024-02-15 18:43:04 +00:00 by msrd0 · 14 comments

(See also https://codeberg.org/forgejo/forgejo/issues/2357)

Description

According to the GitHub docs, the default behaviour if a step does not set its own shell is to use bash and fall back to sh if bash is not available. However, Forgejo Actions just fail if bash is not found:

Screenshot_2024-02-15-16-43-37.png

Forgejo Version

1.21.5+0

Operating System

Debian

How are you running Forgejo?

docker

Database

PostgreSQL

(See also https://codeberg.org/forgejo/forgejo/issues/2357) ### Description According to the [GitHub docs](https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsshell), the default behaviour if a step does not set its own shell is to use `bash` and fall back to `sh` if bash is not available. However, Forgejo Actions just fail if bash is not found: ![Screenshot_2024-02-15-16-43-37.png](https://codeberg.org/forgejo/forgejo/attachments/dc4e75e4-23fd-41ae-9e9a-12afcc1cd907) ### Forgejo Version 1.21.5+0 ### Operating System Debian ### How are you running Forgejo? docker ### Database PostgreSQL
Member

Interestingly, the code suggests it (= act, which the runner uses under the hood to run the workflow) should try sh too. I wonder why that doesn't work... perhaps there's something else that implicitly sets the shell to bash before this code is reached?

Interestingly, the [code](https://code.forgejo.org/forgejo/act/src/commit/d38e0690644fdd20d2a5dcb4bc4368327b019ada/pkg/runner/step_run.go#L180-L194) suggests it (= act, which the runner uses under the hood to run the workflow) *should* try `sh` too. I wonder why that doesn't work... perhaps there's something else that implicitly sets the shell to bash before this code is reached?
earl-warren added the
Kind/Feature
label 2024-02-15 19:15:31 +00:00
Owner

Nitpicking: note that I'm tagging this as a feature rather than a bug, simply because there is no promise to be 100% compatible with GitHub Actions.

Nitpicking: note that I'm tagging this as a feature rather than a bug, simply because there is no promise to be 100% compatible with GitHub Actions.
Member

Can you provide screenshots or the log file? The attachment doesn't exist.

Can you provide screenshots or the log file? The attachment doesn't exist.
Author

Hm, well then codeberg does something weird where it does not serve the image when requested from another domain? Anyways, here's a re-upload:

Screenshot_2024-02-15-16-43-37.png

Not sure if that's the screenshot you're looking for though, let me know if you need something else

Hm, well then codeberg does something weird where it does not serve the image when requested from another domain? Anyways, here's a re-upload: ![Screenshot_2024-02-15-16-43-37.png](/attachments/51e66c61-64bc-412f-bb97-34d0253091f4) Not sure if that's the screenshot you're looking for though, let me know if you need something else
Member

Could you provide the link to your workflow so we can test it out? And what image is your runner using?

I have just finished setting up a workflow to auto build and deploy my website built with Hugo and I don't have this issue, it could be because I use this image instead: https://github.com/catthehacker/docker_images

Could you provide the link to your workflow so we can test it out? And what image is your runner using? I have just finished setting up a workflow to auto build and deploy my website built with Hugo and I don't have this issue, it could be because I use this image instead: https://github.com/catthehacker/docker_images
Author

@Xinayder I'm not going to publish my repository, but I'm using alpine-latest which is mapped to the alpine:latest docker image. Using docker images that come with bash like the ones from catthehacker obviously don't run into the problem that bash is not available.

@Xinayder I'm not going to publish my repository, but I'm using `alpine-latest` which is mapped to the `alpine:latest` docker image. Using docker images that come with bash like the ones from catthehacker obviously don't run into the problem that bash is not available.
Author

This would be the workflow at the time I took the screenshot:

jobs:
  clang-format:
    runs-on: alpine-latest
    steps:
      - name: Install packages
        run: apk add clang17-extra-tools nodejs
      - uses: actions/checkout@v4
      - name: Check files using clang-format diffutils
        run: |
          set -exuo pipefail
          tmpdir=$(mktemp -d)
          mkdir -p $tmpdir
          find . -name '*.c' -or -name '*.h' | while read file; do
            mkdir -p "$tmpdir/$(dirname "$file")"
            clang-format "$1" >"$tmpdir/$1"
          done
          diff -Naur . $tmpdir          

The working version I'm using right now would be the following, where I had to explicitly set the shell I'm using:

jobs:
  clang-format:
    runs-on: alpine-latest
    steps:
      - name: Install packages
        run: apk add clang17-extra-tools git nodejs
        shell: ash -eo pipefail {0}
      - uses: actions/checkout@v4
      - name: Check files using clang-format diffutils
        run: |
          olddir=$(mktemp -d)
          newdir=$(mktemp -d)
          find . -name '*.c' -or -name '*.h' | while read file; do
            mkdir -p "$olddir/$(dirname "$file")"
            mkdir -p "$newdir/$(dirname "$file")"
            cp "$file" "$olddir/$file"
            clang-format "$file" >"$newdir/$file"
          done
          diff -Naur $olddir $newdir                    
        shell: ash -euo pipefail {0}
This would be the workflow at the time I took the screenshot: ```yaml jobs: clang-format: runs-on: alpine-latest steps: - name: Install packages run: apk add clang17-extra-tools nodejs - uses: actions/checkout@v4 - name: Check files using clang-format diffutils run: | set -exuo pipefail tmpdir=$(mktemp -d) mkdir -p $tmpdir find . -name '*.c' -or -name '*.h' | while read file; do mkdir -p "$tmpdir/$(dirname "$file")" clang-format "$1" >"$tmpdir/$1" done diff -Naur . $tmpdir ``` The working version I'm using right now would be the following, where I had to explicitly set the shell I'm using: ```yaml jobs: clang-format: runs-on: alpine-latest steps: - name: Install packages run: apk add clang17-extra-tools git nodejs shell: ash -eo pipefail {0} - uses: actions/checkout@v4 - name: Check files using clang-format diffutils run: | olddir=$(mktemp -d) newdir=$(mktemp -d) find . -name '*.c' -or -name '*.h' | while read file; do mkdir -p "$olddir/$(dirname "$file")" mkdir -p "$newdir/$(dirname "$file")" cp "$file" "$olddir/$file" clang-format "$file" >"$newdir/$file" done diff -Naur $olddir $newdir shell: ash -euo pipefail {0} ```
Member

If this is the case that the alpine-latest tag is mapped to alpine:latest image then there's nothing wrong, as Alpine doesn't ship bash by default. You could probably fix that by adding bash to the list of packages to be installed, unless I misunderstood the problem.

EDIT: okay, the problem is that the runner just fails without falling back to sh.

If this is the case that the `alpine-latest` tag is mapped to `alpine:latest` image then there's nothing wrong, as Alpine doesn't ship `bash` by default. You could probably fix that by adding `bash` to the list of packages to be installed, unless I misunderstood the problem. EDIT: okay, the problem is that the runner just fails without falling back to `sh`.
Author

I know that alpine doesn't ship bash. I'm not asking for forgejo to magically spawn a bash. I'm asking for forgejo to fall back to sh if bash is not available, like it is explained as the "default behaviour" in the github docs I linked in my initial post.

I know that alpine doesn't ship `bash`. I'm not asking for forgejo to magically spawn a `bash`. I'm asking for forgejo to fall back to `sh` if `bash` is not available, like it is explained as the "default behaviour" in the github docs I linked in my initial post.
Author

You could probably fix that by adding bash to the list of packages to be installed, unless I misunderstood the problem.

Well ... that command already needs a shell to execute it.

> You could probably fix that by adding bash to the list of packages to be installed, unless I misunderstood the problem. Well ... that command already needs a shell to execute it.
Member

I think I know where the problem is. Will have a go at it tomorrow, and report back.

I *think* I know where the problem is. Will have a go at it tomorrow, and report back.
Member

I setup a test repo to replicate the issue and I had to explicitly set the shell to bash for it to fail:

image

name: Test shell fallback
on:
  push:
    branches:
      - main
  workflow_dispatch:

jobs:
  clang-format:
    runs-on: self-hosted
    container:
      image: alpine:latest
    steps:
      - name: Install packages
        run: apk add clang17-extra-tools nodejs
      #- uses: actions/checkout@v4
      - name: Check shell
        run: |
          ps -ef | grep $$ | grep -v grep
          echo $0                    
        shell: sh
      - name: Check shell with ash
        run: |
          ps -ef | grep $$ | grep -v grep
          echo $0                    
        shell: ash -euo pipefail {0}
      - name: Check shell with bash
        run: |
          ps -ef | grep $$ | grep -v grep
          echo $0                    
        shell: bash
      - name: Check files using clang-format diffutils
        run: |
          set -exuo pipefail
          tmpdir=$(mktemp -d)
          mkdir -p $tmpdir                    

So, I guess the step shouldn't fail if the shell isn't found, falling back to sh if, in the example above, bash is not found?

EDIT: link to my test repo https://codeberg.org/Xinayder/runner-tests/src/branch/main/.forgejo/workflows/shell.yaml

I setup a test repo to replicate the issue and I had to explicitly set the shell to `bash` for it to fail: ![image](/attachments/48c3f286-5a63-4e9b-9e6c-b7c645eac481) ```yaml name: Test shell fallback on: push: branches: - main workflow_dispatch: jobs: clang-format: runs-on: self-hosted container: image: alpine:latest steps: - name: Install packages run: apk add clang17-extra-tools nodejs #- uses: actions/checkout@v4 - name: Check shell run: | ps -ef | grep $$ | grep -v grep echo $0 shell: sh - name: Check shell with ash run: | ps -ef | grep $$ | grep -v grep echo $0 shell: ash -euo pipefail {0} - name: Check shell with bash run: | ps -ef | grep $$ | grep -v grep echo $0 shell: bash - name: Check files using clang-format diffutils run: | set -exuo pipefail tmpdir=$(mktemp -d) mkdir -p $tmpdir ``` So, I guess the step shouldn't fail if the shell isn't found, falling back to `sh` if, in the example above, `bash` is not found? EDIT: link to my test repo https://codeberg.org/Xinayder/runner-tests/src/branch/main/.forgejo/workflows/shell.yaml
Author

@Xinayder In your example, it is fine to fail as you explicitly request bash. For me, it failed in the first step already, where no shell was explicitly requested. I'll see if I can set up a repro later.

@Xinayder In your example, it is fine to fail as you explicitly request `bash`. For me, it failed in the first step already, where no shell was explicitly requested. I'll see if I can set up a repro later.
Author

@Xinayder I got a working repro: https://msrd0.dev/msrd0/runner-test/actions/runs/1

I've set up the runner using docker-compose:

version: '3.3'

services:

  forgejo_runner:
    image: code.forgejo.org/forgejo/runner:3.3.0
    command: |
      busybox ash -exuo pipefail -c '
        apk add --no-cache sudo;
        addgroup -g 102 docker || true;
        adduser -h /data -D -u 1000 runner || true;
        addgroup runner docker;
        test -e /data/runner.json || cp /runner.json /data/runner.json;
        chown runner:runner /data/runner.json;
        chmod 600 /data/runner.json;
        sudo -u runner forgejo-runner --config /config.yml daemon
      '      
    user: root
    volumes:
      - "/somewhere/data:/data"
      - "./config.yml:/config.yml:ro"
      - "./runner.json:/runner.json:ro"
    networks:
      www:
      docker:
    restart: unless-stopped

  docker:
    image: docker:dind
    command: dockerd --tls=false --host=tcp://[::]:2375 --experimental --ipv6 --fixed-cidr-v6 fd00:fd00::/80 --ip6tables --dns 1.1.1.1
    environment:
      DOCKER_TLS_CERTDIR: ""
    privileged: true
    networks:
      www:
      docker:
        ipv4_address: 10.0.12.10
        ipv6_address: fd00::12:10
    restart: unless-stopped

networks:
  docker:
    internal: true
    driver: bridge
    enable_ipv6: true
    ipam:
      driver: default
      config:
        - subnet: 10.0.12.0/25
        - subnet: fd00::12:0/121
  www:
    driver: bridge
    enable_ipv6: true
    ipam:
      driver: default
      config:
        - subnet: 10.0.12.128/25
        - subnet: fd00::12:80/121

and these are the labels in the config.yml file:

  labels:
    - "ubuntu-latest:docker://ghcr.io/catthehacker/ubuntu:runner-latest"
    - "ubuntu-22.04:docker://ghcr.io/catthehacker/ubuntu:runner-22.04"
    - "ubuntu-20.04:docker://ghcr.io/catthehacker/ubuntu:runner-20.04"
    - "alpine-latest:docker://alpine"
    - "alpine-3.18:docker://alpine:3.18"
    - "alpine-3.19:docker://alpine:3.19"
@Xinayder I got a working repro: https://msrd0.dev/msrd0/runner-test/actions/runs/1 I've set up the runner using docker-compose: ```yml version: '3.3' services: forgejo_runner: image: code.forgejo.org/forgejo/runner:3.3.0 command: | busybox ash -exuo pipefail -c ' apk add --no-cache sudo; addgroup -g 102 docker || true; adduser -h /data -D -u 1000 runner || true; addgroup runner docker; test -e /data/runner.json || cp /runner.json /data/runner.json; chown runner:runner /data/runner.json; chmod 600 /data/runner.json; sudo -u runner forgejo-runner --config /config.yml daemon ' user: root volumes: - "/somewhere/data:/data" - "./config.yml:/config.yml:ro" - "./runner.json:/runner.json:ro" networks: www: docker: restart: unless-stopped docker: image: docker:dind command: dockerd --tls=false --host=tcp://[::]:2375 --experimental --ipv6 --fixed-cidr-v6 fd00:fd00::/80 --ip6tables --dns 1.1.1.1 environment: DOCKER_TLS_CERTDIR: "" privileged: true networks: www: docker: ipv4_address: 10.0.12.10 ipv6_address: fd00::12:10 restart: unless-stopped networks: docker: internal: true driver: bridge enable_ipv6: true ipam: driver: default config: - subnet: 10.0.12.0/25 - subnet: fd00::12:0/121 www: driver: bridge enable_ipv6: true ipam: driver: default config: - subnet: 10.0.12.128/25 - subnet: fd00::12:80/121 ``` and these are the labels in the `config.yml` file: ```yml labels: - "ubuntu-latest:docker://ghcr.io/catthehacker/ubuntu:runner-latest" - "ubuntu-22.04:docker://ghcr.io/catthehacker/ubuntu:runner-22.04" - "ubuntu-20.04:docker://ghcr.io/catthehacker/ubuntu:runner-20.04" - "alpine-latest:docker://alpine" - "alpine-3.18:docker://alpine:3.18" - "alpine-3.19:docker://alpine:3.19" ```
Sign in to join this conversation.
No milestone
No project
No assignees
4 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: forgejo/runner#150
No description provided.