Skip to content

Commit

Permalink
Fix spk export not respecting repository arguments
Browse files Browse the repository at this point in the history
In particular, `spk export --local-repo-only ...` would export packages
from the origin repo.

This necessitated adding another level of "handle" indirection to deal
with the different type flavors of SpfsRepository.

Signed-off-by: J Robert Ray <jrray@jrray.org>
  • Loading branch information
jrray committed Jun 28, 2024
1 parent fed4bcf commit cf2e783
Show file tree
Hide file tree
Showing 11 changed files with 147 additions and 122 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 11 additions & 2 deletions crates/spk-build/src/archive_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use rstest::rstest;
use spk_schema::foundation::option_map;
use spk_schema::ident_ops::NormalizedTagStrategy;
use spk_schema::{recipe, Package};
use spk_storage::export_package;
use spk_storage::fixtures::*;
use spk_storage::{export_package, SpfsRepositoryHandle};

use crate::{BinaryPackageBuilder, BuildSource};

Expand All @@ -28,7 +28,16 @@ async fn test_archive_create_parents() {
.await
.unwrap();
let filename = rt.tmpdir.path().join("deep/nested/path/archive.spk");
export_package::<NormalizedTagStrategy>(&spec.ident().to_any(), filename)
let repo = match &*rt.tmprepo {
spk_solve::RepositoryHandle::SPFS(repo) => SpfsRepositoryHandle::Normalized(repo),
spk_solve::RepositoryHandle::SPFSWithVerbatimTags(repo) => {
SpfsRepositoryHandle::Verbatim(repo)
}
spk_solve::RepositoryHandle::Mem(_) | spk_solve::RepositoryHandle::Runtime(_) => {
panic!("only spfs repositories are supported")
}
};
export_package::<NormalizedTagStrategy>(&[repo], &spec.ident().to_any(), filename)
.await
.expect("export should create dirs as needed");
}
1 change: 1 addition & 0 deletions crates/spk-cli/group3/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,5 @@ tokio = "1.20"
[dev-dependencies]
tar = "0.4.3"
spk-build = { workspace = true }
spk-solve = { workspace = true }
rstest = { workspace = true }
26 changes: 19 additions & 7 deletions crates/spk-cli/group3/src/cmd_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ use std::sync::Arc;

use clap::{Args, ValueHint};
use colored::Colorize;
use miette::Result;
use miette::{bail, Result};
use spk_cli_common::{flags, CommandArgs, Run};
use spk_schema::ident_ops::{NormalizedTagStrategy, VerbatimTagStrategy};
use spk_storage as storage;
use storage::SpfsRepositoryHandle;

