-
Notifications
You must be signed in to change notification settings - Fork 335
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
Add a fix for python 3.12 #1928
Conversation
The python extractor does not yet support 3.12. Check for this and instead make sure we run python 3.11. Only need to check on windows since we are extremely unlikely to be running 3.12 on linux or macos.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clever. Looks reasonable.
Can we have an integration test on Windows that calls setup-python
with 3.12 to exercise this path?
python-setup/check_python12.ps1
Outdated
Write-Host "Python 3.11 detected, using this version instead of 3.12+." | ||
Write-Output "PY_PYTHON3=3.11" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf8 -Append | ||
} else { | ||
Write-Host "FAILURE: Python 3.12+ is not supported. Please install Python 3.11." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional.
Write-Host "FAILURE: Python 3.12+ is not supported. Please install Python 3.11." | |
Write-Host "FAILURE: Python 3.12+ is not supported, and Python 3.11 could not be detected on the system. Please install Python 3.11." |
#! /usr/bin/pwsh | ||
|
||
# If we are running greater than or equal to python 3.12, change py to run version 3.11 | ||
Write-Host "Checking python version" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Write-Host "Checking python version" | |
Write-Host "Checking Python version" |
Super-nit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I think all Python
should be python
.
Should be simple enough to create a test. |
Also: - Update changelog - Fix comments in check_python script
9ebed2f
to
028bacb
Compare
I think we should query a feature flag from the CLI so that we can turn off this workaround from within the CLI once we've a release out that fixes the python extractor. |
Good call. I can check for less than 2.15.0. |
I think the new tool-features feature (see https://github.com/github/codeql-action/pull/1909/files#diff-a2b4f46aa09bd648db18893b59331e2e037dfb60954c80383d820703b09d2519) would be more appropriate to use. |
That requires adding the feature to the CLI directly, no? |
As far as I understand it yes, but only once we're actually shipping new python-3.12 support - which should be doable, as we release the python extractor together with the CLI. |
Ah...I see what you're saying. I choose a feature name now and that will be the feature name in the version of codeql that has the fix. Good idea. |
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
bd30ad1
to
8a81c5b
Compare
8a81c5b
to
669c610
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good modulo question on the changelog file!
@@ -4,7 +4,7 @@ See the [releases page](https://github.com/github/codeql-action/releases) for th | |||
|
|||
## [UNRELEASED] | |||
|
|||
No user facing changes. | |||
- Add a workaround python 3.12, which is not supported in CodeQL CLI version 2.14.6 or earlier. If you are running an analysis on Windows and using python 3.12 or later, the CodeQL Action will switch to running python 3.11. If python 3.11 is not found, then the workflow will fail. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Add a workaround python 3.12, which is not supported in CodeQL CLI version 2.14.6 or earlier. If you are running an analysis on Windows and using python 3.12 or later, the CodeQL Action will switch to running python 3.11. If python 3.11 is not found, then the workflow will fail. | |
- Add a workaround for python 3.12, which is not supported in CodeQL CLI version 2.14.6 or earlier. If you are running an analysis on Windows and using python 3.12 or later, the CodeQL Action will switch to running python 3.11. If python 3.11 is not found in this case, then the workflow will fail. |
Just realized it's missing a word..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks I'll put this in a follow-up.
The python extractor does not yet support 3.12. Check for this and instead make sure we run python 3.11. Only need to check on windows since we are extremely unlikely to be running 3.12 on linux or macos.
Merge / deployment checklist