Handle readme and license files in scarb package (#897)

Resolves #737

scarb/src/core/manifest/mod.rs

@@ -61,7 +61,7 @@ pub struct ManifestMetadata {
     pub homepage: Option<String>,
     pub keywords: Option<Vec<String>>,
     pub license: Option<String>,
-    pub license_file: Option<String>,
+    pub license_file: Option<Utf8PathBuf>,
     pub readme: Option<Utf8PathBuf>,
     pub repository: Option<String>,
     #[serde(rename = "tool")]

scarb/src/core/manifest/toml_manifest.rs

@@ -111,7 +111,7 @@ pub struct PackageInheritableFields {
     pub homepage: Option<String>,
     pub keywords: Option<Vec<String>>,
     pub license: Option<String>,
-    pub license_file: Option<String>,
+    pub license_file: Option<Utf8PathBuf>,
     pub readme: Option<PathOrBool>,
     pub repository: Option<String>,
     pub cairo_version: Option<VersionReq>,
@@ -140,7 +140,7 @@ impl PackageInheritableFields {
     get_field!(documentation, String);
     get_field!(homepage, String);
     get_field!(license, String);
-    get_field!(license_file, String);
+    get_field!(license_file, Utf8PathBuf);
     get_field!(repository, String);
     get_field!(edition, Edition);
 
@@ -188,7 +188,7 @@ pub struct TomlPackage {
     pub homepage: Option<MaybeWorkspaceField<String>>,
     pub keywords: Option<MaybeWorkspaceField<Vec<String>>>,
     pub license: Option<MaybeWorkspaceField<String>>,
-    pub license_file: Option<MaybeWorkspaceField<String>>,
+    pub license_file: Option<MaybeWorkspaceField<Utf8PathBuf>>,
     pub readme: Option<MaybeWorkspaceField<PathOrBool>>,
     pub repository: Option<MaybeWorkspaceField<String>>,
     /// **UNSTABLE** This package does not depend on Cairo's `core`.
@@ -498,7 +498,18 @@ impl TomlManifest {
             license_file: package
                 .license_file
                 .clone()
-                .map(|mw| mw.resolve("license_file", || inheritable_package.license_file()))
+                .map(|mw| match mw {
+                    MaybeWorkspace::Defined(license_rel_path) => {
+                        abs_canonical_path("license", manifest_path, &license_rel_path)
+                    }
+                    MaybeWorkspace::Workspace(_) => mw.resolve("license_file", || {
+                        abs_canonical_path(
+                            "license",
+                            workspace_manifest_path,
+                            &inheritable_package.license_file()?,
+                        )
+                    }),
+                })
                 .transpose()?,
             readme: readme_for_package(
                 manifest_path,
@@ -829,20 +840,17 @@ pub fn readme_for_package(
         Some(PathOrBool::Bool(false)) => None,
     };
 
-    abs_canonical_path(package_root, file_name)
+    file_name
+        .map(|file_name| abs_canonical_path("readme", package_root, file_name))
+        .transpose()
 }
 
-/// Creates the absolute canonical path of the README file and checks if it exists
-fn abs_canonical_path(prefix: &Utf8Path, readme: Option<&Utf8Path>) -> Result<Option<Utf8PathBuf>> {
-    match readme {
-        None => Ok(None),
-        Some(readme) => {
-            let path = prefix.parent().unwrap().join(readme);
-            let path = fsx::canonicalize_utf8(&path)
-                .with_context(|| format!("failed to find the readme at {path}"))?;
-            Ok(Some(path))
-        }
-    }
+/// Creates the absolute canonical path of the file and checks if it exists
+fn abs_canonical_path(file_label: &str, prefix: &Utf8Path, path: &Utf8Path) -> Result<Utf8PathBuf> {
+    let path = prefix.parent().unwrap().join(path);
+    let path = fsx::canonicalize_utf8(&path)
+        .with_context(|| format!("failed to find {file_label} at {path}"))?;
+    Ok(path)
 }
 
 const DEFAULT_README_FILES: &[&str] = &["README.md", "README.txt", "README"];

scarb/src/core/publishing/manifest_normalization.rs

@@ -1,13 +1,16 @@
 use std::collections::BTreeMap;
 
 use anyhow::{bail, Result};
-use camino::{Utf8Path, Utf8PathBuf};
+use camino::Utf8PathBuf;
 use indoc::formatdoc;
 
-use crate::core::{
-    DepKind, DependencyVersionReq, DetailedTomlDependency, ManifestDependency, MaybeWorkspace,
-    Package, PackageName, TargetKind, TomlDependency, TomlManifest, TomlPackage,
-    TomlWorkspaceDependency, TomlWorkspaceField,
+use crate::{
+    core::{
+        DepKind, DependencyVersionReq, DetailedTomlDependency, ManifestDependency, MaybeWorkspace,
+        Package, PackageName, TargetKind, TomlDependency, TomlManifest, TomlPackage,
+        TomlWorkspaceDependency,
+    },
+    DEFAULT_LICENSE_FILE_NAME, DEFAULT_README_FILE_NAME,
 };
 
 pub fn prepare_manifest_for_publish(pkg: &Package) -> Result<TomlManifest> {
@@ -62,12 +65,12 @@ fn generate_package(pkg: &Package) -> Box<TomlPackage> {
         license: metadata.license.clone().map(MaybeWorkspace::Defined),
         license_file: metadata
             .license_file
-            .as_ref()
-            .map(|_| todo!("Packaging packages with a license file is not implemented yet.")),
+            .clone()
+            .map(|_| MaybeWorkspace::Defined(Utf8PathBuf::from(DEFAULT_LICENSE_FILE_NAME))),
         readme: metadata
             .readme
-            .as_ref()
-            .map(|p| map_metadata_file_path(p, pkg)),
+            .clone()
+            .map(|_| MaybeWorkspace::Defined((Utf8PathBuf::from(DEFAULT_README_FILE_NAME)).into())),
         repository: metadata.repository.clone().map(MaybeWorkspace::Defined),
         no_core: summary.no_core.then_some(true),
         cairo_version: metadata.cairo_version.clone().map(MaybeWorkspace::Defined),
@@ -139,28 +142,3 @@ fn generate_dependency(dep: &ManifestDependency) -> Result<TomlDependency> {
         },
     })))
 }
-
-fn map_metadata_file_path<T>(
-    path: &Utf8Path,
-    pkg: &Package,
-) -> MaybeWorkspace<T, TomlWorkspaceField>
-where
-    T: From<Utf8PathBuf>,
-{
-    assert!(
-        path.is_absolute(),
-        "Manifest parser is expected to canonicalize paths for README/LICENSE files."
-    );
-
-    let path = if let Ok(relative_path) = path.strip_prefix(pkg.root()) {
-        relative_path.to_owned()
-    } else {
-        // This path points outside the package root. `scarb package` will copy it
-        // into the root, so we have to adjust the path to this location.
-        path.file_name()
-            .expect("README/LICENSE path must have a file name.")
-            .into()
-    };
-
-    MaybeWorkspace::Defined(T::from(path))
-}

scarb/src/core/publishing/source.rs

@@ -16,6 +16,7 @@ use crate::{DEFAULT_TARGET_DIR_NAME, LOCK_FILE_NAME, MANIFEST_FILE_NAME, SCARB_I
 /// * Skip any subdirectories containing `Scarb.toml`.
 /// * Skip `<root>/target` directory.
 /// * Skip `Scarb.lock` file.
+/// * Skip README and LICENSE files.
 /// * **Skip `Scarb.toml` file, as users of this function may want to generate it themselves.**
 /// * Symlinks within the package directory are followed, while symlinks outside are just skipped.
 /// * Avoid crossing file system boundaries, because it can complicate our lives.
@@ -29,6 +30,14 @@ pub fn list_source_files(pkg: &Package) -> Result<Vec<Utf8PathBuf>> {
 fn push_worktree_files(pkg: &Package, ret: &mut Vec<Utf8PathBuf>) -> Result<()> {
     let filter = {
         let pkg = pkg.clone();
+        let readme = pkg.manifest.metadata.readme.clone().unwrap_or_default();
+        let license_file = pkg
+            .manifest
+            .metadata
+            .license_file
+            .clone()
+            .unwrap_or_default();
+
         move |entry: &DirEntry| -> bool {
             let path = entry.path();
             let is_root = entry.depth() == 0;
@@ -53,6 +62,11 @@ fn push_worktree_files(pkg: &Package, ret: &mut Vec<Utf8PathBuf>) -> Result<()>
                 return false;
             }
 
+            // Skip README and LICENSE files
+            if path == readme || path == license_file {
+                return false;
+            }
+
             true
         }
     };

scarb/src/lib.rs

@@ -32,6 +32,7 @@ pub const DEFAULT_MODULE_MAIN_FILE: &str = "lib.cairo";
 pub const DEFAULT_TESTS_PATH: &str = "tests";
 pub const DEFAULT_TARGET_DIR_NAME: &str = "target";
 pub const SCARB_IGNORE_FILE_NAME: &str = ".scarbignore";
-
 pub static DEFAULT_SOURCE_PATH: Lazy<Utf8PathBuf> =
     Lazy::new(|| ["src", "lib.cairo"].iter().collect());
+pub const DEFAULT_README_FILE_NAME: &str = "README.md";
+pub const DEFAULT_LICENSE_FILE_NAME: &str = "LICENSE";

scarb/src/ops/metadata.rs

@@ -104,7 +104,14 @@ fn collect_package_metadata(package: &Package) -> m::PackageMetadata {
         .homepage(package.manifest.metadata.homepage.clone())
         .keywords(package.manifest.metadata.keywords.clone())
         .license(package.manifest.metadata.license.clone())
-        .license_file(package.manifest.metadata.license_file.clone())
+        .license_file(
+            package
+                .manifest
+                .metadata
+                .license_file
+                .as_ref()
+                .map(ToString::to_string),
+        )
         .readme(
             package
                 .manifest

scarb/src/ops/package.rs

@@ -17,7 +17,10 @@ use crate::core::publishing::source::list_source_files;
 use crate::core::{Package, PackageId, PackageName, Workspace};
 use crate::flock::FileLockGuard;
 use crate::internal::restricted_names;
-use crate::{ops, MANIFEST_FILE_NAME, VCS_INFO_FILE_NAME};
+use crate::{
+    ops, DEFAULT_LICENSE_FILE_NAME, DEFAULT_README_FILE_NAME, MANIFEST_FILE_NAME,
+    VCS_INFO_FILE_NAME,
+};
 
 const VERSION: u8 = 1;
 const VERSION_FILE_NAME: &str = "VERSION";
@@ -228,6 +231,22 @@ fn prepare_archive_recipe(pkg: &Package, opts: &PackageOpts) -> Result<ArchiveRe
         contents: ArchiveFileContents::OnDisk(pkg.manifest_path().to_owned()),
     });
 
+    // Add README file
+    if let Some(readme) = &pkg.manifest.metadata.readme {
+        recipe.push(ArchiveFile {
+            path: DEFAULT_README_FILE_NAME.into(),
+            contents: ArchiveFileContents::OnDisk(readme.clone()),
+        })
+    }
+
+    // Add LICENSE file
+    if let Some(license) = &pkg.manifest.metadata.license_file {
+        recipe.push(ArchiveFile {
+            path: DEFAULT_LICENSE_FILE_NAME.into(),
+            contents: ArchiveFileContents::OnDisk(license.clone()),
+        })
+    }
+
     // Add archive version file.
     recipe.push(ArchiveFile {
         path: VERSION_FILE_NAME.into(),

scarb/tests/metadata.rs

@@ -281,6 +281,7 @@ fn manifest_targets_and_metadata() {
         )
         .unwrap();
     t.child("README.md").touch().unwrap();
+    t.child("license.md").touch().unwrap();
 
     let meta = Scarb::quick_snapbox()
         .arg("--json")
@@ -314,7 +315,11 @@ fn manifest_targets_and_metadata() {
                 "keywords".to_string(),
             ]))
             .license(Some("MIT License".to_string()))
-            .license_file(Some("./license.md".to_string()))
+            .license_file(
+                fsx::canonicalize_utf8(t.join("license.md"))
+                    .unwrap()
+                    .into_string()
+            )
             .readme(
                 fsx::canonicalize_utf8(t.join("README.md"))
                     .unwrap()
@@ -858,7 +863,7 @@ fn infer_readme_simple_bool() {
         .assert()
         .failure()
         .stdout_matches(indoc! {r#"
-            {"type":"error","message":"failed to parse manifest at: [..]/Scarb.toml[..]Caused by:[..]failed to find the readme at [..]/README.md[..]"}
+            {"type":"error","message":"failed to parse manifest at: [..]/Scarb.toml[..]Caused by:[..]failed to find readme at [..]/README.md[..]"}
         "#});
 
     t.child("README.md").touch().unwrap();