Merge pull request #1431 from github/rasmuswl/poetry-always-install-pip

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

CHANGELOG.md

@@ -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
 

lib/analyze.js

@@ -50,6 +50,7 @@ async function setupPythonExtractor(logger) {
         // If CODEQL_PYTHON is not set, no dependencies were installed, so we don't need to do anything
         return;
     }
+    const scriptsFolder = path.resolve(__dirname, "../python-setup");
     let output = "";
     const options = {
         listeners: {
@@ -58,10 +59,7 @@ async function setupPythonExtractor(logger) {
             },
         },
     };
-    await new toolrunner.ToolRunner(codeqlPython, [
+    await new toolrunner.ToolRunner(codeqlPython, [path.join(scriptsFolder, "find_site_packages.py")], options).exec();
-        "-c",
-        "import os; import pip; print(os.path.dirname(os.path.dirname(pip.__file__)))",
-    ], options).exec();
     logger.info(`Setting LGTM_INDEX_IMPORT_PATH=${output}`);
     process.env["LGTM_INDEX_IMPORT_PATH"] = output;
     output = "";

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])

python-setup/tests/from_python_exe.py

@@ -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",
+            os.path.join(os.path.dirname(__file__), "..", "find_site_packages.py")
-            "import os; import pip; print(os.path.dirname(os.path.dirname(pip.__file__)))",
         ],
         stdin=subprocess.DEVNULL,
     )

python-setup/tests/poetry/requests-3/poetry.toml

@@ -1,2 +1,5 @@
 [virtualenvs]
 in-project = true
+
+[virtualenvs.options]
+no-pip = true

src/analyze.ts

@@ -88,6 +88,8 @@ async function setupPythonExtractor(logger: Logger) {
     return;
   }
 
+  const scriptsFolder = path.resolve(__dirname, "../python-setup");
+
   let output = "";
   const options = {
     listeners: {
@@ -99,10 +101,7 @@ async function setupPythonExtractor(logger: Logger) {
 
   await new toolrunner.ToolRunner(
     codeqlPython,
-    [
+    [path.join(scriptsFolder, "find_site_packages.py")],
-      "-c",
-      "import os; import pip; print(os.path.dirname(os.path.dirname(pip.__file__)))",
-    ],
     options
   ).exec();
   logger.info(`Setting LGTM_INDEX_IMPORT_PATH=${output}`);