Skip to content

Commit

Permalink
Remove hard coded pip show in fix_direct_url (#2352)
Browse files Browse the repository at this point in the history
Before uv 0.4.25, `uv pip show` did not support the `--files` argument
which is required by `fix_direct_url`. Now that the flag is supported,
falling back to pip is not necessary.

Error handling is also improved. Previously if pip was not present in
the environment (which is the case for `uv venv`) no error was reported
to the user but `fix_direct_url` will have failed so the package is not
installed in editable mode.

fixes #2346
  • Loading branch information
mbway authored Dec 1, 2024
1 parent b743250 commit 5708c78
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 24 deletions.
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ packages = ["maturin"]
bindings = "bin"

[tool.black]
target_version = ['py37']
target-version = ['py37']
extend-exclude = '''
# Ignore cargo-generate templates
^/src/templates
Expand Down
63 changes: 40 additions & 23 deletions src/develop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::BuildOptions;
use crate::PlatformTag;
use crate::PythonInterpreter;
use crate::Target;
use anyhow::ensure;
use anyhow::{anyhow, bail, Context, Result};
use cargo_options::heading;
use fs_err as fs;
Expand All @@ -30,6 +31,13 @@ enum InstallBackend {
}

impl InstallBackend {
fn name(&self) -> &'static str {
match self {
InstallBackend::Pip { .. } => "pip",
InstallBackend::Uv { .. } => "uv pip",
}
}

fn is_pip(&self) -> bool {
matches!(self, InstallBackend::Pip { .. })
}
Expand Down Expand Up @@ -72,6 +80,24 @@ fn find_uv_bin() -> Result<(PathBuf, Vec<&'static str>)> {
}
}

fn check_pip_exists(python_path: &Path, pip_path: Option<&PathBuf>) -> Result<()> {
let output = if let Some(pip_path) = pip_path {
Command::new(pip_path).args(["--version"]).output()?
} else {
Command::new(python_path)
.args(["-m", "pip", "--version"])
.output()?
};
if output.status.success() {
let version_str =
str::from_utf8(&output.stdout).context("`pip --version` didn't return utf8 output")?;
debug!(version = %version_str, "Found pip");
Ok(())
} else {
bail!("`pip --version` failed with status: {}", output.status);
}
}

/// Detect the Python uv package
fn find_uv_python(python_path: &Path) -> Result<(PathBuf, Vec<&'static str>)> {
let output = Command::new(python_path)
Expand Down Expand Up @@ -185,12 +211,11 @@ fn install_dependencies(
}

#[instrument(skip_all, fields(wheel_filename = %wheel_filename.display()))]
fn pip_install_wheel(
fn install_wheel(
build_context: &BuildContext,
python: &Path,
venv_dir: &Path,
wheel_filename: &Path,
pip_path: Option<PathBuf>,
install_backend: &InstallBackend,
) -> Result<()> {
let mut cmd = install_backend.make_command(python);
Expand All @@ -199,13 +224,15 @@ fn pip_install_wheel(
.arg(dunce::simplified(wheel_filename))
.output()
.context(format!(
"pip install failed (ran {:?} with {:?})",
"{} install failed (ran {:?} with {:?})",
install_backend.name(),
cmd.get_program(),
&cmd.get_args().collect::<Vec<_>>(),
))?;
if !output.status.success() {
bail!(
"pip install in {} failed running {:?}: {}\n--- Stdout:\n{}\n--- Stderr:\n{}\n---\n",
"{} install in {} failed running {:?}: {}\n--- Stdout:\n{}\n--- Stderr:\n{}\n---\n",
install_backend.name(),
venv_dir.display(),
&cmd.get_args().collect::<Vec<_>>(),
output.status,
Expand All @@ -221,17 +248,11 @@ fn pip_install_wheel(
String::from_utf8_lossy(&output.stderr).trim(),
);
}
// pip show --files is not supported by uv, thus
// default to using pip backend all the time
fix_direct_url(
build_context,
python,
&InstallBackend::Pip { path: pip_path },
)?;
fix_direct_url(build_context, python, install_backend)?;
Ok(())
}

/// Each editable-installed python package has a direct_url.json file that includes a file:// URL
/// Each editable-installed python package has a `direct_url.json` file that includes a `file://` URL
/// indicating the location of the source code of that project. The maturin import hook uses this
/// URL to locate and rebuild editable-installed projects.
///
Expand All @@ -246,15 +267,10 @@ fn fix_direct_url(
) -> Result<()> {
println!("✏️ Setting installed package as editable");
let mut cmd = install_backend.make_command(python);
let output = cmd
.args(["show", "--files"])
.arg(&build_context.metadata23.name)
.output()
.context(format!(
"pip show failed (ran {:?} with {:?})",
cmd.get_program(),
&cmd.get_args().collect::<Vec<_>>(),
))?;
let cmd = cmd.args(["show", "--files", &build_context.metadata23.name]);
debug!("running {:?}", cmd);
let output = cmd.output()?;
ensure!(output.status.success(), "failed to list package files");
if let Some(direct_url_path) = parse_direct_url_path(&String::from_utf8_lossy(&output.stdout))?
{
let project_dir = build_context
Expand Down Expand Up @@ -353,6 +369,8 @@ pub fn develop(develop_options: DevelopOptions, venv_dir: &Path) -> Result<()> {
args: uv_args,
}
} else {
check_pip_exists(&interpreter.executable, pip_path.as_ref())
.context("Failed to find pip (if working with a uv venv try `maturin develop --uv`)")?;
InstallBackend::Pip {
path: pip_path.clone(),
}
Expand All @@ -363,12 +381,11 @@ pub fn develop(develop_options: DevelopOptions, venv_dir: &Path) -> Result<()> {
let wheels = build_context.build_wheels()?;
if !skip_install {
for (filename, _supported_version) in wheels.iter() {
pip_install_wheel(
install_wheel(
&build_context,
&python,
venv_dir,
filename,
pip_path.clone(),
&install_backend,
)?;
eprintln!(
Expand Down

0 comments on commit 5708c78

Please sign in to comment.