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

Add registry metadata to published components. #132

Merged
merged 4 commits into from
Sep 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
18 changes: 2 additions & 16 deletions crates/wit/src/commands/add.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
use std::path::PathBuf;

use crate::{
config::{Config, CONFIG_FILE_NAME},
resolve_dependencies,
};
use crate::config::{Config, CONFIG_FILE_NAME};
use anyhow::{bail, Context, Result};
use cargo_component_core::{
command::CommonOptions,
Expand All @@ -13,6 +8,7 @@ use cargo_component_core::{
};
use clap::Args;
use semver::VersionReq;
use std::path::PathBuf;
use warg_protocol::registry::PackageId;

async fn resolve_version(
Expand Down Expand Up @@ -127,16 +123,6 @@ impl AddCommand {
.dependencies
.insert(id.clone(), Dependency::Package(package));

// Resolve all dependencies to ensure that the lockfile is up to date.
resolve_dependencies(
fibonacci1729 marked this conversation as resolved.
Show resolved Hide resolved
&config,
&config_path,
&warg_config,
&terminal,
!self.dry_run,
)
.await?;

format!(
"dependency `{id}` with version `{version}`{dry_run}",
dry_run = if self.dry_run { " (dry run)" } else { "" }
Expand Down
30 changes: 30 additions & 0 deletions crates/wit/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ impl ConfigBuilder {
version: self.version.unwrap_or_else(|| Version::new(0, 1, 0)),
dependencies: Default::default(),
registries: self.registries,
authors: Default::default(),
categories: Default::default(),
description: None,
license: None,
documentation: None,
homepage: None,
repository: None,
}
}
}
Expand All @@ -72,9 +79,32 @@ pub struct Config {
/// The current package version.
pub version: Version,
/// The package dependencies.
#[serde(default, skip_serializing_if = "HashMap::is_empty")]
pub dependencies: HashMap<PackageId, Dependency>,
/// The registries to use for sourcing packages.
#[serde(default, skip_serializing_if = "HashMap::is_empty")]
pub registries: HashMap<String, Url>,
/// The authors of the package.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub authors: Vec<String>,
/// The categories of the package.
#[serde(default, skip_serializing_if = "Vec::is_empty")]
pub categories: Vec<String>,
/// The package description.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub description: Option<String>,
/// The package license.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub license: Option<String>,
/// The package documentation URL.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub documentation: Option<String>,
/// The package homepage URL.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub homepage: Option<String>,
/// The package repository URL.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub repository: Option<String>,
}

impl Config {
Expand Down
52 changes: 52 additions & 0 deletions crates/wit/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use std::{
use warg_client::storage::{ContentStorage, PublishEntry, PublishInfo};
use warg_crypto::signing::PrivateKey;
use warg_protocol::registry::PackageId;
use wasm_metadata::{Link, LinkType, RegistryMetadata};
use wit_component::DecodedWasm;
use wit_parser::{PackageName, Resolve, UnresolvedPackage};

Expand Down Expand Up @@ -273,6 +274,56 @@ struct PublishOptions<'a> {
dry_run: bool,
}

fn add_registry_metadata(config: &Config, bytes: &[u8]) -> Result<Vec<u8>> {
let mut metadata = RegistryMetadata::default();
if !config.authors.is_empty() {
metadata.set_authors(Some(config.authors.clone()));
}

if !config.categories.is_empty() {
metadata.set_categories(Some(config.categories.clone()));
}

metadata.set_description(config.description.clone());

// TODO: registry metadata should have keywords
// if !package.keywords.is_empty() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are keywords distinct from categories? They seem coupled in the original registry discussion.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

crates.io has them as distinct fields, where keywords is for describing the package and the category with a set of predefined slugs to use.

I wasn't following the discussion around the metadata implementation closely, so it sounds like we should be populating this off of keywords instead of categories or perhaps both?

Copy link
Contributor

@fibonacci1729 fibonacci1729 Sep 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! Given what you linked, i think distinct fields make the most sense (and AFAICT upon closer look seems to be what the registry discussion proposes). cc/ @Kylebrown9 to check my interpretation.

Copy link
Member Author

@peterhuene peterhuene Sep 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to switch to populating the categories field from the cargo keywords as I believe that's more of the intention of this field in the registry metadata; the predefined categories list from cargo doesn't map to what we'd want in a component registry anyway.

I've opened bytecodealliance/registry#197 to track the confusion here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've decided to merge this as-is and wait for the resolution on the linked issue; will update cargo-component if things change.

// metadata.set_keywords(Some(package.keywords.clone()));
// }

metadata.set_license(config.license.clone());

let mut links = Vec::new();
if let Some(docs) = &config.documentation {
links.push(Link {
ty: LinkType::Documentation,
value: docs.clone(),
});
}

if let Some(homepage) = &config.homepage {
links.push(Link {
ty: LinkType::Homepage,
value: homepage.clone(),
});
}

if let Some(repo) = &config.repository {
links.push(Link {
ty: LinkType::Repository,
value: repo.clone(),
});
}

if !links.is_empty() {
metadata.set_links(Some(links));
}

metadata
.add_to_wasm(bytes)
.context("failed to add registry metadata to component")
}

async fn publish_wit_package(options: PublishOptions<'_>, terminal: &Terminal) -> Result<()> {
let (id, bytes) = build_wit_package(
options.config,
Expand All @@ -287,6 +338,7 @@ async fn publish_wit_package(options: PublishOptions<'_>, terminal: &Terminal) -
return Ok(());
}

let bytes = add_registry_metadata(options.config, &bytes)?;
let id = options.package.unwrap_or(&id);
let client = create_client(options.warg_config, options.url, terminal)?;

Expand Down
114 changes: 112 additions & 2 deletions crates/wit/tests/publish.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
use std::fs;

use crate::support::*;
use anyhow::Result;
use anyhow::{Context, Result};
use assert_cmd::prelude::*;
use predicates::str::contains;
use warg_client::FileSystemClient;
use semver::Version;
use toml_edit::{value, Array};
use warg_client::{Client, FileSystemClient};
use warg_protocol::registry::PackageId;
use wasm_metadata::LinkType;

mod support;

Expand Down Expand Up @@ -73,3 +79,107 @@ async fn it_does_a_dry_run_publish() -> Result<()> {

Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn it_publishes_with_registry_metadata() -> Result<()> {
let root = create_root()?;
let (_server, config) = spawn_server(&root).await?;
config.write_to_file(&root.join("warg-config.json"))?;

let authors = ["Jane Doe <[email protected]>"];
let categories = ["wasm"];
let description = "A test package";
let license = "Apache-2.0";
let documentation = "https://example.com/docs";
let homepage = "https://example.com/home";
let repository = "https://example.com/repo";

let project = Project::with_root(&root, "foo", "")?;
project.file("baz.wit", "package baz:qux\n")?;

project.update_manifest(|mut doc| {
doc["authors"] = value(Array::from_iter(authors));
doc["categories"] = value(Array::from_iter(categories));
doc["description"] = value(description);
doc["license"] = value(license);
doc["documentation"] = value(documentation);
doc["homepage"] = value(homepage);
doc["repository"] = value(repository);
Ok(doc)
})?;

project
.wit("publish --init")
.env("WIT_PUBLISH_KEY", test_signing_key())
.assert()
.stderr(contains("Published package `baz:qux` v0.1.0"))
.success();

let client = Client::new_with_config(None, &config)?;
let download = client
.download_exact(&PackageId::new("baz:qux")?, &Version::parse("0.1.0")?)
.await?;

let bytes = fs::read(&download.path).with_context(|| {
format!(
"failed to read downloaded package `{path}`",
path = download.path.display()
)
})?;

let metadata = wasm_metadata::RegistryMetadata::from_wasm(&bytes)
.with_context(|| {
format!(
"failed to parse registry metadata from `{path}`",
path = download.path.display()
)
})?
.expect("missing registry metadata");

assert_eq!(
metadata.get_authors().expect("missing authors").as_slice(),
authors
);
assert_eq!(
metadata
.get_categories()
.expect("missing categories")
.as_slice(),
categories
);
assert_eq!(
metadata.get_description().expect("missing description"),
description
);
assert_eq!(metadata.get_license().expect("missing license"), license);

let links = metadata.get_links().expect("missing links");
assert_eq!(links.len(), 3);

assert_eq!(
links
.iter()
.find(|link| link.ty == LinkType::Documentation)
.expect("missing documentation")
.value,
documentation
);
assert_eq!(
links
.iter()
.find(|link| link.ty == LinkType::Homepage)
.expect("missing homepage")
.value,
homepage
);
assert_eq!(
links
.iter()
.find(|link| link.ty == LinkType::Repository)
.expect("missing repository")
.value,
repository
);

Ok(())
}
8 changes: 8 additions & 0 deletions crates/wit/tests/support/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use std::{
};
use tokio::task::JoinHandle;
use tokio_util::sync::CancellationToken;
use toml_edit::Document;
use warg_crypto::signing::PrivateKey;
use warg_server::{policy::content::WasmContentPolicy, Config, Server};
use wasmparser::{Chunk, Encoding, Parser, Payload, Validator, WasmFeatures};
Expand Down Expand Up @@ -186,6 +187,13 @@ impl Project {
cmd.current_dir(&self.root);
cmd
}

pub fn update_manifest(&self, f: impl FnOnce(Document) -> Result<Document>) -> Result<()> {
let manifest_path = self.root.join("wit.toml");
let manifest = fs::read_to_string(&manifest_path)?;
fs::write(manifest_path, f(manifest.parse()?)?.to_string())?;
Ok(())
}
}

pub fn validate_component(path: &Path) -> Result<()> {
Expand Down
24 changes: 24 additions & 0 deletions crates/wit/tests/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ async fn update_without_changes_is_a_noop() -> Result<()> {
.stderr(contains("Added dependency `foo:bar` with version `0.1.0"))
.success();

project
.wit("build")
.assert()
.success()
.stderr(contains("Created package `baz.wasm`"));

project
.wit("update")
.assert()
Expand Down Expand Up @@ -82,6 +88,12 @@ async fn test_update_without_compatible_changes_is_a_noop() -> Result<()> {
.stderr(contains("Added dependency `foo:bar` with version `0.1.0"))
.success();

project
.wit("build")
.assert()
.success()
.stderr(contains("Created package `baz.wasm`"));

fs::write(
root.join("bar/wit.toml"),
"version = \"1.0.0\"\n[dependencies]\n[registries]\n",
Expand Down Expand Up @@ -131,6 +143,12 @@ async fn update_with_compatible_changes() -> Result<()> {
.stderr(contains("Added dependency `foo:bar` with version `1.0.0"))
.success();

project
.wit("build")
.assert()
.success()
.stderr(contains("Created package `baz.wasm`"));

fs::write(
root.join("bar/wit.toml"),
"version = \"1.1.0\"\n[dependencies]\n[registries]\n",
Expand Down Expand Up @@ -183,6 +201,12 @@ async fn update_with_compatible_changes_is_noop_for_dryrun() -> Result<()> {
.stderr(contains("Added dependency `foo:bar` with version `1.0.0"))
.success();

project
.wit("build")
.assert()
.success()
.stderr(contains("Created package `baz.wasm`"));

fs::write(
root.join("bar/wit.toml"),
"version = \"1.1.0\"\n[dependencies]\n[registries]\n",
Expand Down
1 change: 1 addition & 0 deletions src/commands/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ impl PublishCommand {
}

let options = PublishOptions {
package,
registry_url,
init: self.init,
id,
Expand Down
Loading