Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement normalization of manifests for publishing #710

Merged
merged 1 commit into from
Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions scarb/src/core/manifest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use toml::Value;

pub use compiler_config::*;
pub use dependency::*;
pub use maybe_workspace::*;
pub use scripts::*;
pub use summary::*;
pub use target::*;
Expand Down
12 changes: 12 additions & 0 deletions scarb/src/core/manifest/toml_manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,18 @@ impl<'de> Deserialize<'de> for PathOrBool {
}
}

impl From<Utf8PathBuf> for PathOrBool {
fn from(p: Utf8PathBuf) -> Self {
Self::Path(p)
}
}

impl From<bool> for PathOrBool {
fn from(b: bool) -> Self {
Self::Bool(b)
}
}

#[derive(Debug, Default, Clone, Deserialize, Serialize)]
pub struct TomlWorkspaceDependency {
pub workspace: bool,
Expand Down
1 change: 1 addition & 0 deletions scarb/src/core/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ mod dirs;
pub mod errors;
pub(crate) mod manifest;
pub(crate) mod package;
pub(crate) mod publishing;
pub(crate) mod registry;
pub(crate) mod resolver;
pub(crate) mod source;
Expand Down
146 changes: 146 additions & 0 deletions scarb/src/core/publishing/manifest_normalization.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
use std::collections::BTreeMap;

use anyhow::{bail, Result};
use camino::{Utf8Path, Utf8PathBuf};
use indoc::formatdoc;

use crate::core::{
DepKind, DependencyVersionReq, DetailedTomlDependency, ManifestDependency, MaybeWorkspace,
Package, PackageName, TomlDependency, TomlManifest, TomlPackage, TomlWorkspaceDependency,
TomlWorkspaceField,
};

pub fn prepare_manifest_for_publish(pkg: &Package) -> Result<TomlManifest> {
let package = Some(generate_package(pkg));

let dependencies = Some(generate_dependencies(
// NOTE: We deliberately do not ask for `full_dependencies` here, because
// we do not want to emit requirements for built-in packages like `core`.
&pkg.manifest.summary.dependencies,
DepKind::Normal,
)?);

let tool = pkg.manifest.metadata.tool_metadata.clone().map(|m| {
m.into_iter()
.map(|(k, v)| (k, MaybeWorkspace::Defined(v)))
.collect()
});

Ok(TomlManifest {
package,
workspace: None,
dependencies,
lib: None,
// TODO(mkaput): Allow packaging Cairo plugins.
cairo_plugin: None,
test: None,
target: None,
mkaput marked this conversation as resolved.
Show resolved Hide resolved
cairo: None,
profile: None,
scripts: None,
tool,
})
}

fn generate_package(pkg: &Package) -> Box<TomlPackage> {
let summary = &pkg.manifest.summary;
let metadata = &pkg.manifest.metadata;
Box::new(TomlPackage {
name: summary.package_id.name.clone(),
version: MaybeWorkspace::Defined(summary.package_id.version.clone()),
authors: metadata.authors.clone().map(MaybeWorkspace::Defined),
urls: metadata.urls.clone(),
description: metadata.description.clone().map(MaybeWorkspace::Defined),
documentation: metadata.documentation.clone().map(MaybeWorkspace::Defined),
homepage: metadata.homepage.clone().map(MaybeWorkspace::Defined),
keywords: metadata.keywords.clone().map(MaybeWorkspace::Defined),
license: metadata.license.clone().map(MaybeWorkspace::Defined),
// TODO(mkaput): Normalize this the same way as readme is.
license_file: metadata.license_file.clone().map(MaybeWorkspace::Defined),
readme: metadata
.readme
.as_ref()
.map(|p| map_metadata_file_path(p, pkg)),
repository: metadata.repository.clone().map(MaybeWorkspace::Defined),
no_core: summary.no_core.then_some(true),
cairo_version: metadata.cairo_version.clone().map(MaybeWorkspace::Defined),
})
}

fn generate_dependencies(
deps: &[ManifestDependency],
kind: DepKind,
) -> Result<BTreeMap<PackageName, MaybeWorkspace<TomlDependency, TomlWorkspaceDependency>>> {
deps.iter()
.filter(|dep| dep.kind == kind)
.map(|dep| {
let name = dep.name.clone();
let toml_dep = generate_dependency(dep)?;
Ok((name, MaybeWorkspace::Defined(toml_dep)))
})
.collect()
}

fn generate_dependency(dep: &ManifestDependency) -> Result<TomlDependency> {
assert!(
!dep.source_id.is_std(),
"Built-in dependencies should not be included in published manifests."
maciektr marked this conversation as resolved.
Show resolved Hide resolved
);

let version = Some(match &dep.version_req {
DependencyVersionReq::Req(req) => req.clone(),

// Ignore what is in the lock file.
DependencyVersionReq::Locked { req, .. } => req.clone(),

// This case is triggered by dependencies like this:
//
// [dependencies]
// foo = { path = "../foo" }
DependencyVersionReq::Any => {
bail!(formatdoc! {
r#"
dependency `{name}` does not specify a version requirement
note: all dependencies must have a version specified when packaging
note: the `{kind}` specification will be removed from dependency declaration
"#,
name = dep.name,
kind = dep.source_id.kind.primary_field(),
})
}
});

Ok(TomlDependency::Detailed(DetailedTomlDependency {
version,
path: None,
git: None,
branch: None,
tag: None,
rev: None,
}))
}

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.
maciektr marked this conversation as resolved.
Show resolved Hide resolved
path.file_name()
.expect("README/LICENSE path must have a file name.")
.into()
};

