Repository: basilisp-lang/basilisp

Issue:
Issue with basilisp bootstrap on MS-Windows
Hi,

`basilisp bootstrap` fails to install properly on MS-Windows:
```
Error processing line 1 of D:\bas\issue\.venv\basilispbootstrap.pth:

  Traceback (most recent call last):
    File "<frozen site>", line 186, in addpackage
    File "<string>", line 1, in <module>
  ModuleNotFoundError: No module named 'basilisp'
```

To reproduce with Poetry from scartch on windows
1. Create a `new` `issue` venv, `add` Basilisp and enter the `shell`:
```ps1
> poetry new issue
> cd issue
issue> poetry add basilisp
Creating virtualenv issue in D:\bas\issue\.venv
...
issue> poetry shell
(issue-py3.11) issue> basilisp version
Basilisp 0.3.6
```
2. `bootstrap` Basilisp, and check the `version`, the above error message appears because Python fails to load `basilispbootstrap.pth`
```ps1
(issue-py3.11) issue> basilisp bootstrap
Your Python installation has been bootstrapped! You can undo this at any time with with `basilisp bootstrap --uninstall`.
(issue-py3.11) issue> basilisp version
Error processing line 1 of D:\bas\issue\.venv\basilispbootstrap.pth:

  Traceback (most recent call last):
    File "<frozen site>", line 186, in addpackage
    File "<string>", line 1, in <module>
  ModuleNotFoundError: No module named 'basilisp'

Remainder of file ignored
Basilisp 0.3.6
```

The cause is that the `basilisp bootstrap` process added `.pth` file at the root of the virtual environment (`.venv\basilispbootstrap.pth`):

```ps1
issue> cat .venv\basilispbootstrap.pth
import basilisp.sitecustomize
```

At startup, Python attempts to import `basilisp.sitecustomize`, but `basilisp` is not yet in sys.path, causing the failure.

Ideally, instead of placing the `.pth` file in the first directory returned by `site.getsitepackages()` (which is inappropriate on Windows):

```python
>>> import site
>>> site.getsitepackages()
['D:\\bas\\issue\\.venv', 'D:\\bas\\issue\\.venv\\Lib\\site-packages']
```

`basilisp bootstrap` should place it in the `sys.path` directory where `basilisp` is installed, so that `basilisp` is always guaranteed to be available.

