fix!: fallback to sh if bash does not exist #177
sclu1034
viceice
Labels
No labels
Compat/Breaking
Kind/Bug
Kind
Chore
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
No milestone
No project
No assignees
5 participants
No due date set.
Dependencies
No dependencies set.
Reference
forgejo/act!177
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "earl-warren/act:wip-sh-fallback"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
It is a breaking change because it changes how the shell is determined.
Before, if
jobs.<job_id>.container.imageis set and the shell is not specified, it defaults toshinstead ofbash.After, regardless of
jobs.<job_id>.container.image, if the shell is not specified, it defaults tobashif available, otherwise it defaults tosh.Tests run at https://code.forgejo.org/forgejo/act/actions/runs/963/jobs/1#jobstep-4-2333
Closes forgejo/runner#150
39f64b89a784cff685c884cff685c86019db73816019db73814b7f0e93e9WIP: fix!: fallback to sh if bash does not exist [skip cascade]to fix!: fallback to sh if bash does not exist [skip cascade]@ -199,1 +197,3 @@step.Shell = "sh"} else {shell_fallback := `if which bash >/dev/null 2>/dev/null; thenwhichis probably good enough, though the POSIX way would becommand -vI confess I did not know about this:
You suggest replacing with
if command -v bash; then?4e25206774..bf530e002e4b7f0e93e94e252067744e25206774bf530e002efix!: fallback to sh if bash does not exist [skip cascade]to fix!: fallback to sh if bash does not existbf530e002e9628be0a7acascading-pr updated at forgejo/runner#692
@ -589,0 +599,4 @@env[k] = v}name := "throwaway-script.sh"Should use a randomly generated name, e.g. like
os.CreateTempdoes, to avoid avenues of code injection when someone knows the file name.Would also allow multiple to run in parallel, if that ever came up.
sounds like a good idea
0fe66981cd..b937bc1f75worried about the removed pwsh tests, that hould be done in a seperate pr with a description why.
otherwise LGTM
@ -1,25 +0,0 @@on: pushenv:MY_SHELL: pwshwhy removing all pwsh tests? too slow?
They require proprietary software (well, a mix of MIT and proprietary really).
Hum....
@Crown0815 I'm confused there. Is there a Free Software version of pwsh that can be downloaded from somewhere that does not include proprietary components?
I see https://github.com/PowerShell/PowerShell/releases/tag/v7.5.2 and there is a .deb there.
@earl-warren wrote in #177 (comment):
Just wanted to send you the link. This is it.
I will add the tests back.
@viceice @Crown0815 adding them to the general purpose image used for CI seems sensible right now. Do you concur?
forgejo/ci-image-builder#6
sure, here's how I do it on Ubuntu
github.com/containerbase/base@a7d11772ea/src/usr/local/containerbase/tools/v2/powershell.shRestored and simplified pwsh tests.
9628be0a7a..64666db7b0fix!: fallback to sh if bash does not existto fix!: fallback to sh if bash does not exist [skip cascade]9628be0a7a64666db7b0fix!: fallback to sh if bash does not exist [skip cascade]to fix!: fallback to sh if bash does not existfix!: fallback to sh if bash does not existto fix!: fallback to sh if bash does not exist [skip cascade]64666db7b00fe66981cd0fe66981cdb937bc1f75As there was no function to create a random name in ACT, I added one in a separate commit and used it where code as duplicated. This should also be reviewed.
!177 (commit
1f841f15bb)fix!: fallback to sh if bash does not exist [skip cascade]to fix!: fallback to sh if bash does not existb937bc1f7592219f9e8fcascading-pr updated at forgejo/runner#692
@ -229,4 +229,1 @@{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 pwshshould we use our CI Image for these tests or doesn't change it anything for code coverage?
It changes nothing in this context. I assume this image is just ubuntu + pwsh as the name suggests and I don't see that it would change anything in term of coverage.