Document behaviour of SSH functionality

[daften]

Mainly how it differs from SSH functionality in robo core and why (stopOnFail flag and concatenation of commands)

[daften]

@Jelle-S something for monday? :)

[daften]

https://github.com/digipolisgent/robo-digipolis-deploy/blob/bf749564bd83e99e09fc038b5f783a29726c43f6/src/Ssh.php#L285 shows that when stoponfail is true, the result is an error, however. when stoponfail is false and there is an error message, stoponfail isn't taken into account.

[Jelle-S]

Ok, so should we change this behavior? Meaning that if stopOnFail is false, we always return Result::success($this)? Or should we document the current behavior?

What Robo themselves do is pretty weird too tho, see https://github.com/consolidation/Robo/blob/b027871e12c7433457fb326be64fc81198d03f51/src/Task/CommandStack.php

Looking at the code, when $this->stopOnFail === false, they concatenate the commands with &&, meaning that when a command fails, the rest will not be executed and exit code 1 (or exit code > 0) will be returned. When $this->stopOnFail === true, they basically do the exact same thing, but explicitly execute the commands one by one. So basically, $this->stopOnFail means nothing here.

The for the ssh tasks defined by Robo (https://github.com/consolidation/Robo/blob/047af4b4465e5381d5ee691a7ee8194ede890c9c/src/Task/Remote/Ssh.php#L210) the behavior is different too. Based on $this->stopOnFail they concatenate the commands by && ($this->stopOnFail === true) or by ; ($this->stopOnFail === false. So that behavior is different from the one in the CommandStack.

I personally think the behavior is the Robo CommandStack is wrong and probably a bug. So we should decide whether we want to keep our current behavior and document it (if a command fails with $this->stopOnFail === false we will still execute all following command but will also return an error result to indicate that at least one command failed.) or whether we want to implement it the way it is implemented in the Robo ssh task (if $this->stopOnFail === false we execute all commands and always return an exit code of 0).

Any thoughts @daften?

[matthijs-va]

FYI: consolidation/robo#697