Merge pull request science-computing#450 from primeos-work/rework-and…

…-improve-package-version-constraints Rework and improve the code around package version and package version constraints
primeos-work · Jan 14, 2025 · 778d37f · 778d37f
2 parents f7c68cb + 4fb877a
commit 778d37f
Show file tree

Hide file tree

Showing 20 changed files with 285 additions and 124 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git 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
@@ -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,15 +880,15 @@ pub fn cli() -> Command {
                 )
 
                 .arg(Arg::new("package_name")
-                    .required(false)
+                    .required(true)
                     .index(1)
                     .value_name("PKG")
                     .help("The name of the package")
                     .requires("package_version")
                 )
 
                 .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
@@ -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::<String>("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
@@ -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::<String>("package_version_constraint")
+        let version = matches
+            .get_one::<String>("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
@@ -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::<String>("env_filter")

diff --git 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
@@ -37,7 +37,7 @@ pub async fn lint(
         .map(|s| s.to_owned())
         .map(PackageName::from);
     let pvers = matches
-        .get_one::<String>("package_version")
+        .get_one::<String>("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
@@ -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
@@ -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
@@ -217,7 +217,7 @@ pub async fn download(
         .map(|s| s.to_owned())
         .map(PackageName::from);
     let pvers = matches
-        .get_one::<String>("package_version")
+        .get_one::<String>("package_version_constraint")
         .map(|s| s.to_owned())
         .map(PackageVersionConstraint::try_from)
         .transpose()?;