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

Manage special bash options when no shell is defined in a workflow #2448

Closed
sebastien-perpignane opened this issue Sep 10, 2024 · 10 comments · Fixed by #2449
Closed

Manage special bash options when no shell is defined in a workflow #2448

sebastien-perpignane opened this issue Sep 10, 2024 · 10 comments · Fixed by #2449
Labels
kind/bug Something isn't working

Comments

@sebastien-perpignane
Copy link
Contributor

sebastien-perpignane commented Sep 10, 2024

Bug report info

act version:            0.2.66
GOOS:                   linux
GOARCH:                 amd64
NumCPU:                 4
Docker host:            DOCKER_HOST environment variable is not set
Sockets found:
        /var/run/docker.sock
Config files:           
        /home/derek/.config/act/actrc:
                -P ubuntu-latest=catthehacker/ubuntu:act-latest
                -P ubuntu-22.04=catthehacker/ubuntu:act-22.04
                -P ubuntu-20.04=catthehacker/ubuntu:act-20.04
                -P ubuntu-18.04=catthehacker/ubuntu:act-18.04
Build info:
        Go version:            go1.23.0
        Module path:           command-line-arguments
        Main version:          
        Main path:             
        Main checksum:         
        Build settings:
                -buildmode:           exe
                -compiler:            gc
                -ldflags:             -X main.version=0.2.66
                DefaultGODEBUG:       asynctimerchan=1,gotypesalias=0,httplaxcontentlength=1,httpmuxgo121=1,httpservecontentkeepheaders=1,tls10server=1,tls3des=1,tlskyber=0,tlsrsakex=1,tlsunsafeekm=1,winreadlinkvolume=0,winsymlink=0,x509keypairleaf=0,x509negativeserial=1
                CGO_ENABLED:          1
                CGO_CFLAGS:           
                CGO_CPPFLAGS:         
                CGO_CXXFLAGS:         
                CGO_LDFLAGS:          
                GOARCH:               amd64
                GOOS:                 linux
                GOAMD64:              v1
Docker Engine:
        Engine version:        27.2.1
        Engine runtime:        runc
        Cgroup version:        1
        Cgroup driver:         cgroupfs
        Storage driver:        overlay2
        Registry URI:          https://index.docker.io/v1/
        OS:                    Linux Mint 20.3
        OS type:               linux
        OS version:            20.3
        OS arch:               x86_64
        OS kernel:             5.4.0-193-generic
        OS CPU:                4
        OS memory:             15880 MB
        Security options:
                name=apparmor
                name=seccomp,profile=builtin

Command used with act

act -v -P ubuntu-latest=localhost:5000/act-ubuntu-mvn:latest --remote-name github -j check-dependencies  workflowdispatch

Describe issue

The job fails when executing this bash expression on a file that does not contain any dependency which is updatable:

nb_updatable_dependencies=$(cat updatable.txt | grep . | grep -v '^No dependencies' | wc -l)

It fails because bash is called with :

-o pipefail

This option is not used in github actions.

This issue was already fixed : #528
But the "-o pipefail" option came back in this PR: #575

Link to GitHub repository

https://github.com/sebastien-perpignane/cardgame

Workflow content

name: "Check dependencies"

on:
  schedule:
    - cron: '35 1 * * *'

  workflow_dispatch:

jobs:
  check-dependencies:
    uses: sebastien-perpignane/my-workflows/.github/workflows/check-dependencies.yml@main
    with:
      java-version: '21'
      distribution: 'temurin'

Relevant log output

