Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Install Python 3 tools before installing Python 2 tools. #339

Merged
merged 2 commits into from
Dec 7, 2020

Conversation

chrisgavin
Copy link
Contributor

This change is designed to reduce the fallout of issues with Python 2 dependency installation, since Python 2 is both less popular than Python 3 and also end-of-life meaning there are more likely to be issues with it as tools like Pip drop support for it.

Firstly it moves all the Python 2 steps to the end. This ensures that if they fail the script will have already done the Python 3 dependency installation, which is the one we likely care most about. It also wraps the Python 2 dependency installation in a check that Python 2 is actually present to avoid spurious warnings when you're only using Python 3 which is going to become more and more common.

I note that [there is a PowerShell version of this script, which I have not updated. This is because the PowerShell version already totally ignores all exit codes, so is already fairly safe when it comes to the Python 2 steps failing (although ignoring all exit codes obviously has its own problems).

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.

@chrisgavin chrisgavin requested a review from RasmusWL December 7, 2020 14:07
@chrisgavin chrisgavin marked this pull request as ready for review December 7, 2020 14:22
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.

Nice, thanks 👍

I was slightly worried what would happen if we try to analyze some project as Python 2, and this step failed because python2 command wasn't available. But I guess we will figure out quick enough when trying to invoke the extractor with python2 😄

As a side question, do you know if python2 is available by default in environment the codeql-action runs on when it's based off Ubuntu 20.04 instead of Ubuntu 16.04?

@chrisgavin
Copy link
Contributor Author

I was slightly worried what would happen if we try to analyze some project as Python 2, and this step failed because python2 command wasn't available. But I guess we will figure out quick enough when trying to invoke the extractor with python2 😄

Haha, yep my thoughts too.

As a side question, do you know if python2 is available by default in environment the codeql-action runs on when it's based off Ubuntu 20.04 instead of Ubuntu 16.04?

Yes, python2 is still present, it's just Pip for Python 2 that's not installed by default anymore.

@chrisgavin chrisgavin merged commit 8cbc02a into main Dec 7, 2020
@chrisgavin chrisgavin deleted the split-python2-python3-install branch December 7, 2020 15:55
@github-actions github-actions bot mentioned this pull request Dec 14, 2020
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.

2 participants