Skip to content

Commit

Permalink
container: Move more metadata into ExportOpts
Browse files Browse the repository at this point in the history
This drops two extra arguments we added over time; in
a few places before we had e.g. `None, None, None` being passed
which just looks awkward.  And we also threaded through all 3
in various places.

The `ExportOpts` just needs to grow a lifetime argument, but that
turned out to not be too bad when I realized we could use the
elided lifetime `<'_>` in all methods that use it.
  • Loading branch information
cgwalters committed Sep 11, 2023
1 parent 2781808 commit f1713aa
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 68 deletions.
4 changes: 2 additions & 2 deletions lib/src/chunking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ impl Chunking {
pub fn from_mapping(
repo: &ostree::Repo,
rev: &str,
meta: ObjectMetaSized,
meta: &ObjectMetaSized,
max_layers: &Option<NonZeroU32>,
prior_build_metadata: Option<&oci_spec::image::ImageManifest>,
) -> Result<Self> {
Expand All @@ -287,7 +287,7 @@ impl Chunking {
#[allow(clippy::or_fun_call)]
pub fn process_mapping(
&mut self,
meta: ObjectMetaSized,
meta: &ObjectMetaSized,
max_layers: &Option<NonZeroU32>,
prior_build_metadata: Option<&oci_spec::image::ImageManifest>,
) -> Result<()> {
Expand Down
3 changes: 1 addition & 2 deletions lib/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -650,8 +650,7 @@ async fn container_export(
skip_compression: compression_fast, // TODO rename this in the struct at the next semver break
..Default::default()
};
let pushed =
crate::container::encapsulate(repo, rev, &config, None, Some(opts), None, imgref).await?;
let pushed = crate::container::encapsulate(repo, rev, &config, Some(opts), imgref).await?;
println!("{}", pushed);
Ok(())
}
Expand Down
62 changes: 19 additions & 43 deletions lib/src/container/encapsulate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,6 @@ fn build_oci(
tag: Option<&str>,
config: &Config,
opts: ExportOpts,
prior_build: Option<&oci_image::ImageManifest>,
contentmeta: Option<crate::chunking::ObjectMetaSized>,
) -> Result<ImageReference> {
if !ocidir_path.exists() {
std::fs::create_dir(ocidir_path).context("Creating OCI dir")?;
Expand Down Expand Up @@ -230,14 +228,16 @@ fn build_oci(

let mut manifest = ocidir::new_empty_manifest().build().unwrap();

let chunking = contentmeta
let chunking = opts
.contentmeta
.as_ref()
.map(|meta| {
crate::chunking::Chunking::from_mapping(
repo,
commit,
meta,
&opts.max_layers,
prior_build,
opts.prior_build,
)
})
.transpose()?;
Expand Down Expand Up @@ -316,14 +316,12 @@ pub(crate) fn parse_oci_path_and_tag(path: &str) -> (&str, Option<&str>) {
}

/// Helper for `build()` that avoids generics
#[instrument(skip(repo, contentmeta))]
#[instrument(skip(repo, config, opts))]
async fn build_impl(
repo: &ostree::Repo,
ostree_ref: &str,
config: &Config,
prior_build: Option<&oci_image::ImageManifest>,
opts: Option<ExportOpts>,
contentmeta: Option<ObjectMetaSized>,
opts: Option<ExportOpts<'_, '_>>,
dest: &ImageReference,
) -> Result<String> {
let mut opts = opts.unwrap_or_default();
Expand All @@ -332,16 +330,8 @@ async fn build_impl(
}
let digest = if dest.transport == Transport::OciDir {
let (path, tag) = parse_oci_path_and_tag(dest.name.as_str());
let _copied: ImageReference = build_oci(
repo,
ostree_ref,
Path::new(path),
tag,
config,
opts,
prior_build,
contentmeta,
)?;
let _copied: ImageReference =
build_oci(repo, ostree_ref, Path::new(path), tag, config, opts)?;
None
} else {
let tempdir = tempfile::tempdir_in("/var/tmp")?;
Expand All @@ -350,16 +340,7 @@ async fn build_impl(

// Minor TODO: refactor to avoid clone
let authfile = opts.authfile.clone();
let tempoci = build_oci(
repo,
ostree_ref,
Path::new(tempdest),
None,
config,
opts,
prior_build,
contentmeta,
)?;
let tempoci = build_oci(repo, ostree_ref, Path::new(tempdest), None, config, opts)?;

let digest = skopeo::copy(&tempoci, dest, authfile.as_deref()).await?;
Some(digest)
Expand All @@ -381,7 +362,7 @@ async fn build_impl(
/// Options controlling commit export into OCI
#[derive(Clone, Debug, Default)]
#[non_exhaustive]
pub struct ExportOpts {
pub struct ExportOpts<'m, 'o> {
/// If true, do not perform gzip compression of the tar layers.
pub skip_compression: bool,
/// A set of commit metadata keys to copy as image labels.
Expand All @@ -395,9 +376,15 @@ pub struct ExportOpts {
// TODO semver-break: remove this
/// Use only the standard OCI version label
pub no_legacy_version_label: bool,
/// A reference to the metadata for a previous build; used to optimize
/// the packing structure.
pub prior_build: Option<&'m oci_image::ImageManifest>,
/// Metadata mapping between objects and their owning component/package;
/// used to optimize packing.
pub contentmeta: Option<&'o ObjectMetaSized>,
}

impl ExportOpts {
impl<'m, 'o> ExportOpts<'m, 'o> {
/// Return the gzip compression level to use, as configured by the export options.
fn compression(&self) -> Compression {
if self.skip_compression {
Expand All @@ -415,19 +402,8 @@ pub async fn encapsulate<S: AsRef<str>>(
repo: &ostree::Repo,
ostree_ref: S,
config: &Config,
prior_build: Option<&oci_image::ImageManifest>,
opts: Option<ExportOpts>,
contentmeta: Option<ObjectMetaSized>,
opts: Option<ExportOpts<'_, '_>>,
dest: &ImageReference,
) -> Result<String> {
build_impl(
repo,
ostree_ref.as_ref(),
config,
prior_build,
opts,
contentmeta,
dest,
)
.await
build_impl(repo, ostree_ref.as_ref(), config, opts, dest).await
}
3 changes: 1 addition & 2 deletions lib/src/fixture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -679,15 +679,14 @@ impl Fixture {
.context("Computing sizes")?;
let opts = ExportOpts {
max_layers: std::num::NonZeroU32::new(PKGS_V0_LEN as u32),
contentmeta: Some(&contentmeta),
..Default::default()
};
let digest = crate::container::encapsulate(
self.srcrepo(),
self.testref(),
&config,
None,
Some(opts),
Some(contentmeta),
&imgref,
)
.await
Expand Down
24 changes: 5 additions & 19 deletions lib/tests/it/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,13 +457,12 @@ async fn impl_test_container_import_export(chunked: bool) -> Result<()> {
opts.copy_meta_keys = vec!["buildsys.checksum".to_string()];
opts.copy_meta_opt_keys = vec!["nosuchvalue".to_string()];
opts.max_layers = std::num::NonZeroU32::new(PKGS_V0_LEN as u32);
opts.contentmeta = contentmeta.as_ref();
let digest = ostree_ext::container::encapsulate(
fixture.srcrepo(),
fixture.testref(),
&config,
None,
Some(opts),
contentmeta,
&srcoci_imgref,
)
.await
Expand Down Expand Up @@ -515,8 +514,6 @@ async fn impl_test_container_import_export(chunked: bool) -> Result<()> {
fixture.testref(),
&config,
None,
None,
None,
&ociarchive_dest,
)
.await
Expand Down Expand Up @@ -626,8 +623,6 @@ async fn test_unencapsulate_unbootable() -> Result<()> {
fixture.testref(),
&config,
None,
None,
None,
&srcoci_imgref,
)
.await
Expand Down Expand Up @@ -962,8 +957,6 @@ async fn test_container_write_derive() -> Result<()> {
..Default::default()
},
None,
None,
None,
&ImageReference {
transport: Transport::OciDir,
name: base_oci_path.to_string(),
Expand Down Expand Up @@ -1346,17 +1339,10 @@ async fn test_container_import_export_registry() -> Result<()> {
cmd: Some(vec!["/bin/bash".to_string()]),
..Default::default()
};
let digest = ostree_ext::container::encapsulate(
fixture.srcrepo(),
testref,
&config,
None,
None,
None,
&src_imgref,
)
.await
.context("exporting to registry")?;
let digest =
ostree_ext::container::encapsulate(fixture.srcrepo(), testref, &config, None, &src_imgref)
.await
.context("exporting to registry")?;
let mut digested_imgref = src_imgref.clone();
digested_imgref.name = format!("{}@{}", src_imgref.name, digest);

Expand Down

0 comments on commit f1713aa

Please sign in to comment.