[check-dependencies/Check maven dependency updates/check-dependency-updates]   🐳  docker exec cmd=[bash --noprofile --norc -e -o pipefail /var/run/act/workflow/2] user= workdir=
[check-dependencies/Check maven dependency updates/check-dependency-updates] [DEBUG] Exec command '[bash --noprofile --norc -e -o pipefail /var/run/act/workflow/2]'
[check-dependencies/Check maven dependency updates/check-dependency-updates] [DEBUG] Working directory '/home/derek/workspace/cardgame'
| [INFO] Scanning for projects...
| [INFO] 
| [INFO] -------------------< sebastien.perpignane:cardgame >--------------------
| [INFO] Building cardgame 0.0.1-SNAPSHOT
| [INFO]   from pom.xml
| [INFO] --------------------------------[ jar ]---------------------------------
| [INFO] 
| [INFO] --- versions:2.17.1:display-dependency-updates (default-cli) @ cardgame ---
| [INFO] No dependencies in Dependencies have newer versions.
| [INFO] 
| [INFO] No dependencies in Plugin Dependencies have newer versions.
| [INFO] 
| [INFO] ------------------------------------------------------------------------
| [INFO] BUILD SUCCESS
| [INFO] ------------------------------------------------------------------------
| [INFO] Total time:  1.957 s
| [INFO] Finished at: 2024-09-10T20:07:03Z
| [INFO] ------------------------------------------------------------------------
[check-dependencies/Check maven dependency updates/check-dependency-updates]   ❌  Failure - Main Check dependencies
[check-dependencies/Check maven dependency updates/check-dependency-updates] exitcode '1': failure

Additional information

Dockerfile to build the custom image I use (act-ubuntu-mvn image) -> https://gist.github.com/sebastien-perpignane/ade17b1922d35147d08cb08169a6a388

I submitted a PR as it is a very simple fix, even for someone who does not know Go, like me :)

@ChristopherHX
Copy link
Contributor

If you add defaults.run.shell: bash (short hand syntax, not valid yml) then pipefail will be used by GitHub Actions.

For your example is your suggestion not really correct, because they use the sh template (that only has -e) for bash if you don't tell the workflow to use bash on linux/mac (due to a bug?)

See here explicit bash has that flag https://github.com/actions/runner/blob/ddf41af7678a8bc585d9e6b8ab70d941cdb687b2/src/Runner.Worker/Handlers/ScriptHandlerHelpers.cs#L18

@sebastien-perpignane
Copy link
Contributor Author

sebastien-perpignane commented Sep 10, 2024

Hello @ChristopherHX

Thanks a lot for your quick answer.

You're right.
I just changed my callable workflow with
shell: bash
and now it fails for the same reason when I run it in github.

Then, what would you think about running bash without pipefail in act when no explicit shell is declared on a step ?I guess it would make act better synchronized with the github actions behavior, right ?

Something like this :

case "":
    shellCommand = "bash -e {0}"
case "sh":
    shellCommand = "sh -e {0}"
case "bash":
    shellCommand = "bash --noprofile --norc -e -o pipefail {0}"

I'm just trying to find a solution to make act behavior as close as possible to github actions one. It will help me to convince my team mates to use act to win some time when implementing GitHub jobs ;)

@sebastien-perpignane
Copy link
Contributor Author

Hi again,

It does not seem to be due to a bug as it is a documented behavior:

https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#defaultsrunshell

Supported platform shell parameter Description Command run internally
Linux / macOS unspecified The default shell on non-Windows platforms. Note that this runs a different command to when bash is specified explicitly. If bash is not found in the path, this is treated as sh. bash -e {0}

@ChristopherHX
Copy link
Contributor

It does not seem to be due to a bug as it is a documented behavior:

Or an old bug, then they merged a PR to document the current state. That wouldn't be the first time, where a fix would potential break existing workflows and then nothing is changed.

Then, what would you think about running bash without pipefail in act when no explicit shell is declared on a step ?I guess it would make act better synchronized with the github actions behavior, right ?

Something like this :

case "":
    shellCommand = "bash -e {0}"
case "sh":
    shellCommand = "sh -e {0}"
case "bash":
    shellCommand = "bash --noprofile --norc -e -o pipefail {0}"

I'm just trying to find a solution to make act behavior as close as possible to github actions one. It will help me to convince my team mates to use act to win some time when implementing GitHub jobs ;)

