Skip to content

Commit

Permalink
Match dependency readmes by package id
Browse files Browse the repository at this point in the history 
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.

<details>
  <summary>Relevant `cargo metadata` excerpt</summary>

  ```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
    }
  ]
  ```

</details>
  • Loading branch information
@konstin
konstin committed Jan 19, 2024
1 parent 3492c97 commit cd2ad95
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 14 deletions.
2 changes: 1 addition & 1 deletion src/module_writer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
)
})?
Expand Down
4 changes: 2 additions & 2 deletions src/project_layout.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: {:?}",
Expand Down Expand Up @@ -248,7 +248,7 @@ 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,
));
Expand Down
32 changes: 21 additions & 11 deletions src/source_distribution.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 _;
Expand Down Expand Up @@ -242,14 +242,23 @@ fn find_path_deps(cargo_metadata: &Metadata) -> Result<HashMap<String, PathDepen
package
.readme
.as_ref()
.map(|readme| (package.name.clone(), readme.clone().into_std_path_buf()))
.map(|readme| (package.id.clone(), readme.clone().into_std_path_buf()))
})
.collect::<HashMap<String, PathBuf>>();
.collect::<HashMap<PackageId, PathBuf>>();
// 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) {
Expand All @@ -269,9 +278,10 @@ fn find_path_deps(cargo_metadata: &Metadata) -> Result<HashMap<String, PathDepen
.with_context(|| {
format!(
"Failed to resolve workspace root for {} at '{}'",
dep_name, dep_manifest_path
dep_id, dep_manifest_path
)
})?;

path_deps.insert(
dep_name.clone(),
PathDependency {
Expand All @@ -280,7 +290,7 @@ fn find_path_deps(cargo_metadata: &Metadata) -> Result<HashMap<String, PathDepen
.workspace_root
.clone()
.into_std_path_buf(),
readme: pkg_readmes.get(dep_name.as_str()).cloned(),
readme: pkg_readmes.get(dep_id).cloned(),
},
);
if let Some(dep_package) = cargo_metadata
Expand Down Expand Up @@ -398,7 +408,7 @@ fn add_cargo_package_files_to_sdist(
let abs_readme = path_dep_manifest_dir
.join(readme)
.normalize()
.with_context(|| format!("failed to normalize path `{}`", readme.display()))?
.with_context(|| format!("failed to normalize readme path `{}`", readme.display()))?
.into_path_buf();
let relative_readme = abs_readme.strip_prefix(&sdist_root).unwrap();
writer.add_file(root_dir.join(relative_readme), &abs_readme)?;
Expand All @@ -419,7 +429,7 @@ fn add_cargo_package_files_to_sdist(
// Add the main crate
let abs_manifest_path = manifest_path
.normalize()
.with_context(|| format!("failed to normalize path `{}`", manifest_path.display()))?
.with_context(|| format!("failed to normalize manifest path `{}`", manifest_path.display()))?
.into_path_buf();
let abs_manifest_dir = abs_manifest_path.parent().unwrap();
let main_crate = build_context.cargo_metadata.root_package().unwrap();
Expand All @@ -441,7 +451,7 @@ fn add_cargo_package_files_to_sdist(
let abs_readme = abs_manifest_dir
.join(readme)
.normalize()
.with_context(|| format!("failed to normalize path `{}`", readme))?
.with_context(|| format!("failed to normalize readme path `{}`", readme))?
.into_path_buf();
let relative_readme = abs_readme.strip_prefix(&sdist_root).unwrap();
writer.add_file(root_dir.join(relative_readme), &abs_readme)?;
Expand Down Expand Up @@ -575,7 +585,7 @@ pub fn source_distribution(
.normalize()
.with_context(|| {
format!(
"failed to normalize path `{}`",
"failed to normalize pyproject.toml path `{}`",
build_context.pyproject_toml_path.display()
)
})?
Expand Down Expand Up @@ -605,7 +615,7 @@ pub fn source_distribution(
.normalize()
.with_context(|| {
format!(
"failed to normalize path `{}`",
"failed to normalize pyproject.toml path `{}`",
build_context.pyproject_toml_path.display()
)
})?
Expand Down

0 comments on commit cd2ad95

Please sign in to comment.