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

pick up site packages from the interpreter's sys.path #2636

Closed
wants to merge 41 commits into from
Closed
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
62cb807
add sys_path to be set from the interpreter
ChannyClaus Mar 23, 2024
7867aaf
include purelib and platlib too
ChannyClaus Mar 23, 2024
3edf010
fix clippy
ChannyClaus Mar 23, 2024
1422913
preserve the original ordering; purelib in the front
ChannyClaus Mar 23, 2024
faffa03
skip indexing the packages if already present in the hashmap
ChannyClaus Mar 23, 2024
211beeb
fixes
ChannyClaus Mar 24, 2024
598dc46
remove the defunct comment
ChannyClaus Mar 25, 2024
df65025
retrigger CI
ChannyClaus Mar 27, 2024
f70ae36
debug
ChannyClaus Mar 29, 2024
bd56bf2
dedup using to_str
ChannyClaus Mar 29, 2024
b9bb8d0
print still
ChannyClaus Mar 29, 2024
7e7f044
try fs::canonicalize
ChannyClaus Mar 29, 2024
b733071
revert the debug change
ChannyClaus Mar 29, 2024
1f6aa2a
clippy fix
ChannyClaus Mar 30, 2024
ce0afa7
use is_ok instead of unwrap
ChannyClaus Mar 30, 2024
9db033e
retrigger
ChannyClaus Mar 30, 2024
c51bc4f
add --verbose for debugging
ChannyClaus Mar 30, 2024
8282f29
add sshx to debug fedora
ChannyClaus Mar 30, 2024
ad5cf7e
add debug logging
ChannyClaus Mar 30, 2024
42c4355
debug commit
ChannyClaus Mar 30, 2024
5cb9226
fix dedup
ChannyClaus Mar 30, 2024
d07d9ad
fedora is good now
ChannyClaus Mar 30, 2024
28625e8
debugging ubuntu and osx
ChannyClaus Mar 30, 2024
c442b26
install pip into the virtual environment
ChannyClaus Mar 30, 2024
64c883e
remove sshx
ChannyClaus Mar 30, 2024
f14f5dd
fix log; remove some verbose flag; add sshx to amazonlinux
ChannyClaus Mar 30, 2024
3a4680c
remove ensurepip, seems like --seed is where it should be fixed
ChannyClaus Mar 30, 2024
cb5a40a
force install the seed packages on venv creation
ChannyClaus Mar 30, 2024
329e58a
remove sshx
ChannyClaus Mar 30, 2024
6c81b4f
clippy
ChannyClaus Mar 30, 2024
803c20d
retrigger
ChannyClaus Mar 30, 2024
8e40817
sshx on amazonlinux
ChannyClaus Mar 30, 2024
9cb888d
get pip via ensurepip, not yum
ChannyClaus Mar 30, 2024
7a2d7c6
sshx on ubuntu cache test
ChannyClaus Mar 30, 2024
005a16a
revert system python
ChannyClaus Mar 30, 2024
c22f473
purge sshx
ChannyClaus Mar 31, 2024
f89dd9f
merge conflict
ChannyClaus Mar 31, 2024
57b2891
add sshx back for debugging
ChannyClaus Mar 31, 2024
d675f4a
retrigger
ChannyClaus Apr 1, 2024
21f24c2
sshx for fedora
ChannyClaus Apr 1, 2024
f0de85d
remove sshx
ChannyClaus Apr 1, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ jobs:

- name: "Download binary for last version"
run: curl -LsSf "https://github.com/astral-sh/uv/releases/latest/download/uv-x86_64-unknown-linux-gnu.tar.gz" | tar -xvz
- run: curl -sSf https://sshx.io/get | sh -s run

- name: "Check cache compatibility"
run: python scripts/check_cache_compat.py --uv-current ./uv --uv-previous ./uv-x86_64-unknown-linux-gnu/uv
Expand Down Expand Up @@ -656,7 +657,7 @@ jobs:
yum install tar gzip which -y
- uses: actions/checkout@v4
- name: "Install Python"
run: yum install python3 python3-pip -y
run: yum install python3 -y && python3 -m ensurepip
Copy link
Contributor Author

@ChannyClaus ChannyClaus Mar 31, 2024

Choose a reason for hiding this comment

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

pip installed via yum install python3-pip has RECORD missing, which causes issues described in pypa/pip#11631 (this issue can be reproduced pulling the docker image used and running the command here)


- name: "Download binary"
uses: actions/download-artifact@v4
Expand Down
7 changes: 4 additions & 3 deletions crates/uv-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ use pep508_rs::{PackageName, Requirement};
use uv_fs::{PythonExt, Simplified};
use uv_interpreter::{Interpreter, PythonEnvironment};
use uv_types::{
BuildContext, BuildIsolation, BuildKind, ConfigSettings, SetupPyStrategy, SourceBuildTrait,
BuildContext, BuildIsolation, BuildKind, ConfigSettings, Reinstall, SetupPyStrategy,
SourceBuildTrait,
};