#[cfg(test)]
#[path = "./cmd_export_test.rs"]
Expand Down Expand Up @@ -54,14 +55,26 @@ impl Run for Export {
let options = self.options.get_options()?;

let names_and_repos = self.repos.get_repos_for_non_destructive_operation().await?;
let repos = names_and_repos
let repo_handles = names_and_repos
.into_iter()
.map(|(_, r)| Arc::new(r))
.collect::<Vec<_>>();
let repos = repo_handles
.iter()
.map(|repo| match &**repo {
storage::RepositoryHandle::SPFS(repo) => Ok(SpfsRepositoryHandle::Normalized(repo)),
storage::RepositoryHandle::SPFSWithVerbatimTags(repo) => {
Ok(SpfsRepositoryHandle::Verbatim(repo))
}
storage::RepositoryHandle::Mem(_) | storage::RepositoryHandle::Runtime(_) => {
bail!("Only spfs repositories are supported")
}
})
.collect::<Result<Vec<_>>>()?;

let pkg = self
.requests
.parse_idents(&options, [self.package.as_str()], &repos)
.parse_idents(&options, [self.package.as_str()], repo_handles.as_slice())
.await?
.pop()
.unwrap();
Expand All @@ -73,12 +86,11 @@ impl Run for Export {
let filename = self.filename.clone().unwrap_or_else(|| {
std::path::PathBuf::from(format!("{}_{}{build}.spk", pkg.name(), pkg.version()))
});
// TODO: this doesn't take the repos as an argument, but probably
// should. It assumes/uses 'local' and 'origin' repos internally.
let res = if self.legacy_spk_version_tags_for_writes {
storage::export_package::<VerbatimTagStrategy>(&pkg, &filename).await
storage::export_package::<VerbatimTagStrategy>(repos.as_slice(), &pkg, &filename).await
} else {
storage::export_package::<NormalizedTagStrategy>(&pkg, &filename).await
storage::export_package::<NormalizedTagStrategy>(repos.as_slice(), &pkg, &filename)
.await
};
if let Err(spk_storage::Error::PackageNotFound(_)) = res {
tracing::warn!("Ensure that you are specifying at least a package and");
Expand Down
21 changes: 6 additions & 15 deletions crates/spk-cli/group3/src/cmd_export_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,18 @@
// https://github.com/imageworks/spk

use rstest::rstest;
use spfs::config::Remote;
use spfs::prelude::*;
use spfs::RemoteAddress;
use spk_build::{BinaryPackageBuilder, BuildSource};
use spk_schema::foundation::option_map;
use spk_schema::ident_ops::NormalizedTagStrategy;
use spk_schema::{recipe, Package};
use spk_storage::fixtures::*;
use spk_storage::SpfsRepositoryHandle;

#[rstest]
#[tokio::test]
async fn test_export_works_with_missing_builds() {
let mut rt = spfs_runtime().await;

// exporting requires a configured "origin" repo.
let remote_repo = spfsrepo().await;
rt.add_remote_repo(
"origin",
Remote::Address(RemoteAddress {
address: remote_repo.address().clone(),
}),
)
.unwrap();
let rt = spfs_runtime().await;

let spec = recipe!(
{
Expand Down Expand Up @@ -53,7 +42,7 @@ async fn test_export_works_with_missing_builds() {
// Now that these two builds are created, remove the `spk/pkg` tags for one
// of them. The publish is still expected to succeed; it should publish
// the remaining valid build.
match &*rt.tmprepo {
let repo = match &*rt.tmprepo {
spk_storage::RepositoryHandle::SPFS(spfs) => {
for spec in [
format!("{}", blue_spec.ident().build()),
Expand All @@ -66,13 +55,15 @@ async fn test_export_works_with_missing_builds() {
.unwrap();
spfs.remove_tag_stream(&tag).await.unwrap();
}
SpfsRepositoryHandle::Normalized(spfs)
}
_ => panic!("only implemented for spfs repos"),
}
};

let filename = rt.tmpdir.path().join("archive.spk");
filename.ensure();
spk_storage::export_package::<NormalizedTagStrategy>(
&[repo],
red_spec.ident().clone().to_version().to_any(None),
&filename,
)
Expand Down
12 changes: 11 additions & 1 deletion crates/spk-cli/group3/src/cmd_import_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use spk_schema::foundation::option_map;
use spk_schema::ident_ops::NormalizedTagStrategy;
use spk_schema::{recipe, Package};
use spk_storage::fixtures::*;
use spk_storage::SpfsRepositoryHandle;

#[rstest]
#[tokio::test]
Expand All @@ -30,7 +31,16 @@ async fn test_archive_io() {

let filename = rt.tmpdir.path().join("archive.spk");
filename.ensure();
spk_storage::export_package::<NormalizedTagStrategy>(spec.ident().to_any(), &filename)
let repo = match &*rt.tmprepo {
spk_solve::RepositoryHandle::SPFS(repo) => SpfsRepositoryHandle::Normalized(repo),
spk_solve::RepositoryHandle::SPFSWithVerbatimTags(repo) => {
SpfsRepositoryHandle::Verbatim(repo)
}
spk_solve::RepositoryHandle::Mem(_) | spk_solve::RepositoryHandle::Runtime(_) => {
panic!("only spfs repositories are supported")
}
};
spk_storage::export_package::<NormalizedTagStrategy>(&[repo], spec.ident().to_any(), &filename)
.await
.expect("failed to export");
let mut actual = Vec::new();
Expand Down
1 change: 1 addition & 0 deletions crates/spk-storage/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,4 @@ tracing = { workspace = true }
tracing-subscriber = "0.3.17"
ulid = { workspace = true }
url = "2.2"
variantly = { workspace = true }
1 change: 1 addition & 0 deletions crates/spk-storage/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,6 @@ pub use storage::{
RepositoryHandle,
RuntimeRepository,
SpfsRepository,
SpfsRepositoryHandle,
Storage,
};
Loading

0 comments on commit cf2e783

Please sign in to comment.