From cb7a038ba0966443667810a57759e8eb6f922209 Mon Sep 17 00:00:00 2001 From: messense Date: Thu, 8 Jun 2023 05:46:48 +0000 Subject: [PATCH 1/2] Refactor `develop` to take a `DevelopOptions` --- src/develop.rs | 48 +++++++++++++++++++++++++++++++++------- src/lib.rs | 2 +- src/main.rs | 47 +++++---------------------------------- tests/cmd/develop.stdout | 2 -- tests/common/develop.rs | 16 +++++++------- 5 files changed, 55 insertions(+), 60 deletions(-) diff --git a/src/develop.rs b/src/develop.rs index bdc0190ff..436957db8 100644 --- a/src/develop.rs +++ b/src/develop.rs @@ -10,19 +10,51 @@ use std::path::Path; use std::process::Command; use tempfile::TempDir; +/// Install the crate as module in the current virtualenv +#[derive(Debug, clap::Parser)] +pub struct DevelopOptions { + /// Which kind of bindings to use + #[arg( + short = 'b', + long = "bindings", + alias = "binding-crate", + value_parser = ["pyo3", "pyo3-ffi", "rust-cpython", "cffi", "uniffi", "bin"] + )] + pub bindings: Option, + /// Pass --release to cargo + #[arg(short = 'r', long)] + pub release: bool, + /// Strip the library for minimum file size + #[arg(long)] + pub strip: bool, + /// Install extra requires aka. optional dependencies + /// + /// Use as `--extras=extra1,extra2` + #[arg( + short = 'E', + long, + value_delimiter = ',', + action = clap::ArgAction::Append + )] + pub extras: Vec, + /// `cargo rustc` options + #[command(flatten)] + pub cargo_options: CargoOptions, +} + /// Installs a crate by compiling it and copying the shared library to site-packages. /// Also adds the dist-info directory to make sure pip and other tools detect the library /// /// Works only in a virtualenv. #[allow(clippy::too_many_arguments)] -pub fn develop( - bindings: Option, - cargo_options: CargoOptions, - venv_dir: &Path, - release: bool, - strip: bool, - extras: Vec, -) -> Result<()> { +pub fn develop(develop_options: DevelopOptions, venv_dir: &Path) -> Result<()> { + let DevelopOptions { + bindings, + release, + strip, + extras, + cargo_options, + } = develop_options; let mut target_triple = cargo_options.target.as_ref().map(|x| x.to_string()); let target = Target::from_target_triple(cargo_options.target)?; let python = target.get_venv_python(venv_dir); diff --git a/src/lib.rs b/src/lib.rs index 3998fe558..dc89994e1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -27,7 +27,7 @@ pub use crate::build_context::{BridgeModel, BuildContext, BuiltWheelMetadata}; pub use crate::build_options::{BuildOptions, CargoOptions}; pub use crate::cargo_toml::CargoToml; pub use crate::compile::{compile, BuildArtifact}; -pub use crate::develop::develop; +pub use crate::develop::{develop, DevelopOptions}; pub use crate::metadata::{Metadata21, WheelMetadata}; pub use crate::module_writer::{ write_dist_info, ModuleWriter, PathWriter, SDistWriter, WheelWriter, diff --git a/src/main.rs b/src/main.rs index f8e1c4641..b8aadcc9a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -12,8 +12,8 @@ use clap::{Parser, Subcommand}; #[cfg(feature = "scaffolding")] use maturin::{ci::GenerateCI, init_project, new_project, GenerateProjectOptions}; use maturin::{ - develop, write_dist_info, BridgeModel, BuildOptions, CargoOptions, PathWriter, PlatformTag, - PythonInterpreter, Target, + develop, write_dist_info, BridgeModel, BuildOptions, CargoOptions, DevelopOptions, PathWriter, + PlatformTag, PythonInterpreter, Target, }; #[cfg(feature = "upload")] use maturin::{upload_ui, PublishOpt}; @@ -73,36 +73,7 @@ enum Opt { }, #[command(name = "develop", alias = "dev")] /// Install the crate as module in the current virtualenv - /// - /// Note that this command doesn't create entrypoints - Develop { - /// Which kind of bindings to use - #[arg( - short = 'b', - long = "bindings", - alias = "binding-crate", - value_parser = ["pyo3", "pyo3-ffi", "rust-cpython", "cffi", "uniffi", "bin"] - )] - bindings: Option, - /// Pass --release to cargo - #[arg(short = 'r', long)] - release: bool, - /// Strip the library for minimum file size - #[arg(long)] - strip: bool, - /// Install extra requires aka. optional dependencies - /// - /// Use as `--extras=extra1,extra2` - #[arg( - short = 'E', - long, - value_delimiter = ',', - action = clap::ArgAction::Append - )] - extras: Vec, - #[command(flatten)] - cargo_options: CargoOptions, - }, + Develop(DevelopOptions), /// Build only a source distribution (sdist) without compiling. /// /// Building a source distribution requires a pyproject.toml with a `[build-system]` table. @@ -404,16 +375,10 @@ fn run() -> Result<()> { eprintln!(" - {interpreter}"); } } - Opt::Develop { - bindings, - release, - strip, - extras, - cargo_options, - } => { - let target = Target::from_target_triple(cargo_options.target.clone())?; + Opt::Develop(develop_options) => { + let target = Target::from_target_triple(develop_options.cargo_options.target.clone())?; let venv_dir = detect_venv(&target)?; - develop(bindings, cargo_options, &venv_dir, release, strip, extras)?; + develop(develop_options, &venv_dir)?; } Opt::SDist { manifest_path, out } => { let build_options = BuildOptions { diff --git a/tests/cmd/develop.stdout b/tests/cmd/develop.stdout index a53d6afb8..59b52431f 100644 --- a/tests/cmd/develop.stdout +++ b/tests/cmd/develop.stdout @@ -1,7 +1,5 @@ Install the crate as module in the current virtualenv -Note that this command doesn't create entrypoints - Usage: maturin[EXE] develop [OPTIONS] [ARGS]... Arguments: diff --git a/tests/common/develop.rs b/tests/common/develop.rs index 735c5cbad..aaaea8d29 100644 --- a/tests/common/develop.rs +++ b/tests/common/develop.rs @@ -1,6 +1,6 @@ use crate::common::{check_installed, create_conda_env, create_virtualenv, maybe_mock_cargo}; use anyhow::Result; -use maturin::{develop, CargoOptions}; +use maturin::{develop, CargoOptions, DevelopOptions}; use std::path::{Path, PathBuf}; use std::process::Command; use std::str; @@ -44,19 +44,19 @@ pub fn test_develop( } let manifest_file = package.join("Cargo.toml"); - develop( + let develop_options = DevelopOptions { bindings, - CargoOptions { + release: false, + strip: false, + extras: Vec::new(), + cargo_options: CargoOptions { manifest_path: Some(manifest_file), quiet: true, target_dir: Some(PathBuf::from(format!("test-crates/targets/{unique_name}"))), ..Default::default() }, - &venv_dir, - false, - cfg!(feature = "faster-tests"), - vec![], - )?; + }; + develop(develop_options, &venv_dir)?; check_installed(package, &python)?; Ok(()) From 0e675f0baa0732aac9319467344635ac6275bcec Mon Sep 17 00:00:00 2001 From: messense Date: Thu, 8 Jun 2023 05:56:18 +0000 Subject: [PATCH 2/2] Add `--skip-install` option to `maturin develop` --- src/develop.rs | 58 +++++++++++++++++++++++----------------- tests/cmd/develop.stdout | 5 ++++ tests/common/develop.rs | 1 + 3 files changed, 39 insertions(+), 25 deletions(-) diff --git a/src/develop.rs b/src/develop.rs index 436957db8..db85eb83c 100644 --- a/src/develop.rs +++ b/src/develop.rs @@ -37,6 +37,11 @@ pub struct DevelopOptions { action = clap::ArgAction::Append )] pub extras: Vec, + /// Skip installation, only build the extension module inplace + /// + /// Only works with mixed Rust/Python project layout + #[arg(long)] + pub skip_install: bool, /// `cargo rustc` options #[command(flatten)] pub cargo_options: CargoOptions, @@ -53,6 +58,7 @@ pub fn develop(develop_options: DevelopOptions, venv_dir: &Path) -> Result<()> { release, strip, extras, + skip_install, cargo_options, } = develop_options; let mut target_triple = cargo_options.target.as_ref().map(|x| x.to_string()); @@ -149,22 +155,23 @@ pub fn develop(develop_options: DevelopOptions, venv_dir: &Path) -> Result<()> { } let wheels = build_context.build_wheels()?; - for (filename, _supported_version) in wheels.iter() { - let command = [ - "-m", - "pip", - "--disable-pip-version-check", - "install", - "--no-deps", - "--force-reinstall", - ]; - let output = Command::new(&python) - .args(command) - .arg(dunce::simplified(filename)) - .output() - .context(format!("pip install failed with {python:?}"))?; - if !output.status.success() { - bail!( + if !skip_install { + for (filename, _supported_version) in wheels.iter() { + let command = [ + "-m", + "pip", + "--disable-pip-version-check", + "install", + "--no-deps", + "--force-reinstall", + ]; + let output = Command::new(&python) + .args(command) + .arg(dunce::simplified(filename)) + .output() + .context(format!("pip install failed with {python:?}"))?; + if !output.status.success() { + bail!( "pip install in {} failed running {:?}: {}\n--- Stdout:\n{}\n--- Stderr:\n{}\n---\n", venv_dir.display(), &command, @@ -172,18 +179,19 @@ pub fn develop(develop_options: DevelopOptions, venv_dir: &Path) -> Result<()> { String::from_utf8_lossy(&output.stdout).trim(), String::from_utf8_lossy(&output.stderr).trim(), ); - } - if !output.stderr.is_empty() { + } + if !output.stderr.is_empty() { + eprintln!( + "⚠️ Warning: pip raised a warning running {:?}:\n{}", + &command, + String::from_utf8_lossy(&output.stderr).trim(), + ); + } eprintln!( - "⚠️ Warning: pip raised a warning running {:?}:\n{}", - &command, - String::from_utf8_lossy(&output.stderr).trim(), + "🛠 Installed {}-{}", + build_context.metadata21.name, build_context.metadata21.version ); } - eprintln!( - "🛠 Installed {}-{}", - build_context.metadata21.name, build_context.metadata21.version - ); } Ok(()) diff --git a/tests/cmd/develop.stdout b/tests/cmd/develop.stdout index 59b52431f..83c6cb027 100644 --- a/tests/cmd/develop.stdout +++ b/tests/cmd/develop.stdout @@ -23,6 +23,11 @@ Options: Use as `--extras=extra1,extra2` + --skip-install + Skip installation, only build the extension module inplace + + Only works with mixed Rust/Python project layout + -q, --quiet Do not print cargo log messages diff --git a/tests/common/develop.rs b/tests/common/develop.rs index aaaea8d29..b58100aaa 100644 --- a/tests/common/develop.rs +++ b/tests/common/develop.rs @@ -49,6 +49,7 @@ pub fn test_develop( release: false, strip: false, extras: Vec::new(), + skip_install: false, cargo_options: CargoOptions { manifest_path: Some(manifest_file), quiet: true,