From e302648843fef1f5f7334bc4f75e2a757185b8da Mon Sep 17 00:00:00 2001 From: konstin Date: Fri, 19 Jan 2024 13:17:42 +0100 Subject: [PATCH] Match dependency readmes by package id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have a project which depends on a vendored version of `pep440_rs`, while some of our dependencies depend on the crates.io version. The published version uses `Readme.md`, while the vendored one follows a different convention and uses `README.md`. This causes `maturin sdist` to fail: ``` 🍹 Building a mixed python/rust project 🔗 Found bin bindings 📡 Using build options bindings from pyproject.toml 💥 maturin failed Caused by: Failed to build source distribution Caused by: failed to normalize path `Readme.md` Caused by: No such file or directory (os error 2) ``` Internally, maturin uses the package name to match dependencies with their resolved readme. This failed in our cased since the second entry for `pep440_rs` with the crates.io readme was overwriting the first entry with the project local readme. The fix is to match by package id instead. The implementation is somewhat awkward, since there's no API to get the package id directly, so we again have to get list of dependency package ids and dependency `Dependency`s and match them by name, assuming in a single `[dependency]` each package name must be unique.
Relevant `cargo metadata` excerpt ```javascript [ { "name": "pep440_rs", "version": "0.3.12", "id": "pep440_rs 0.3.12 (path+file:///home/konsti/projects/internal/crates/pep440-rs)", "license": "Apache-2.0 OR BSD-2-Clause", "license_file": null, "description": "A library for python version numbers and specifiers, implementing PEP 440", "source": null, "dependencies": ["..."], "targets": ["..."], "features": { "...": [ "..." ] }, "manifest_path": "/home/konsti/projects/internal/crates/pep440-rs/Cargo.toml", "metadata": null, "publish": null, "authors": [ "Puffin" ], "categories": [], "keywords": [], "readme": "README.md", "repository": "https://github.com/astral-sh/internal", "homepage": "https://pypi.org/project/internal-alpha/", "documentation": "https://pypi.org/project/internal-alpha/", "edition": "2021", "links": null, "default_run": null, "rust_version": "1.74" }, { "name": "pep440_rs", "version": "0.3.12", "id": "pep440_rs 0.3.12 (registry+https://github.com/rust-lang/crates.io-index)", "license": "Apache-2.0 OR BSD-2-Clause", "license_file": null, "description": "A library for python version numbers and specifiers, implementing PEP 440", "source": "registry+https://github.com/rust-lang/crates.io-index", "dependencies": ["..."], "targets": ["..."], "features": { "...": [ "..." ] }, "manifest_path": "/home/konsti/.cargo/registry/src/index.crates.io-6f17d22bba15001f/pep440_rs-0.3.12/Cargo.toml", "metadata": null, "publish": null, "authors": [], "categories": [], "keywords": [], "readme": "Readme.md", "repository": "https://github.com/konstin/pep440-rs", "homepage": null, "documentation": null, "edition": "2021", "links": null, "default_run": null, "rust_version": null } ] ```
--- src/module_writer.rs | 2 +- src/project_layout.rs | 6 +++-- src/source_distribution.rs | 54 ++++++++++++++++++++++++++++++-------- 3 files changed, 48 insertions(+), 14 deletions(-) diff --git a/src/module_writer.rs b/src/module_writer.rs index 0c13e0105..0c9946884 100644 --- a/src/module_writer.rs +++ b/src/module_writer.rs @@ -302,7 +302,7 @@ impl WheelWriter { .normalize() .with_context(|| { format!( - "failed to normalize path `{}`", + "failed to normalize python dir path `{}`", project_layout.python_dir.display() ) })? diff --git a/src/project_layout.rs b/src/project_layout.rs index affb25d80..e3ef0958a 100644 --- a/src/project_layout.rs +++ b/src/project_layout.rs @@ -209,7 +209,7 @@ impl ProjectResolver { if let Some(path) = cargo_manifest_path { let path = path .normalize() - .with_context(|| format!("failed to normalize path `{}`", path.display()))? + .with_context(|| format!("failed to normalize manifest path `{}`", path.display()))? .into_path_buf(); debug!( "Using cargo manifest path from command line argument: {:?}", @@ -248,7 +248,9 @@ impl ProjectResolver { debug!("Using cargo manifest path from pyproject.toml {:?}", path); return Ok(( path.normalize() - .with_context(|| format!("failed to normalize path `{}`", path.display()))? + .with_context(|| { + format!("failed to normalize manifest path `{}`", path.display()) + })? .into_path_buf(), pyproject_file, )); diff --git a/src/source_distribution.rs b/src/source_distribution.rs index 1cdc7c8b6..56a4e3b30 100644 --- a/src/source_distribution.rs +++ b/src/source_distribution.rs @@ -2,7 +2,7 @@ use crate::module_writer::{add_data, ModuleWriter}; use crate::pyproject_toml::SdistGenerator; use crate::{pyproject_toml::Format, BuildContext, PyProjectToml, SDistWriter}; use anyhow::{bail, Context, Result}; -use cargo_metadata::{Metadata, MetadataCommand}; +use cargo_metadata::{Metadata, MetadataCommand, PackageId}; use fs_err as fs; use ignore::overrides::Override; use normpath::PathExt as _; @@ -242,14 +242,40 @@ fn find_path_deps(cargo_metadata: &Metadata) -> Result>(); + .collect::>(); // scan the dependency graph for path dependencies let mut path_deps = HashMap::new(); let mut stack: Vec<&cargo_metadata::Package> = vec![root]; while let Some(top) = stack.pop() { - for dependency in &top.dependencies { + // There seems to be no API to get the package id of a dependency, so collect the package ids from resolve + // and match them up with the `Dependency`s from `Package.dependencies`. + let dep_ids = &cargo_metadata + .resolve + .as_ref() + .unwrap() + .nodes + .iter() + .find(|node| node.id == top.id) + .unwrap() + .dependencies; + for dep_id in dep_ids { + // Assumption: Each package name can only occur once in a `[dependencies]` table. + let dependency = top + .dependencies + .iter() + .find(|package| { + // Package ids are opaque and there seems to be no way to query their name. + let dep_name = &cargo_metadata + .packages + .iter() + .find(|package| &package.id == dep_id) + .unwrap() + .name; + &package.name == dep_name + }) + .unwrap(); if let Some(path) = &dependency.path { let dep_name = dependency.rename.as_ref().unwrap_or(&dependency.name); if path_deps.contains_key(dep_name) { @@ -269,9 +295,10 @@ fn find_path_deps(cargo_metadata: &Metadata) -> Result Result