Problem: macOS doesn't have /usr/bin/bash #306
Conversation
Solution: Specify no shell, default to /bin/sh
There was a problem hiding this comment.
Great idea; neat to see someone trying to run this action on MacOS for the first time. Please take a look at the contributing guide, and feel free to let us know if you have any feedback or encounter any issues. This change implies an update to the "Supported Runners" section of the README.
| @@ -8,7 +8,7 @@ const execBashCommand = async (command: string): Promise<string> => { | |||
| info(command); | |||
| let output = ""; | |||
| try { | |||
| const result = await execAsPromised(command, { shell: "/usr/bin/bash" }); | |||
| const result = await execAsPromised(command); | |||
There was a problem hiding this comment.
According to the Node.js docs:
Although Microsoft specifies
%COMSPEC%must contain the path to'cmd.exe'in the root environment, child processes are not always subject to the same requirement. Thus, inchild_processfunctions where a shell can be spawned,'cmd.exe'is used as a fallback ifprocess.env.ComSpecis unavailable.
So, I believe this change either eliminates or at least reduces Windows support. I wonder if "/bin/sh" or similar might work on all platforms. The most expedient way I can think of to test alternate shell paths would be to use a matrix strategy to run the Save Cache and Restore Cache jobs on ubuntu-22.04, macos-12, and windows-2022. This would necessitate replacing docker-cache-test-${{ github.run_id }}-${{ github.run_attempt }} with docker-cache-test-${{ matrix.os }}-${{ github.run_id }}-${{ github.run_attempt }} to ensure no unintended collisions in the cache key used by the Save Cache and Restore Cache steps.
There was a problem hiding this comment.
I am not sure I understand the docs in that part very well (as in, how the removal of shell: "/usr/bin/bash" affects Windows operations).
There was a problem hiding this comment.
I take it to mean that Windows typically defaults to a non-Bash shell, such as PowerShell or Command Prompt.
There was a problem hiding this comment.
If you specify /usr/bin/bash, that wouldn't work on Windows anyway, would it? (I am unfortunately not very familiar with intricacies of Windows toolchains)
There was a problem hiding this comment.
Yes, it does; that's actually why we bothered to specify the shell in the first place. Windows ships with Bash these days.
There was a problem hiding this comment.
Unfortunately, the macos-12 image doesn't ship with Docker, so docker-cache can't be used on macOS. @mwarres is adding Windows support in #347 though.
There was a problem hiding this comment.
Windows support has been released in 0.3.0. I am closing this pull request as untenable for now. Please feel free to reach back out if GitHub Actions adds Docker to their macOS runners or you find some other way to achieve macOS support.
There was a problem hiding this comment.
There was a problem hiding this comment.
Can this be merged? Self-hosted runners are not affected by the above issue.
There was a problem hiding this comment.
Not as is:
I believe this change either eliminates or at least reduces Windows support.
I am happy to accept a pull request that adds support for self-hosted macOS runners in a reasonably general way without eliminating Windows support.
Solution: Specify no shell, default to /bin/sh