add option to skip dependency installation [#77 reopen] #98
Conversation
src/index.js
Outdated
| if (core.getInput("install-dependencies") === "true") { | ||
| const optionalSudoPrefix = core.getInput("sudo") === "true" ? "sudo " : ""; | ||
| await execShellCommand(optionalSudoPrefix + "apt-get update"); | ||
| await execShellCommand(optionalSudoPrefix + "apt-get install -y openssh-client xz-utils"); | ||
| } |
There was a problem hiding this comment.
Hmm. What about macOS and Windows? If anybody wants to use this Action on their self-hosted runner, they might want to skip installing the dependencies there, too.
I would have expected the if(...install-dependencies...) to span even the line if (process.platform === "darwin") { above.
There was a problem hiding this comment.
I've got your point. I agree with you!
I will change codes and push.
| @@ -32,7 +32,20 @@ describe('Tmate GitHub integration', () => { | |||
| const customConnectionString = "foobar" | |||
| execShellCommand.mockReturnValue(Promise.resolve(customConnectionString)) | |||
| await run() | |||
| expect(execShellCommand).toHaveBeenNthCalledWith(1, "pacman -Sy --noconfirm tmate") | |||
| expect(execShellCommand).toHaveBeenNthCalledWith(1, "pacman -Sy --noconfirm tmate"); | |||
| expect(core.info).toHaveBeenNthCalledWith(1, `Web shell: ${customConnectionString}`); | |||
There was a problem hiding this comment.
What I am concerned about a little bit is that main-loop test is duplicated many times.
Main-loop implementation is not platform-specific, so it seems testing main-loop only once is sufficient.
|
Only remaining question is: do we want this new option documented in the |
dscho
force-pushed
the
skip_dependency_installation
branch
from
5647912
to
f1b2ac7
Compare
I got a bit tired of waiting, so I allowed myself to force-push a fix. |
|
@dscho |
apt-get is not available on certain distributions so it's better to have the option to leave it out so we can still have the ability to run this action without having to be on ubuntu. Signed-off-by: Eli Uriegas <eliuriegas@fb.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho
force-pushed
the
skip_dependency_installation
branch
from
6a028f1
to
fa7a79e
Compare
|
@noymer and here's the new version: https://github.com/mxschmitt/action-tmate/releases/tag/v3.9. Congratulations! 🎉 |
This is copy of #77 and modified according to #77 comments.
In some situations,
apt-getcan't be called without sudo, but sudo is not permitted to user.So, skipping dependency installation is necessary in this situation.