Isolated import hook changes (#1958)

* small fixes to test-crates

* added pyo3-mixed-with-path-dep test crate

* moved fixing of direct_url.json into maturin itself. Also refactored develop.rs

* renamed document to match title

* small fixes

* support windows style paths when fixing direct_url.json

* fixes to test crate

* updated guide

* updated lockfile

* updated test crate lock files

* removed lock file that was supposed to be missing

* updated link in SUMMARY.md

* added tests for pyo3-mixed-with-path-dep

---------

Co-authored-by: messense <[email protected]>

Author: mbway

Cargo.toml

@@ -82,7 +82,8 @@ normpath = "1.1.1"
 path-slash = "0.2.1"
 pep440_rs = { version = "0.5.0", features = ["serde", "tracing"] }
 pep508_rs = { version = "0.4.2", features = ["serde", "tracing"] }
-time = "0.3.34"
+time = "0.3.17"
+url = "2.5.0"
 unicode-xid = { version = "0.2.4", optional = true }
 
 # cli
@@ -127,8 +128,7 @@ rustls-pemfile = { version = "2.1.0", optional = true }
 keyring = { version = "2.3.2", default-features = false, features = [
     "linux-no-secret-service",
 ], optional = true }
-wild = { version = "2.2.1", optional = true }
-url = { version = "2.3.0", optional = true }
+wild = { version = "2.1.0", optional = true }
 
 [dev-dependencies]
 expect-test = "1.4.1"
@@ -154,10 +154,10 @@ upload = [
     "configparser",
     "bytesize",
     "dialoguer/password",
-    "url",
     "wild",
     "dep:dirs",
 ]
+
 # keyring doesn't support *BSD so it's not enabled in `full` by default
 password-storage = ["upload", "keyring"]
 

guide/src/SUMMARY.md

@@ -11,7 +11,7 @@
 - [Python Metadata](./metadata.md)
 - [Configuration](./config.md)
 - [Environment Variables](./environment-variables.md)
-- [Local Development](./develop.md)
+- [Local Development](./local_development.md)
 - [Distribution](./distribution.md)
 - [Sphinx Integration](./sphinx.md)
 

guide/src/develop.md renamed to guide/src/local_development.md

@@ -117,15 +117,26 @@ requires = ["maturin>=1.0,<2.0"]
 build-backend = "maturin"
 ```
 
-Editable installs right now is only useful in mixed Rust/Python project so you
-don't have to recompile and reinstall when only Python source code changes. For
-example when using pip you can make an editable installation with
+Editable installs can be used with mixed Rust/Python projects so you
+don't have to recompile and reinstall when only Python source code changes.
+They can also be used with mixed and pure projects together with the
+import hook so that recompilation/re-installation occurs automatically
+when Python or Rust source code changes.
+
+To install a package in editable mode with pip:
 
 ```bash
+cd my-project
 pip install -e .
 ```
+or
+```bash
+cd my-project
+maturin develop
+```
 
-Then Python source code changes will take effect immediately.
+Then Python source code changes will take effect immediately because the interpreter looks
+for the modules directly in the project source tree.
 
 ## Import Hook
 

src/develop.rs

@@ -1,16 +1,20 @@
 use crate::build_options::CargoOptions;
 use crate::target::Arch;
+use crate::BuildContext;
 use crate::BuildOptions;
 use crate::PlatformTag;
 use crate::PythonInterpreter;
 use crate::Target;
 use anyhow::{anyhow, bail, Context, Result};
 use cargo_options::heading;
 use pep508_rs::{MarkerExpression, MarkerOperator, MarkerTree, MarkerValue};
+use regex::Regex;
+use std::fs;
 use std::path::Path;
 use std::path::PathBuf;
 use std::process::Command;
 use tempfile::TempDir;
+use url::Url;
 
 /// Install the crate as module in the current virtualenv
 #[derive(Debug, clap::Parser)]
@@ -72,6 +76,143 @@ fn make_pip_command(python_path: &Path, pip_path: Option<&Path>) -> Command {
     }
 }
 
+fn install_dependencies(
+    build_context: &BuildContext,
+    extras: &[String],
+    interpreter: &PythonInterpreter,
+    pip_path: Option<&Path>,
+) -> Result<()> {
+    if !build_context.metadata21.requires_dist.is_empty() {
+        let mut args = vec!["install".to_string()];
+        args.extend(build_context.metadata21.requires_dist.iter().map(|x| {
+            let mut pkg = x.clone();
+            // Remove extra marker to make it installable with pip
+            // Keep in sync with `Metadata21::merge_pyproject_toml()`!
+            for extra in extras {
+                pkg.marker = pkg.marker.and_then(|marker| -> Option<MarkerTree> {
+                    match marker.clone() {
+                        MarkerTree::Expression(MarkerExpression {
+                            l_value: MarkerValue::Extra,
+                            operator: MarkerOperator::Equal,
+                            r_value: MarkerValue::QuotedString(extra_value),
+                        }) if &extra_value == extra => None,
+                        MarkerTree::And(and) => match &*and {
+                            [existing, MarkerTree::Expression(MarkerExpression {
+                                l_value: MarkerValue::Extra,
+                                operator: MarkerOperator::Equal,
+                                r_value: MarkerValue::QuotedString(extra_value),
+                            })] if extra_value == extra => Some(existing.clone()),
+                            _ => Some(marker),
+                        },
+                        _ => Some(marker),
+                    }
+                });
+            }
+            pkg.to_string()
+        }));
+        let status = make_pip_command(&interpreter.executable, pip_path)
+            .args(&args)
+            .status()
+            .context("Failed to run pip install")?;
+        if !status.success() {
+            bail!(r#"pip install finished with "{}""#, status)
+        }
+    }
+    Ok(())
+}
+
+fn pip_install_wheel(
+    build_context: &BuildContext,
+    python: &Path,
+    venv_dir: &Path,
+    pip_path: Option<&Path>,
+    wheel_filename: &Path,
+) -> Result<()> {
+    let mut pip_cmd = make_pip_command(python, pip_path);
+    let output = pip_cmd
+        .args(["install", "--no-deps", "--force-reinstall"])
+        .arg(dunce::simplified(wheel_filename))
+        .output()
+        .context(format!(
+            "pip install failed (ran {:?} with {:?})",
+            pip_cmd.get_program(),
+            &pip_cmd.get_args().collect::<Vec<_>>(),
+        ))?;
+    if !output.status.success() {
+        bail!(
+            "pip install in {} failed running {:?}: {}\n--- Stdout:\n{}\n--- Stderr:\n{}\n---\n",
+            venv_dir.display(),
+            &pip_cmd.get_args().collect::<Vec<_>>(),
+            output.status,
+            String::from_utf8_lossy(&output.stdout).trim(),
+            String::from_utf8_lossy(&output.stderr).trim(),
+        );
+    }
+    if !output.stderr.is_empty() {
+        eprintln!(
+            "⚠️ Warning: pip raised a warning running {:?}:\n{}",
+            &pip_cmd.get_args().collect::<Vec<_>>(),
+            String::from_utf8_lossy(&output.stderr).trim(),
+        );
+    }
+    fix_direct_url(build_context, python, pip_path)?;
+    Ok(())
+}
+
+/// 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.
+///
+/// When a maturin package is installed using `pip install -e`, pip takes care of writing the
+/// correct URL, however when a maturin package is installed with `maturin develop`, the URL is
+/// set to the path to the temporary wheel file created during installation.
+fn fix_direct_url(
+    build_context: &BuildContext,
+    python: &Path,
+    pip_path: Option<&Path>,
+) -> Result<()> {
+    println!("✏️  Setting installed package as editable");
+    let mut pip_cmd = make_pip_command(python, pip_path);
+    let output = pip_cmd
+        .args(["show", "--files"])
+        .arg(&build_context.metadata21.name)
+        .output()
+        .context(format!(
+            "pip show failed (ran {:?} with {:?})",
+            pip_cmd.get_program(),
+            &pip_cmd.get_args().collect::<Vec<_>>(),
+        ))?;
+    if let Some(direct_url_path) = parse_direct_url_path(&String::from_utf8_lossy(&output.stdout))?
+    {
+        let project_dir = build_context
+            .pyproject_toml_path
+            .parent()
+            .ok_or_else(|| anyhow!("failed to get project directory"))?;
+        let uri = Url::from_file_path(project_dir)
+            .map_err(|_| anyhow!("failed to convert project directory to file URL"))?;
+        let content = format!("{{\"dir_info\": {{\"editable\": true}}, \"url\": \"{uri}\"}}");
+        fs::write(direct_url_path, content)?;
+    }
+    Ok(())
+}
+
+fn parse_direct_url_path(pip_show_output: &str) -> Result<Option<PathBuf>> {
+    if let Some(Some(location)) = Regex::new(r"Location: ([^\r\n]*)")?
+        .captures(pip_show_output)
+        .map(|c| c.get(1))
+    {
+        if let Some(Some(direct_url_path)) = Regex::new(r"  (.*direct_url.json)")?
+            .captures(pip_show_output)
+            .map(|c| c.get(1))
+        {
+            return Ok(Some(
+                PathBuf::from(location.as_str()).join(direct_url_path.as_str()),
+            ));
+        }
+    }
+    Ok(None)
+}
+
 /// 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
 ///
@@ -137,74 +278,18 @@ pub fn develop(develop_options: DevelopOptions, venv_dir: &Path) -> Result<()> {
             || anyhow!("Expected `python` to be a python interpreter inside a virtualenv ಠ_ಠ"),
         )?;
 
-    // Install dependencies
-    if !build_context.metadata21.requires_dist.is_empty() {
-        let mut args = vec!["install".to_string()];
-        args.extend(build_context.metadata21.requires_dist.iter().map(|x| {
-            let mut pkg = x.clone();
-            // Remove extra marker to make it installable with pip
-            // Keep in sync with `Metadata21::merge_pyproject_toml()`!
-            for extra in &extras {
-                pkg.marker = pkg.marker.and_then(|marker| -> Option<MarkerTree> {
-                    match marker.clone() {
-                        MarkerTree::Expression(MarkerExpression {
-                            l_value: MarkerValue::Extra,
-                            operator: MarkerOperator::Equal,
-                            r_value: MarkerValue::QuotedString(extra_value),
-                        }) if &extra_value == extra => None,
-                        MarkerTree::And(and) => match &*and {
-                            [existing, MarkerTree::Expression(MarkerExpression {
-                                l_value: MarkerValue::Extra,
-                                operator: MarkerOperator::Equal,
-                                r_value: MarkerValue::QuotedString(extra_value),
-                            })] if extra_value == extra => Some(existing.clone()),
-                            _ => Some(marker),
-                        },
-                        _ => Some(marker),
-                    }
-                });
-            }
-            pkg.to_string()
-        }));
-        let status = make_pip_command(&interpreter.executable, pip_path.as_deref())
-            .args(&args)
-            .status()
-            .context("Failed to run pip install")?;
-        if !status.success() {
-            bail!(r#"pip install finished with "{}""#, status)
-        }
-    }
+    install_dependencies(&build_context, &extras, &interpreter, pip_path.as_deref())?;
 
     let wheels = build_context.build_wheels()?;
     if !skip_install {
         for (filename, _supported_version) in wheels.iter() {
-            let mut pip_cmd = make_pip_command(&python, pip_path.as_deref());
-            let output = pip_cmd
-                .args(["install", "--no-deps", "--force-reinstall"])
-                .arg(dunce::simplified(filename))
-                .output()
-                .context(format!(
-                    "pip install failed (ran {:?} with {:?})",
-                    pip_cmd.get_program(),
-                    &pip_cmd.get_args().collect::<Vec<_>>(),
-                ))?;
-            if !output.status.success() {
-                bail!(
-                "pip install in {} failed running {:?}: {}\n--- Stdout:\n{}\n--- Stderr:\n{}\n---\n",
-                venv_dir.display(),
-                &pip_cmd.get_args().collect::<Vec<_>>(),
-                output.status,
-                String::from_utf8_lossy(&output.stdout).trim(),
-                String::from_utf8_lossy(&output.stderr).trim(),
-            );
-            }
-            if !output.stderr.is_empty() {
-                eprintln!(
-                    "⚠️ Warning: pip raised a warning running {:?}:\n{}",
-                    &pip_cmd.get_args().collect::<Vec<_>>(),
-                    String::from_utf8_lossy(&output.stderr).trim(),
-                );
-            }
+            pip_install_wheel(
+                &build_context,
+                &python,
+                venv_dir,
+                pip_path.as_deref(),
+                filename,
+            )?;
             eprintln!(
                 "🛠 Installed {}-{}",
                 build_context.metadata21.name, build_context.metadata21.version
@@ -214,3 +299,79 @@ pub fn develop(develop_options: DevelopOptions, venv_dir: &Path) -> Result<()> {
 
     Ok(())
 }
+
+#[cfg(test)]
+mod test {
+    use std::path::PathBuf;
+
+    use super::parse_direct_url_path;
+
+    #[test]
+    #[cfg(not(target_os = "windows"))]
+    fn test_parse_direct_url() {
+        let example_with_direct_url = "\
+Name: my-project
+Version: 0.1.0
+Location: /foo bar/venv/lib/pythonABC/site-packages
+Editable project location: /tmp/temporary.whl
+Files:
+  my_project-0.1.0+abc123de.dist-info/INSTALLER
+  my_project-0.1.0+abc123de.dist-info/METADATA
+  my_project-0.1.0+abc123de.dist-info/RECORD
+  my_project-0.1.0+abc123de.dist-info/REQUESTED
+  my_project-0.1.0+abc123de.dist-info/WHEEL
+  my_project-0.1.0+abc123de.dist-info/direct_url.json
+  my_project-0.1.0+abc123de.dist-info/entry_points.txt
+  my_project.pth
+";
+        let expected_path = PathBuf::from("/foo bar/venv/lib/pythonABC/site-packages/my_project-0.1.0+abc123de.dist-info/direct_url.json");
+        assert_eq!(
+            parse_direct_url_path(example_with_direct_url).unwrap(),
+            Some(expected_path)
+        );
+
+        let example_without_direct_url = "\
+Name: my-project
+Version: 0.1.0
+Location: /foo bar/venv/lib/pythonABC/site-packages
+Files:
+  my_project-0.1.0+abc123de.dist-info/INSTALLER
+  my_project-0.1.0+abc123de.dist-info/METADATA
+  my_project-0.1.0+abc123de.dist-info/RECORD
+  my_project-0.1.0+abc123de.dist-info/REQUESTED
+  my_project-0.1.0+abc123de.dist-info/WHEEL
+  my_project-0.1.0+abc123de.dist-info/entry_points.txt
+  my_project.pth
+";
+
+        assert_eq!(
+            parse_direct_url_path(example_without_direct_url).unwrap(),
+            None
+        );
+    }
+
+    #[test]
+    #[cfg(target_os = "windows")]
+    fn test_parse_direct_url_windows() {
+        let example_with_direct_url_windows = "\
+Name: my-project\r
+Version: 0.1.0\r
+Location: C:\\foo bar\\venv\\Lib\\site-packages\r
+Files:\r
+  my_project-0.1.0+abc123de.dist-info\\INSTALLER\r
+  my_project-0.1.0+abc123de.dist-info\\METADATA\r
+  my_project-0.1.0+abc123de.dist-info\\RECORD\r
+  my_project-0.1.0+abc123de.dist-info\\REQUESTED\r
+  my_project-0.1.0+abc123de.dist-info\\WHEEL\r
+  my_project-0.1.0+abc123de.dist-info\\direct_url.json\r
+  my_project-0.1.0+abc123de.dist-info\\entry_points.txt\r
+  my_project.pth\r
+";
+
+        let expected_path = PathBuf::from("C:\\foo bar\\venv\\Lib\\site-packages\\my_project-0.1.0+abc123de.dist-info\\direct_url.json");
+        assert_eq!(
+            parse_direct_url_path(example_with_direct_url_windows).unwrap(),
+            Some(expected_path)
+        );
+    }
+}