MaybeWorkspace::Defined(T::from(path))
}
1 change: 1 addition & 0 deletions scarb/src/core/publishing/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pub mod manifest_normalization;
15 changes: 15 additions & 0 deletions scarb/src/core/source/id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,10 @@ impl SourceId {
}
}

pub fn is_std(self) -> bool {
self.kind == SourceKind::Std
}

pub fn to_pretty_url(self) -> String {
match &self.kind {
SourceKind::Path => format!("{PATH_SOURCE_PROTOCOL}+{}", self.url),
Expand Down Expand Up @@ -358,6 +362,17 @@ impl<'de> Deserialize<'de> for SourceId {
}
}

impl SourceKind {
pub fn primary_field(&self) -> &str {
match self {
SourceKind::Path => "path",
SourceKind::Git(_) => "git",
SourceKind::Registry => "registry",
SourceKind::Std => "std",
}
}
}

#[cfg(test)]
mod tests {
use test_case::test_case;
Expand Down
36 changes: 31 additions & 5 deletions scarb/src/ops/package.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
use std::collections::BTreeMap;
use std::fs::File;
use std::io::{Seek, SeekFrom};
use std::io::{Seek, SeekFrom, Write};

use anyhow::{ensure, Context, Result};
use camino::Utf8PathBuf;
use indoc::writedoc;

use scarb_ui::components::Status;
use scarb_ui::{HumanBytes, HumanCount};

use crate::core::publishing::manifest_normalization::prepare_manifest_for_publish;
use crate::core::{Package, PackageId, PackageName, Workspace};
use crate::flock::FileLockGuard;
use crate::internal::fsx;
Expand All @@ -30,7 +32,6 @@ type ArchiveRecipe = Vec<ArchiveFile>;
struct ArchiveFile {
/// The relative path in the archive (not including top-level package name directory).
path: Utf8PathBuf,
#[allow(dead_code)]
/// The contents of the file.
contents: ArchiveFileContents,
}
Expand Down Expand Up @@ -213,9 +214,34 @@ fn check_no_reserved_files(recipe: &ArchiveRecipe) -> Result<()> {
Ok(())
}

fn normalize_manifest(_pkg: Package) -> Result<Vec<u8>> {
// TODO(mkaput): Implement this properly.
Ok("[package]".to_string().into_bytes())
fn normalize_manifest(pkg: Package) -> Result<Vec<u8>> {
let mut buf = Vec::new();

writedoc!(
&mut buf,
r##"
# Code generated by scarb package -p {package_name}; DO NOT EDIT.
#
# When uploading packages to the registry Scarb will automatically
# "normalize" {toml} files for maximal compatibility
# with all versions of Scarb and also rewrite `path` dependencies
# to registry dependencies.
#
# If you are reading this file be aware that the original {toml}
# will likely look very different (and much more reasonable).
# See {orig} for the original contents.
"##,
package_name = pkg.id.name,
toml = MANIFEST_FILE_NAME,
orig = ORIGINAL_MANIFEST_FILE_NAME,
)?;
writeln!(&mut buf)?;

let manifest = prepare_manifest_for_publish(&pkg)?;
let toml = toml::to_string_pretty(&manifest)?;
writeln!(&mut buf, "{toml}")?;

Ok(buf)
}

/// Compress and package the recipe, and write it into the given file.
Expand Down
6 changes: 3 additions & 3 deletions scarb/tests/git_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,10 +190,10 @@ fn fetch_with_short_ssh_git() {
error: failed to parse manifest at: [..]

Caused by:
TOML parse error at line 2, column 24
TOML parse error at line 6, column 7
|
2 | dependencies = { dep = { git = "[email protected]:a/dep" } }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6 | dep = { git = "[email protected]:a/dep" }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
data did not match any variant of untagged enum TomlDependency
"#});
}
Expand Down
Loading