-
Notifications
You must be signed in to change notification settings - Fork 193
Migrate from bash to sh in snyk/actions/setup #159
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
base: master
Are you sure you want to change the base?
Conversation
|
Hey @fabasoad - thanks for your contribution. I am not 100% sure I trust my own tests enough to merge this. Will think a bit about it. According to Snyk's breaking change policy this could also be seen as a breaking change, considering how all downstream users of this action are currently using it. Just a heads-up that this might be a bit before you hear something. |
|
Hi @dotkas! yeah, makes sense. I just stepped on this issues a few times in the past and thought would be good to fix it. Later on dependency on |
|
TIL, that GitHub actually has Linux arm64 and macOS Intel runners: https://docs.github.com/en/actions/how-tos/using-github-hosted-runners/using-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories I will add some tests. |
|
Your PR has not had any activity for 30 days. In 2 days I'll close it. Make some activity to remove this. |
|
Hi @dotkas! WDYT about this PR? Would it be OK to move it forward or should we close it? |
|
Hi @fabasoad I still think it's good, but our internal processes are taking way longer than I had expected. I am sorry for the delay on it. |
|
Your PR has not had any activity for 30 days. In 2 days I'll close it. Make some activity to remove this. |
|
Your PR has not had any activity for 30 days. In 2 days I'll close it. Make some activity to remove this. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Your PR has not had any activity for 30 days. In 2 days I'll close it. Make some activity to remove this. |
|
Your PR has not had any activity for 30 days. In 2 days I'll close it. Make some activity to remove this. |
Problem
Currently,
snyk/actions/setupsupportsAlpineOS. There are no Alpine OS GitHub hosted runners, so usual use case is to usealpinedocker image (e.g.alpine:latest). Official Alpine docker image does not includebashmeaning that user has to install it in advance every time.Solution
Since
snyk/actions/setupis a composite action that runs shell script it is not difficult to rewritebashtosh. It will remove the dependency onbash. Furthermore, it is not difficult to do as currentsetup_snyk.shshell script is already POSIX compliant. Only a few minor changes required to make this GitHub Action work withoutbashdependency.Validation results
I've run this GitHub Action before fix and after fix, and everything works as expected.
Before fix
CI pipeline
Workflow configuration
Logs
After fix
CI pipeline
Workflow configuration
Logs