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

python-setup: Handle poetry virtualenvs.options.no-pip = true #1431

Merged
merged 8 commits into from Jan 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Expand Up @@ -2,7 +2,7 @@

## [UNRELEASED]

No user facing changes.
- Python automatic dependency installation will no longer fail for projects using Poetry that specify `virtualenvs.options.no-pip = true` in their `poetry.toml`. [#1431](https://github.com/github/codeql-action/pull/1431).

## 2.1.38 - 12 Jan 2023

Expand Down
6 changes: 2 additions & 4 deletions lib/analyze.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/analyze.js.map

Large diffs are not rendered by default.

34 changes: 34 additions & 0 deletions python-setup/find_site_packages.py
@@ -0,0 +1,34 @@
"""
Print the path to the site-packages directory for the current Python environment.
"""
from __future__ import print_function

try:
import pip
import os
print(os.path.dirname(os.path.dirname(pip.__file__)))
except ImportError:
import sys
print("DEBUG: could not import pip", file=sys.stderr)
# if you use poetry with `virtualenvs.options.no-pip = true` you might end up with a
# virtualenv without pip, so the above trick doesn't actually work. See
# https://python-poetry.org/docs/configuration/#virtualenvsoptionsno-pip
#
# A possible option is to install `pip` into the virtualenv created by poetry
# (`poetry add pip`), but it turns out that doesn't always work :( for the test
# poetry/requests-3, I was not allowed to install pip! So I did not pursue this
# option further.
#
# Instead, testing `site.getsitepackages()` contains has the right path, whereas
# `site.getusersitepackages()` is about the system python (very confusing).
#
# We can't use the environment variable POETRY_VIRTUALENVS_OPTIONS_NO_PIP because it
# does not work, see https://github.com/python-poetry/poetry/issues/5906
import site

if sys.platform.startswith("win32"):
# On windows, the last entry of `site.getsitepackages()` has the right path
print(site.getsitepackages()[-1])
else:
# on unix, the first entry of `site.getsitepackages()` has the right path
print(site.getsitepackages()[0])
3 changes: 1 addition & 2 deletions python-setup/tests/from_python_exe.py
Expand Up @@ -9,8 +9,7 @@ def get_details(path_to_python_exe: str) -> Tuple[str, str]:
import_path = subprocess.check_output(
[
path_to_python_exe,
"-c",
"import os; import pip; print(os.path.dirname(os.path.dirname(pip.__file__)))",
os.path.join(os.path.dirname(__file__), "..", "find_site_packages.py")
],
stdin=subprocess.DEVNULL,
)
Expand Down
3 changes: 3 additions & 0 deletions python-setup/tests/poetry/requests-3/poetry.toml
@@ -1,2 +1,5 @@
[virtualenvs]
in-project = true

[virtualenvs.options]
no-pip = true
7 changes: 3 additions & 4 deletions src/analyze.ts
Expand Up @@ -88,6 +88,8 @@ async function setupPythonExtractor(logger: Logger) {
return;
}

const scriptsFolder = path.resolve(__dirname, "../python-setup");
Copy link
Contributor

Choose a reason for hiding this comment

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

You will need to rebuild the javascript files in order to get this to pass.

Copy link
Contributor

Choose a reason for hiding this comment

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

npm run build

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks <3


let output = "";
const options = {
listeners: {
Expand All @@ -99,10 +101,7 @@ async function setupPythonExtractor(logger: Logger) {

await new toolrunner.ToolRunner(
codeqlPython,
[
"-c",
"import os; import pip; print(os.path.dirname(os.path.dirname(pip.__file__)))",
],
[path.join(scriptsFolder, "find_site_packages.py")],
options
).exec();
logger.info(`Setting LGTM_INDEX_IMPORT_PATH=${output}`);
Expand Down