[red-knot] Add support for `--system-site-packages` virtual environments (#12759)

crates/red_knot/src/main.rs

@@ -10,7 +10,7 @@ use salsa::plumbing::ZalsaDatabase;
 use red_knot_python_semantic::{ProgramSettings, SearchPathSettings};
 use red_knot_server::run_server;
 use red_knot_workspace::db::RootDatabase;
-use red_knot_workspace::site_packages::site_packages_dirs_of_venv;
+use red_knot_workspace::site_packages::VirtualEnvironment;
 use red_knot_workspace::watch;
 use red_knot_workspace::watch::WorkspaceWatcher;
 use red_knot_workspace::workspace::WorkspaceMetadata;
@@ -164,16 +164,9 @@ fn run() -> anyhow::Result<ExitStatus> {
 
     // TODO: Verify the remaining search path settings eagerly.
     let site_packages = venv_path
-        .map(|venv_path| {
+        .map(|path| {
-            let venv_path = SystemPath::absolute(venv_path, &cli_base_path);
+            VirtualEnvironment::new(path, &OsSystem::new(cli_base_path))
-
+                .and_then(|venv| venv.site_packages_directories(&system))
-            if system.is_directory(&venv_path) {
-                Ok(site_packages_dirs_of_venv(&venv_path, &system)?)
-            } else {
-                Err(anyhow!(
-                    "Provided venv-path {venv_path} is not a directory!"
-                ))
-            }
         })
         .transpose()?
         .unwrap_or_default();

crates/red_knot_python_semantic/src/module_resolver/resolver.rs

@@ -145,10 +145,6 @@ fn try_resolve_module_resolution_settings(
         tracing::info!("Custom typeshed directory: {custom_typeshed}");
     }
 
-    if !site_packages.is_empty() {
-        tracing::info!("Site-packages directories: {site_packages:?}");
-    }
-
     let system = db.system();
     let files = db.files();
 

crates/red_knot_python_semantic/src/python_version.rs

@@ -59,6 +59,15 @@ pub struct PythonVersion {
     pub minor: u8,
 }
 
+impl PythonVersion {
+    pub fn free_threaded_build_available(self) -> bool {
+        self >= PythonVersion {
+            major: 3,
+            minor: 13,
+        }
+    }
+}
+
 impl TryFrom<(&str, &str)> for PythonVersion {
     type Error = std::num::ParseIntError;
 

crates/red_knot_workspace/resources/test/unix-uv-venv/CACHEDIR.TAG

@@ -1 +0,0 @@
-Signature: 8a477f597d28d172789f06886806bc55

crates/red_knot_workspace/resources/test/unix-uv-venv/bin/python

@@ -1 +0,0 @@
-/Users/alexw/.pyenv/versions/3.12.4/bin/python3.12

crates/red_knot_workspace/resources/test/unix-uv-venv/bin/python3

@@ -1 +0,0 @@
-python

crates/red_knot_workspace/resources/test/unix-uv-venv/bin/python3.12

@@ -1 +0,0 @@
-python

crates/red_knot_workspace/resources/test/unix-uv-venv/lib/python3.12/site-packages/_virtualenv.pth

@@ -1 +0,0 @@
-import _virtualenv

crates/red_knot_workspace/resources/test/unix-uv-venv/lib/python3.12/site-packages/_virtualenv.py

@@ -1,103 +0,0 @@
-"""Patches that are applied at runtime to the virtual environment."""
-
-from __future__ import annotations
-
-import os
-import sys
-
-VIRTUALENV_PATCH_FILE = os.path.join(__file__)
-
-
-def patch_dist(dist):
-    """
-    Distutils allows user to configure some arguments via a configuration file:
-    https://docs.python.org/3/install/index.html#distutils-configuration-files.
-
-    Some of this arguments though don't make sense in context of the virtual environment files, let's fix them up.
-    """  # noqa: D205
-    # we cannot allow some install config as that would get packages installed outside of the virtual environment
-    old_parse_config_files = dist.Distribution.parse_config_files
-
-    def parse_config_files(self, *args, **kwargs):
-        result = old_parse_config_files(self, *args, **kwargs)
-        install = self.get_option_dict("install")
-
-        if "prefix" in install:  # the prefix governs where to install the libraries
-            install["prefix"] = VIRTUALENV_PATCH_FILE, os.path.abspath(sys.prefix)
-        for base in ("purelib", "platlib", "headers", "scripts", "data"):
-            key = f"install_{base}"
-            if key in install:  # do not allow global configs to hijack venv paths
-                install.pop(key, None)
-        return result
-
-    dist.Distribution.parse_config_files = parse_config_files
-
-
-# Import hook that patches some modules to ignore configuration values that break package installation in case
-# of virtual environments.
-_DISTUTILS_PATCH = "distutils.dist", "setuptools.dist"
-# https://docs.python.org/3/library/importlib.html#setting-up-an-importer
-
-
-class _Finder:
-    """A meta path finder that allows patching the imported distutils modules."""
-
-    fullname = None
-
-    # lock[0] is threading.Lock(), but initialized lazily to avoid importing threading very early at startup,
-    # because there are gevent-based applications that need to be first to import threading by themselves.
-    # See https://github.com/pypa/virtualenv/issues/1895 for details.
-    lock = []  # noqa: RUF012
-
-    def find_spec(self, fullname, path, target=None):  # noqa: ARG002
-        if fullname in _DISTUTILS_PATCH and self.fullname is None:
-            # initialize lock[0] lazily
-            if len(self.lock) == 0:
-                import threading
-
-                lock = threading.Lock()
-                # there is possibility that two threads T1 and T2 are simultaneously running into find_spec,
-                # observing .lock as empty, and further going into hereby initialization. However due to the GIL,
-                # list.append() operation is atomic and this way only one of the threads will "win" to put the lock
-                # - that every thread will use - into .lock[0].
-                # https://docs.python.org/3/faq/library.html#what-kinds-of-global-value-mutation-are-thread-safe
-                self.lock.append(lock)
-
-            from functools import partial
-            from importlib.util import find_spec
-
-            with self.lock[0]:
-                self.fullname = fullname
-                try:
-                    spec = find_spec(fullname, path)
-                    if spec is not None:
-                        # https://www.python.org/dev/peps/pep-0451/#how-loading-will-work
-                        is_new_api = hasattr(spec.loader, "exec_module")
-                        func_name = "exec_module" if is_new_api else "load_module"
-                        old = getattr(spec.loader, func_name)
-                        func = self.exec_module if is_new_api else self.load_module
-                        if old is not func:
-                            try:  # noqa: SIM105
-                                setattr(spec.loader, func_name, partial(func, old))
-                            except AttributeError:
-                                pass  # C-Extension loaders are r/o such as zipimporter with <3.7
-                        return spec
-                finally:
-                    self.fullname = None
-        return None
-
-    @staticmethod
-    def exec_module(old, module):
-        old(module)
-        if module.__name__ in _DISTUTILS_PATCH:
-            patch_dist(module)
-
-    @staticmethod
-    def load_module(old, name):
-        module = old(name)
-        if module.__name__ in _DISTUTILS_PATCH:
-            patch_dist(module)
-        return module
-
-
-sys.meta_path.insert(0, _Finder())

crates/red_knot_workspace/resources/test/unix-uv-venv/pyvenv.cfg

@@ -1,6 +0,0 @@
-home = /Users/alexw/.pyenv/versions/3.12.4/bin
-implementation = CPython
-uv = 0.2.32
-version_info = 3.12.4
-include-system-site-packages = false
-relocatable = false

crates/red_knot_workspace/src/site_packages.rs

@@ -8,43 +8,270 @@
 //! reasonably ask us to type-check code assuming that the code runs
 //! on Linux.)
 
+use std::fmt;
 use std::io;
+use std::num::NonZeroUsize;
+use std::ops::Deref;
 
+use red_knot_python_semantic::PythonVersion;
 use ruff_db::system::{System, SystemPath, SystemPathBuf};
 
+type SitePackagesDiscoveryResult<T> = Result<T, SitePackagesDiscoveryError>;
+
+/// Abstraction for a Python virtual environment.
+///
+/// Most of this information is derived from the virtual environment's `pyvenv.cfg` file.
+/// The format of this file is not defined anywhere, and exactly which keys are present
+/// depends on the tool that was used to create the virtual environment.
+#[derive(Debug)]
+pub struct VirtualEnvironment {
+    venv_path: SysPrefixPath,
+    base_executable_home_path: PythonHomePath,
+    include_system_site_packages: bool,
+
+    /// The version of the Python executable that was used to create this virtual environment.
+    ///
+    /// The Python version is encoded under different keys and in different formats
+    /// by different virtual-environment creation tools,
+    /// and the key is never read by the standard-library `site.py` module,
+    /// so it's possible that we might not be able to find this information
+    /// in an acceptable format under any of the keys we expect.
+    /// This field will be `None` if so.
+    version: Option<PythonVersion>,
+}
+
+impl VirtualEnvironment {
+    pub fn new(
+        path: impl AsRef<SystemPath>,
+        system: &dyn System,
+    ) -> SitePackagesDiscoveryResult<Self> {
+        Self::new_impl(path.as_ref(), system)
+    }
+
+    fn new_impl(path: &SystemPath, system: &dyn System) -> SitePackagesDiscoveryResult<Self> {
+        fn pyvenv_cfg_line_number(index: usize) -> NonZeroUsize {
+            index.checked_add(1).and_then(NonZeroUsize::new).unwrap()
+        }
+
+        let venv_path = SysPrefixPath::new(path, system)?;
+        let pyvenv_cfg_path = venv_path.join("pyvenv.cfg");
+        tracing::debug!("Attempting to parse virtual environment metadata at {pyvenv_cfg_path}");
+
+        let pyvenv_cfg = system
+            .read_to_string(&pyvenv_cfg_path)
+            .map_err(SitePackagesDiscoveryError::NoPyvenvCfgFile)?;
+
+        let mut include_system_site_packages = false;
+        let mut base_executable_home_path = None;
+        let mut version_info_string = None;
+
+        // A `pyvenv.cfg` file *looks* like a `.ini` file, but actually isn't valid `.ini` syntax!
+        // The Python standard-library's `site` module parses these files by splitting each line on
+        // '=' characters, so that's what we should do as well.
+        //
+        // See also: https://snarky.ca/how-virtual-environments-work/
+        for (index, line) in pyvenv_cfg.lines().enumerate() {
+            if let Some((key, value)) = line.split_once('=') {
+                let key = key.trim();
+                if key.is_empty() {
+                    return Err(SitePackagesDiscoveryError::PyvenvCfgParseError(
+                        pyvenv_cfg_path,
+                        PyvenvCfgParseErrorKind::MalformedKeyValuePair {
+                            line_number: pyvenv_cfg_line_number(index),
+                        },
+                    ));
+                }
+
+                let value = value.trim();
+                if value.is_empty() {
+                    return Err(SitePackagesDiscoveryError::PyvenvCfgParseError(
+                        pyvenv_cfg_path,
+                        PyvenvCfgParseErrorKind::MalformedKeyValuePair {
+                            line_number: pyvenv_cfg_line_number(index),
+                        },
+                    ));
+                }
+
+                if value.contains('=') {
+                    return Err(SitePackagesDiscoveryError::PyvenvCfgParseError(
+                        pyvenv_cfg_path,
+                        PyvenvCfgParseErrorKind::TooManyEquals {
+                            line_number: pyvenv_cfg_line_number(index),
+                        },
+                    ));
+                }
+
+                match key {
+                    "include-system-site-packages" => {
+                        include_system_site_packages = value.eq_ignore_ascii_case("true");
+                    }
+                    "home" => base_executable_home_path = Some(value),
+                    // `virtualenv` and `uv` call this key `version_info`,
+                    // but the stdlib venv module calls it `version`
+                    "version" | "version_info" => version_info_string = Some(value),
+                    _ => continue,
+                }
+            }
+        }
+
+        // The `home` key is read by the standard library's `site.py` module,
+        // so if it's missing from the `pyvenv.cfg` file
+        // (or the provided value is invalid),
+        // it's reasonable to consider the virtual environment irredeemably broken.
+        let Some(base_executable_home_path) = base_executable_home_path else {
+            return Err(SitePackagesDiscoveryError::PyvenvCfgParseError(
+                pyvenv_cfg_path,
+                PyvenvCfgParseErrorKind::NoHomeKey,
+            ));
+        };
+        let base_executable_home_path = PythonHomePath::new(base_executable_home_path, system)
+            .map_err(|io_err| {
+                SitePackagesDiscoveryError::PyvenvCfgParseError(
+                    pyvenv_cfg_path,
+                    PyvenvCfgParseErrorKind::InvalidHomeValue(io_err),
+                )
+            })?;
+
+        // but the `version`/`version_info` key is not read by the standard library,
+        // and is provided under different keys depending on which virtual-environment creation tool
+        // created the `pyvenv.cfg` file. Lenient parsing is appropriate here:
+        // the file isn't really *invalid* if it doesn't have this key,
+        // or if the value doesn't parse according to our expectations.
+        let version = version_info_string.and_then(|version_string| {
+            let mut version_info_parts = version_string.split('.');
+            let (major, minor) = (version_info_parts.next()?, version_info_parts.next()?);
+            PythonVersion::try_from((major, minor)).ok()
+        });
+
+        let metadata = Self {
+            venv_path,
+            base_executable_home_path,
+            include_system_site_packages,
+            version,
+        };
+
+        tracing::trace!("Resolved metadata for virtual environment: {metadata:?}");
+        Ok(metadata)
+    }
+
+    /// Return a list of `site-packages` directories that are available from this virtual environment
+    ///
+    /// See the documentation for `site_packages_dir_from_sys_prefix` for more details.
+    pub fn site_packages_directories(
+        &self,
+        system: &dyn System,
+    ) -> SitePackagesDiscoveryResult<Vec<SystemPathBuf>> {
+        let VirtualEnvironment {
+            venv_path,
+            base_executable_home_path,
+            include_system_site_packages,
+            version,
+        } = self;
+
+        let mut site_packages_directories = vec![site_packages_directory_from_sys_prefix(
+            venv_path, *version, system,
+        )?];
+
+        if *include_system_site_packages {
+            let system_sys_prefix =
+                SysPrefixPath::from_executable_home_path(base_executable_home_path);
+
+            // If we fail to resolve the `sys.prefix` path from the base executable home path,
+            // or if we fail to resolve the `site-packages` from the `sys.prefix` path,
+            // we should probably print a warning but *not* abort type checking
+            if let Some(sys_prefix_path) = system_sys_prefix {
+                match site_packages_directory_from_sys_prefix(&sys_prefix_path, *version, system) {
+                    Ok(site_packages_directory) => {
+                        site_packages_directories.push(site_packages_directory);
+                    }
+                    Err(error) => tracing::warn!(
+                        "{error}. System site-packages will not be used for module resolution."
+                    ),
+                }
+            } else {
+                tracing::warn!(
+                    "Failed to resolve `sys.prefix` of the system Python installation \
+from the `home` value in the `pyvenv.cfg` file at {}. \
+System site-packages will not be used for module resolution.",
+                    venv_path.join("pyvenv.cfg")
+                );
+            }
+        }
+
+        tracing::debug!("Resolved site-packages directories for this virtual environment are: {site_packages_directories:?}");
+        Ok(site_packages_directories)
+    }
+}
+
+#[derive(Debug, thiserror::Error)]
+pub enum SitePackagesDiscoveryError {
+    #[error("Invalid --venv-path argument: {0} could not be canonicalized")]
+    VenvDirCanonicalizationError(SystemPathBuf, #[source] io::Error),
+    #[error("Invalid --venv-path argument: {0} does not point to a directory on disk")]
+    VenvDirIsNotADirectory(SystemPathBuf),
+    #[error("--venv-path points to a broken venv with no pyvenv.cfg file")]
+    NoPyvenvCfgFile(#[source] io::Error),
+    #[error("Failed to parse the pyvenv.cfg file at {0} because {1}")]
+    PyvenvCfgParseError(SystemPathBuf, PyvenvCfgParseErrorKind),
+    #[error("Failed to search the `lib` directory of the Python installation at {1} for `site-packages`")]
+    CouldNotReadLibDirectory(#[source] io::Error, SysPrefixPath),
+    #[error("Could not find the `site-packages` directory for the Python installation at {0}")]
+    NoSitePackagesDirFound(SysPrefixPath),
+}
+
+/// The various ways in which parsing a `pyvenv.cfg` file could fail
+#[derive(Debug)]
+pub enum PyvenvCfgParseErrorKind {
+    TooManyEquals { line_number: NonZeroUsize },
+    MalformedKeyValuePair { line_number: NonZeroUsize },
+    NoHomeKey,
+    InvalidHomeValue(io::Error),
+}
+
+impl fmt::Display for PyvenvCfgParseErrorKind {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::TooManyEquals { line_number } => {
+                write!(f, "line {line_number} has too many '=' characters")
+            }
+            Self::MalformedKeyValuePair { line_number } => write!(
+                f,
+                "line {line_number} has a malformed `<key> = <value>` pair"
+            ),
+            Self::NoHomeKey => f.write_str("the file does not have a `home` key"),
+            Self::InvalidHomeValue(io_err) => {
+                write!(
+                    f,
+                    "the following error was encountered \
+when trying to resolve the `home` value to a directory on disk: {io_err}"
+                )
+            }
+        }
+    }
+}
+
 /// Attempt to retrieve the `site-packages` directory
 /// associated with a given Python installation.
 ///
-/// `sys_prefix_path` is equivalent to the value of [`sys.prefix`]
+/// The location of the `site-packages` directory can vary according to the
-/// at runtime in Python. For the case of a virtual environment, where a
+/// Python version that this installation represents. The Python version may
-/// Python binary is at `/.venv/bin/python`, `sys.prefix` is the path to
+/// or may not be known at this point, which is why the `python_version`
-/// the virtual environment the Python binary lies inside, i.e. `/.venv`,
+/// parameter is an `Option`.
-/// and `site-packages` will be at `.venv/lib/python3.X/site-packages`.
+fn site_packages_directory_from_sys_prefix(
-/// System Python installations generally work the same way: if a system
+    sys_prefix_path: &SysPrefixPath,
-/// Python installation lies at `/opt/homebrew/bin/python`, `sys.prefix`
+    python_version: Option<PythonVersion>,
-/// will be `/opt/homebrew`, and `site-packages` will be at
-/// `/opt/homebrew/lib/python3.X/site-packages`.
-///
-/// This routine does not verify that `sys_prefix_path` points
-/// to an existing directory on disk; it is assumed that this has already
-/// been checked.
-///
-/// [`sys.prefix`]: https://docs.python.org/3/library/sys.html#sys.prefix
-fn site_packages_dir_from_sys_prefix(
-    sys_prefix_path: &SystemPath,
     system: &dyn System,
-) -> Result<SystemPathBuf, SitePackagesDiscoveryError> {
+) -> SitePackagesDiscoveryResult<SystemPathBuf> {
-    tracing::debug!("Searching for site-packages directory in '{sys_prefix_path}'");
+    tracing::debug!("Searching for site-packages directory in {sys_prefix_path}");
 
     if cfg!(target_os = "windows") {
-        let site_packages = sys_prefix_path.join("Lib/site-packages");
+        let site_packages = sys_prefix_path.join(r"Lib\site-packages");
-
+        return system
-        return if system.is_directory(&site_packages) {
+            .is_directory(&site_packages)
-            tracing::debug!("Resolved site-packages directory to '{site_packages}'");
+            .then_some(site_packages)
-            Ok(site_packages)
+            .ok_or(SitePackagesDiscoveryError::NoSitePackagesDirFound(
-        } else {
+                sys_prefix_path.to_owned(),
-            Err(SitePackagesDiscoveryError::NoSitePackagesDirFound)
+            ));
-        };
     }
 
     // In the Python standard library's `site.py` module (used for finding `site-packages`
@@ -69,7 +296,38 @@ fn site_packages_dir_from_sys_prefix(
     //
     // [the non-Windows branch]: https://github.com/python/cpython/blob/a8be8fc6c4682089be45a87bd5ee1f686040116c/Lib/site.py#L401-L410
     // [the `sys`-module documentation]: https://docs.python.org/3/library/sys.html#sys.platlibdir
-    for entry_result in system.read_directory(&sys_prefix_path.join("lib"))? {
+
+    // If we were able to figure out what Python version this installation is,
+    // we should be able to avoid iterating through all items in the `lib/` directory:
+    if let Some(version) = python_version {
+        let expected_path = sys_prefix_path.join(format!("lib/python{version}/site-packages"));
+        if system.is_directory(&expected_path) {
+            return Ok(expected_path);
+        }
+        if version.free_threaded_build_available() {
+            // Nearly the same as `expected_path`, but with an additional `t` after {version}:
+            let alternative_path =
+                sys_prefix_path.join(format!("lib/python{version}t/site-packages"));
+            if system.is_directory(&alternative_path) {
+                return Ok(alternative_path);
+            }
+        }
+    }
+
+    // Either we couldn't figure out the version before calling this function
+    // (e.g., from a `pyvenv.cfg` file if this was a venv),
+    // or we couldn't find a `site-packages` folder at the expected location given
+    // the parsed version
+    //
+    // Note: the `python3.x` part of the `site-packages` path can't be computed from
+    // the `--target-version` the user has passed, as they might be running Python 3.12 locally
+    // even if they've requested that we type check their code "as if" they're running 3.8.
+    for entry_result in system
+        .read_directory(&sys_prefix_path.join("lib"))
+        .map_err(|io_err| {
+            SitePackagesDiscoveryError::CouldNotReadLibDirectory(io_err, sys_prefix_path.to_owned())
+        })?
+    {
         let Ok(entry) = entry_result else {
             continue;
         };
@@ -80,16 +338,6 @@ fn site_packages_dir_from_sys_prefix(
 
         let mut path = entry.into_path();
 
-        // The `python3.x` part of the `site-packages` path can't be computed from
-        // the `--target-version` the user has passed, as they might be running Python 3.12 locally
-        // even if they've requested that we type check their code "as if" they're running 3.8.
-        //
-        // The `python3.x` part of the `site-packages` path *could* be computed
-        // by parsing the virtual environment's `pyvenv.cfg` file.
-        // Right now that seems like overkill, but in the future we may need to parse
-        // the `pyvenv.cfg` file anyway, in which case we could switch to that method
-        // rather than iterating through the whole directory until we find
-        // an entry where the last component of the path starts with `python3.`
         let name = path
             .file_name()
             .expect("File name to be non-null because path is guaranteed to be a child of `lib`");
@@ -100,55 +348,494 @@ fn site_packages_dir_from_sys_prefix(
 
         path.push("site-packages");
         if system.is_directory(&path) {
-            tracing::debug!("Resolved site-packages directory to '{path}'");
             return Ok(path);
         }
     }
-    Err(SitePackagesDiscoveryError::NoSitePackagesDirFound)
+    Err(SitePackagesDiscoveryError::NoSitePackagesDirFound(
+        sys_prefix_path.to_owned(),
+    ))
 }
 
-#[derive(Debug, thiserror::Error)]
+/// A path that represents the value of [`sys.prefix`] at runtime in Python
-pub enum SitePackagesDiscoveryError {
+/// for a given Python executable.
-    #[error("Failed to search the virtual environment directory for `site-packages`")]
+///
-    CouldNotReadLibDirectory(#[from] io::Error),
+/// For the case of a virtual environment, where a
-    #[error("Could not find the `site-packages` directory in the virtual environment")]
+/// Python binary is at `/.venv/bin/python`, `sys.prefix` is the path to
-    NoSitePackagesDirFound,
+/// the virtual environment the Python binary lies inside, i.e. `/.venv`,
-}
+/// and `site-packages` will be at `.venv/lib/python3.X/site-packages`.
+/// System Python installations generally work the same way: if a system
+/// Python installation lies at `/opt/homebrew/bin/python`, `sys.prefix`
+/// will be `/opt/homebrew`, and `site-packages` will be at
+/// `/opt/homebrew/lib/python3.X/site-packages`.
+///
+/// [`sys.prefix`]: https://docs.python.org/3/library/sys.html#sys.prefix
+#[derive(Debug, PartialEq, Eq, Clone)]
+pub struct SysPrefixPath(SystemPathBuf);
 
-/// Given a validated, canonicalized path to a virtual environment,
+impl SysPrefixPath {
-/// return a list of `site-packages` directories that are available from that environment.
+    fn new(
-///
+        unvalidated_path: impl AsRef<SystemPath>,
-/// See the documentation for `site_packages_dir_from_sys_prefix` for more details.
-///
-/// TODO: Currently we only ever return 1 path from this function:
-/// the `site-packages` directory that is actually inside the virtual environment.
-/// Some `site-packages` directories are able to also access system `site-packages` and
-/// user `site-packages`, however.
-pub fn site_packages_dirs_of_venv(
-    venv_path: &SystemPath,
         system: &dyn System,
-) -> Result<Vec<SystemPathBuf>, SitePackagesDiscoveryError> {
+    ) -> SitePackagesDiscoveryResult<Self> {
-    Ok(vec![site_packages_dir_from_sys_prefix(venv_path, system)?])
+        Self::new_impl(unvalidated_path.as_ref(), system)
+    }
+
+    fn new_impl(
+        unvalidated_path: &SystemPath,
+        system: &dyn System,
+    ) -> SitePackagesDiscoveryResult<Self> {
+        // It's important to resolve symlinks here rather than simply making the path absolute,
+        // since system Python installations often only put symlinks in the "expected"
+        // locations for `home` and `site-packages`
+        let canonicalized = system
+            .canonicalize_path(unvalidated_path)
+            .map_err(|io_err| {
+                SitePackagesDiscoveryError::VenvDirCanonicalizationError(
+                    unvalidated_path.to_path_buf(),
+                    io_err,
+                )
+            })?;
+        system
+            .is_directory(&canonicalized)
+            .then_some(Self(canonicalized))
+            .ok_or_else(|| {
+                SitePackagesDiscoveryError::VenvDirIsNotADirectory(unvalidated_path.to_path_buf())
+            })
+    }
+
+    fn from_executable_home_path(path: &PythonHomePath) -> Option<Self> {
+        // No need to check whether `path.parent()` is a directory:
+        // the parent of a canonicalised path that is known to exist
+        // is guaranteed to be a directory.
+        if cfg!(target_os = "windows") {
+            Some(Self(path.to_path_buf()))
+        } else {
+            path.parent().map(|path| Self(path.to_path_buf()))
+        }
+    }
+}
+
+impl Deref for SysPrefixPath {
+    type Target = SystemPath;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl fmt::Display for SysPrefixPath {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "`sys.prefix` path {}", self.0)
+    }
+}
+
+/// The value given by the `home` key in `pyvenv.cfg` files.
+///
+/// This is equivalent to `{sys_prefix_path}/bin`, and points
+/// to a directory in which a Python executable can be found.
+/// Confusingly, it is *not* the same as the [`PYTHONHOME`]
+/// environment variable that Python provides! However, it's
+/// consistent among all mainstream creators of Python virtual
+/// environments (the stdlib Python `venv` module, the third-party
+/// `virtualenv` library, and `uv`), was specified by
+/// [the original PEP adding the `venv` module],
+/// and it's one of the few fields that's read by the Python
+/// standard library's `site.py` module.
+///
+/// Although it doesn't appear to be specified anywhere,
+/// all existing virtual environment tools always use an absolute path
+/// for the `home` value, and the Python standard library also assumes
+/// that the `home` value will be an absolute path.
+///
+/// Other values, such as the path to the Python executable or the
+/// base-executable `sys.prefix` value, are either only provided in
+/// `pyvenv.cfg` files by some virtual-environment creators,
+/// or are included under different keys depending on which
+/// virtual-environment creation tool you've used.
+///
+/// [`PYTHONHOME`]: https://docs.python.org/3/using/cmdline.html#envvar-PYTHONHOME
+/// [the original PEP adding the `venv` module]: https://peps.python.org/pep-0405/
+#[derive(Debug, PartialEq, Eq)]
+struct PythonHomePath(SystemPathBuf);
+
+impl PythonHomePath {
+    fn new(path: impl AsRef<SystemPath>, system: &dyn System) -> io::Result<Self> {
+        let path = path.as_ref();
+        // It's important to resolve symlinks here rather than simply making the path absolute,
+        // since system Python installations often only put symlinks in the "expected"
+        // locations for `home` and `site-packages`
+        let canonicalized = system.canonicalize_path(path)?;
+        system
+            .is_directory(&canonicalized)
+            .then_some(Self(canonicalized))
+            .ok_or_else(|| io::Error::new(io::ErrorKind::Other, "not a directory"))
+    }
+}
+
+impl Deref for PythonHomePath {
+    type Target = SystemPath;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl fmt::Display for PythonHomePath {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "`home` location {}", self.0)
+    }
+}
+
+impl PartialEq<SystemPath> for PythonHomePath {
+    fn eq(&self, other: &SystemPath) -> bool {
+        &*self.0 == other
+    }
+}
+
+impl PartialEq<SystemPathBuf> for PythonHomePath {
+    fn eq(&self, other: &SystemPathBuf) -> bool {
+        self == &**other
+    }
 }
 
 #[cfg(test)]
 mod tests {
-    use ruff_db::system::{OsSystem, System, SystemPath};
+    use ruff_db::system::TestSystem;
 
-    use crate::site_packages::site_packages_dirs_of_venv;
+    use super::*;
+
+    struct VirtualEnvironmentTester {
+        system: TestSystem,
+        minor_version: u8,
+        free_threaded: bool,
+        system_site_packages: bool,
+        pyvenv_cfg_version_field: Option<&'static str>,
+    }
+
+    impl VirtualEnvironmentTester {
+        /// Builds a mock virtual environment, and returns the path to the venv
+        fn build_mock_venv(&self) -> SystemPathBuf {
+            let VirtualEnvironmentTester {
+                system,
+                minor_version,
+                system_site_packages,
+                free_threaded,
+                pyvenv_cfg_version_field,
+            } = self;
+            let memory_fs = system.memory_file_system();
+            let unix_site_packages = if *free_threaded {
+                format!("lib/python3.{minor_version}t/site-packages")
+            } else {
+                format!("lib/python3.{minor_version}/site-packages")
+            };
+
+            let system_install_sys_prefix =
+                SystemPathBuf::from(&*format!("/Python3.{minor_version}"));
+            let (system_home_path, system_exe_path, system_site_packages_path) =
+                if cfg!(target_os = "windows") {
+                    let system_home_path = system_install_sys_prefix.clone();
+                    let system_exe_path = system_home_path.join("python.exe");
+                    let system_site_packages_path =
+                        system_install_sys_prefix.join(r"Lib\site-packages");
+                    (system_home_path, system_exe_path, system_site_packages_path)
+                } else {
+                    let system_home_path = system_install_sys_prefix.join("bin");
+                    let system_exe_path = system_home_path.join("python");
+                    let system_site_packages_path =
+                        system_install_sys_prefix.join(&unix_site_packages);
+                    (system_home_path, system_exe_path, system_site_packages_path)
+                };
+            memory_fs.write_file(system_exe_path, "").unwrap();
+            memory_fs
+                .create_directory_all(&system_site_packages_path)
+                .unwrap();
+
+            let venv_sys_prefix = SystemPathBuf::from("/.venv");
+            let (venv_exe, site_packages_path) = if cfg!(target_os = "windows") {
+                (
+                    venv_sys_prefix.join(r"Scripts\python.exe"),
+                    venv_sys_prefix.join(r"Lib\site-packages"),
+                )
+            } else {
+                (
+                    venv_sys_prefix.join("bin/python"),
+                    venv_sys_prefix.join(&unix_site_packages),
+                )
+            };
+            memory_fs.write_file(&venv_exe, "").unwrap();
+            memory_fs.create_directory_all(&site_packages_path).unwrap();
+
+            let pyvenv_cfg_path = venv_sys_prefix.join("pyvenv.cfg");
+            let mut pyvenv_cfg_contents = format!("home = {system_home_path}\n");
+            if let Some(version_field) = pyvenv_cfg_version_field {
+                pyvenv_cfg_contents.push_str(version_field);
+                pyvenv_cfg_contents.push('\n');
+            }
+            // Deliberately using weird casing here to test that our pyvenv.cfg parsing is case-insensitive:
+            if *system_site_packages {
+                pyvenv_cfg_contents.push_str("include-system-site-packages = TRuE\n");
+            }
+            memory_fs
+                .write_file(pyvenv_cfg_path, &pyvenv_cfg_contents)
+                .unwrap();
+
+            venv_sys_prefix
+        }
+
+        fn test(self) {
+            let venv_path = self.build_mock_venv();
+            let venv = VirtualEnvironment::new(venv_path.clone(), &self.system).unwrap();
+
+            assert_eq!(
+                venv.venv_path,
+                SysPrefixPath(self.system.canonicalize_path(&venv_path).unwrap())
+            );
+            assert_eq!(venv.include_system_site_packages, self.system_site_packages);
+
+            if self.pyvenv_cfg_version_field.is_some() {
+                assert_eq!(
+                    venv.version,
+                    Some(PythonVersion {
+                        major: 3,
+                        minor: self.minor_version
+                    })
+                );
+            } else {
+                assert_eq!(venv.version, None);
+            }
+
+            let expected_home = if cfg!(target_os = "windows") {
+                SystemPathBuf::from(&*format!(r"\Python3.{}", self.minor_version))
+            } else {
+                SystemPathBuf::from(&*format!("/Python3.{}/bin", self.minor_version))
+            };
+            assert_eq!(venv.base_executable_home_path, expected_home);
+
+            let site_packages_directories = venv.site_packages_directories(&self.system).unwrap();
+            let expected_venv_site_packages = if cfg!(target_os = "windows") {
+                SystemPathBuf::from(r"\.venv\Lib\site-packages")
+            } else if self.free_threaded {
+                SystemPathBuf::from(&*format!(
+                    "/.venv/lib/python3.{}t/site-packages",
+                    self.minor_version
+                ))
+            } else {
+                SystemPathBuf::from(&*format!(
+                    "/.venv/lib/python3.{}/site-packages",
+                    self.minor_version
+                ))
+            };
+
+            let expected_system_site_packages = if cfg!(target_os = "windows") {
+                SystemPathBuf::from(&*format!(
+                    r"\Python3.{}\Lib\site-packages",
+                    self.minor_version
+                ))
+            } else if self.free_threaded {
+                SystemPathBuf::from(&*format!(
+                    "/Python3.{minor_version}/lib/python3.{minor_version}t/site-packages",
+                    minor_version = self.minor_version
+                ))
+            } else {
+                SystemPathBuf::from(&*format!(
+                    "/Python3.{minor_version}/lib/python3.{minor_version}/site-packages",
+                    minor_version = self.minor_version
+                ))
+            };
+
+            if self.system_site_packages {
+                assert_eq!(
+                    &site_packages_directories,
+                    &[expected_venv_site_packages, expected_system_site_packages]
+                );
+            } else {
+                assert_eq!(&site_packages_directories, &[expected_venv_site_packages]);
+            }
+        }
+    }
 
     #[test]
-    // Windows venvs have different layouts, and we only have a Unix venv committed for now.
+    fn can_find_site_packages_directory_no_version_field_in_pyvenv_cfg() {
-    // This test is skipped on Windows until we commit a Windows venv.
+        let tester = VirtualEnvironmentTester {
-    #[cfg_attr(target_os = "windows", ignore = "Windows has a different venv layout")]
+            system: TestSystem::default(),
-    fn can_find_site_packages_dir_in_committed_venv() {
+            minor_version: 12,
-        let path_to_venv = SystemPath::new("resources/test/unix-uv-venv");
+            free_threaded: false,
-        let system = OsSystem::default();
+            system_site_packages: false,
+            pyvenv_cfg_version_field: None,
+        };
+        tester.test();
+    }
 
-        // if this doesn't hold true, the premise of the test is incorrect.
+    #[test]
-        assert!(system.is_directory(path_to_venv));
+    fn can_find_site_packages_directory_venv_style_version_field_in_pyvenv_cfg() {
+        let tester = VirtualEnvironmentTester {
+            system: TestSystem::default(),
+            minor_version: 12,
+            free_threaded: false,
+            system_site_packages: false,
+            pyvenv_cfg_version_field: Some("version = 3.12"),
+        };
+        tester.test();
+    }
 
-        let site_packages_dirs = site_packages_dirs_of_venv(path_to_venv, &system).unwrap();
+    #[test]
-        assert_eq!(site_packages_dirs.len(), 1);
+    fn can_find_site_packages_directory_uv_style_version_field_in_pyvenv_cfg() {
+        let tester = VirtualEnvironmentTester {
+            system: TestSystem::default(),
+            minor_version: 12,
+            free_threaded: false,
+            system_site_packages: false,
+            pyvenv_cfg_version_field: Some("version_info = 3.12"),
+        };
+        tester.test();
+    }
+
+    #[test]
+    fn can_find_site_packages_directory_virtualenv_style_version_field_in_pyvenv_cfg() {
+        let tester = VirtualEnvironmentTester {
+            system: TestSystem::default(),
+            minor_version: 12,
+            free_threaded: false,
+            system_site_packages: false,
+            pyvenv_cfg_version_field: Some("version_info = 3.12.0rc2"),
+        };
+        tester.test();
+    }
+
+    #[test]
+    fn can_find_site_packages_directory_freethreaded_build() {
+        let tester = VirtualEnvironmentTester {
+            system: TestSystem::default(),
+            minor_version: 13,
+            free_threaded: true,
+            system_site_packages: false,
+            pyvenv_cfg_version_field: Some("version_info = 3.13"),
+        };
+        tester.test();
+    }
+
+    #[test]
+    fn finds_system_site_packages() {
+        let tester = VirtualEnvironmentTester {
+            system: TestSystem::default(),
+            minor_version: 13,
+            free_threaded: true,
+            system_site_packages: true,
+            pyvenv_cfg_version_field: Some("version_info = 3.13"),
+        };
+        tester.test();
+    }
+
+    #[test]
+    fn reject_venv_that_does_not_exist() {
+        let system = TestSystem::default();
+        assert!(matches!(
+            VirtualEnvironment::new("/.venv", &system),
+            Err(SitePackagesDiscoveryError::VenvDirIsNotADirectory(_))
+        ));
+    }
+
+    #[test]
+    fn reject_venv_with_no_pyvenv_cfg_file() {
+        let system = TestSystem::default();
+        system
+            .memory_file_system()
+            .create_directory_all("/.venv")
+            .unwrap();
+        assert!(matches!(
+            VirtualEnvironment::new("/.venv", &system),
+            Err(SitePackagesDiscoveryError::NoPyvenvCfgFile(_))
+        ));
+    }
+
+    #[test]
+    fn parsing_pyvenv_cfg_with_too_many_equals() {
+        let system = TestSystem::default();
+        let memory_fs = system.memory_file_system();
+        let pyvenv_cfg_path = SystemPathBuf::from("/.venv/pyvenv.cfg");
+        memory_fs
+            .write_file(&pyvenv_cfg_path, "home = bar = /.venv/bin")
+            .unwrap();
+        let venv_result = VirtualEnvironment::new("/.venv", &system);
+        assert!(matches!(
+            venv_result,
+            Err(SitePackagesDiscoveryError::PyvenvCfgParseError(
+                path,
+                PyvenvCfgParseErrorKind::TooManyEquals { line_number }
+            ))
+            if path == pyvenv_cfg_path && Some(line_number) == NonZeroUsize::new(1)
+        ));
+    }
+
+    #[test]
+    fn parsing_pyvenv_cfg_with_key_but_no_value_fails() {
+        let system = TestSystem::default();
+        let memory_fs = system.memory_file_system();
+        let pyvenv_cfg_path = SystemPathBuf::from("/.venv/pyvenv.cfg");
+        memory_fs.write_file(&pyvenv_cfg_path, "home =").unwrap();
+        let venv_result = VirtualEnvironment::new("/.venv", &system);
+        assert!(matches!(
+            venv_result,
+            Err(SitePackagesDiscoveryError::PyvenvCfgParseError(
+                path,
+                PyvenvCfgParseErrorKind::MalformedKeyValuePair { line_number }
+            ))
+            if path == pyvenv_cfg_path && Some(line_number) == NonZeroUsize::new(1)
+        ));
+    }
+
+    #[test]
+    fn parsing_pyvenv_cfg_with_value_but_no_key_fails() {
+        let system = TestSystem::default();
+        let memory_fs = system.memory_file_system();
+        let pyvenv_cfg_path = SystemPathBuf::from("/.venv/pyvenv.cfg");
+        memory_fs
+            .write_file(&pyvenv_cfg_path, "= whatever")
+            .unwrap();
+        let venv_result = VirtualEnvironment::new("/.venv", &system);
+        assert!(matches!(
+            venv_result,
+            Err(SitePackagesDiscoveryError::PyvenvCfgParseError(
+                path,
+                PyvenvCfgParseErrorKind::MalformedKeyValuePair { line_number }
+            ))
+            if path == pyvenv_cfg_path && Some(line_number) == NonZeroUsize::new(1)
+        ));
+    }
+
+    #[test]
+    fn parsing_pyvenv_cfg_with_no_home_key_fails() {
+        let system = TestSystem::default();
+        let memory_fs = system.memory_file_system();
+        let pyvenv_cfg_path = SystemPathBuf::from("/.venv/pyvenv.cfg");
+        memory_fs.write_file(&pyvenv_cfg_path, "").unwrap();
+        let venv_result = VirtualEnvironment::new("/.venv", &system);
+        assert!(matches!(
+            venv_result,
+            Err(SitePackagesDiscoveryError::PyvenvCfgParseError(
+                path,
+                PyvenvCfgParseErrorKind::NoHomeKey
+            ))
+            if path == pyvenv_cfg_path
+        ));
+    }
+
+    #[test]
+    fn parsing_pyvenv_cfg_with_invalid_home_key_fails() {
+        let system = TestSystem::default();
+        let memory_fs = system.memory_file_system();
+        let pyvenv_cfg_path = SystemPathBuf::from("/.venv/pyvenv.cfg");
+        memory_fs
+            .write_file(&pyvenv_cfg_path, "home = foo")
+            .unwrap();
+        let venv_result = VirtualEnvironment::new("/.venv", &system);
+        assert!(matches!(
+            venv_result,
+            Err(SitePackagesDiscoveryError::PyvenvCfgParseError(
+                path,
+                PyvenvCfgParseErrorKind::InvalidHomeValue(_)
+            ))
+            if path == pyvenv_cfg_path
+        ));
     }
 }