From 102787727e9b9129c6e49985669ec349c5d1c4ea Mon Sep 17 00:00:00 2001 From: konstin Date: Tue, 20 Aug 2024 01:51:33 +0200 Subject: [PATCH] Place source dist readmes next to Cargo.toml To fix #2154. --- src/source_distribution.rs | 112 ++++++++++++++++++++++++++++--------- 1 file changed, 86 insertions(+), 26 deletions(-) diff --git a/src/source_distribution.rs b/src/source_distribution.rs index 6f8e8ded..d2edb41f 100644 --- a/src/source_distribution.rs +++ b/src/source_distribution.rs @@ -8,9 +8,11 @@ use ignore::overrides::Override; use normpath::PathExt as _; use path_slash::PathExt as _; use std::collections::HashMap; +use std::ffi::OsStr; use std::path::{Path, PathBuf}; use std::process::Command; use std::str; +use toml_edit::DocumentMut; use tracing::debug; /// Path dependency information. @@ -45,13 +47,14 @@ fn parse_toml_file(path: &Path, kind: &str) -> Result { /// We only want to add path dependencies that are actually used /// to reduce the size of the source distribution. fn rewrite_cargo_toml( - manifest_path: impl AsRef, + document: &mut DocumentMut, + manifest_path: &Path, known_path_deps: &HashMap, -) -> Result { - let manifest_path = manifest_path.as_ref(); - debug!("Rewriting Cargo.toml at {}", manifest_path.display()); - let mut document = parse_toml_file(manifest_path, "Cargo.toml")?; - +) -> Result<()> { + debug!( + "Rewriting Cargo.toml `workspace.members` at {}", + manifest_path.display() + ); // Update workspace members if let Some(workspace) = document.get_mut("workspace").and_then(|x| x.as_table_mut()) { if let Some(members) = workspace.get_mut("members").and_then(|x| x.as_array()) { @@ -100,7 +103,29 @@ fn rewrite_cargo_toml( } } } - Ok(document.to_string()) + Ok(()) +} + +fn rewrite_cargo_toml_readme( + document: &mut DocumentMut, + manifest_path: &Path, + readme_name: Option<&str>, +) -> Result<()> { + debug!( + "Rewriting Cargo.toml `package.readme` at {}", + manifest_path.display() + ); + + if let Some(readme_name) = readme_name { + let project = document.get_mut("package").with_context(|| { + format!( + "Missing `[package]` table in Cargo.toml with readme at {}", + manifest_path.display() + ) + })?; + project["readme"] = toml_edit::value(readme_name); + } + Ok(()) } /// When `pyproject.toml` is inside the Cargo workspace root, @@ -148,6 +173,7 @@ fn add_crate_to_source_distribution( writer: &mut SDistWriter, manifest_path: impl AsRef, prefix: impl AsRef, + readme: Option<&Path>, known_path_deps: &HashMap, root_crate: bool, skip_cargo_toml: bool, @@ -216,15 +242,33 @@ fn add_crate_to_source_distribution( let cargo_toml_path = prefix.join(manifest_path.file_name().unwrap()); + let readme_name = readme + .as_ref() + .map(|readme| { + readme + .file_name() + .and_then(OsStr::to_str) + .with_context(|| format!("Missing readme filename for {}", manifest_path.display())) + }) + .transpose()?; + if root_crate { - let rewritten_cargo_toml = rewrite_cargo_toml(manifest_path, known_path_deps)?; + let mut document = parse_toml_file(manifest_path, "Cargo.toml")?; + rewrite_cargo_toml_readme(&mut document, manifest_path, readme_name)?; + rewrite_cargo_toml(&mut document, manifest_path, known_path_deps)?; writer.add_bytes( cargo_toml_path, Some(manifest_path), - rewritten_cargo_toml.as_bytes(), + document.to_string().as_bytes(), )?; } else if !skip_cargo_toml { - writer.add_file(cargo_toml_path, manifest_path)?; + let mut document = parse_toml_file(manifest_path, "Cargo.toml")?; + rewrite_cargo_toml_readme(&mut document, manifest_path, readme_name)?; + writer.add_bytes( + cargo_toml_path, + Some(manifest_path), + document.to_string().as_bytes(), + )?; } for (target, source) in target_source { @@ -409,21 +453,23 @@ fn add_cargo_package_files_to_sdist( path_dep_manifest_dir.strip_prefix(&sdist_root).unwrap(); // we may need to rewrite workspace Cargo.toml later so don't add it to sdist yet let skip_cargo_toml = workspace_manifest_path == path_dep.manifest_path; + add_crate_to_source_distribution( writer, &path_dep.manifest_path, root_dir.join(relative_path_dep_manifest_dir), + path_dep.readme.as_deref(), &known_path_deps, false, skip_cargo_toml, ) - .with_context(|| { - format!( - "Failed to add local dependency {} at {} to the source distribution", - name, - path_dep.manifest_path.display() - ) - })?; + .with_context(|| { + format!( + "Failed to add local dependency {} at {} to the source distribution", + name, + path_dep.manifest_path.display() + ) + })?; // Handle possible relative readme field in Cargo.toml if let Some(readme) = path_dep.readme.as_ref() { let readme = path_dep_manifest_dir.join(readme); @@ -431,8 +477,14 @@ fn add_cargo_package_files_to_sdist( .normalize() .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)?; + // Add readme next to Cargo.toml so we don't get collisions between crates using readmes + // higher up the file tree. + writer.add_file( + root_dir + .join(relative_path_dep_manifest_dir) + .join(readme.file_name().unwrap()), + &abs_readme, + )?; } // Handle different workspace manifest if &path_dep.workspace_root != workspace_root { @@ -468,6 +520,7 @@ fn add_cargo_package_files_to_sdist( writer, manifest_path, root_dir.join(relative_main_crate_manifest_dir), + None, &known_path_deps, true, false, @@ -479,8 +532,9 @@ fn add_cargo_package_files_to_sdist( .normalize() .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)?; + // Add readme next to Cargo.toml so we don't get collisions between crates using readmes + // higher up the file tree. + writer.add_file(root_dir.join(readme.file_name().unwrap()), &abs_readme)?; } // Add Cargo.lock file and workspace Cargo.toml @@ -524,11 +578,17 @@ fn add_cargo_package_files_to_sdist( readme: None, }, ); - let workspace_cargo_toml = rewrite_cargo_toml(&workspace_manifest_path, &deps_to_keep)?; + let mut document = + parse_toml_file(workspace_manifest_path.as_std_path(), "Cargo.toml")?; + rewrite_cargo_toml( + &mut document, + workspace_manifest_path.as_std_path(), + &deps_to_keep, + )?; writer.add_bytes( root_dir.join(relative_workspace_cargo_toml), Some(workspace_manifest_path.as_std_path()), - workspace_cargo_toml.as_bytes(), + document.to_string().as_bytes(), )?; } } else if cargo_lock_required { @@ -655,9 +715,9 @@ pub fn source_distribution( writer.add_file(root_dir.join(readme), pyproject_dir.join(readme))?; } if let Some(pyproject_toml::License::Table { - file: Some(license), - text: None, - }) = project.license.as_ref() + file: Some(license), + text: None, + }) = project.license.as_ref() { writer.add_file(root_dir.join(license), pyproject_dir.join(license))?; }