This looks like a good way to handle this and I give my 👍, however I remember the code may have somewhere a if shell == "" then shell = "bash" that would escape your change.

@sebastien-perpignane
Copy link
Contributor Author

however I remember the code may have somewhere a if shell == "" then shell = "bash" that would escape your change.

I'll check this, thanks for the tip.

sebastien-perpignane added a commit to sebastien-perpignane/act that referenced this issue Sep 12, 2024
sebastien-perpignane added a commit to sebastien-perpignane/act that referenced this issue Sep 12, 2024
sebastien-perpignane added a commit to sebastien-perpignane/act that referenced this issue Sep 12, 2024
@sebastien-perpignane sebastien-perpignane changed the title bash "-o pipefail" is not used in Github Actions Manage special bash options when no shell is defined in a workflow Sep 12, 2024
@sebastien-perpignane
Copy link
Contributor Author

sebastien-perpignane commented Sep 12, 2024

Hi @ChristopherHX

Please have a look at my PR. I changed the code according to what we agreed.

I tested on my workflow:

  • bash -e {0} when no shell is defined
  • bash --noprofile --norc -e -o pipefail {0} when a shell is defined

as expected.

@ChristopherHX
Copy link
Contributor

ChristopherHX commented Sep 13, 2024

only the lint error tab vs. spaces and my suggestion to ignore WorkflowShell for yaml are blocking my 👍

Se tests failed as well,not shure what they do


--- FAIL: TestRunEvent/defaults-run (3.95s)
=== RUN   TestStepRun
--- FAIL: TestStepRun (0.04s)

@sebastien-perpignane
Copy link
Contributor Author

sebastien-perpignane commented Sep 13, 2024

Thanks for the quick review !

first time ever I code with Go, I'm learning ^^

I'll take all the suggestions in consideration and fix the tests, no problem

sebastien-perpignane added a commit to sebastien-perpignane/act that referenced this issue Sep 14, 2024
@sebastien-perpignane
Copy link
Contributor Author

Hey @ChristopherHX thanks for running the checks, linter is happy now, I'm working on test failures.

sebastien-perpignane added a commit to sebastien-perpignane/act that referenced this issue Sep 14, 2024
sebastien-perpignane added a commit to sebastien-perpignane/act that referenced this issue Sep 14, 2024
…defined - bonus

fix inverted expected and actual in TestGetGitHubContext assertions
sebastien-perpignane added a commit to sebastien-perpignane/act that referenced this issue Sep 15, 2024
sebastien-perpignane added a commit to sebastien-perpignane/act that referenced this issue Sep 15, 2024
sebastien-perpignane added a commit to sebastien-perpignane/act that referenced this issue Sep 15, 2024
…defined - bonus

fix inverted expected and actual in TestGetGitHubContext assertions
sebastien-perpignane added a commit to sebastien-perpignane/act that referenced this issue Sep 15, 2024
@sebastien-perpignane
Copy link
Contributor Author

I finally managed to make the tests work properly on my laptop.
With my last commit, all is green locally.
Please run the check actions when you can.

sebastien-perpignane added a commit to sebastien-perpignane/act that referenced this issue Sep 15, 2024
…defined

* bash without "-o pipefail" option when "bash" is not explicitely
defined in the workflow
* bonus: fix inverted expected and actual in TestGetGitHubContext assertions
sebastien-perpignane added a commit to sebastien-perpignane/act that referenced this issue Sep 15, 2024
…defined

* bash without "-o pipefail" option when "bash" is not explicitely
defined in the workflow
* bonus: fix inverted expected and actual in TestGetGitHubContext assertions
mergify bot pushed a commit that referenced this issue Sep 19, 2024
#2449)

* bash without "-o pipefail" option when "bash" is not explicitely
defined in the workflow
* bonus: fix inverted expected and actual in TestGetGitHubContext assertions
@mergify mergify bot closed this as completed in #2449 Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants