Skip to content

Commit

Permalink
Adds default filter using current host's distro setting, adds --nohos…
Browse files Browse the repository at this point in the history
…t option to disable this.

Using --filter-by opt=value will also override the default filter.

Signed-off-by: David Gilligan-Cook <dcook@imageworks.com>
  • Loading branch information
dcookspi committed Mar 13, 2024
1 parent 00fdc1a commit 4610f10
Show file tree
Hide file tree
Showing 2 changed files with 175 additions and 19 deletions.
27 changes: 23 additions & 4 deletions crates/spk-cli/group2/src/cmd_ls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ use spk_schema::foundation::ident_component::ComponentSet;
use spk_schema::foundation::name::{PkgName, PkgNameBuf};
use spk_schema::ident::{parse_ident, AnyIdent};
use spk_schema::ident_ops::parsing::{ident_parts, IdentParts, KNOWN_REPOSITORY_NAMES};
use spk_schema::name::OptNameBuf;
use spk_schema::name::{OptName, OptNameBuf};
use spk_schema::option_map::HOST_OPTIONS;
use spk_schema::spec_ops::WithVersion;
use spk_schema::{Deprecate, OptionMap, Package, Spec};
use spk_storage as storage;
Expand Down Expand Up @@ -140,9 +141,11 @@ pub struct Ls<Output: Default = Console> {
#[clap(long, name = "OPT=VALUE")]
filter_by: Option<OptFilter>,

// TODO: disable the always check the host options setting, which I haven't implemented yet
// #[cla[(long)]
// nohost: bool
/// Disable only showing items that have a build that matches the
/// current host's distro host option.
#[clap(long)]
nohost: bool,

/// Given a name, list versions. Given a name/version list builds.
///
/// If nothing is provided, list all available packages.
Expand All @@ -164,6 +167,22 @@ impl<T: Output> Run for Ls<T> {
return self.list_recursively(repos).await;
}

// Set the default filter to the current host's distro option.
// --nohost and --filter-by will override this. Note: this
// only uses the 'distro' option.
// TODO: longer term make this configurable and support multiple filters
if !self.nohost && self.filter_by.is_none() {
let host_options = HOST_OPTIONS.get()?;
if let Some(value) = host_options.get(OptName::distro()) {
self.filter_by = Some(OptFilter {
name: OptName::distro().into(),
op: FilterOperator::MustMatchNameValue,
value: value.to_string(),
});
}
}
tracing::debug!("Filter is: {:?}", self.filter_by);

let mut results = Vec::new();
match &self.package {
None => {
Expand Down
167 changes: 152 additions & 15 deletions crates/spk-cli/group2/src/cmd_ls_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ use spfs::storage::EntryType;
use spfs::RemoteAddress;
use spk_schema::foundation::ident_component::Component;
use spk_schema::ident_ops::VerbatimTagStrategy;
use spk_schema::name::OptName;
use spk_schema::recipe;
use spk_solve::spec;
use spk_storage::fixtures::*;
use spk_storage::RepositoryHandle;

use super::{Ls, Output, Run};
use crate::cmd_ls::HOST_OPTIONS;

#[derive(Default)]
struct OutputToVec {
Expand Down Expand Up @@ -57,10 +59,10 @@ async fn test_ls_trivially_works() {
assert_eq!(opt.ls.output.vec.len(), 0);
}

/// `spk ls` is expected to list packages in the configured remote
/// repositories.
/// `spk ls --nohost` is expected to list all packages in the configured
/// remote repositories.
#[tokio::test]
async fn test_ls_shows_remote_packages() {
async fn test_ls_shows_remote_packages_with_no_host() {
let mut rt = spfs_runtime().await;
let remote_repo = spfsrepo().await;

Expand Down Expand Up @@ -88,7 +90,136 @@ async fn test_ls_shows_remote_packages() {
.await
.unwrap();

let mut opt = Opt::try_parse_from([] as [&str; 0]).unwrap();
let mut opt = Opt::try_parse_from(["ls", "--nohost"]).unwrap();
opt.ls.run().await.unwrap();
assert_ne!(opt.ls.output.vec.len(), 0);
}

/// `spk ls` is expected to list packages in the configured remote
/// repositories that match the default filter for the current host
#[tokio::test]
async fn test_ls_shows_remote_packages_with_default_filter() {
let mut rt = spfs_runtime().await;
let remote_repo = spfsrepo().await;

// Populate the "origin" repo with one package.
// The "local" repo is empty.

rt.add_remote_repo(
"origin",
Remote::Address(RemoteAddress {
address: remote_repo.address().clone(),
}),
)
.unwrap();

let recipe = recipe!({"pkg": "my-pkg/1.0.0"});
remote_repo.publish_recipe(&recipe).await.unwrap();
let host_options = HOST_OPTIONS.get().unwrap();
let spec = spec!({"pkg": "my-pkg/1.0.0/BGSHW3CN",
"build": {
"options": [
{"var": format!("distro/{}", host_options.get(OptName::distro()).unwrap()) }
]
}});
remote_repo
.publish_package(
&spec,
&vec![(Component::Run, empty_layer_digest())]
.into_iter()
.collect(),
)
.await
.unwrap();

let mut opt = Opt::try_parse_from(["ls"]).unwrap();
opt.ls.run().await.unwrap();
assert_ne!(opt.ls.output.vec.len(), 0);
}

/// `spk ls --filter-by opt=value` is expected to list packages in the
/// configured remote repositories that match the filter, which should
/// override the default host distro filter
#[tokio::test]
async fn test_ls_shows_remote_packages_with_filter_by() {
let mut rt = spfs_runtime().await;
let remote_repo = spfsrepo().await;

// Populate the "origin" repo with one package.
// The "local" repo is empty.

rt.add_remote_repo(
"origin",
Remote::Address(RemoteAddress {
address: remote_repo.address().clone(),
}),
)
.unwrap();

let recipe = recipe!({"pkg": "my-pkg/1.0.0"});
remote_repo.publish_recipe(&recipe).await.unwrap();
let spec = spec!({"pkg": "my-pkg/1.0.0/BGSHW3CN",
"build": {
"options": [
{"var": "distro/futureOs" },
{"var": "testopt/testvalue" }

Check warning on line 165 in crates/spk-cli/group2/src/cmd_ls_test.rs

View workflow job for this annotation

GitHub Actions / spellcheck

Unknown word (testopt)

Check warning on line 165 in crates/spk-cli/group2/src/cmd_ls_test.rs

View workflow job for this annotation

GitHub Actions / spellcheck

Unknown word (testvalue)

Check warning on line 165 in crates/spk-cli/group2/src/cmd_ls_test.rs

View workflow job for this annotation

GitHub Actions / spellcheck

Unknown word (testopt)

Check warning on line 165 in crates/spk-cli/group2/src/cmd_ls_test.rs

View workflow job for this annotation

GitHub Actions / spellcheck

Unknown word (testvalue)
]
}});
remote_repo
.publish_package(
&spec,
&vec![(Component::Run, empty_layer_digest())]
.into_iter()
.collect(),
)
.await
.unwrap();

let mut opt = Opt::try_parse_from(["ls", "--filter-by", "testopt=testvalue"]).unwrap();

Check warning on line 178 in crates/spk-cli/group2/src/cmd_ls_test.rs

View workflow job for this annotation

GitHub Actions / spellcheck

Unknown word (testopt)

Check warning on line 178 in crates/spk-cli/group2/src/cmd_ls_test.rs

View workflow job for this annotation

GitHub Actions / spellcheck

Unknown word (testvalue)

Check warning on line 178 in crates/spk-cli/group2/src/cmd_ls_test.rs

View workflow job for this annotation

GitHub Actions / spellcheck

Unknown word (testopt)

Check warning on line 178 in crates/spk-cli/group2/src/cmd_ls_test.rs

View workflow job for this annotation

GitHub Actions / spellcheck

Unknown word (testvalue)
opt.ls.run().await.unwrap();
assert_ne!(opt.ls.output.vec.len(), 0);
}

/// `spk ls --filter-by opt?=value` is expected to list packages in the
/// configured remote repositories that match the filter, which should
/// override the default host distro filter
#[tokio::test]
async fn test_ls_shows_remote_packages_with_filter_by_ok_if_name_missing() {
let mut rt = spfs_runtime().await;
let remote_repo = spfsrepo().await;

// Populate the "origin" repo with one package.
// The "local" repo is empty.

rt.add_remote_repo(
"origin",
Remote::Address(RemoteAddress {
address: remote_repo.address().clone(),
}),
)
.unwrap();

let recipe = recipe!({"pkg": "my-pkg/1.0.0"});
remote_repo.publish_recipe(&recipe).await.unwrap();
let spec = spec!({"pkg": "my-pkg/1.0.0/BGSHW3CN",
"build": {
"options": [
{"var": "testopt/testvalue" }

Check warning on line 207 in crates/spk-cli/group2/src/cmd_ls_test.rs

View workflow job for this annotation

GitHub Actions / spellcheck

Unknown word (testopt)

Check warning on line 207 in crates/spk-cli/group2/src/cmd_ls_test.rs

View workflow job for this annotation

GitHub Actions / spellcheck

Unknown word (testvalue)

Check warning on line 207 in crates/spk-cli/group2/src/cmd_ls_test.rs

View workflow job for this annotation

GitHub Actions / spellcheck

Unknown word (testopt)

Check warning on line 207 in crates/spk-cli/group2/src/cmd_ls_test.rs

View workflow job for this annotation

GitHub Actions / spellcheck

Unknown word (testvalue)
]
}});
remote_repo
.publish_package(
&spec,
&vec![(Component::Run, empty_layer_digest())]
.into_iter()
.collect(),
)
.await
.unwrap();

// Filtering by a opt name, distro, that the package does not
// have. This is ok with the ?= operator.
let mut opt = Opt::try_parse_from(["ls", "--filter-by", "distro?=testvalue"]).unwrap();

Check warning on line 222 in crates/spk-cli/group2/src/cmd_ls_test.rs

View workflow job for this annotation

GitHub Actions / spellcheck

Unknown word (testvalue)

Check warning on line 222 in crates/spk-cli/group2/src/cmd_ls_test.rs

View workflow job for this annotation

GitHub Actions / spellcheck

Unknown word (testvalue)
opt.ls.run().await.unwrap();
assert_ne!(opt.ls.output.vec.len(), 0);
}
Expand Down Expand Up @@ -137,7 +268,7 @@ async fn test_ls_shows_local_and_remote_packages() {
.await
.unwrap();

let mut opt = Opt::try_parse_from([] as [&str; 0]).unwrap();
let mut opt = Opt::try_parse_from(["ls", "--nohost"]).unwrap();
opt.ls.run().await.unwrap();
assert_eq!(opt.ls.output.vec.len(), 2);
}
Expand Down Expand Up @@ -185,7 +316,7 @@ async fn test_ls_dash_l_shows_local_packages_only() {
.await
.unwrap();

let mut opt = Opt::try_parse_from(["ls", "-L"]).unwrap();
let mut opt = Opt::try_parse_from(["ls", "-L", "--nohost"]).unwrap();
opt.ls.run().await.unwrap();
assert_eq!(opt.ls.output.vec.len(), 1);
assert_eq!(opt.ls.output.vec.first().unwrap(), "my-local-pkg");
Expand Down Expand Up @@ -235,7 +366,7 @@ async fn test_ls_dash_r_shows_local_and_remote_packages() {
.await
.unwrap();

let mut opt = Opt::try_parse_from(["ls", "-r", "origin"]).unwrap();
let mut opt = Opt::try_parse_from(["ls", "-r", "origin", "--nohost"]).unwrap();
opt.ls.run().await.unwrap();
assert_eq!(opt.ls.output.vec.len(), 2);
}
Expand Down Expand Up @@ -284,7 +415,7 @@ async fn test_ls_dash_dash_no_local_repo_shows_remote_packages_only() {
.await
.unwrap();

let mut opt = Opt::try_parse_from(["ls", "--no-local-repo"]).unwrap();
let mut opt = Opt::try_parse_from(["ls", "--no-local-repo", "--nohost"]).unwrap();
opt.ls.run().await.unwrap();
assert_eq!(opt.ls.output.vec.len(), 1);
assert_eq!(opt.ls.output.vec.first().unwrap(), "my-remote-pkg");
Expand Down Expand Up @@ -334,7 +465,7 @@ async fn test_ls_dash_dash_disable_repo_shows_local_packages_only() {
.await
.unwrap();

let mut opt = Opt::try_parse_from(["ls", "--disable-repo", "origin"]).unwrap();
let mut opt = Opt::try_parse_from(["ls", "--disable-repo", "origin", "--nohost"]).unwrap();
opt.ls.run().await.unwrap();
assert_eq!(opt.ls.output.vec.len(), 1);
assert_eq!(opt.ls.output.vec.first().unwrap(), "my-local-pkg");
Expand Down Expand Up @@ -365,7 +496,7 @@ async fn test_ls_succeeds_for_package_with_no_version_spec() {
.await
.unwrap();

let mut opt = Opt::try_parse_from(["ls", "my-pkg"]).unwrap();
let mut opt = Opt::try_parse_from(["ls", "my-pkg", "--nohost"]).unwrap();
opt.ls.run().await.unwrap();
assert_eq!(
opt.ls.output.warnings.len(),
Expand Down Expand Up @@ -405,7 +536,7 @@ async fn test_ls_hides_deprecated_version() {
.unwrap();

// `ls` without showing deprecated
let mut opt = Opt::try_parse_from(["ls", "my-pkg"]).unwrap();
let mut opt = Opt::try_parse_from(["ls", "my-pkg", "--nohost"]).unwrap();
opt.ls.run().await.unwrap();
assert_eq!(
opt.ls.output.warnings.len(),
Expand All @@ -421,7 +552,7 @@ async fn test_ls_hides_deprecated_version() {
);

// `ls` with showing deprecated
let mut opt = Opt::try_parse_from(["ls", "--deprecated", "my-pkg"]).unwrap();
let mut opt = Opt::try_parse_from(["ls", "--deprecated", "my-pkg", "--nohost"]).unwrap();
opt.ls.run().await.unwrap();
assert_eq!(
opt.ls.output.warnings.len(),
Expand Down Expand Up @@ -475,7 +606,7 @@ async fn test_ls_shows_partially_deprecated_version() {
.unwrap();

// `ls` without showing deprecated
let mut opt = Opt::try_parse_from(["ls", "my-pkg"]).unwrap();
let mut opt = Opt::try_parse_from(["ls", "my-pkg", "--nohost"]).unwrap();
opt.ls.run().await.unwrap();
assert_eq!(
opt.ls.output.warnings.len(),
Expand All @@ -489,7 +620,7 @@ async fn test_ls_shows_partially_deprecated_version() {
assert_eq!(opt.ls.output.vec.first().unwrap(), "1.0.0");

// `ls` with showing deprecated
let mut opt = Opt::try_parse_from(["ls", "--deprecated", "my-pkg"]).unwrap();
let mut opt = Opt::try_parse_from(["ls", "--deprecated", "my-pkg", "--nohost"]).unwrap();
opt.ls.run().await.unwrap();
assert_eq!(
opt.ls.output.warnings.len(),
Expand Down Expand Up @@ -549,7 +680,13 @@ async fn test_ls_succeeds_for_package_saved_with_legacy_version_tag() {
_ => panic!("expected SPFSWithVerbatimTags"),
}

let mut opt = Opt::try_parse_from(["ls", "--legacy-spk-version-tags", "my-local-pkg"]).unwrap();
let mut opt = Opt::try_parse_from([
"ls",
"--legacy-spk-version-tags",
"my-local-pkg",
"--nohost",
])
.unwrap();
opt.ls.run().await.unwrap();
assert_eq!(opt.ls.output.vec.len(), 1);
}

0 comments on commit 4610f10

Please sign in to comment.