Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 13 commits into from
Aug 9, 2024
Merged
15 changes: 4 additions & 11 deletions crates/red_knot/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels a bit surprising that VirtualEnvironment is part of the site_packages module. Shouldn' we rename the site_packages module to venv?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That might make @carljm upset, since he keeps pointing out that several of the routines in this module really work with any Python installation, not just virtual environments 😆

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yeah. Not sure. The hierarchy just feels slightly off to me. But we can also revisit this later.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it would depend how you do it :) I was actually going to make a similar comment and decided not to. But I would not want to just rename this module, I would want two modules, one which is specific to venvs and one which contains stuff that works on Python installations in general. But I'm also fine with just keeping it all in one module for now, which is why I didn't make that comment :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll leave this for now; we can come back to it later

use red_knot_workspace::watch;
use red_knot_workspace::watch::WorkspaceWatcher;
use red_knot_workspace::workspace::WorkspaceMetadata;
Expand Down Expand Up @@ -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| {
let venv_path = SystemPath::absolute(venv_path, &cli_base_path);

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!"
))
}
.map(|path| {
VirtualEnvironment::new(path, &OsSystem::new(cli_base_path))
.and_then(|venv| venv.site_packages_directories(&system))
})
.transpose()?
.unwrap_or_default();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
9 changes: 9 additions & 0 deletions crates/red_knot_python_semantic/src/python_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Loading
Loading