Combine stdout and stderr when getting version from requried commands

[jordiclariana]

What issue type does this pull request address? (keep at least one, remove the others) /kind enhancement

What does this pull request do? Which issues does it resolve? (use resolves #<issue_number> if possible) Some required commands output their version on stderr instead of stdout. Devspace is not handling stderr when running command version so it is not possible to assess that the command is installed with its right version. This change changes the behavior so stdout and stderr are combined and used the same to look for the version string/

Please provide a short message that should be published in the DevSpace release notes Combine stdout and stderr for required commands when getting their version

What else do we need to know? This can potentially break compatibility since some programs can mix version data between stdout and stderr output. I assume this is an edge case that should not be common at all but is still possible.

[netlify[bot]]

✅ Deploy Preview for devspace-docs canceled.

Built without sensitive environment variables

Name	Link
🔨 Latest commit	ef5a4130c6ff11ca72184fe794b2aeeceed93ae9
🔍 Latest deploy log	https://app.netlify.com/sites/devspace-docs/deploys/6409bdfe03648b0008e42fba

[jordiclariana]

Hi @lizardruss, here're some examples I'm working on:

Full disclosure: I'm also contributing to this PR which adds Tanka support to devspace.

• Tanka: tk --version
• jsonnet-bundler: jb --version

I tend to agree with you that combining stdout and stderr is not in general a good thing to do, I've written my opinion on that in my initial comment. But I think that in this case is the less dangerous one. We are using a well-known particular switch (version, --version, -V, ...) to get the version, and stdout and stderr mixing should not be something that might collide with each other in this case. Most likely, either there is stdout only or stderr only. That is at least my rationality.

As an alternative, we might add another field under require.command[] that specifies which output we want to capture, but I thought that can be overkill for such a task. I can develop it though if you think is better.

[jordiclariana]

I missed this in my comment:

It's possible to add a small script to invoke the command and redirect stderr to stdout

I guess you're referring on having a wrapper to capture the stderr and throw it into stdout. That is possible, although

1. either a wrapper with the name of the original tool needs to created per tool (since we use require.commands[].name as the executable to run)

require:
  commands:
    - name: tk_wrapper.sh
      versionArgs: ["--version"]
    - name: jb_wrapper.sh
      versionArgs: ["--version"]

2. or use the same name for all commands with different versionArgs:

require:
  commands:
    - name: wrapper.sh
      versionArgs: ["tk", "--version"]
    - name: wrapper.sh
      versionArgs: ["jb", "--version"]

On option 1 I think it can be too cumbersome to maintain. Option 2 I have doubts it might work (right now I don't know if name collision is checked). In any case, none of these look to me elegant solutions, although I agree it will maintain compatibility (if there is any non-compatibility on my original approach).