Skip to content

Commit d902df4

Browse files
src: start using a bit of refcounting
Our `SplitStreamWriter` and `oci::ImageOp` structs contain simple references to `Repository` which results in some awkward lifetime rules on those structs. We can simplify things substantially if we lean into ref-counting a bit more. I'm not yet ready to declare that Repository is always refcounted, but for operations involving splitstreams (including oci downloads) it is now required. The ergonomics of this change surprised me. The Deref trait on `Arc<>` and the ability to define `self: &Arc<Self>` methods makes this all quite nice to use. Signed-off-by: Allison Karlitskaya <[email protected]>
1 parent 087c628 commit d902df4

File tree

5 files changed

+33
-29
lines changed

5 files changed

+33
-29
lines changed

src/bin/cfsctl.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::path::PathBuf;
1+
use std::{path::PathBuf, sync::Arc};
22

33
use anyhow::Result;
44
use clap::{Parser, Subcommand};
@@ -141,7 +141,7 @@ async fn main() -> Result<()> {
141141
Command::Oci { cmd: oci_cmd } => match oci_cmd {
142142
OciCommand::ImportLayer { name, sha256 } => {
143143
let object_id = oci::import_layer(
144-
&repo,
144+
&Arc::new(repo),
145145
&parse_sha256(sha256)?,
146146
name.as_deref(),
147147
&mut std::io::stdin(),
@@ -159,11 +159,11 @@ async fn main() -> Result<()> {
159159
println!("{}", image_id.to_hex());
160160
}
161161
OciCommand::Pull { ref image, name } => {
162-
oci::pull(&repo, image, name.as_deref()).await?
162+
oci::pull(&Arc::new(repo), image, name.as_deref()).await?
163163
}
164164
OciCommand::Seal { verity, ref name } => {
165165
let (sha256, verity) = oci::seal(
166-
&repo,
166+
&Arc::new(repo),
167167
name,
168168
verity.map(Sha256HashValue::from_hex).transpose()?.as_ref(),
169169
)?;

src/oci/mod.rs

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::process::Command;
33
pub mod image;
44
pub mod tar;
55

6-
use std::{collections::HashMap, io::Read, iter::zip, path::Path};
6+
use std::{collections::HashMap, io::Read, iter::zip, path::Path, sync::Arc};
77

88
use anyhow::{bail, ensure, Context, Result};
99
use async_compression::tokio::bufread::{GzipDecoder, ZstdDecoder};
@@ -23,7 +23,7 @@ use crate::{
2323
};
2424

2525
pub fn import_layer<ObjectID: FsVerityHashValue>(
26-
repo: &Repository<ObjectID>,
26+
repo: &Arc<Repository<ObjectID>>,
2727
sha256: &Sha256Digest,
2828
name: Option<&str>,
2929
tar_stream: &mut impl Read,
@@ -44,8 +44,8 @@ pub fn ls_layer<ObjectID: FsVerityHashValue>(
4444
Ok(())
4545
}
4646

47-
struct ImageOp<'repo, ObjectID: FsVerityHashValue> {
48-
repo: &'repo Repository<ObjectID>,
47+
struct ImageOp<ObjectID: FsVerityHashValue> {
48+
repo: Arc<Repository<ObjectID>>,
4949
proxy: ImageProxy,
5050
img: OpenedImage,
5151
progress: MultiProgress,
@@ -67,8 +67,8 @@ fn sha256_from_digest(digest: &str) -> Result<Sha256Digest> {
6767

6868
type ContentAndVerity<ObjectID> = (Sha256Digest, ObjectID);
6969

70-
impl<'repo, ObjectID: FsVerityHashValue> ImageOp<'repo, ObjectID> {
71-
async fn new(repo: &'repo Repository<ObjectID>, imgref: &str) -> Result<Self> {
70+
impl<ObjectID: FsVerityHashValue> ImageOp<ObjectID> {
71+
async fn new(repo: &Arc<Repository<ObjectID>>, imgref: &str) -> Result<Self> {
7272
// See https://github.com/containers/skopeo/issues/2563
7373
let skopeo_cmd = if imgref.starts_with("containers-storage:") {
7474
let mut cmd = Command::new("podman");
@@ -87,7 +87,7 @@ impl<'repo, ObjectID: FsVerityHashValue> ImageOp<'repo, ObjectID> {
8787
let img = proxy.open_image(imgref).await.context("Opening image")?;
8888
let progress = MultiProgress::new();
8989
Ok(ImageOp {
90-
repo,
90+
repo: Arc::clone(repo),
9191
proxy,
9292
img,
9393
progress,
@@ -142,7 +142,7 @@ impl<'repo, ObjectID: FsVerityHashValue> ImageOp<'repo, ObjectID> {
142142
}
143143

144144
pub async fn ensure_config(
145-
&self,
145+
self: &Arc<Self>,
146146
manifest_layers: &[Descriptor],
147147
descriptor: &Descriptor,
148148
) -> Result<ContentAndVerity<ObjectID>> {
@@ -192,7 +192,7 @@ impl<'repo, ObjectID: FsVerityHashValue> ImageOp<'repo, ObjectID> {
192192
}
193193
}
194194

195-
pub async fn pull(&self) -> Result<ContentAndVerity<ObjectID>> {
195+
pub async fn pull(self: &Arc<Self>) -> Result<ContentAndVerity<ObjectID>> {
196196
let (_manifest_digest, raw_manifest) = self
197197
.proxy
198198
.fetch_manifest_raw_oci(&self.img)
@@ -213,11 +213,11 @@ impl<'repo, ObjectID: FsVerityHashValue> ImageOp<'repo, ObjectID> {
213213
/// Pull the target image, and add the provided tag. If this is a mountable
214214
/// image (i.e. not an artifact), it is *not* unpacked by default.
215215
pub async fn pull(
216-
repo: &Repository<impl FsVerityHashValue>,
216+
repo: &Arc<Repository<impl FsVerityHashValue>>,
217217
imgref: &str,
218218
reference: Option<&str>,
219219
) -> Result<()> {
220-
let op = ImageOp::new(repo, imgref).await?;
220+
let op = Arc::new(ImageOp::new(repo, imgref).await?);
221221
let (sha256, id) = op
222222
.pull()
223223
.await
@@ -280,7 +280,7 @@ pub fn open_config_shallow<ObjectID: FsVerityHashValue>(
280280
}
281281

282282
pub fn write_config<ObjectID: FsVerityHashValue>(
283-
repo: &Repository<ObjectID>,
283+
repo: &Arc<Repository<ObjectID>>,
284284
config: &ImageConfiguration,
285285
refs: DigestMap<ObjectID>,
286286
) -> Result<ContentAndVerity<ObjectID>> {
@@ -294,7 +294,7 @@ pub fn write_config<ObjectID: FsVerityHashValue>(
294294
}
295295

296296
pub fn seal<ObjectID: FsVerityHashValue>(
297-
repo: &Repository<ObjectID>,
297+
repo: &Arc<Repository<ObjectID>>,
298298
name: &str,
299299
verity: Option<&ObjectID>,
300300
) -> Result<ContentAndVerity<ObjectID>> {
@@ -421,7 +421,7 @@ mod test {
421421
let layer_id: [u8; 32] = context.finalize().into();
422422

423423
let repo_dir = tempdir();
424-
let repo = Repository::<Sha256HashValue>::open_path(CWD, &repo_dir).unwrap();
424+
let repo = Arc::new(Repository::<Sha256HashValue>::open_path(CWD, &repo_dir).unwrap());
425425
let id = import_layer(&repo, &layer_id, Some("name"), &mut layer.as_slice()).unwrap();
426426

427427
let mut dump = String::new();

src/oci/tar.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ pub fn split(
7575

7676
pub async fn split_async(
7777
mut tar_stream: impl AsyncRead + Unpin,
78-
writer: &mut SplitStreamWriter<'_, impl FsVerityHashValue>,
78+
writer: &mut SplitStreamWriter<impl FsVerityHashValue>,
7979
) -> Result<()> {
8080
while let Some(header) = read_header_async(&mut tar_stream).await? {
8181
// the header always gets stored as inline data

src/repository.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use std::{
55
io::{Read, Write},
66
os::fd::{AsFd, OwnedFd},
77
path::{Path, PathBuf},
8+
sync::Arc,
89
};
910

1011
use anyhow::{bail, ensure, Context, Result};
@@ -173,7 +174,7 @@ impl<ObjectID: FsVerityHashValue> Repository<ObjectID> {
173174
/// You should write the data to the returned object and then pass it to .store_stream() to
174175
/// store the result.
175176
pub fn create_stream(
176-
&self,
177+
self: &Arc<Self>,
177178
sha256: Option<Sha256Digest>,
178179
maps: Option<DigestMap<ObjectID>>,
179180
) -> SplitStreamWriter<ObjectID> {
@@ -285,7 +286,7 @@ impl<ObjectID: FsVerityHashValue> Repository<ObjectID> {
285286
/// On success, the object ID of the new object is returned. It is expected that this object
286287
/// ID will be used when referring to the stream from other linked streams.
287288
pub fn ensure_stream(
288-
&self,
289+
self: &Arc<Self>,
289290
sha256: &Sha256Digest,
290291
callback: impl FnOnce(&mut SplitStreamWriter<ObjectID>) -> Result<()>,
291292
reference: Option<&str>,

src/splitstream.rs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@
33
* See doc/splitstream.md
44
*/
55

6-
use std::io::{BufReader, Read, Write};
6+
use std::{
7+
io::{BufReader, Read, Write},
8+
sync::Arc,
9+
};
710

811
use anyhow::{bail, Result};
912
use sha2::{Digest, Sha256};
@@ -60,14 +63,14 @@ impl<ObjectID: FsVerityHashValue> DigestMap<ObjectID> {
6063
}
6164
}
6265

63-
pub struct SplitStreamWriter<'a, ObjectID: FsVerityHashValue> {
64-
repo: &'a Repository<ObjectID>,
66+
pub struct SplitStreamWriter<ObjectID: FsVerityHashValue> {
67+
repo: Arc<Repository<ObjectID>>,
6568
inline_content: Vec<u8>,
66-
writer: Encoder<'a, Vec<u8>>,
69+
writer: Encoder<'static, Vec<u8>>,
6770
pub sha256: Option<(Sha256, Sha256Digest)>,
6871
}
6972

70-
impl<ObjectID: FsVerityHashValue> std::fmt::Debug for SplitStreamWriter<'_, ObjectID> {
73+
impl<ObjectID: FsVerityHashValue> std::fmt::Debug for SplitStreamWriter<ObjectID> {
7174
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
7275
// writer doesn't impl Debug
7376
f.debug_struct("SplitStreamWriter")
@@ -78,9 +81,9 @@ impl<ObjectID: FsVerityHashValue> std::fmt::Debug for SplitStreamWriter<'_, Obje
7881
}
7982
}
8083

81-
impl<'a, ObjectID: FsVerityHashValue> SplitStreamWriter<'a, ObjectID> {
84+
impl<ObjectID: FsVerityHashValue> SplitStreamWriter<ObjectID> {
8285
pub fn new(
83-
repo: &'a Repository<ObjectID>,
86+
repo: &Arc<Repository<ObjectID>>,
8487
refs: Option<DigestMap<ObjectID>>,
8588
sha256: Option<Sha256Digest>,
8689
) -> Self {
@@ -98,7 +101,7 @@ impl<'a, ObjectID: FsVerityHashValue> SplitStreamWriter<'a, ObjectID> {
98101
}
99102

100103
Self {
101-
repo,
104+
repo: Arc::clone(repo),
102105
inline_content: vec![],
103106
writer,
104107
sha256: sha256.map(|x| (Sha256::new(), x)),

0 commit comments

Comments
 (0)