From e779f77264b017cd17f2a54587fb5812084ece30 Mon Sep 17 00:00:00 2001 From: Chris Barnes Date: Wed, 7 Oct 2020 16:47:30 +0100 Subject: [PATCH 1/5] Configurable description_content_type No validation is performed. The defaults are unchanged (and inconsistent with setuptools/PyPI). Basic test for checking metadata gets through. --- src/cargo_toml.rs | 1 + src/metadata.rs | 133 +++++++++++++++++++++++++++++++++++----------- 2 files changed, 102 insertions(+), 32 deletions(-) diff --git a/src/cargo_toml.rs b/src/cargo_toml.rs index 61333373f..540815d7e 100644 --- a/src/cargo_toml.rs +++ b/src/cargo_toml.rs @@ -119,6 +119,7 @@ pub struct RemainingCoreMetadata { pub requires_external: Option>, pub project_url: Option>, pub provides_extra: Option>, + pub description_content_type: Option, } #[cfg(test)] diff --git a/src/metadata.rs b/src/metadata.rs index f5d10fcc2..7ab41ce53 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -76,23 +76,26 @@ impl Metadata21 { None }; - let description_content_type = if description.is_some() { - // I'm not hundred percent sure if that's the best preset - Some("text/markdown; charset=UTF-8; variant=GFM".to_owned()) - } else { - None - }; - let classifier = cargo_toml.classifier(); - let extra_metadata = cargo_toml.remaining_core_metadata(); - let author_email = if authors.contains('@') { Some(authors.clone()) } else { None }; + let extra_metadata = cargo_toml.remaining_core_metadata(); + + // See https://packaging.python.org/specifications/core-metadata/#description-content-type + let description_content_type = extra_metadata.description_content_type.or_else(|| { + if description.is_some() { + // I'm not hundred percent sure if that's the best preset + Some("text/markdown; charset=UTF-8; variant=GFM".to_owned()) + } else { + None + } + }); + Ok(Metadata21 { metadata_version: "2.1".to_owned(), @@ -243,16 +246,7 @@ mod test { use indoc::indoc; use std::io::Write; - #[test] - fn test_metadata_from_cargo_toml() { - let readme = indoc!( - r#" - # Some test package - - This is the readme for a test package - "# - ); - + fn assert_metadata_from_cargo_toml(readme: &str, cargo_toml: &str, expected: &str) { let mut readme_md = tempfile::NamedTempFile::new().unwrap(); let readme_path = if cfg!(windows) { @@ -263,6 +257,39 @@ mod test { readme_md.write_all(readme.as_bytes()).unwrap(); + let cargo_toml = cargo_toml.replace("readme.md", &readme_path); + + let cargo_toml: CargoToml = toml::from_str(&cargo_toml).unwrap(); + + let metadata = + Metadata21::from_cargo_toml(&cargo_toml, &readme_md.path().parent().unwrap()).unwrap(); + + let actual = metadata.to_file_contents(); + + if actual.trim() != expected.trim() { + panic!( + "Actual metadata differed from expected\nEXPECTED:\n{}\n\nGOT:\n{}", + expected, actual + ); + } + + assert_eq!(actual.trim(), expected.trim()); + + if metadata.get_dist_info_dir() != PathBuf::from("info_project-0.1.0.dist-info") { + panic!("Dist info dir differed from expected"); + } + } + + #[test] + fn test_metadata_from_cargo_toml() { + let readme = indoc!( + r#" + # Some test package + + This is the readme for a test package + "# + ); + let cargo_toml = indoc!( r#" [package] @@ -285,13 +312,7 @@ mod test { classifier = ["Programming Language :: Python"] requires-dist = ["flask~=1.1.0", "toml==0.10.0"] "# - ) - .replace("readme.md", &readme_path); - - let cargo_toml: CargoToml = toml::from_str(&cargo_toml).unwrap(); - - let metadata = - Metadata21::from_cargo_toml(&cargo_toml, &readme_md.path().parent().unwrap()).unwrap(); + ); let expected = indoc!( r#" @@ -314,13 +335,61 @@ mod test { "# ); - let actual = metadata.to_file_contents(); + assert_metadata_from_cargo_toml(readme, cargo_toml, expected); + } - assert_eq!(actual.trim(), expected.trim()); + #[test] + fn test_metadata_from_cargo_toml_rst() { + let readme = indoc!( + r#" + # Some test package + "# + ); + + let cargo_toml = indoc!( + r#" + [package] + authors = ["konstin "] + name = "info-project" + version = "0.1.0" + description = "A test project" + homepage = "https://example.org" + readme = "readme.md" + keywords = ["ffi", "test"] + + [lib] + crate-type = ["cdylib"] + name = "pyo3_pure" + + [package.metadata.maturin.scripts] + ph = "maturin:print_hello" + + [package.metadata.maturin] + classifier = ["Programming Language :: Python"] + requires-dist = ["flask~=1.1.0", "toml==0.10.0"] + description-content-type = "text/x-rst" + "# + ); + + let expected = indoc!( + r#" + Metadata-Version: 2.1 + Name: info-project + Version: 0.1.0 + Classifier: Programming Language :: Python + Requires-Dist: flask~=1.1.0 + Requires-Dist: toml==0.10.0 + Summary: A test project + Keywords: ffi test + Home-Page: https://example.org + Author: konstin + Author-Email: konstin + Description-Content-Type: text/x-rst + + # Some test package + "# + ); - assert_eq!( - metadata.get_dist_info_dir(), - PathBuf::from("info_project-0.1.0.dist-info") - ) + assert_metadata_from_cargo_toml(readme, cargo_toml, expected); } } From e1b32c1433d5b074b6559cc6699575a91bb1dbfc Mon Sep 17 00:00:00 2001 From: Chris Barnes Date: Thu, 8 Oct 2020 13:46:16 +0100 Subject: [PATCH 2/5] use assert_eq string formatting --- src/metadata.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/metadata.rs b/src/metadata.rs index 7ab41ce53..5b6d4351b 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -269,9 +269,15 @@ mod test { if actual.trim() != expected.trim() { panic!( "Actual metadata differed from expected\nEXPECTED:\n{}\n\nGOT:\n{}", - expected, actual - ); - } + expected, + actual + ); + assert_eq!( + metadata.get_dist_info_dir(), + PathBuf::from("info_project-0.1.0.dist-info"), + "Dist info dir differed from expected" + ); + } assert_eq!(actual.trim(), expected.trim()); From caa5f8ddda11bdf41adb32913a2d68ca389ac803 Mon Sep 17 00:00:00 2001 From: Chris Barnes Date: Thu, 8 Oct 2020 13:46:29 +0100 Subject: [PATCH 3/5] make rst test more rst-y --- src/metadata.rs | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/metadata.rs b/src/metadata.rs index 5b6d4351b..046be3a37 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -266,9 +266,10 @@ mod test { let actual = metadata.to_file_contents(); - if actual.trim() != expected.trim() { - panic!( - "Actual metadata differed from expected\nEXPECTED:\n{}\n\nGOT:\n{}", + assert_eq!( + actual.trim(), + expected.trim(), + "Actual metadata differed from expected\nEXPECTED:\n{}\n\nGOT:\n{}", expected, actual ); @@ -279,13 +280,6 @@ mod test { ); } - assert_eq!(actual.trim(), expected.trim()); - - if metadata.get_dist_info_dir() != PathBuf::from("info_project-0.1.0.dist-info") { - panic!("Dist info dir differed from expected"); - } - } - #[test] fn test_metadata_from_cargo_toml() { let readme = indoc!( @@ -348,7 +342,8 @@ mod test { fn test_metadata_from_cargo_toml_rst() { let readme = indoc!( r#" - # Some test package + Some test package + ================= "# ); @@ -360,7 +355,7 @@ mod test { version = "0.1.0" description = "A test project" homepage = "https://example.org" - readme = "readme.md" + readme = "readme.rst" keywords = ["ffi", "test"] [lib] @@ -392,7 +387,8 @@ mod test { Author-Email: konstin Description-Content-Type: text/x-rst - # Some test package + Some test package + ================= "# ); From b40da7c7b598f6617d82b1ec7571fe03e45b5f2b Mon Sep 17 00:00:00 2001 From: Chris Barnes Date: Thu, 8 Oct 2020 13:55:06 +0100 Subject: [PATCH 4/5] check hardcoded test values --- src/metadata.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/metadata.rs b/src/metadata.rs index 046be3a37..91955c379 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -257,12 +257,13 @@ mod test { readme_md.write_all(readme.as_bytes()).unwrap(); - let cargo_toml = cargo_toml.replace("readme.md", &readme_path); + let toml_with_path = cargo_toml.replace("README_PATH", &readme_path); - let cargo_toml: CargoToml = toml::from_str(&cargo_toml).unwrap(); + let cargo_toml_struct: CargoToml = toml::from_str(&toml_with_path).unwrap(); let metadata = - Metadata21::from_cargo_toml(&cargo_toml, &readme_md.path().parent().unwrap()).unwrap(); + Metadata21::from_cargo_toml(&cargo_toml_struct, &readme_md.path().parent().unwrap()) + .unwrap(); let actual = metadata.to_file_contents(); @@ -273,6 +274,13 @@ mod test { expected, actual ); + + // get_dist_info_dir test checks against hard-coded values - check that they are as expected in the source first + assert!( + cargo_toml.contains("name = \"info-project\"") + && cargo_toml.contains("version = \"0.1.0\""), + "cargo_toml name and version string do not match hardcoded values, test will fail", + ); assert_eq!( metadata.get_dist_info_dir(), PathBuf::from("info_project-0.1.0.dist-info"), @@ -298,7 +306,7 @@ mod test { version = "0.1.0" description = "A test project" homepage = "https://example.org" - readme = "readme.md" + readme = "README_PATH" keywords = ["ffi", "test"] [lib] @@ -355,7 +363,7 @@ mod test { version = "0.1.0" description = "A test project" homepage = "https://example.org" - readme = "readme.rst" + readme = "README_PATH" keywords = ["ffi", "test"] [lib] From fef38f82fbc4712b2a7a2b29b3536c69729b61a2 Mon Sep 17 00:00:00 2001 From: Chris Barnes Date: Thu, 8 Oct 2020 15:00:18 +0100 Subject: [PATCH 5/5] Guess description-content-type from filename ... if not explicitly given. BREAKING: when no type is given AND the file does not have a recognised extension, maturin will now default to plaintext. This makes more sense, but is technically a breaking change. --- src/metadata.rs | 89 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 64 insertions(+), 25 deletions(-) diff --git a/src/metadata.rs b/src/metadata.rs index 91955c379..87f1345a0 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -54,6 +54,27 @@ pub struct Metadata21 { pub provides_extra: Vec, } +const PLAINTEXT_CONTENT_TYPE: &str = "text/plain; charset=UTF-8"; +const GFM_CONTENT_TYPE: &str = "text/markdown; charset=UTF-8; variant=GFM"; + +/// Guess a Description-Content-Type based on the file extension, +/// defaulting to plaintext if extension is unknown or empty. +/// +/// See https://packaging.python.org/specifications/core-metadata/#description-content-type +fn path_to_content_type(path: &PathBuf) -> String { + path.extension() + .map_or(String::from(PLAINTEXT_CONTENT_TYPE), |ext| { + let ext = ext.to_string_lossy().to_lowercase(); + let type_str = match ext.as_str() { + "rst" => "text/x-rst; charset=UTF-8", + "md" => GFM_CONTENT_TYPE, + "markdown" => GFM_CONTENT_TYPE, + _ => PLAINTEXT_CONTENT_TYPE, + }; + String::from(type_str) + }) +} + impl Metadata21 { /// Uses a Cargo.toml to create the metadata for python packages /// @@ -64,18 +85,6 @@ impl Metadata21 { ) -> Result { let authors = cargo_toml.package.authors.join(", "); - // See https://packaging.python.org/specifications/core-metadata/#description - let description = if let Some(ref readme) = cargo_toml.package.readme { - Some( - read_to_string(manifest_path.as_ref().join(readme)).context(format!( - "Failed to read readme specified in Cargo.toml, which should be at {}", - manifest_path.as_ref().join(readme).display() - ))?, - ) - } else { - None - }; - let classifier = cargo_toml.classifier(); let author_email = if authors.contains('@') { @@ -86,15 +95,23 @@ impl Metadata21 { let extra_metadata = cargo_toml.remaining_core_metadata(); - // See https://packaging.python.org/specifications/core-metadata/#description-content-type - let description_content_type = extra_metadata.description_content_type.or_else(|| { - if description.is_some() { - // I'm not hundred percent sure if that's the best preset - Some("text/markdown; charset=UTF-8; variant=GFM".to_owned()) - } else { - None - } - }); + let description: Option; + let description_content_type: Option; + // See https://packaging.python.org/specifications/core-metadata/#description + if let Some(ref readme) = cargo_toml.package.readme { + let readme_path = manifest_path.as_ref().join(readme); + description = Some(read_to_string(&readme_path).context(format!( + "Failed to read readme specified in Cargo.toml, which should be at {}", + readme_path.display() + ))?); + + description_content_type = extra_metadata + .description_content_type + .or_else(|| Some(path_to_content_type(&readme_path))); + } else { + description = None; + description_content_type = None; + }; Ok(Metadata21 { metadata_version: "2.1".to_owned(), @@ -257,7 +274,7 @@ mod test { readme_md.write_all(readme.as_bytes()).unwrap(); - let toml_with_path = cargo_toml.replace("README_PATH", &readme_path); + let toml_with_path = cargo_toml.replace("REPLACE_README_PATH", &readme_path); let cargo_toml_struct: CargoToml = toml::from_str(&toml_with_path).unwrap(); @@ -306,7 +323,7 @@ mod test { version = "0.1.0" description = "A test project" homepage = "https://example.org" - readme = "README_PATH" + readme = "REPLACE_README_PATH" keywords = ["ffi", "test"] [lib] @@ -335,7 +352,7 @@ mod test { Home-Page: https://example.org Author: konstin Author-Email: konstin - Description-Content-Type: text/markdown; charset=UTF-8; variant=GFM + Description-Content-Type: text/plain; charset=UTF-8 # Some test package @@ -363,7 +380,7 @@ mod test { version = "0.1.0" description = "A test project" homepage = "https://example.org" - readme = "README_PATH" + readme = "REPLACE_README_PATH" keywords = ["ffi", "test"] [lib] @@ -402,4 +419,26 @@ mod test { assert_metadata_from_cargo_toml(readme, cargo_toml, expected); } + + #[test] + fn test_path_to_content_type() { + for (filename, expected) in vec![ + ("r.md", GFM_CONTENT_TYPE), + ("r.markdown", GFM_CONTENT_TYPE), + ("r.mArKdOwN", GFM_CONTENT_TYPE), + ("r.rst", "text/x-rst; charset=UTF-8"), + ("r.somethingelse", PLAINTEXT_CONTENT_TYPE), + ("r", PLAINTEXT_CONTENT_TYPE), + ] { + let result = path_to_content_type(&PathBuf::from(filename)); + assert_eq!( + result.as_str(), + expected, + "Wrong content type for file '{}'. Expected '{}', got '{}'", + filename, + expected, + result + ); + } + } }