Fix semver checks

[Lorak-mmk]

Previously semver checks CI used /dev/tty which seems unavailable in Github Actions.
Changed it to /proc/self/fd/2 which I expect to be available pretty much anywhere.
Changed this CI step so that the job fails on errors. It wouldn't prevent #936 , because this command is part of pipeline, so it won't be affected by set -e, but I think it's a good change anyway.

Fixes: #936

Pre-review checklist

• I have split my patch into logically separate commits.
• All commit messages clearly explain what they change and why.
• I added relevant tests for new features and bug fixes.
• All commits compile, pass static checks and pass test.
• PR description sums up the changes and reasons why they should be introduced.
• I have provided docstrings for the public items that I want to introduce.
• I have adjusted the documentation in ./docs/source/.
• I added appropriate Fixes: annotations to PR description.

[Lorak-mmk]

Tested that it works now in #943

[piodul]

It wouldn't prevent #936 , because this command is part of pipeline, so it won't be affected by set -e, but I think it's a good change anyway.

So, what kind of new errors will we catch now? Why can't we catch errors such as those which manifested in #936 ?

[Lorak-mmk]

It wouldn't prevent #936 , because this command is part of pipeline, so it won't be affected by set -e, but I think it's a good change anyway.

So, what kind of new errors will we catch now? Why can't we catch errors such as those which manifested in #936 ?

Not sure if any, as the rest of the script is just a bunch of echo's - so set -e / absence of continue-on-error probably won't help much.

I'm not sure how to best prevent such errors. I guess we could manually check output of steps of the pipeline using PIPESTATUS array and manually error out if second or third step failed. One disadvantage is that it would be more bash code that could fail. Would you like me to do this?

[piodul]

I'm not sure how to best prevent such errors. I guess we could manually check output of steps of the pipeline using PIPESTATUS array and manually error out if second or third step failed. One disadvantage is that it would be more bash code that could fail. Would you like me to do this?

I educated myself on bash error handling a little bit and I think we both make it work and make it readable. I applied what I learnt to your code and I think it should look like this after modifications:

set -e # So that failed commands exit the script
set -o pipefail # So that if a command in a pipe fails, the whole command fails

echo "output<<SEMVER_STDOUT_EOF" >> $GITHUB_OUTPUT
SEMVER_REV_OUTPUT="$(semver-rev rev="$PR_BASE" 2>&1)" && true # "&& true" preserves exit code but cancels effects of set -e
exitcode=$?

# Weird sed strip ANSI colors from output
# If any of the commands below fail, `set -e` and `set -o pipefail` should exit the script
echo "${SEMVER_REV_OUTPUT}" | tee /proc/self/fd/2 | sed -r "s/\x1B\[([0-9]{1,2}(;[0-9]{1,2})?)?[mGK]//g" >> $GITHUB_OUTPUT
echo "Semver checks exitcode: " $exitcode
echo "SEMVER_STDOUT_EOF" >> $GITHUB_OUTPUT
echo "exitcode=$exitcode" >> $GITHUB_OUTPUT

Note that I didn't test this code, this is just to demonstrate the ideas.

[wprzytula]

SEMVER_REV_OUTPUT="$(semver-rev rev="$PR_BASE" 2>&1)" && true # "&& true" preserves exit code but cancels effects of set -e

Are you sure you meant && true, not || true ? IIUC, what we want to achieve here is that even if semver-rev command fails, set -e won't trigger an exit at this point.

[Lorak-mmk]

SEMVER_REV_OUTPUT="$(semver-rev rev="$PR_BASE" 2>&1)" && true # "&& true" preserves exit code but cancels effects of set -e

Are you sure you meant && true, not || true ? IIUC, what we want to achieve here is that even if semver-rev command fails, set -e won't trigger an exit at this point.

I tried it and && seems correct. There is a slight error in @piodul's code - " shouldn't be there, so the line should look like this:

SEMVER_REV_OUTPUT=$(semver-rev rev="$PR_BASE" 2>&1) && true # "&& true" preserves exit code but cancels effects of set -e

If we used ||, it would not preserve exit code.

karolbaryla@localhost:~$ set -e
karolbaryla@localhost:~$ set_status() { return "$1"; }
karolbaryla@localhost:~$ $(set_status 2) && true
karolbaryla@localhost:~$ echo $?
2

karolbaryla@localhost:~$ set -e
karolbaryla@localhost:~$ set_status() { return "$1"; }
karolbaryla@localhost:~$ $(set_status 2) || true
karolbaryla@localhost:~$ echo $?
0

[piodul]

SEMVER_REV_OUTPUT="$(semver-rev rev="$PR_BASE" 2>&1)" && true # "&& true" preserves exit code but cancels effects of set -e

Are you sure you meant && true, not || true ? IIUC, what we want to achieve here is that even if semver-rev command fails, set -e won't trigger an exit at this point.

Yes, I meant && true. https://stackoverflow.com/questions/2870992/automatic-exit-from-bash-shell-script-on-error/2871034#2871034

[Lorak-mmk]

Updated the PR according to @piodul 's suggestion

[piodul]

@Lorak-mmk although this is already merged, please update the PR description to reflect reality (the "It wouldn't prevent" part looks outdated)