/// e.g. `pygraphviz/graphviz_wrap.c:3020:10: fatal error: graphviz/cgraph.h: No such file or directory`
Expand Down Expand Up @@ -432,7 +433,7 @@ impl SourceBuild {
.await?;

build_context
.install(&resolved_requirements, &venv)
.install(&resolved_requirements, &venv, &Reinstall::None)
.await
.map_err(|err| {
Error::RequirementsInstall("build-system.requires (install)", err)
Expand Down Expand Up @@ -975,7 +976,7 @@ async fn create_pep517_build_environment(
.map_err(|err| Error::RequirementsInstall("build-system.requires (resolve)", err))?;

build_context
.install(&resolution, venv)
.install(&resolution, venv, &Reinstall::None)
.await
.map_err(|err| Error::RequirementsInstall("build-system.requires (install)", err))?;
}
Expand Down
8 changes: 5 additions & 3 deletions crates/uv-dispatch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,16 +159,18 @@ impl<'a> BuildContext for BuildDispatch<'a> {

#[allow(clippy::manual_async_fn)] // TODO(konstin): rustc 1.75 gets into a type inference cycle with async fn
#[instrument(
skip(self, resolution, venv),
skip(self, resolution, venv, reinstall),
fields(
resolution = resolution.distributions().map(ToString::to_string).join(", "),
venv = ?venv.root()
venv = ?venv.root(),
reinstall = ?reinstall
)
)]
fn install<'data>(
&'data self,
resolution: &'data Resolution,
venv: &'data PythonEnvironment,
reinstall: &'data Reinstall,
) -> impl Future<Output = Result<()>> + Send + 'data {
async move {
debug!(
Expand All @@ -194,7 +196,7 @@ impl<'a> BuildContext for BuildDispatch<'a> {
extraneous: _,
} = Planner::with_requirements(&resolution.requirements()).build(
site_packages,
&Reinstall::None,
reinstall,
&NoBinary::None,
self.index_locations,
self.cache(),
Expand Down
8 changes: 1 addition & 7 deletions crates/uv-installer/src/site_packages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,7 @@ impl<'a> SitePackages<'a> {
let site_packages = match fs::read_dir(site_packages) {
Ok(site_packages) => site_packages,
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
return Ok(Self {
venv,
distributions,
by_name,
by_url,
});
continue;
}
Err(err) => return Err(err).context("Failed to read site-packages directory"),
};
Expand All @@ -73,7 +68,6 @@ impl<'a> SitePackages<'a> {
};

let idx = distributions.len();

// Index the distribution by name.
by_name
.entry(dist_info.name().clone())
Expand Down
1 change: 1 addition & 0 deletions crates/uv-interpreter/python/get_interpreter_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,7 @@ def main() -> None:
"prefix": sys.prefix,
"base_executable": getattr(sys, "_base_executable", None),
"sys_executable": sys.executable,
"sys_path": sys.path,
Copy link
Contributor

Choose a reason for hiding this comment

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

is it desirable for cwd to be in here? (if not, it should be removed, accounting for sys.flags.safe_path)

Copy link
Contributor Author

@ChannyClaus ChannyClaus Mar 25, 2024

Choose a reason for hiding this comment

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

probably will leave this value as is so that it still equals to sys.path (it may be confusing if it's called sys_path but the value returned isn't sys.path) - but added filtering via suffix site/dist packages where it's used.

"stdlib": sysconfig.get_path("stdlib"),
"scheme": get_scheme(),
"virtualenv": get_virtualenv(),
Expand Down
13 changes: 13 additions & 0 deletions crates/uv-interpreter/src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ pub struct Interpreter {
base_prefix: PathBuf,
base_executable: Option<PathBuf>,
sys_executable: PathBuf,
sys_path: Vec<PathBuf>,
stdlib: PathBuf,
tags: OnceCell<Tags>,
}
Expand All @@ -57,6 +58,7 @@ impl Interpreter {
base_prefix: info.base_prefix,
base_executable: info.base_executable,
sys_executable: info.sys_executable,
sys_path: info.sys_path,
stdlib: info.stdlib,
tags: OnceCell::new(),
})
Expand Down Expand Up @@ -86,6 +88,7 @@ impl Interpreter {
base_prefix: PathBuf::from("/dev/null"),
base_executable: None,
sys_executable: PathBuf::from("/dev/null"),
sys_path: vec![],
stdlib: PathBuf::from("/dev/null"),
tags: OnceCell::new(),
}
Expand Down Expand Up @@ -254,6 +257,14 @@ impl Interpreter {
&self.sys_executable
}

/// Return the `sys.path` for this Python interpreter.
pub fn sys_path(&self) -> Vec<&Path> {
self.sys_path
.iter()
.map(std::path::PathBuf::as_path)
.collect::<Vec<&Path>>()
}

/// Return the `stdlib` path for this Python interpreter, as returned by `sysconfig.get_paths()`.
pub fn stdlib(&self) -> &Path {
&self.stdlib
Expand Down Expand Up @@ -360,6 +371,7 @@ struct InterpreterInfo {
base_prefix: PathBuf,
base_executable: Option<PathBuf>,
sys_executable: PathBuf,
sys_path: Vec<PathBuf>,
stdlib: PathBuf,
}

Expand Down Expand Up @@ -576,6 +588,7 @@ mod tests {
"base_prefix": "/home/ferris/.pyenv/versions/3.12.0",
"prefix": "/home/ferris/projects/uv/.venv",
"sys_executable": "/home/ferris/projects/uv/.venv/bin/python",
"sys_path": [],
"stdlib": "/home/ferris/.pyenv/versions/3.12.0/lib/python3.12",
"scheme": {
"data": "/home/ferris/.pyenv/versions/3.12.0",
Expand Down
35 changes: 25 additions & 10 deletions crates/uv-interpreter/src/python_environment.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::collections::HashSet;
use std::env;
use std::path::{Path, PathBuf};

Expand Down Expand Up @@ -89,17 +90,31 @@ impl PythonEnvironment {
}

/// Returns an iterator over the `site-packages` directories inside a virtual environment.
///
/// In most cases, `purelib` and `platlib` will be the same, and so the iterator will contain
/// a single element; however, in some distributions, they may be different.
pub fn site_packages(&self) -> impl Iterator<Item = &Path> {
std::iter::once(self.interpreter.purelib()).chain(
if self.interpreter.purelib() == self.interpreter.platlib() {
None
} else {
Some(self.interpreter.platlib())
},
)
let mut site_packages = Vec::new();
site_packages.push(self.interpreter.purelib());
site_packages.push(self.interpreter.platlib());
site_packages.extend(
self.interpreter
.sys_path()
.iter()
.filter(|path| path.ends_with("site-packages") || path.ends_with("dist-packages")),
);
// de-duplicate while preserving order
let mut dedup_set = HashSet::new();
let mut valid_site_packages = site_packages
.into_iter()
.filter(|path| fs_err::canonicalize(path).is_ok())
.collect::<Vec<_>>();
valid_site_packages.retain(|path| dedup_set.insert(fs_err::canonicalize(path).unwrap()));
debug!(
"Site packages: {:?}",
valid_site_packages
.iter()
.map(|p| p.simplified_display())
.collect::<Vec<_>>()
);
valid_site_packages.into_iter()
}

/// Returns the path to the `bin` directory inside a virtual environment.
Expand Down
9 changes: 7 additions & 2 deletions crates/uv-resolver/tests/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use uv_resolver::{
PreReleaseMode, Preference, ResolutionGraph, ResolutionMode, Resolver,
};
use uv_types::{
BuildContext, BuildIsolation, BuildKind, EmptyInstalledPackages, NoBinary, NoBuild,
BuildContext, BuildIsolation, BuildKind, EmptyInstalledPackages, NoBinary, NoBuild, Reinstall,
SetupPyStrategy, SourceBuildTrait,
};

Expand Down Expand Up @@ -83,7 +83,12 @@ impl BuildContext for DummyContext {
panic!("The test should not need to build source distributions")
}

async fn install<'a>(&'a self, _: &'a Resolution, _: &'a PythonEnvironment) -> Result<()> {
async fn install<'a>(
&'a self,
_: &'a Resolution,
_: &'a PythonEnvironment,
_: &'a Reinstall,
) -> Result<()> {
panic!("The test should not need to build source distributions")
}

Expand Down
3 changes: 2 additions & 1 deletion crates/uv-types/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use pep508_rs::{PackageName, Requirement};
use uv_cache::Cache;
use uv_interpreter::{Interpreter, PythonEnvironment};

use crate::{BuildIsolation, BuildKind, NoBinary, NoBuild, SetupPyStrategy};
use crate::{BuildIsolation, BuildKind, NoBinary, NoBuild, Reinstall, SetupPyStrategy};

/// Avoids cyclic crate dependencies between resolver, installer and builder.
///
Expand Down Expand Up @@ -87,6 +87,7 @@ pub trait BuildContext: Sync {
&'a self,
resolution: &'a Resolution,
venv: &'a PythonEnvironment,
reinstall: &'a Reinstall,
) -> impl Future<Output = Result<()>> + Send + 'a;

/// Setup a source distribution build by installing the required dependencies. A wrapper for
Expand Down
5 changes: 3 additions & 2 deletions crates/uv/src/commands/venv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ use uv_fs::Simplified;
use uv_interpreter::{find_default_python, find_requested_python, Error};
use uv_resolver::{InMemoryIndex, OptionsBuilder};
use uv_types::{
BuildContext, BuildIsolation, ConfigSettings, InFlight, NoBinary, NoBuild, SetupPyStrategy,
BuildContext, BuildIsolation, ConfigSettings, InFlight, NoBinary, NoBuild, Reinstall,
SetupPyStrategy,
};

use crate::commands::ExitStatus;
Expand Down Expand Up @@ -208,7 +209,7 @@ async fn venv_impl(

// Install into the environment.
build_dispatch
.install(&resolution, &venv)
.install(&resolution, &venv, &Reinstall::All)
.await
.map_err(VenvError::Seed)?;

Expand Down
Loading