Change EnvironmentOptions::venv-path to Option<SystemPathBuf> (#15631)

## Summary

The `Options` struct is intended to capture the user's configuration
options but
`EnvironmentOptions::venv_path` supports both a `SitePackages::Known`
and `SitePackages::Derived`.

Users should only be able to provide `SitePackages::Derived`—they
specify a path to a venv, and Red Knot derives the path to the
site-packages directory. We'll only use the `Known` variant once we
automatically discover the Python installation.

That's why this PR changes `EnvironmentOptions::venv_path` from
`Option<SitePackages>` to `Option<SystemPathBuf>`.

This requires making some changes to the file watcher test, and I
decided to use `extra_paths` over venv path
because our venv validation is annoyingly correct -- making mocking a
venv rather involved.

## Test Plan

`cargo test`
This commit is contained in:
Micha Reiser 2025-01-21 15:10:41 +01:00 committed by GitHub
parent 4366473d9b
commit 067c6de465
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 19 additions and 27 deletions

View file

@ -10,7 +10,6 @@ use red_knot_project::metadata::options::{EnvironmentOptions, Options};
use red_knot_project::watch;
use red_knot_project::watch::ProjectWatcher;
use red_knot_project::{ProjectDatabase, ProjectMetadata};
use red_knot_python_semantic::SitePackages;
use red_knot_server::run_server;
use ruff_db::diagnostic::Diagnostic;
use ruff_db::system::{OsSystem, System, SystemPath, SystemPathBuf};
@ -77,9 +76,7 @@ impl Args {
venv_path: self
.venv_path
.as_ref()
.map(|venv_path| SitePackages::Derived {
venv_path: SystemPath::absolute(venv_path, cli_cwd),
}),
.map(|venv_path| SystemPath::absolute(venv_path, cli_cwd)),
typeshed: self
.typeshed
.as_ref()

View file

@ -8,9 +8,7 @@ use red_knot_project::metadata::options::{EnvironmentOptions, Options};
use red_knot_project::metadata::pyproject::{PyProject, Tool};
use red_knot_project::watch::{directory_watcher, ChangeEvent, ProjectWatcher};
use red_knot_project::{Db, ProjectDatabase, ProjectMetadata};
use red_knot_python_semantic::{
resolve_module, ModuleName, PythonPlatform, PythonVersion, SitePackages,
};
use red_knot_python_semantic::{resolve_module, ModuleName, PythonPlatform, PythonVersion};
use ruff_db::files::{system_path_to_file, File, FileError};
use ruff_db::source::source_text;
use ruff_db::system::{OsSystem, SystemPath, SystemPathBuf};
@ -324,7 +322,6 @@ where
.extra_paths
.iter()
.chain(program_settings.search_paths.typeshed.as_ref())
.chain(program_settings.search_paths.site_packages.paths())
{
std::fs::create_dir_all(path.as_std_path())
.with_context(|| format!("Failed to create search path `{path}`"))?;
@ -794,7 +791,7 @@ fn search_path() -> anyhow::Result<()> {
let mut case = setup_with_options([("bar.py", "import sub.a")], |root_path, _project_path| {
Some(Options {
environment: Some(EnvironmentOptions {
venv_path: Some(SitePackages::Known(vec![root_path.join("site_packages")])),
extra_paths: Some(vec![root_path.join("site_packages")]),
..EnvironmentOptions::default()
}),
..Options::default()
@ -835,7 +832,7 @@ fn add_search_path() -> anyhow::Result<()> {
// Register site-packages as a search path.
case.update_options(Options {
environment: Some(EnvironmentOptions {
venv_path: Some(SitePackages::Known(vec![site_packages.clone()])),
extra_paths: Some(vec![site_packages.clone()]),
..EnvironmentOptions::default()
}),
..Options::default()
@ -858,7 +855,7 @@ fn remove_search_path() -> anyhow::Result<()> {
let mut case = setup_with_options([("bar.py", "import sub.a")], |root_path, _project_path| {
Some(Options {
environment: Some(EnvironmentOptions {
venv_path: Some(SitePackages::Known(vec![root_path.join("site_packages")])),
extra_paths: Some(vec![root_path.join("site_packages")]),
..EnvironmentOptions::default()
}),
..Options::default()
@ -1381,9 +1378,8 @@ mod unix {
|_root, project| {
Some(Options {
environment: Some(EnvironmentOptions {
venv_path: Some(SitePackages::Known(vec![
project.join(".venv/lib/python3.12/site-packages")
])),
extra_paths: Some(vec![project.join(".venv/lib/python3.12/site-packages")]),
python_version: Some(PythonVersion::PY312),
..EnvironmentOptions::default()
}),
..Options::default()

View file

@ -74,7 +74,9 @@ impl Options {
extra_paths: extra_paths.unwrap_or_default(),
src_roots,
typeshed,
site_packages: python.unwrap_or(SitePackages::Known(vec![])),
site_packages: python
.map(|venv_path| SitePackages::Derived { venv_path })
.unwrap_or(SitePackages::Known(vec![])),
}
}
}
@ -98,7 +100,7 @@ pub struct EnvironmentOptions {
// TODO: Rename to python, see https://github.com/astral-sh/ruff/issues/15530
/// The path to the user's `site-packages` directory, where third-party packages from ``PyPI`` are installed.
pub venv_path: Option<SitePackages>,
pub venv_path: Option<SystemPathBuf>,
}
#[derive(Debug, Default, Clone, Eq, PartialEq, Combine, Serialize, Deserialize)]

View file

@ -222,8 +222,14 @@ impl SearchPaths {
static_paths.push(stdlib_path);
let site_packages_paths = match site_packages_paths {
SitePackages::Derived { venv_path } => VirtualEnvironment::new(venv_path, system)
.and_then(|venv| venv.site_packages_directories(system))?,
SitePackages::Derived { venv_path } => {
// TODO: We may want to warn here if the venv's python version is older
// than the one resolved in the program settings because it indicates
// that the `target-version` is incorrectly configured or that the
// venv is out of date.
VirtualEnvironment::new(venv_path, system)
.and_then(|venv| venv.site_packages_directories(system))?
}
SitePackages::Known(paths) => paths
.iter()
.map(|path| canonicalize(path, system))

View file

@ -134,12 +134,3 @@ pub enum SitePackages {
/// Resolved site packages paths
Known(Vec<SystemPathBuf>),
}
impl SitePackages {
pub fn paths(&self) -> &[SystemPathBuf] {
match self {
SitePackages::Derived { venv_path } => std::slice::from_ref(venv_path),
SitePackages::Known(paths) => paths,
}
}
}