diff --git a/Cargo.lock b/Cargo.lock index 206886c4..3ee5113e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -248,6 +248,7 @@ dependencies = [ "resiter", "rlimit", "rustversion", + "semver", "serde", "serde_json", "sha1", diff --git a/Cargo.toml b/Cargo.toml index 6b862373..e43eb700 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -60,6 +60,7 @@ reqwest = { version = "0.12", features = [ "stream" ] } resiter = "0.5" rlimit = "0.10" rustversion = "1" +semver = "1" serde = "1" serde_json = "1" sha1 = "0.10" diff --git a/src/cli.rs b/src/cli.rs index 3f3af5c5..47f7a0a5 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -47,7 +47,7 @@ pub fn cli() -> Command { .long("package") .short('p') .value_name("PKG") - .help("Only list releases for package PKG"), + .help("List only releases for package PKG"), ) .arg( Arg::new("limit") @@ -412,7 +412,7 @@ pub fn cli() -> Command { .required(true) .index(1) .value_name("UUID") - .help("The id of the Job") + .help("The job to print the log of") .value_parser(uuid::Uuid::parse_str) ) ) @@ -579,11 +579,11 @@ pub fn cli() -> Command { .value_name("PACKAGE_NAME") .help("The name of the package") ) - .arg(Arg::new("package_version_constraint") + .arg(Arg::new("package_version") .required(true) .index(2) - .value_name("VERSION_CONSTRAINT") - .help("A version constraint to search for (optional), e.g., '=1.0.0'") + .value_name("PACKAGE_VERSION") + .help("The version of the package") ) ) @@ -599,7 +599,7 @@ pub fn cli() -> Command { .required(false) .index(2) .value_name("VERSION_CONSTRAINT") - .help("A version constraint to search for (optional), e.g., '=1.0.0'") + .help("A version constraint to match the package version against (optional), e.g., '=1.0.0'") ) .arg(Arg::new("no_script_filter") .action(ArgAction::SetTrue) @@ -645,7 +645,7 @@ pub fn cli() -> Command { .required(false) .index(2) .value_name("VERSION_CONSTRAINT") - .help("A version constraint to search for (optional), e.g., '=1.0.0'") + .help("A version constraint to match the package version against (optional), e.g., '=1.0.0'") ) .arg(Arg::new("terse") @@ -766,11 +766,11 @@ pub fn cli() -> Command { .value_name("PKG") .help("Verify the sources of this package (optional, if left out, all packages are checked)") ) - .arg(Arg::new("package_version") + .arg(Arg::new("package_version_constraint") .required(false) .index(2) - .value_name("VERSION") - .help("Verify the sources of this package version (optional, if left out, all packages are checked)") + .value_name("VERSION_CONSTRAINT") + .help("Verify the sources of matching package versions (optional, if left out, all versions are checked)") ) .arg(Arg::new("matching") @@ -794,13 +794,13 @@ pub fn cli() -> Command { .required(false) .index(1) .value_name("PKG") - .help("Verify the sources of this package (optional, if left out, all packages are checked)") + .help("Show the URL of this package (or all packages, if omitted)") ) - .arg(Arg::new("package_version") + .arg(Arg::new("package_version_constraint") .required(false) .index(2) - .value_name("VERSION") - .help("Verify the sources of this package version (optional, if left out, all packages are checked)") + .value_name("VERSION_CONSTRAINT") + .help("Show the URLs of matching package versions (or all versions, if omitted)") ) ) .subcommand(Command::new("download") @@ -811,24 +811,24 @@ pub fn cli() -> Command { .value_name("PKG") .help("Download the sources of this package") ) - .arg(Arg::new("package_version") + .arg(Arg::new("package_version_constraint") .required(false) .index(2) .value_name("VERSION_CONSTRAINT") - .help("Download the sources of this package version (optional, if left out, all packages are downloaded)") + .help("Download the sources of matching package versions (optional, if left out, all versions are downloaded)") ) .arg(Arg::new("force") .action(ArgAction::SetTrue) .required(false) .long("force") - .help("Overwrite existing cache entry") + .help("Overwrite existing cache entry (the downloaded source file(s) will be deleted before the re-download starts)") ) .arg(Arg::new("matching") .required(false) .long("matching") .value_name("REGEX") - .help("Download all packages matching a regex with their name") + .help("Download all packages where the package name matches REGEX") ) .group(ArgGroup::new("download-one-or-many") @@ -850,13 +850,13 @@ pub fn cli() -> Command { .required(false) .index(1) .value_name("PKG") - .help("Get the source file paths for this package") + .help("Get the source file paths for this package (or all packages, if omitted)") ) - .arg(Arg::new("package_version") + .arg(Arg::new("package_version_constraint") .required(false) .index(2) - .value_name("VERSION") - .help("Get the source file paths for the package in this version") + .value_name("VERSION_CONSTRAINT") + .help("Get the source file paths for the package in matching versions (or all versions, if omitted)") ) ) ) @@ -880,7 +880,7 @@ pub fn cli() -> Command { ) .arg(Arg::new("package_name") - .required(false) + .required(true) .index(1) .value_name("PKG") .help("The name of the package") @@ -888,7 +888,7 @@ pub fn cli() -> Command { ) .arg(Arg::new("package_version") - .required(false) + .required(true) .index(2) .value_name("VERSION") .help("The exact version of the package (string match)") @@ -902,7 +902,7 @@ pub fn cli() -> Command { .required(true) .index(1) .value_name("SUBMIT") - .help("The submit uuid from which to release a package") + .help("The submit UUID from which to release a package") .value_parser(uuid::Uuid::parse_str) ) .arg(Arg::new("release_store_name") @@ -919,18 +919,7 @@ pub fn cli() -> Command { .required(false) .index(2) .value_name("PKG") - .help("The name of the package") - .conflicts_with("all-packages") - ) - .arg(Arg::new("all-packages") - .required(false) - .long("all") - .help("Release all packages") - .conflicts_with("package_name") - ) - .group(ArgGroup::new("package") - .args(["package_name", "all-packages"]) - .required(true) // one of these is required + .help("The name of the package (or, if omitted, release all packages of the submit)") ) .arg(Arg::new("package_version") .required(false) @@ -968,13 +957,13 @@ pub fn cli() -> Command { .required(false) .index(1) .value_name("NAME") - .help("Package name to lint (if not present, every package will be linted") + .help("Package name to lint (if not present, every package will be linted)") ) - .arg(Arg::new("package_version") + .arg(Arg::new("package_version_constraint") .required(false) .index(2) .value_name("VERSION_CONSTRAINT") - .help("A version constraint to search for (optional), e.g., '=1.0.0'") + .help("A version constraint to match the package version against (optional), e.g., '=1.0.0'") ) ) @@ -984,9 +973,9 @@ pub fn cli() -> Command { .required(true) .index(1) .value_name("NAME") - .help("Package name to lint (if not present, every package will be linted") + .help("Package name to print the dependency tree of") ) - .arg(Arg::new("package_version") + .arg(Arg::new("package_version_constraint") .required(false) .index(2) .value_name("VERSION_CONSTRAINT") diff --git a/src/commands/dependencies_of.rs b/src/commands/dependencies_of.rs index 2700054d..ee46024e 100644 --- a/src/commands/dependencies_of.rs +++ b/src/commands/dependencies_of.rs @@ -12,6 +12,7 @@ use std::io::Write; +use anyhow::Context; use anyhow::Result; use clap::ArgMatches; use futures::stream::StreamExt; @@ -21,6 +22,7 @@ use tracing::trace; use crate::commands::util::getbool; use crate::config::*; use crate::package::PackageName; +use crate::package::PackageVersionConstraint; use crate::repository::Repository; use crate::ui::*; @@ -39,8 +41,20 @@ pub async fn dependencies_of( .map(PackageName::from) .unwrap(); trace!("Checking for package with name = {}", name); + let version_constraint = matches + .get_one::("package_version_constraint") + .map(|s| s.to_owned()) + .map(PackageVersionConstraint::try_from) + .transpose() + .context("Parsing package version constraint")?; + trace!( + "Checking for package with version constraint = {:?}", + version_constraint + ); - crate::util::filters::build_package_filter_by_name(name) + crate::util::filters::build_package_filter_by_name(name).and( + crate::util::filters::build_package_filter_by_version_constraint(version_constraint), + ) }; let format = config.package_print_format(); diff --git a/src/commands/env_of.rs b/src/commands/env_of.rs index 7e7173a1..d44f7a2d 100644 --- a/src/commands/env_of.rs +++ b/src/commands/env_of.rs @@ -15,7 +15,7 @@ use clap::ArgMatches; use tracing::trace; use crate::package::PackageName; -use crate::package::PackageVersionConstraint; +use crate::package::PackageVersion; use crate::repository::Repository; /// Implementation of the "env_of" subcommand @@ -29,19 +29,20 @@ pub async fn env_of(matches: &ArgMatches, repo: Repository) -> Result<()> { .map(|s| s.to_owned()) .map(PackageName::from) .unwrap(); - let constraint = matches - .get_one::("package_version_constraint") + let version = matches + .get_one::("package_version") .map(|s| s.to_owned()) - .map(PackageVersionConstraint::try_from) + .map(PackageVersion::try_from) .unwrap()?; trace!( "Checking for package with name = {} and version = {:?}", name, - constraint + version ); - crate::util::filters::build_package_filter_by_name(name) - .and(crate::util::filters::build_package_filter_by_version_constraint(constraint)) + crate::util::filters::build_package_filter_by_name(name).and( + crate::util::filters::build_package_filter_by_version(version), + ) }; let mut stdout = std::io::stdout(); diff --git a/src/commands/find_artifact.rs b/src/commands/find_artifact.rs index b42130c3..2349291f 100644 --- a/src/commands/find_artifact.rs +++ b/src/commands/find_artifact.rs @@ -50,8 +50,7 @@ pub async fn find_artifact( .map(|s| s.to_owned()) .map(PackageVersionConstraint::try_from) .transpose() - .context("Parsing package version constraint") - .context("A valid package version constraint looks like this: '=1.0.0'")?; + .context("Parsing package version constraint")?; let env_filter = matches .get_many::("env_filter") diff --git a/src/commands/find_pkg.rs b/src/commands/find_pkg.rs index c998b4f1..ba157199 100644 --- a/src/commands/find_pkg.rs +++ b/src/commands/find_pkg.rs @@ -39,8 +39,7 @@ pub async fn find_pkg( .map(|s| s.to_owned()) .map(PackageVersionConstraint::try_from) .transpose() - .context("Parsing package version constraint") - .context("A valid package version constraint looks like this: '=1.0.0'")?; + .context("Parsing package version constraint")?; let iter = repo .packages() diff --git a/src/commands/lint.rs b/src/commands/lint.rs index 8d3463f5..ddbb97b4 100644 --- a/src/commands/lint.rs +++ b/src/commands/lint.rs @@ -37,7 +37,7 @@ pub async fn lint( .map(|s| s.to_owned()) .map(PackageName::from); let pvers = matches - .get_one::("package_version") + .get_one::("package_version_constraint") .map(|s| s.to_owned()) .map(PackageVersionConstraint::try_from) .transpose()?; diff --git a/src/commands/metrics.rs b/src/commands/metrics.rs index a610aa56..32f9d3b3 100644 --- a/src/commands/metrics.rs +++ b/src/commands/metrics.rs @@ -128,24 +128,27 @@ pub async fn metrics( r#" Butido release {release} - {configured_endpoints} Configured endpoints - {configured_images} Configured images - {configured_release_stores} Configured release stores - {configured_phases} Configures phases + Configuration: + - {configured_endpoints} endpoints + - {configured_images} images + - {configured_release_stores} release stores + - {configured_phases} phases - {nfiles} files in repository - {repo_packages} packages in repository + Repository: + - {nfiles} files + - {repo_packages} packages - {n_artifacts} artifacts in database - {n_endpoints} endpoints in database - {n_envvars} envvars in database - {n_githashes} githashes in database - {n_images} images in database - {n_jobs} jobs in database - {n_packages} packages in database - {n_releasestores} releasestores in database - {n_releases} releases in database - {n_submits} submits in database + Database: + - {n_artifacts} artifacts + - {n_endpoints} endpoints + - {n_envvars} envvars + - {n_githashes} githashes + - {n_images} images + - {n_jobs} jobs + - {n_packages} packages + - {n_releasestores} releasestores + - {n_releases} releases + - {n_submits} submits "#, release = clap::crate_version!(), configured_endpoints = config.docker().endpoints().len(), diff --git a/src/commands/release.rs b/src/commands/release.rs index ac950efb..81664230 100644 --- a/src/commands/release.rs +++ b/src/commands/release.rs @@ -119,6 +119,10 @@ async fn new_release( }; debug!("Artifacts = {:?}", arts); + if arts.is_empty() { + return Err(anyhow!("No matching artifacts found to release")); + } + arts.iter() .filter_map(|art| { art.path_buf() diff --git a/src/commands/source/download.rs b/src/commands/source/download.rs index c0520c1b..ea3176b5 100644 --- a/src/commands/source/download.rs +++ b/src/commands/source/download.rs @@ -217,7 +217,7 @@ pub async fn download( .map(|s| s.to_owned()) .map(PackageName::from); let pvers = matches - .get_one::("package_version") + .get_one::("package_version_constraint") .map(|s| s.to_owned()) .map(PackageVersionConstraint::try_from) .transpose()?; diff --git a/src/commands/source/mod.rs b/src/commands/source/mod.rs index ae4a47a7..7fa0985f 100644 --- a/src/commands/source/mod.rs +++ b/src/commands/source/mod.rs @@ -64,7 +64,7 @@ pub async fn verify( .map(|s| s.to_owned()) .map(PackageName::from); let pvers = matches - .get_one::("package_version") + .get_one::("package_version_constraint") .map(|s| s.to_owned()) .map(PackageVersionConstraint::try_from) .transpose()?; @@ -180,7 +180,7 @@ pub async fn url(matches: &ArgMatches, repo: Repository) -> Result<()> { .map(|s| s.to_owned()) .map(PackageName::from); let pvers = matches - .get_one::("package_version") + .get_one::("package_version_constraint") .map(|s| s.to_owned()) .map(PackageVersionConstraint::try_from) .transpose()?; @@ -216,7 +216,7 @@ async fn of(matches: &ArgMatches, config: &Configuration, repo: Repository) -> R .map(|s| s.to_owned()) .map(PackageName::from); let pvers = matches - .get_one::("package_version") + .get_one::("package_version_constraint") .map(|s| s.to_owned()) .map(PackageVersionConstraint::try_from) .transpose()?; diff --git a/src/commands/tree_of.rs b/src/commands/tree_of.rs index 0ca2648e..fd002d77 100644 --- a/src/commands/tree_of.rs +++ b/src/commands/tree_of.rs @@ -35,7 +35,7 @@ pub async fn tree_of(matches: &ArgMatches, repo: Repository, config: &Configurat .map(|s| s.to_owned()) .map(PackageName::from); let pvers = matches - .get_one::("package_version") + .get_one::("package_version_constraint") .map(|s| s.to_owned()) .map(PackageVersionConstraint::try_from) .transpose()?; diff --git a/src/package/dag.rs b/src/package/dag.rs index dea1ed94..230076de 100644 --- a/src/package/dag.rs +++ b/src/package/dag.rs @@ -31,7 +31,7 @@ use crate::package::condition::ConditionData; use crate::package::dependency::ParseDependency; use crate::package::Package; use crate::package::PackageName; -use crate::package::PackageVersionConstraint; +use crate::package::PackageVersion; use crate::repository::Repository; #[derive(Debug, Getters)] @@ -63,12 +63,12 @@ impl Dag { dependency: &D, dependency_type: DependencyType, conditional_data: &ConditionData<'_>, - ) -> Result<(bool, PackageName, PackageVersionConstraint, DependencyType)> { + ) -> Result<(bool, PackageName, PackageVersion, DependencyType)> { // Check whether the condition of the dependency matches our data let take = dependency.check_condition(conditional_data)?; let (name, version) = dependency.parse_as_name_and_version()?; - // (dependency check result, name of the dependency, version constraint of the + // (dependency check result, name of the dependency, version of the // dependency, and type (build/runtime)) Ok((take, name, version, dependency_type)) } @@ -83,7 +83,7 @@ impl Dag { fn get_package_dependencies<'a>( package: &'a Package, conditional_data: &'a ConditionData<'_>, - ) -> impl Iterator> + 'a + ) -> impl Iterator> + 'a { trace!("Collecting the dependencies of {package:?} {conditional_data:?}"); package @@ -121,16 +121,16 @@ impl Dag { conditional_data: &ConditionData<'_>, ) -> Result<()> { get_package_dependencies(p, conditional_data) - .and_then_ok(|(name, constr, kind)| { + .and_then_ok(|(name, version, kind)| { trace!( "Processing the following dependency of {} {}: {} {} {:?}", p.name(), p.version(), name, - constr, + version, kind ); - let packs = repo.find_with_version(&name, &constr); + let packs = repo.find_with_version(&name, &version); trace!( "Found the following matching packages in the repo: {:?}", packs @@ -141,7 +141,7 @@ impl Dag { p.name(), p.version(), name, - constr + version )); } @@ -154,10 +154,11 @@ impl Dag { }) { // TODO: It should be sufficient to process a single package of `packs`. // The `packs` vector contains a list of all packages in the repo that - // match the dependency specification (PackageName and - // PackageVersionConstraint). All packages must have the same name so only - // the version can differ -> we could simply pick the package with the most - // recent version and optionally omit a warning (or even abort with an error). + // match the dependency specification (PackageName and PackageVersion). + // TODO: Support PackageVersionConstraint: All packages must have the same + // name so only the version can differ -> we could simply pick the package + // with the most recent version and optionally omit a warning (or even + // abort with an error). packs.into_iter().try_for_each(|p| { let _ = progress.as_ref().map(|p| p.tick()); @@ -186,11 +187,11 @@ impl Dag { ) -> Result<()> { for (package, idx) in mappings { get_package_dependencies(package, conditional_data) - .and_then_ok(|(dep_name, dep_constr, dep_kind)| { + .and_then_ok(|(dep_name, dep_version, dep_kind)| { mappings .iter() .filter(|(pkg, _)| { - *pkg.name() == dep_name && dep_constr.matches(pkg.version()) + *pkg.name() == dep_name && *pkg.version() == dep_version }) .try_for_each(|(dep, dep_idx)| { dag.add_edge(*idx, *dep_idx, dep_kind.clone()) diff --git a/src/package/dependency/build.rs b/src/package/dependency/build.rs index ecb05f01..1b3195ed 100644 --- a/src/package/dependency/build.rs +++ b/src/package/dependency/build.rs @@ -15,7 +15,7 @@ use serde::Serialize; use crate::package::dependency::condition::Condition; use crate::package::dependency::ParseDependency; use crate::package::PackageName; -use crate::package::PackageVersionConstraint; +use crate::package::PackageVersion; /// A dependency that is packaged and is only required during build time #[derive(Serialize, Deserialize, Clone, Debug, Hash, Eq, PartialEq, Ord, PartialOrd)] @@ -35,7 +35,7 @@ impl AsRef for BuildDependency { } impl ParseDependency for BuildDependency { - fn parse_as_name_and_version(&self) -> Result<(PackageName, PackageVersionConstraint)> { + fn parse_as_name_and_version(&self) -> Result<(PackageName, PackageVersion)> { crate::package::dependency::parse_package_dependency_string_into_name_and_version( self.as_ref(), ) diff --git a/src/package/dependency/mod.rs b/src/package/dependency/mod.rs index 7f6c8c45..87dbff5c 100644 --- a/src/package/dependency/mod.rs +++ b/src/package/dependency/mod.rs @@ -14,7 +14,7 @@ use lazy_static::lazy_static; use regex::Regex; use crate::package::PackageName; -use crate::package::PackageVersionConstraint; +use crate::package::PackageVersion; mod build; pub use build::*; @@ -25,7 +25,7 @@ pub use runtime::*; pub mod condition; pub trait ParseDependency { - fn parse_as_name_and_version(&self) -> Result<(PackageName, PackageVersionConstraint)>; + fn parse_as_name_and_version(&self) -> Result<(PackageName, PackageVersion)>; } lazy_static! { @@ -43,12 +43,12 @@ lazy_static! { /// TODO: Reimplement using pom crate pub(in crate::package::dependency) fn parse_package_dependency_string_into_name_and_version( s: &str, -) -> Result<(PackageName, PackageVersionConstraint)> { +) -> Result<(PackageName, PackageVersion)> { let caps = crate::package::dependency::DEPENDENCY_PARSING_RE .captures(s) .ok_or_else(|| { anyhow!( - "Could not parse into package name and package version constraint: '{}'", + "Could not parse into package name and package version: '{}'", s ) })?; @@ -63,13 +63,14 @@ pub(in crate::package::dependency) fn parse_package_dependency_string_into_name_ .map(|m| String::from(m.as_str())) .ok_or_else(|| anyhow!("Could not parse version: '{}'", s))?; - let v = PackageVersionConstraint::try_from(vers).map_err(|e| { + // TODO: This is here temporarily to keep the version validation: + let _ = crate::package::PackageVersionConstraint::try_from(vers.clone()).map_err(|e| { e.context(anyhow!( "Could not parse the following package dependency string: {}", s )) })?; - Ok((PackageName::from(name), v)) + Ok((PackageName::from(name), PackageVersion::from(vers))) } #[cfg(test)] @@ -88,20 +89,17 @@ mod tests { let dependency_specification = format!("{name} ={version}"); let dep = Dependency::from(dependency_specification.clone()); - let (dep_name, dep_version_constraint) = dep.parse_as_name_and_version().unwrap(); + let (dep_name, dep_version) = dep.parse_as_name_and_version().unwrap(); - let version_constraint = PackageVersionConstraint::from_version( - String::from("="), - PackageVersion::from(version), - ); + let version = PackageVersion::from(version); assert_eq!( dep_name, PackageName::from(name), "Name check failed for input: {dependency_specification}" ); assert_eq!( - dep_version_constraint, version_constraint, - "Version constraint check failed for input: {dependency_specification}" + dep_version, version, + "Version check failed for input: {dependency_specification}" ); } @@ -138,6 +136,21 @@ mod tests { dep_parse_test("7z", "42"); } + #[test] + fn test_dependency_version_with_constraint() { + let name = "foobar"; + let version_constraint = "=1.42.37"; + + let dep = Dependency::from(format!("{name} {version_constraint}")); + let (dep_name, dep_version) = dep.parse_as_name_and_version().unwrap(); + + assert_eq!(dep_name, PackageName::from(name.to_string())); + assert_eq!( + dep_version, + PackageVersion::from(version_constraint.to_string()), + ); + } + #[test] fn test_complex_dependency_parsing() { dep_parse_test("0ad_", "42"); @@ -153,6 +166,5 @@ mod tests { dep_parse_expect_err("a *"); dep_parse_expect_err("a >2"); dep_parse_expect_err("a <2"); - dep_parse_expect_err("a 42"); } } diff --git a/src/package/dependency/runtime.rs b/src/package/dependency/runtime.rs index f0f80999..d8395448 100644 --- a/src/package/dependency/runtime.rs +++ b/src/package/dependency/runtime.rs @@ -15,7 +15,7 @@ use serde::Serialize; use crate::package::dependency::condition::Condition; use crate::package::dependency::ParseDependency; use crate::package::PackageName; -use crate::package::PackageVersionConstraint; +use crate::package::PackageVersion; /// A dependency that is packaged and is required during runtime #[derive(Serialize, Deserialize, Clone, Debug, Hash, Eq, PartialEq, Ord, PartialOrd)] @@ -48,7 +48,7 @@ impl From for Dependency { } impl ParseDependency for Dependency { - fn parse_as_name_and_version(&self) -> Result<(PackageName, PackageVersionConstraint)> { + fn parse_as_name_and_version(&self) -> Result<(PackageName, PackageVersion)> { crate::package::dependency::parse_package_dependency_string_into_name_and_version( self.as_ref(), ) diff --git a/src/package/version.rs b/src/package/version.rs index 6b9c6c86..6f260849 100644 --- a/src/package/version.rs +++ b/src/package/version.rs @@ -15,8 +15,10 @@ use anyhow::Context; use anyhow::Error; use anyhow::Result; use pom::parser::Parser as PomParser; +use regex::Regex; use serde::Deserialize; use serde::Serialize; +use tracing::info; use crate::util::parser::*; @@ -27,10 +29,35 @@ pub struct PackageVersionConstraint { } impl PackageVersionConstraint { + fn get_default_constraint() -> String { + // We default to `Op::Exact` (=) to enable partial version specification + // (e.g., only the major and minor or only the major version): + // https://docs.rs/semver/latest/semver/enum.Op.html#opexact + // Our own implementation of "=" (s. below) allows only a single version to match + // while `semver::Op::Exact` can result in multiple versions matching if only a + // "partial" version is provided. + String::from("=") + } + fn parser<'a>() -> PomParser<'a, u8, Self> { - (pom::parser::sym(b'=') + PackageVersion::parser()) + (pom::parser::sym(b'=').opt() + PackageVersion::parser()) .convert(|(constraint, version)| { - String::from_utf8(vec![constraint]).map(|c| (c, version)) + if let Some(c) = constraint { + String::from_utf8(vec![c]) + .map(|c| (c, version)) + .map_err(Error::from) + } else { + semver::VersionReq::parse(&(Self::get_default_constraint() + &version)) + .map(|_| ("".to_string(), version.clone())) + .map_err(Error::from) + // TODO: Drop this (for backward compatibility, we temporarily fallback to + // the old behaviour (as if the constraint `=` was specified) if the + // provided version cannot be parsed by the semver crate - this is required + // for somewhat "exotic" versions like the old OpenSSL 1.1.1w, web browsers + // with a fourth version number, or (unstable) releases based on the date): + .inspect_err(|e| info!("Couldn't parse version \"{version}\" as SemVer ({e}) -> falling back to strict version matching (={version})")) + .map_or(Ok(("=".to_string(), version)), Ok) + } }) .map(|(constraint, version)| PackageVersionConstraint { constraint, @@ -39,10 +66,35 @@ impl PackageVersionConstraint { } pub fn matches(&self, v: &PackageVersion) -> bool { - self.version == *v + use semver::{Version, VersionReq}; + match self.constraint.as_str() { + "" => { + let constraint = + VersionReq::parse(&(Self::get_default_constraint() + self.version.as_str())) + .unwrap(); + let version = Version::parse(v.as_str()) + .with_context(|| anyhow!("Failed to parse the package version as semver::Version")) + .or_else(|eo| v.clone().try_into().map_err(|e: Error| e.context(eo))) + .with_context(|| anyhow!("Also failed to parse the package version using our own SemVer converter")) + .unwrap_or_else(|e| panic!( + "Failed to parse the package version \"{}\" as SemVer to check if it matches \"{}\". Error: {:#}", + v, + constraint, + e + )); + + constraint.matches(&version) + } + "=" => self.version == *v, + _ => panic!( + "Internal error: Unsupported version constraint: {} (version: {})", + self.constraint, self.version + ), + } } #[cfg(test)] + #[allow(unused)] pub fn from_version(constraint: String, version: PackageVersion) -> Self { PackageVersionConstraint { constraint, @@ -66,7 +118,7 @@ impl TryFrom<&str> for PackageVersionConstraint { PackageVersionConstraint::parser() .parse(s.as_bytes()) .context(anyhow!("Failed to parse the following package version constraint: {}", s)) - .context("A package version constraint must have a comparator (only `=` is currently supported) and a version string, like so: =0.1.0") + .context("A package version constraint must have a version and an optional comparator (only `=` is currently supported, which is also the default), e.g.: =0.1.0") .map_err(Error::from) } } @@ -108,7 +160,69 @@ impl AsRef for PackageVersion { impl From for PackageVersion { fn from(s: String) -> Self { - PackageVersion(s) + PackageVersion(s.trim_start_matches('=').to_string()) + } +} + +impl TryInto for PackageVersion { + type Error = anyhow::Error; + + // TODO: Improve or replace this spaghetti code that we only use as a fallback (the unwrap()s + // should be safe as long as the static regex guarantees the assumptions): + fn try_into(self) -> Result { + // Warning: This regex must remain compatible to the one in the PackageVersion::parser below!: + let version_regex = Regex::new("^(?:([[:digit:]]+)|[-_.]|[[:alpha:]]+)").unwrap(); + // This regex is based on PackageVersion::parser() below. We use a capture group to extract + // the numbers and the "(?:exp)" syntax is for a non-capturing group. If it matches we'll + // have the entire match in \0 and if it's a number it'll be in \1. + + // Our input (version string): + let mut version_str = self.0.as_str(); + // Our results (extracted version numbers): + let mut versions = Vec::::new(); + + // This loop is dangerous... Ensure that the match gets removed from version_str in every + // iteration to avoid an endless loop! + while let Some(captures) = version_regex.captures(version_str) { + // For debugging: println!("{:?}", captures); + + let match_str = captures.get(0).unwrap().as_str(); // Unwrap safe as \0 always exists + version_str = &version_str[match_str.len()..]; + + if let Some(version_match) = captures.get(1) { + // We have a non-empty (version) number match + let version = version_match.as_str().parse()?; + versions.push(version); + } + } + + if version_str.is_empty() { + // Return what is hopefully the corresponding SemVer (the minor and patch version + // default to zero if they couldn't be extracted from PackageVersion): + Ok(semver::Version::new( + *versions.first().unwrap_or(&0), + *versions.get(1).unwrap_or(&0), + *versions.get(2).unwrap_or(&0), + )) + } else { + // We couldn't parse the entire version string -> report an error: + Err(anyhow!( + "The following rest of the package version couldn't be parsed: {}", + version_str + )) + .with_context(|| { + anyhow!( + "The regex \"{}\" for parsing the PackageVersion didn't match", + version_regex + ) + }) + .with_context(|| { + anyhow!( + "The PackageVersion \"{}\" couldn't be converted into a semver::Version", + self + ) + }) + } } } @@ -126,6 +240,10 @@ mod tests { #[test] fn test_parse_version_1() { + assert!(PackageVersion::parser().parse(b"1").is_ok()); + assert!(PackageVersion::parser().parse(b"1.42").is_ok()); + assert!(PackageVersion::parser().parse(b"1.42.37").is_ok()); + assert!(PackageVersion::parser().parse(b"").is_err()); assert!(PackageVersion::parser().parse(b"=").is_err()); assert!(PackageVersion::parser().parse(b"*1").is_err()); @@ -137,6 +255,10 @@ mod tests { assert!(PackageVersion::parser().parse(b"=a1").is_err()); assert!(PackageVersion::parser().parse(b"a").is_err()); + assert!(PackageVersionConstraint::parser().parse(b"1").is_ok()); + assert!(PackageVersionConstraint::parser().parse(b"1.42").is_ok()); + assert!(PackageVersionConstraint::parser().parse(b"1.42.37").is_ok()); + assert!(PackageVersionConstraint::parser().parse(b"").is_err()); assert!(PackageVersionConstraint::parser().parse(b"=").is_err()); assert!(PackageVersionConstraint::parser().parse(b"*1").is_err()); @@ -146,7 +268,6 @@ mod tests { assert!(PackageVersionConstraint::parser().parse(b"=.a").is_err()); assert!(PackageVersionConstraint::parser().parse(b"=.1").is_err()); assert!(PackageVersionConstraint::parser().parse(b"=a1").is_err()); - assert!(PackageVersionConstraint::parser().parse(b"1").is_err()); assert!(PackageVersionConstraint::parser().parse(b"a").is_err()); } diff --git a/src/repository/repository.rs b/src/repository/repository.rs index f6db4d52..bc801f2d 100644 --- a/src/repository/repository.rs +++ b/src/repository/repository.rs @@ -259,11 +259,11 @@ impl Repository { pub fn find_with_version<'a>( &'a self, name: &PackageName, - vc: &PackageVersionConstraint, + version: &PackageVersion, ) -> Vec<&'a Package> { self.inner .iter() - .filter(|((n, v), _)| n == name && vc.matches(v)) + .filter(|((n, v), _)| n == name && v == version) .map(|(_, p)| p) .collect() } @@ -393,9 +393,9 @@ pub mod tests { let repo = Repository::from(btree); - let constraint = PackageVersionConstraint::from_version(String::from("="), pversion("2")); + let version = pversion("=2"); - let ps = repo.find_with_version(&pname("a"), &constraint); + let ps = repo.find_with_version(&pname("a"), &version); assert_eq!(ps.len(), 1); let p = ps.first().unwrap(); @@ -409,9 +409,8 @@ pub mod tests { use crate::package::Package; fn get_pkg(repo: &Repository, name: &str, version: &str) -> Package { - let constraint = - PackageVersionConstraint::from_version(String::from("="), pversion(version)); - let pkgs = repo.find_with_version(&pname(name), &constraint); + let version = pversion(version); + let pkgs = repo.find_with_version(&pname(name), &version); assert_eq!(pkgs.len(), 1, "Failed to find pkg: {name} ={version}"); (*pkgs.first().unwrap()).clone() } diff --git a/src/util/filters.rs b/src/util/filters.rs index ff32b288..3bb4f375 100644 --- a/src/util/filters.rs +++ b/src/util/filters.rs @@ -16,6 +16,7 @@ use tracing::trace; use crate::package::Package; use crate::package::PackageName; +use crate::package::PackageVersion; use crate::package::PackageVersionConstraint; use crate::package::ParseDependency; @@ -79,12 +80,28 @@ pub fn build_package_filter_by_name(name: PackageName) -> impl filters::filter:: } } +pub fn build_package_filter_by_version( + version: PackageVersion, +) -> impl filters::filter::Filter { + move |p: &Package| { + trace!("Checking {:?} -> version == {}", p, version); + *p.version() == version + } +} + pub fn build_package_filter_by_version_constraint( - constraint: PackageVersionConstraint, + version_constraint: Option, ) -> impl filters::filter::Filter { move |p: &Package| { - trace!("Checking {:?} -> version matches {:?}", p, constraint); - constraint.matches(p.version()) + trace!( + "Checking {:?} -> version matches constraint: {:?}", + p, + version_constraint + ); + version_constraint + .as_ref() + .map(|v| v.matches(p.version())) + .unwrap_or(true) } }