From 2c675d46c672c472275a60002a45b82ee9235633 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Fri, 3 Nov 2023 15:40:43 +0000 Subject: [PATCH 1/7] extract method to parse semver compatible strings --- tooling/nargo_toml/src/semver.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tooling/nargo_toml/src/semver.rs b/tooling/nargo_toml/src/semver.rs index de722f06bd8..09f6ecc29ea 100644 --- a/tooling/nargo_toml/src/semver.rs +++ b/tooling/nargo_toml/src/semver.rs @@ -3,14 +3,19 @@ use nargo::{ package::{Dependency, Package}, workspace::Workspace, }; -use semver::{Version, VersionReq}; +use semver::{Error, Version, VersionReq}; + +// Parse a semver compatible version string +pub(crate) fn parse_semver_compatible_version(version: &str) -> Result { + Version::parse(version) +} // Check that all of the packages in the workspace are compatible with the current compiler version pub(crate) fn semver_check_workspace( workspace: Workspace, current_compiler_version: String, ) -> Result<(), ManifestError> { - let version = Version::parse(¤t_compiler_version) + let version = parse_semver_compatible_version(¤t_compiler_version) .expect("The compiler version is not a valid semver version"); for package in &workspace.members { semver_check_package(package, &version).map_err(ManifestError::SemverError)?; @@ -83,6 +88,7 @@ mod tests { entry_path: PathBuf::new(), name: CrateName::from_str("test").unwrap(), dependencies: BTreeMap::new(), + package_version: Some("1.0".to_string()), }; if let Err(err) = semver_check_package(&package, &compiler_version) { panic!("semver check should have passed. compiler version is 0.1.0 and required version from the package is 0.1.0\n error: {err:?}") @@ -113,6 +119,7 @@ mod tests { entry_path: PathBuf::new(), name: CrateName::from_str("test").unwrap(), dependencies: BTreeMap::new(), + package_version: Some("1.0".to_string()), }; let valid_dependency = Package { @@ -122,6 +129,7 @@ mod tests { entry_path: PathBuf::new(), name: CrateName::from_str("good_dependency").unwrap(), dependencies: BTreeMap::new(), + package_version: Some("1.0".to_string()), }; let invalid_dependency = Package { compiler_required_version: Some("0.2.0".to_string()), @@ -130,6 +138,7 @@ mod tests { entry_path: PathBuf::new(), name: CrateName::from_str("bad_dependency").unwrap(), dependencies: BTreeMap::new(), + package_version: Some("1.0".to_string()), }; package.dependencies.insert( @@ -169,6 +178,7 @@ mod tests { entry_path: PathBuf::new(), name: CrateName::from_str("test").unwrap(), dependencies: BTreeMap::new(), + package_version: Some("1.0".to_string()), }; if let Err(err) = semver_check_package(&package, &compiler_version) { @@ -187,6 +197,7 @@ mod tests { entry_path: PathBuf::new(), name: CrateName::from_str("test").unwrap(), dependencies: BTreeMap::new(), + package_version: Some("1.0".to_string()), }; if let Err(err) = semver_check_package(&package, &compiler_version) { From 35d6d34b88e659c58dfde421962b4154cb8190c0 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Fri, 3 Nov 2023 15:41:06 +0000 Subject: [PATCH 2/7] add package_version to metadata and add validation checks --- tooling/nargo_toml/src/lib.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tooling/nargo_toml/src/lib.rs b/tooling/nargo_toml/src/lib.rs index 223ed2da081..df47a932a21 100644 --- a/tooling/nargo_toml/src/lib.rs +++ b/tooling/nargo_toml/src/lib.rs @@ -8,6 +8,7 @@ use std::{ path::{Component, Path, PathBuf}, }; +use errors::SemverError; use fm::{NormalizePath, FILE_EXTENSION}; use nargo::{ package::{Dependency, Package, PackageType}, @@ -99,7 +100,7 @@ struct PackageConfig { impl PackageConfig { fn resolve_to_package(&self, root_dir: &Path) -> Result { - let name = if let Some(name) = &self.package.name { + let name: CrateName = if let Some(name) = &self.package.name { name.parse().map_err(|_| ManifestError::InvalidPackageName { toml: root_dir.join("Nargo.toml"), name: name.into(), @@ -163,7 +164,18 @@ impl PackageConfig { } }; + // If there is a package version, ensure that it is semver compatible + if let Some(package_version) = &self.package.package_version { + semver::parse_semver_compatible_version(package_version).map_err(|err| { + ManifestError::SemverError(SemverError::CouldNotParsePackageVersion { + package_name: name.to_string(), + error: err.to_string(), + }) + })?; + } + Ok(Package { + package_version: self.package.package_version.clone(), compiler_required_version: self.package.compiler_version.clone(), root_dir: root_dir.to_path_buf(), entry_path, @@ -225,6 +237,7 @@ struct WorkspaceConfig { #[derive(Default, Debug, Deserialize, Clone)] struct PackageMetadata { name: Option, + package_version: Option, #[serde(alias = "type")] package_type: Option, entry: Option, From 316443b1ccfeeabccf3f95cff33c463209fe5217 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Fri, 3 Nov 2023 15:41:24 +0000 Subject: [PATCH 3/7] add package version to package --- tooling/nargo/src/package.rs | 1 + tooling/nargo_toml/src/errors.rs | 2 ++ 2 files changed, 3 insertions(+) diff --git a/tooling/nargo/src/package.rs b/tooling/nargo/src/package.rs index 94c7c5b9c98..ec52f09637a 100644 --- a/tooling/nargo/src/package.rs +++ b/tooling/nargo/src/package.rs @@ -43,6 +43,7 @@ impl Dependency { #[derive(Clone)] pub struct Package { + pub package_version: Option, // A semver string which specifies the compiler version required to compile this package pub compiler_required_version: Option, pub root_dir: PathBuf, diff --git a/tooling/nargo_toml/src/errors.rs b/tooling/nargo_toml/src/errors.rs index fdbdc317bf9..b1940554f3b 100644 --- a/tooling/nargo_toml/src/errors.rs +++ b/tooling/nargo_toml/src/errors.rs @@ -81,4 +81,6 @@ pub enum SemverError { }, #[error("Could not parse the required compiler version for package {package_name} in Nargo.toml. Error: {error}")] CouldNotParseRequiredVersion { package_name: String, error: String }, + #[error("Could not parse the package version for package {package_name} in Nargo.toml. Error: {error}")] + CouldNotParsePackageVersion { package_name: String, error: String }, } From ffbc12561c5c1714bd178f2d2d72f0f83a351060 Mon Sep 17 00:00:00 2001 From: kevaundray Date: Fri, 3 Nov 2023 15:45:46 +0000 Subject: [PATCH 4/7] add package_version to one of the examples --- tooling/nargo_cli/tests/execution_success/1_mul/Nargo.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tooling/nargo_cli/tests/execution_success/1_mul/Nargo.toml b/tooling/nargo_cli/tests/execution_success/1_mul/Nargo.toml index a0fd8d98027..ee10d44733a 100644 --- a/tooling/nargo_cli/tests/execution_success/1_mul/Nargo.toml +++ b/tooling/nargo_cli/tests/execution_success/1_mul/Nargo.toml @@ -3,4 +3,6 @@ name = "1_mul" type = "bin" authors = [""] +package_version = "0.1.0" + [dependencies] From 80292d4a4f39613f5f8988f467f094829253805a Mon Sep 17 00:00:00 2001 From: kevaundray Date: Fri, 3 Nov 2023 15:49:27 +0000 Subject: [PATCH 5/7] clippy --- tooling/nargo_toml/src/errors.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tooling/nargo_toml/src/errors.rs b/tooling/nargo_toml/src/errors.rs index b1940554f3b..76df986921c 100644 --- a/tooling/nargo_toml/src/errors.rs +++ b/tooling/nargo_toml/src/errors.rs @@ -71,6 +71,7 @@ pub enum ManifestError { SemverError(SemverError), } +#[allow(clippy::enum_variant_names)] #[derive(Error, Debug, PartialEq, Eq, Clone)] pub enum SemverError { #[error("Incompatible compiler version in package {package_name}. Required compiler version is {required_compiler_version} but the compiler version is {compiler_version_found}.\n Update the compiler_version field in Nargo.toml to >={compiler_version_found} or compile this project with version {compiler_version_found}")] From 23a90618bd72c6e064d12f584882b41f64b3b3d1 Mon Sep 17 00:00:00 2001 From: Tom French Date: Tue, 28 Nov 2023 16:16:53 +0000 Subject: [PATCH 6/7] chore: replace `package_version` with `version` --- test_programs/execution_success/1_mul/Nargo.toml | 3 ++- tooling/nargo/src/package.rs | 2 +- tooling/nargo_toml/src/lib.rs | 8 ++++---- tooling/nargo_toml/src/semver.rs | 12 ++++++------ 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/test_programs/execution_success/1_mul/Nargo.toml b/test_programs/execution_success/1_mul/Nargo.toml index ee10d44733a..46af37dd863 100644 --- a/test_programs/execution_success/1_mul/Nargo.toml +++ b/test_programs/execution_success/1_mul/Nargo.toml @@ -1,8 +1,9 @@ [package] name = "1_mul" +version = "0.1.0" type = "bin" authors = [""] -package_version = "0.1.0" + [dependencies] diff --git a/tooling/nargo/src/package.rs b/tooling/nargo/src/package.rs index ec52f09637a..ecbf3585210 100644 --- a/tooling/nargo/src/package.rs +++ b/tooling/nargo/src/package.rs @@ -43,7 +43,7 @@ impl Dependency { #[derive(Clone)] pub struct Package { - pub package_version: Option, + pub version: Option, // A semver string which specifies the compiler version required to compile this package pub compiler_required_version: Option, pub root_dir: PathBuf, diff --git a/tooling/nargo_toml/src/lib.rs b/tooling/nargo_toml/src/lib.rs index df47a932a21..141cb3411b3 100644 --- a/tooling/nargo_toml/src/lib.rs +++ b/tooling/nargo_toml/src/lib.rs @@ -165,8 +165,8 @@ impl PackageConfig { }; // If there is a package version, ensure that it is semver compatible - if let Some(package_version) = &self.package.package_version { - semver::parse_semver_compatible_version(package_version).map_err(|err| { + if let Some(version) = &self.package.version { + semver::parse_semver_compatible_version(version).map_err(|err| { ManifestError::SemverError(SemverError::CouldNotParsePackageVersion { package_name: name.to_string(), error: err.to_string(), @@ -175,7 +175,7 @@ impl PackageConfig { } Ok(Package { - package_version: self.package.package_version.clone(), + version: self.package.version.clone(), compiler_required_version: self.package.compiler_version.clone(), root_dir: root_dir.to_path_buf(), entry_path, @@ -237,7 +237,7 @@ struct WorkspaceConfig { #[derive(Default, Debug, Deserialize, Clone)] struct PackageMetadata { name: Option, - package_version: Option, + version: Option, #[serde(alias = "type")] package_type: Option, entry: Option, diff --git a/tooling/nargo_toml/src/semver.rs b/tooling/nargo_toml/src/semver.rs index 09f6ecc29ea..6acc68afa47 100644 --- a/tooling/nargo_toml/src/semver.rs +++ b/tooling/nargo_toml/src/semver.rs @@ -88,7 +88,7 @@ mod tests { entry_path: PathBuf::new(), name: CrateName::from_str("test").unwrap(), dependencies: BTreeMap::new(), - package_version: Some("1.0".to_string()), + version: Some("1.0".to_string()), }; if let Err(err) = semver_check_package(&package, &compiler_version) { panic!("semver check should have passed. compiler version is 0.1.0 and required version from the package is 0.1.0\n error: {err:?}") @@ -119,7 +119,7 @@ mod tests { entry_path: PathBuf::new(), name: CrateName::from_str("test").unwrap(), dependencies: BTreeMap::new(), - package_version: Some("1.0".to_string()), + version: Some("1.0".to_string()), }; let valid_dependency = Package { @@ -129,7 +129,7 @@ mod tests { entry_path: PathBuf::new(), name: CrateName::from_str("good_dependency").unwrap(), dependencies: BTreeMap::new(), - package_version: Some("1.0".to_string()), + version: Some("1.0".to_string()), }; let invalid_dependency = Package { compiler_required_version: Some("0.2.0".to_string()), @@ -138,7 +138,7 @@ mod tests { entry_path: PathBuf::new(), name: CrateName::from_str("bad_dependency").unwrap(), dependencies: BTreeMap::new(), - package_version: Some("1.0".to_string()), + version: Some("1.0".to_string()), }; package.dependencies.insert( @@ -178,7 +178,7 @@ mod tests { entry_path: PathBuf::new(), name: CrateName::from_str("test").unwrap(), dependencies: BTreeMap::new(), - package_version: Some("1.0".to_string()), + version: Some("1.0".to_string()), }; if let Err(err) = semver_check_package(&package, &compiler_version) { @@ -197,7 +197,7 @@ mod tests { entry_path: PathBuf::new(), name: CrateName::from_str("test").unwrap(), dependencies: BTreeMap::new(), - package_version: Some("1.0".to_string()), + version: Some("1.0".to_string()), }; if let Err(err) = semver_check_package(&package, &compiler_version) { From bef952b53d07f474e3e7ae296c9cd6e225dcadbe Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Tue, 28 Nov 2023 16:17:35 +0000 Subject: [PATCH 7/7] Update test_programs/execution_success/1_mul/Nargo.toml --- test_programs/execution_success/1_mul/Nargo.toml | 2 -- 1 file changed, 2 deletions(-) diff --git a/test_programs/execution_success/1_mul/Nargo.toml b/test_programs/execution_success/1_mul/Nargo.toml index 46af37dd863..94b36157cca 100644 --- a/test_programs/execution_success/1_mul/Nargo.toml +++ b/test_programs/execution_success/1_mul/Nargo.toml @@ -4,6 +4,4 @@ version = "0.1.0" type = "bin" authors = [""] - - [dependencies]