Skip to content

Conversation

@cdivitotawela
Copy link
Contributor

@cdivitotawela cdivitotawela commented Mar 25, 2022

Fix for #996

When the script executed on a runner which does not have python2, script incorrectly execute the code inside the if condition. This script runs using /bin/sh in Ubuntu and cause the problem. Fix for this issue to use different syntax in output redirection.

Issue is reproduced using running the job on Ubuntu container. Fix also verified in the action https://github.com/cdivitotawela/codeql-issue/actions/runs/2038530172

Tested using action file https://github.com/cdivitotawela/codeql-issue/blob/main/.github/workflows/investigate.yaml

Merge / deployment checklist

@cdivitotawela cdivitotawela requested a review from a team as a code owner March 25, 2022 06:42
Copy link
Member

@RasmusWL RasmusWL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I guess I did not confirm that the command -v worked as intended back then. I have been able to reproduce with the code below (on Ubuntu 21.10)

#!/bin/sh

if command -v nonexistingcommand &> /dev/null; then
    echo "old true"
else
    echo "old false"
fi

if command -v nonexistingcommand > /dev/null 2>&1; then
    echo "new true"
else
    echo "new false"
fi

which outputs

old true
new false

I can't say I fully understand this (in particular what the old version actually did), but thanks for the fix 💪

@RasmusWL RasmusWL changed the title #996: Fix python_setup/install_tool.sh Fix python_setup/install_tool.sh when python2 not present Mar 25, 2022
@cklin cklin closed this Mar 25, 2022
@cklin cklin reopened this Mar 25, 2022
cdivitotawela and others added 2 commits March 28, 2022 07:23
When the script  executed on a runner which does not have python2, script incorrectly execute the code inside the if condition. This script runs using /bin/sh in Ubuntu and cause the problem. Fix for this issue to use different syntax in output redirection.

Issue is reproduced using running the job on ubunutu container. Fix also verified in the action https://github.com/cdivitotawela/codeql-issue/actions/runs/2038007502
Copy link
Contributor

@edoardopirovano edoardopirovano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me too, thanks for the fix!

@edoardopirovano edoardopirovano merged commit fdc2a90 into github:main Mar 29, 2022
This was referenced Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants