diff --git a/crates/spk-cli/group3/src/cmd_export.rs b/crates/spk-cli/group3/src/cmd_export.rs index 7fd6214983..a06ad36b06 100644 --- a/crates/spk-cli/group3/src/cmd_export.rs +++ b/crates/spk-cli/group3/src/cmd_export.rs @@ -10,6 +10,10 @@ use colored::Colorize; use spk_cli_common::{flags, CommandArgs, Run}; use spk_storage::{self as storage}; +#[cfg(test)] +#[path = "./cmd_export_test.rs"] +mod cmd_export_test; + /// Export a package as a tar file #[derive(Args)] pub struct Export { diff --git a/crates/spk-cli/group3/src/cmd_export_test.rs b/crates/spk-cli/group3/src/cmd_export_test.rs new file mode 100644 index 0000000000..2a25af3f95 --- /dev/null +++ b/crates/spk-cli/group3/src/cmd_export_test.rs @@ -0,0 +1,124 @@ +use rstest::rstest; +use spfs::config::Remote; +use spfs::RemoteAddress; +use spk_build::{BinaryPackageBuilder, BuildSource}; +use spk_schema::foundation::option_map; +use spk_schema::{recipe, Package}; +use spk_storage::fixtures::*; + +#[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 spec = recipe!( + { + "pkg": "spk-export-test/0.0.1", + "build": { + "options": [ + {"var": "color"}, + ], + "script": "touch /spfs/file.txt", + }, + } + ); + rt.tmprepo.publish_recipe(&spec).await.unwrap(); + let (blue_spec, _) = BinaryPackageBuilder::from_recipe(spec.clone()) + .with_source(BuildSource::LocalPath(".".into())) + .build_and_publish(option_map! {"color" => "blue"}, &*rt.tmprepo) + .await + .unwrap(); + let (red_spec, _) = BinaryPackageBuilder::from_recipe(spec) + .with_source(BuildSource::LocalPath(".".into())) + .build_and_publish(option_map! {"color" => "red"}, &*rt.tmprepo) + .await + .unwrap(); + + // 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 { + spk_storage::RepositoryHandle::SPFS(spfs) => { + for spec in [ + format!("{}", blue_spec.ident().build()), + format!("{}/build", blue_spec.ident().build()), + format!("{}/run", blue_spec.ident().build()), + ] { + let tag = spfs::tracking::TagSpec::parse(format!( + "spk/pkg/spk-export-test/0.0.1/{spec}", + )) + .unwrap(); + spfs.remove_tag_stream(&tag).await.unwrap(); + } + } + _ => panic!("only implemented for spfs repos"), + } + + let filename = rt.tmpdir.path().join("archive.spk"); + filename.ensure(); + spk_storage::export_package( + red_spec.ident().clone().to_version().to_any(None), + &filename, + ) + .await + .expect("failed to export"); + let mut actual = Vec::new(); + let mut tarfile = tar::Archive::new(std::fs::File::open(&filename).unwrap()); + for entry in tarfile.entries().unwrap() { + let filename = entry.unwrap().path().unwrap().to_string_lossy().to_string(); + if filename.contains('/') && !filename.contains("tags") { + // ignore specific object data for this test + continue; + } + actual.push(filename); + } + actual.sort(); + assert_eq!( + actual, + vec![ + "VERSION".to_string(), + "objects".to_string(), + "payloads".to_string(), + "renders".to_string(), + "tags".to_string(), + "tags/spk".to_string(), + "tags/spk/pkg".to_string(), + "tags/spk/pkg/spk-export-test".to_string(), + "tags/spk/pkg/spk-export-test/0.0.1".to_string(), + format!( + "tags/spk/pkg/spk-export-test/0.0.1/{}", + red_spec.ident().build() + ), + format!( + "tags/spk/pkg/spk-export-test/0.0.1/{}.tag", + red_spec.ident().build() + ), + format!( + "tags/spk/pkg/spk-export-test/0.0.1/{}/build.tag", + red_spec.ident().build() + ), + format!( + "tags/spk/pkg/spk-export-test/0.0.1/{}/run.tag", + red_spec.ident().build() + ), + "tags/spk/spec".to_string(), + "tags/spk/spec/spk-export-test".to_string(), + "tags/spk/spec/spk-export-test/0.0.1".to_string(), + "tags/spk/spec/spk-export-test/0.0.1.tag".to_string(), + format!( + "tags/spk/spec/spk-export-test/0.0.1/{}.tag", + red_spec.ident().build() + ), + ] + ); +} diff --git a/crates/spk-storage/src/storage/archive.rs b/crates/spk-storage/src/storage/archive.rs index a92416a124..9f621b421e 100644 --- a/crates/spk-storage/src/storage/archive.rs +++ b/crates/spk-storage/src/storage/archive.rs @@ -77,37 +77,89 @@ where to_transfer.insert(pkg.with_build(None)); } - for pkg in to_transfer.into_iter() { - if pkg.is_embedded() { + for transfer_pkg in to_transfer.into_iter() { + if transfer_pkg.is_embedded() { // Don't attempt to export an embedded package; the stub // will be recreated if exporting its provider. continue; } - let local_err = match copy_any(pkg.clone(), &local_repo, &target_repo).await { + enum CopyResult { + VersionNotFound, + BuildNotFound, + Err(Error), + } + + impl CopyResult { + fn or(self, other: CopyResult) -> Option { + if let CopyResult::Err(err) = self { + Some(err) + } else if let CopyResult::Err(err) = other { + Some(err) + } else { + None + } + } + } + + let local_err = match copy_any(transfer_pkg.clone(), &local_repo, &target_repo).await { Ok(_) => continue, Err(Error::SpkValidatorsError( - spk_schema::validators::Error::PackageNotFoundError(_), - )) => None, - Err(err) => Some(err), + spk_schema::validators::Error::PackageNotFoundError(ident), + )) => { + if ident.build().is_some() { + CopyResult::BuildNotFound + } else { + CopyResult::VersionNotFound + } + } + Err(err) => CopyResult::Err(err), }; if remote_repo.is_err() { return remote_repo.map(|_| ()); } - let remote_err = - match copy_any(pkg.clone(), remote_repo.as_ref().unwrap(), &target_repo).await { - Ok(_) => continue, - Err(Error::SpkValidatorsError( - spk_schema::validators::Error::PackageNotFoundError(_), - )) => None, - Err(err) => Some(err), - }; + let remote_err = match copy_any( + transfer_pkg.clone(), + remote_repo.as_ref().unwrap(), + &target_repo, + ) + .await + { + Ok(_) => continue, + Err(Error::SpkValidatorsError( + spk_schema::validators::Error::PackageNotFoundError(ident), + )) => { + if ident.build().is_some() { + CopyResult::BuildNotFound + } else { + CopyResult::VersionNotFound + } + } + Err(err) => CopyResult::Err(err), + }; + + // `list_package_builds` can return builds that only exist as spfs tags + // under `spk/spec`, meaning the build doesn't really exist. Ignore + // `PackageNotFoundError` about these ... unless the build was + // explicitly named to be archived. + // + // Consider changing `list_package_builds` so it doesn't do that + // anymore, although it has a comment that it is doing so + // intentionally. Maybe it should return a richer type that describes + // if only the "spec build" exists and that info could be used here. + if matches!(local_err, CopyResult::BuildNotFound) + && matches!(remote_err, CopyResult::BuildNotFound) + && pkg.build().is_none() + { + continue; + } + // we will hide the remote_err in cases when both failed, // because the remote was always a fallback and fixing the // local error is preferred - return Err(local_err - .or(remote_err) - .unwrap_or_else(|| spk_schema::validators::Error::PackageNotFoundError(pkg).into())); + return Err(local_err.or(remote_err).unwrap_or_else(|| { + spk_schema::validators::Error::PackageNotFoundError(transfer_pkg).into() + })); } tracing::info!(path=?filename, "building archive");