From 66f3b08fe65594a57624d05650a30c2c1a14069a Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Tue, 30 Apr 2024 08:43:25 +0200 Subject: [PATCH 1/2] [skip ci] feat(wasmer_config): Make `Package` fields non-mandatory This commit makes some fields of the `wasmer_config::package::Package` type non mandatory. In particular, `name`, `version` and `description` aren't mandatory anymore. This introduces *two ways* to describe unnamed packages - no `package` section in the manifest and `package` section with no `name` field in it, and a third kind of package in the package taxonomy, which I deliberately name, here _unversioned packages_, that is packages with a `name` (and any other field) but no `version`. --- lib/cli/src/commands/app/create.rs | 49 ++++++------ lib/cli/src/commands/app/deploy.rs | 109 +++++++++++++++----------- lib/cli/src/commands/package/build.rs | 14 ++-- lib/cli/src/commands/publish.rs | 83 ++++++++++---------- lib/config/src/package/mod.rs | 11 +-- lib/registry/src/package/builder.rs | 26 +++--- lib/registry/src/publish.rs | 33 +++++--- 7 files changed, 185 insertions(+), 140 deletions(-) diff --git a/lib/cli/src/commands/app/create.rs b/lib/cli/src/commands/app/create.rs index 37a64dac050..517c3f1b196 100644 --- a/lib/cli/src/commands/app/create.rs +++ b/lib/cli/src/commands/app/create.rs @@ -559,31 +559,36 @@ impl AppCreator { ), }; - let full = format!("{}@{}", pkg.name, pkg.version); - let mut pkg_ident = NamedPackageIdent::from_str(&pkg.name) - .with_context(|| format!("local package manifest has invalid name: '{full}'"))?; - - // Pin the version. - pkg_ident.tag = Some(Tag::from_str(&pkg.version.to_string()).unwrap()); - - if self.interactive { - eprintln!("Found local package: '{}'", full.green()); - - let msg = format!("Use package '{pkg_ident}'"); - - let theme = dialoguer::theme::ColorfulTheme::default(); - let should_use = Confirm::with_theme(&theme) - .with_prompt(&msg) - .interact_opt()? - .unwrap_or_default(); - - if should_use { - Some(pkg_ident) + if let (Some(name), Some(version)) = (pkg.name, pkg.version) { + let full = format!("{}@{}", name, version); + let mut pkg_ident = NamedPackageIdent::from_str(&name).with_context(|| { + format!("local package manifest has invalid name: '{full}'") + })?; + + // Pin the version. + pkg_ident.tag = Some(Tag::from_str(&version.to_string()).unwrap()); + + if self.interactive { + eprintln!("Found local package: '{}'", full.green()); + + let msg = format!("Use package '{pkg_ident}'"); + + let theme = dialoguer::theme::ColorfulTheme::default(); + let should_use = Confirm::with_theme(&theme) + .with_prompt(&msg) + .interact_opt()? + .unwrap_or_default(); + + if should_use { + Some(pkg_ident) + } else { + None + } } else { - None + Some(pkg_ident) } } else { - Some(pkg_ident) + None } } else { None diff --git a/lib/cli/src/commands/app/deploy.rs b/lib/cli/src/commands/app/deploy.rs index d5e3a422fd4..934c3caa059 100644 --- a/lib/cli/src/commands/app/deploy.rs +++ b/lib/cli/src/commands/app/deploy.rs @@ -327,58 +327,73 @@ impl AsyncCliCommand for CmdAppDeploy { if let Ok(Some((manifest_path, manifest))) = load_package_manifest(&base_dir_path) { if let Some(package) = &manifest.package { - if package.name == n.full_name() { - eprintln!( - "Found local package (manifest path: {}).", - manifest_path.display() - ); - eprintln!("The `package` field in `app.yaml` specified the same named package ({}).", package.name); - eprintln!("This behaviour is deprecated."); - let theme = dialoguer::theme::ColorfulTheme::default(); - if self.non_interactive { - eprintln!("Hint: replace `package: {}` with `package: .` to replicate the intended behaviour.", n); - anyhow::bail!("deprecated deploy behaviour") - } else if Confirm::with_theme(&theme) - .with_prompt("Change package to '.' in app.yaml?") - .interact()? - { - app_config.package = PackageSource::Path(String::from(".")); - // We have to write it right now. - let new_config_raw = serde_yaml::to_string(&app_config)?; - std::fs::write(&app_config_path, new_config_raw).with_context( - || { - format!( - "Could not write file: '{}'", - app_config_path.display() - ) - }, - )?; - - log::info!( - "Using package {} ({})", - app_config.package, - n.full_name() + if let Some(name) = &package.name { + if name == &n.full_name() { + eprintln!( + "Found local package (manifest path: {}).", + manifest_path.display() ); + eprintln!("The `package` field in `app.yaml` specified the same named package ({}).", name); + eprintln!("This behaviour is deprecated."); + + let theme = dialoguer::theme::ColorfulTheme::default(); + if self.non_interactive { + eprintln!("Hint: replace `package: {}` with `package: .` to replicate the intended behaviour.", n); + anyhow::bail!("deprecated deploy behaviour") + } else if Confirm::with_theme(&theme) + .with_prompt("Change package to '.' in app.yaml?") + .interact()? + { + app_config.package = PackageSource::Path(String::from(".")); + // We have to write it right now. + let new_config_raw = serde_yaml::to_string(&app_config)?; + std::fs::write(&app_config_path, new_config_raw).with_context( + || { + format!( + "Could not write file: '{}'", + app_config_path.display() + ) + }, + )?; + + log::info!( + "Using package {} ({})", + app_config.package, + n.full_name() + ); - let package_id = self.publish(owner.clone(), manifest_path).await?; - - app_config.package = package_id.into(); - - DeployAppOpts { - app: &app_config, - original_config: Some( - app_config.clone().to_yaml_value().unwrap(), - ), - allow_create: true, - make_default: !self.no_default, - owner: Some(owner), - wait, - } - } else { - eprintln!( + let package_id = + self.publish(owner.clone(), manifest_path).await?; + + app_config.package = package_id.into(); + + DeployAppOpts { + app: &app_config, + original_config: Some( + app_config.clone().to_yaml_value().unwrap(), + ), + allow_create: true, + make_default: !self.no_default, + owner: Some(owner), + wait, + } + } else { + eprintln!( "{}: the package will not be published and the deployment will fail if the package does not already exist.", "Warning".yellow().bold() ); + DeployAppOpts { + app: &app_config, + original_config: Some( + app_config.clone().to_yaml_value().unwrap(), + ), + allow_create: true, + make_default: !self.no_default, + owner: Some(owner), + wait, + } + } + } else { DeployAppOpts { app: &app_config, original_config: Some( diff --git a/lib/cli/src/commands/package/build.rs b/lib/cli/src/commands/package/build.rs index 940f26dd299..67093906ce4 100644 --- a/lib/cli/src/commands/package/build.rs +++ b/lib/cli/src/commands/package/build.rs @@ -55,11 +55,15 @@ impl PackageBuild { let pkg = webc::wasmer_package::Package::from_manifest(manifest_path)?; let pkg_hash = PackageHash::from_sha256_bytes(pkg.webc_hash()); let name = if let Some(manifest_pkg) = manifest.package { - format!( - "{}-{}.webc", - manifest_pkg.name.replace('/', "-"), - manifest_pkg.version - ) + if let Some(name) = manifest_pkg.name { + if let Some(version) = manifest_pkg.version { + format!("{}-{}.webc", name.replace('/', "-"), version) + } else { + format!("{}-{}.webc", name.replace('/', "-"), pkg_hash) + } + } else { + format!("{}.webc", pkg_hash) + } } else { format!("{}.webc", pkg_hash) }; diff --git a/lib/cli/src/commands/publish.rs b/lib/cli/src/commands/publish.rs index 7c47fafa8a7..17aff62cb48 100644 --- a/lib/cli/src/commands/publish.rs +++ b/lib/cli/src/commands/publish.rs @@ -123,51 +123,54 @@ impl AsyncCliCommand for Publish { let mut version = self.version.clone(); if let Some(ref mut pkg) = manifest.package { - let mut latest_version = { - let v = wasmer_api::query::get_package_version( - &client, - pkg.name.clone(), - "latest".into(), - ) - .await?; - if let Some(v) = v { - semver::Version::parse(&v.version) - .with_context(|| "While parsing registry version of package")? - } else { - pkg.version.clone() - } - }; - - if pkg.version < latest_version { - if self.bump { - latest_version.patch += 1; - version = Some(latest_version); - } else if interactive { - latest_version.patch += 1; - let theme = dialoguer::theme::ColorfulTheme::default(); - if Confirm::with_theme(&theme) - .with_prompt(format!( - "Do you want to bump the package to a new version? ({} -> {})", - pkg.version, latest_version - )) - .interact() - .unwrap_or_default() - { + if let (Some(pkg_name), Some(pkg_version)) = (&pkg.name, &pkg.version) { + let pkg_name = pkg_name.clone(); + let pkg_version = pkg_version.clone(); + + let mut latest_version = { + let v = wasmer_api::query::get_package_version( + &client, + pkg_name.clone(), + "latest".into(), + ) + .await?; + if let Some(v) = v { + semver::Version::parse(&v.version) + .with_context(|| "While parsing registry version of package")? + } else { + pkg_version.clone() + } + }; + + if pkg_version < latest_version { + if self.bump { + latest_version.patch += 1; version = Some(latest_version); + } else if interactive { + latest_version.patch += 1; + let theme = dialoguer::theme::ColorfulTheme::default(); + if Confirm::with_theme(&theme) + .with_prompt(format!( + "Do you want to bump the package to a new version? ({} -> {})", + pkg_version, latest_version + )) + .interact() + .unwrap_or_default() + { + pkg.version = Some(latest_version); + } + } else if latest_version > pkg_version { + eprintln!("Registry has a newer version of this package."); + eprintln!( + "If a package with version {} already exists, publishing will fail.", + pkg_version + ); } - } else if latest_version > pkg.version { - eprintln!("Registry has a newer version of this package."); - eprintln!( - "If a package with version {} already exists, publishing will fail.", - pkg.version - ); } - } - // If necessary, update the manifest. - if let Some(version) = version.clone() { + // If necessary, update the manifest. if version != pkg.version { - pkg.version = version; + pkg.version = version.clone(); let contents = toml::to_string(&manifest).with_context(|| { format!( diff --git a/lib/config/src/package/mod.rs b/lib/config/src/package/mod.rs index 1a3ca32a746..398ca12e6eb 100644 --- a/lib/config/src/package/mod.rs +++ b/lib/config/src/package/mod.rs @@ -113,13 +113,14 @@ const LICENSE_PATHS: &[&str; 3] = &["LICENSE", "LICENSE.md", "COPYING"]; #[non_exhaustive] pub struct Package { /// The package's name in the form `namespace/name`. - #[builder(setter(into))] - pub name: String, + #[builder(setter(into, strip_option), default)] + pub name: Option, /// The package's version number. - pub version: Version, + #[builder(setter(into, strip_option), default)] + pub version: Option, /// A brief description of the package. - #[builder(setter(into))] - pub description: String, + #[builder(setter(into, strip_option), default)] + pub description: Option, /// A SPDX license specifier for this package. #[builder(setter(into, strip_option), default)] pub license: Option, diff --git a/lib/registry/src/package/builder.rs b/lib/registry/src/package/builder.rs index 9af14587dc0..56c13eac0ec 100644 --- a/lib/registry/src/package/builder.rs +++ b/lib/registry/src/package/builder.rs @@ -119,13 +119,13 @@ impl Publish { if let Some(package_name) = self.package_name.as_ref() { if let Some(ref mut package) = manifest.package { - package.name = package_name.clone(); + package.name = Some(package_name.clone()); } } if let Some(version) = self.version.as_ref() { if let Some(ref mut package) = manifest.package { - package.version = version.clone(); + package.version = Some(version.clone()); } } @@ -673,16 +673,20 @@ mod validate { ) -> Result { match &manifest.package { Some(pkg) => { - let result = - crate::query_package_from_registry(registry, &pkg.name, None, Some(auth_token)); - - match result { - Ok(package_version) => Ok(package_version.is_private != pkg.private), - Err(QueryPackageError::NoPackageFound { .. }) => { - // The package hasn't been published yet - Ok(false) + if let Some(ref name) = pkg.name { + let result = + crate::query_package_from_registry(registry, name, None, Some(auth_token)); + + match result { + Ok(package_version) => Ok(package_version.is_private != pkg.private), + Err(QueryPackageError::NoPackageFound { .. }) => { + // The package hasn't been published yet + Ok(false) + } + Err(e) => Err(e.into()), } - Err(e) => Err(e.into()), + } else { + Ok(false) } } diff --git a/lib/registry/src/publish.rs b/lib/registry/src/publish.rs index ca101ddb907..570d3864ec4 100644 --- a/lib/registry/src/publish.rs +++ b/lib/registry/src/publish.rs @@ -130,21 +130,25 @@ pub async fn try_chunked_uploading( upload_package(&signed_url.url, archive_path, archived_data_size, timeout).await?; - let name = package.as_ref().map(|p| p.name.clone()); + let name = package.as_ref().and_then(|p| p.name.clone()); let namespace = match patch_namespace { Some(n) => Some(n), - None => package - .as_ref() - .map(|p| String::from(p.name.split_once('/').unwrap().0)), + None => package.as_ref().and_then(|p| { + p.name + .as_ref() + .map(|p| String::from(p.split_once('/').unwrap().0)) + }), }; let q = PublishPackageMutationChunked::build_query(publish_package_mutation_chunked::Variables { name, namespace, - version: package.as_ref().map(|p| p.version.to_string()), - description: package.as_ref().map(|p| p.description.clone()), + version: package + .as_ref() + .and_then(|p| p.version.as_ref().map(|v| v.to_string())), + description: package.as_ref().and_then(|p| p.description.clone()), manifest: manifest_string.to_string(), license: package.as_ref().and_then(|p| p.license.clone()), license_file: license_file.to_owned(), @@ -215,7 +219,12 @@ pub async fn try_chunked_uploading( let package_ident = PackageIdent::Named(NamedPackageIdent::from_str(&format!( "{}@{}", - package.name, package.version + package + .name + .expect("Unnamed package was published as named"), + package + .version + .expect("Unversioned package was published as versioned") ))?); eprintln!("Package published successfully"); // println!("🚀 Successfully published package `{}`", package_ident); @@ -306,8 +315,12 @@ fn google_signed_url( timeout: Duration, ) -> Result { let get_google_signed_url = GetSignedUrl::build_query(get_signed_url::Variables { - name: package.as_ref().map(|p| p.name.to_string()), - version: package.as_ref().map(|p| p.version.to_string()), + name: package + .as_ref() + .and_then(|p| p.name.as_ref().map(|n| n.to_string())), + version: package + .as_ref() + .and_then(|p| p.version.as_ref().map(|v| v.to_string())), filename: match package { Some(_) => None, None => Some(format!("unnamed_package_{}", rand::random::())), @@ -325,7 +338,7 @@ fn google_signed_url( let url = _response.url.ok_or_else(|| match package { Some(pkg) => { anyhow::anyhow!( - "could not get signed url for package {}@{}", + "could not get signed url for package {:?}@{:?}", pkg.name, pkg.version ) From 601e549ed4c4456a9061892c9d743573aa181533 Mon Sep 17 00:00:00 2001 From: Edoardo Marangoni Date: Tue, 30 Apr 2024 09:11:30 +0200 Subject: [PATCH 2/2] fix(wasmer-config): Fix tests --- lib/config/src/package/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/config/src/package/mod.rs b/lib/config/src/package/mod.rs index 398ca12e6eb..cedc4d00260 100644 --- a/lib/config/src/package/mod.rs +++ b/lib/config/src/package/mod.rs @@ -976,9 +976,9 @@ mod tests { fn test_to_string() { Manifest { package: Some(Package { - name: "package/name".to_string(), - version: Version::parse("1.0.0").unwrap(), - description: "test".to_string(), + name: Some("package/name".to_string()), + version: Some(Version::parse("1.0.0").unwrap()), + description: Some("test".to_string()), license: None, license_file: None, readme: None,