Skip to content

Commit

Permalink
Merge pull request #1431 from github/rasmuswl/poetry-always-install-pip
Browse files Browse the repository at this point in the history
python-setup: Handle poetry `virtualenvs.options.no-pip = true`
  • Loading branch information
RasmusWL authored Jan 16, 2023
2 parents 2073a69 + 5ed1e98 commit 32be38e
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 12 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
[virtualenvs]
in-project = true

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

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

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

0 comments on commit 32be38e

Please sign in to comment.