Possible options I can think of for fixing `basilisp bootstrap`
1. Traverse `site.getsitepackages()` and place the `.pth` file in the directory containing `basilisp`.
2.  Use the [sysconfig.get_paths()](https://docs.python.org/3/library/sysconfig.html#installation-paths) `purelib` value which directly points to the correct `site-packages` directory:
>    stdlib: directory containing the standard Python library files that are not platform-specific.
>   platstdlib: directory containing the standard Python library files that are platform-specific.
>    platlib: directory for site-specific, platform-specific files.
>    purelib: directory for site-specific, non-platform-specific files (‘pure’ Python).
>    include: directory for non-platform-specific header files for the Python C-API.
>    platinclude: directory for platform-specific header files for the Python C-API.
>    scripts: directory for script files.
>    data: directory for data files.

```python
>>> import sysconfig
>>> sysconfig.get_paths()["purelib"]
'D:\\bas\\issue\\.venv\\Lib\\site-packages'
```

I prefer option (2) for its simplicity.

Do you have a preference or another suggestion?

Thanks

Hints from issue comments:
@chrisrink10: I was unaware of `sysconfig` when I added this feature. That seems like a good option to me. Thanks for researching and reporting the issue.@ikappaki: Thanks, I'll raise a PR to fix this as such.

Solution patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 37be0c5..17dec7a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed 
  * Fix a regression introduced in #1176 where the testrunner couldn't handle relative paths in `sys.path`, causing `basilisp test` to fail when no arugments were provided (#1204)
  * Fix a bug where `basilisp.process/exec` could deadlock reading process output if that output exceeded the buffer size (#1202)
+ * Fix `basilisp boostrap` issue on MS-Windows where the boostrap file loaded too early, before Basilisp was in `sys.path` (#1208)
 
 ## [v0.3.6]
 ### Added
diff --git a/src/basilisp/cli.py b/src/basilisp/cli.py
index b25e9bc..d761d6e 100644
--- a/src/basilisp/cli.py
+++ b/src/basilisp/cli.py
@@ -425,11 +425,12 @@ def bootstrap_basilisp_installation(_, args: argparse.Namespace) -> None:
         ):
             print_("No Basilisp bootstrap files were found.")
         else:
-            for file in removed:
-                print_(f"Removed '{file}'")
+            if removed is not None:
+                print_(f"Removed '{removed}'")
     else:
-        basilisp.bootstrap_python(site_packages=args.site_packages)
+        path = basilisp.bootstrap_python(site_packages=args.site_packages)
         print_(
+            f"(Added {path})\n\n"
             "Your Python installation has been bootstrapped! You can undo this at any "
             "time with with `basilisp bootstrap --uninstall`."
         )
@@ -473,7 +474,6 @@ def _add_bootstrap_subcommand(parser: argparse.ArgumentParser) -> None:
     # Not intended to be used by end users.
     parser.add_argument(
         "--site-packages",
-        action="append",
         help=argparse.SUPPRESS,
     )
 
diff --git a/src/basilisp/main.py b/src/basilisp/main.py
index dd18801..ea436d0 100644
--- a/src/basilisp/main.py
+++ b/src/basilisp/main.py
@@ -1,5 +1,6 @@
 import importlib
-import site
+import os
+import sysconfig
 from pathlib import Path
 from typing import Optional
 
@@ -56,42 +57,41 @@ def bootstrap(
         getattr(mod, fn_name)()
 
 
-def bootstrap_python(site_packages: Optional[list[str]] = None) -> None:
-    """Bootstrap a Python installation by installing a ``.pth`` file in the first
-    available ``site-packages`` directory (as by
-    :external:py:func:`site.getsitepackages`).
+def bootstrap_python(site_packages: Optional[str] = None) -> str:
+    """Bootstrap a Python installation by installing a ``.pth`` file
+    in ``site-packages`` directory (corresponding to "purelib" in
+    :external:py:func:`sysconfig.get_paths`). Returns the path to the
+    ``.pth`` file.
 
     Subsequent startups of the Python interpreter will have Basilisp already
     bootstrapped and available to run."""
     if site_packages is None:  # pragma: no cover
-        site_packages = site.getsitepackages()
+        site_packages = sysconfig.get_paths()["purelib"]
 
-    assert site_packages, "Expected at least one site-package directory"
+    assert site_packages, "Expected a site-package directory"
 
-    for d in site_packages:
-        p = Path(d)
-        with open(p / "basilispbootstrap.pth", mode="w") as f:
-            f.write("import basilisp.sitecustomize")
-        break
+    pth = Path(site_packages) / "basilispbootstrap.pth"
+    with open(pth, mode="w") as f:
+        f.write("import basilisp.sitecustomize")
 
+    return str(pth)
 
-def unbootstrap_python(site_packages: Optional[list[str]] = None) -> list[str]:
-    """Remove any `basilispbootstrap.pth` files found in any Python site-packages
-    directory (as by :external:py:func:`site.getsitepackages`). Return a list of
-    removed filenames."""
+
+def unbootstrap_python(site_packages: Optional[str] = None) -> Optional[str]:
+    """Remove the `basilispbootstrap.pth` file found in the Python site-packages
+    directory (corresponding to "purelib" in :external:py:func:`sysconfig.get_paths`).
+    Return the path of the removed file, if any."""
     if site_packages is None:  # pragma: no cover
-        site_packages = site.getsitepackages()
-
-    assert site_packages, "Expected at least one site-package directory"
-
-    removed = []
-    for d in site_packages:
-        p = Path(d)
-        for file in p.glob("basilispbootstrap.pth"):
-            try:
-                file.unlink()
-            except FileNotFoundError:  # pragma: no cover
-                pass
-            else:
-                removed.append(str(file))
+        site_packages = sysconfig.get_paths()["purelib"]
+
+    assert site_packages, "Expected a site-package directory"
+
+    removed = None
+    pth = Path(site_packages) / "basilispbootstrap.pth"
+    try:
+        os.unlink(pth)
+    except FileNotFoundError:  # pragma: no cover
+        pass
+    else:
+        removed = str(pth)
     return removed
diff --git a/tox.ini b/tox.ini
index 868a76b..9e6e201 100644
--- a/tox.ini
+++ b/tox.ini
@@ -20,6 +20,8 @@ commands =
              -m pytest \
              --import-mode=importlib \
              --junitxml={toxinidir}/junit/pytest/{envname}.xml \
+             # also enable pytest marked as slow \
+             --run-slow \
              {posargs}
 
 [testenv:coverage]


Test patch:
diff --git a/tests/basilisp/cli_test.py b/tests/basilisp/cli_test.py
index 8072196..7072455 100644
--- a/tests/basilisp/cli_test.py
+++ b/tests/basilisp/cli_test.py
@@ -10,6 +10,7 @@ import subprocess
 import sys
 import tempfile
 import time
+import venv
 from collections.abc import Sequence
 from threading import Thread
 from typing import Optional
@@ -108,6 +109,7 @@ class TestBootstrap:
         assert bootstrap_file.read_text() == "import basilisp.sitecustomize"
 
         assert res.out == (
+            f"(Added {bootstrap_file})\n\n"
             "Your Python installation has been bootstrapped! You can undo this at any "
             "time with with `basilisp bootstrap --uninstall`.\n"
         )
@@ -135,6 +137,41 @@ class TestBootstrap:
         res = capsys.readouterr()
         assert res.out == ""
 
+    @pytest.mark.slow
+    def test_install_import(self, tmp_path: pathlib.Path):
+        venv_path = tmp_path / "venv"
+        venv.create(venv_path, with_pip=True)
+
+        venv_bin = venv_path / ("Scripts" if sys.platform == "win32" else "bin")
+        pip_path = venv_bin / "pip"
+        python_path = venv_bin / "python"
+        basilisp_path = venv_bin / "basilisp"
+
+        result = subprocess.run(
+            [pip_path, "install", "."], capture_output=True, text=True, cwd=os.getcwd()
+        )
+
+        lpy_file = tmp_path / "boottest.lpy"
+        lpy_file.write_text("(ns boottest) (defn abc [] (println (+ 155 4)))")
+
+        cmd_import = [python_path, "-c", "import boottest; boottest.abc()"]
+        result = subprocess.run(
+            cmd_import, capture_output=True, text=True, cwd=tmp_path
+        )
+        assert "No module named 'boottest'" in result.stderr, result
+
+        result = subprocess.run(
+            [basilisp_path, "bootstrap"], capture_output=True, text=True, cwd=tmp_path
+        )
+        assert (
+            "Your Python installation has been bootstrapped!" in result.stdout
+        ), result
+
+        result = subprocess.run(
+            cmd_import, capture_output=True, text=True, cwd=tmp_path
+        )
+        assert result.stdout.strip() == "159", result
+
     def test_nothing_to_uninstall(self, tmp_path: pathlib.Path, run_cli, capsys):
         bootstrap_file = tmp_path / "basilispbootstrap.pth"
         assert not bootstrap_file.exists()
diff --git a/tests/conftest.py b/tests/conftest.py
index c6481d5..162f6b8 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1 +1,19 @@
+import pytest
+
 pytest_plugins = ["pytester"]
+
+
+def pytest_configure(config):
+    config.addinivalue_line("markers", "slow: Marks tests as slow")
+
+
+def pytest_addoption(parser):
+    parser.addoption("--run-slow", action="store_true", help="Run slow tests")
+
+
+@pytest.fixture(autouse=True)
+def skip_slow(request):
+    if request.node.get_closest_marker("slow") and not request.config.getoption(
+        "--run-slow"
+    ):
+        pytest.skip("Skipping slow test. Use --run-slow to enable.")