Using shell scripts in .github/workflow

[Sdowney]

I’ve seen a few projects in the wild putting the code for part of a CI action in a shell script, rather than putting it inline in the yaml for the action. On the pro side that makes it much more testable outside a github action, on the con side it’s a layer of indirection and one more thing to support.

Does anyone have strong feelings about not doing it, before I dive in and pull out some of the optional run blocks into a separate shell script?

For example: draft/.github/workflows/check.yml at main · cplusplus/draft · GitHub

  - name: check-output.sh
    run: ../tools/check-output.sh

rather than putting all of draft/tools/check-source.sh at main · cplusplus/draft · GitHub inline which clearly would not scale.

or

.github/workflows/validate-header-guards.yml

  - name: Check that there are no duplicated filenames
    run: |
      ./tools/scripts/checkDuplicateFilenames.py

We’re doing similar things in places, but I’m starting to thing that yaml may not be the best place to write shell scripts?

[chayden83]

I actually think moving script logic out of the CI YAML files is a good idea. I remember when I was learning GitLab CI there were a number of arguments and articles that I found compelling on that front. Ultimately, the strongest argument for me was the testing argument that @Sdowney mentioned.

[Jeff-Garland]

I think the only problem is we can’t count on things like bash being everywhere – so it’s need to be something like python.

[Sdowney]

The script that’s embedded in the yaml is sh or bash now, so it wouldn’t be a regression. I have no objection to python of course. But I think it’s changing from one very technically difficult to run locally implementation for one that is possible for more people?

[Jeff-Garland]

Hmm I wonder if I’m thinking about this wrong – if this is exclusively to run in github land then maybe it’s fine?

[neatudarius]

I would address you other related question: how do we manage to reuse CI flows of not by using scripts?

E.g

• infra repo: ci_build_and_test.sh
• optional: has a simple ci_build_and_test.sh file which runs the script from other repo
• Same for exemplar!

I think we should aime to not have duplicated scripts.

[Jeff-Garland]

@neatudarius

optional: has a simple ci_build_and_test.sh file which runs the script from other repo

I seem to recall @dsankel being much against doing this since the ‘no access to the internet’ installs will not be able to use. But again, if the context here is github workflows I think that doesn’t apply because these aren’t run by hand but only in a github pipeline, right?

[river]

My take is… I think 90% of CI scripts should not be so long that they need to be in a separate file. If it’s give or take <10 lines, the indirection is not worth it.

GitHub actions are not testable without just triggering the workflow manually or spamming commit at another branch.

So are most bash script.

Which chunks of scripts are you referring to

[Sdowney]

This gets repeated more than a few times:

.github/workflows/ci.yml

and

.github/workflows/ci.yml

is on the edge of what I’d like to factor out, and actually shellcheck before we find some interpolation vulnerability?

Although the first, on the other hand, in practice, I duplicate in Makefile. On the gripping hand, DRY.

[Jeff-Garland]

My $.02 is that is clearly enough to factor out…

[river]

@Sdowney

set -x
cmake --build .build --config Asan --target all_verify_interface_header_sets -- -k 0
cmake --build .build --config Asan --target all -- -k 0

I don’t think this is necessary to be put in another file? It’s 3 lines… I don’t think the indirection is worth it.

@Sdowney

run: |
            issue_num=$(gh issue list -s open -S "[SCHEDULED-BUILD] Build & Test failure" -L 1 --json number | jq 'if length == 0 then -1 else .[0].number end')
          
            body="**Build-and-Test Failure Report**
            - **Time of Failure**: $(date -u '+%B %d, %Y, %H:%M %Z')
            - **Commit**: [${{ github.sha }}](${{ github.server_url }}/${{ github.repository }}/commit/${{ github.sha }})
            - **Action Run**: [View logs](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }})
          
            The scheduled build-and-test triggered by cron has failed.
            Please investigate the logs and recent changes associated with this commit or rerun the workflow if you believe this is an error."
          
            if [[ $issue_num -eq -1 ]]; then
              gh issue create --repo ${{ github.repository }} --title "[SCHEDULED-BUILD] Build & Test failure" --body "$body"
            else
              gh issue comment --repo ${{ github.repository }} $issue_num --body "$body"
            fi

I agree this should be refactored out. This is universal among all libraries, I can go factor this out as a GitHub Actions Package.

@neatudarius

I would address you other related question: how do we manage to reuse CI flows of not by using scripts?

E.g

• infra repo: ci_build_and_test.sh
• optional: has a simple ci_build_and_test.sh file which runs the script from other repo
• Same for exemplar!

I think we should aime to not have duplicated scripts.

There’s a more GitHub Action integrated way to do this, see: we can package them as GitHub Actions dependency. I am happy to help with this.