Skip to content

Commit 7ec8e8b

Browse files
committed
Refactor rendering logic into separate methods
This change was prompted by wanting to refactor the code to work towards having different trait bounds for the functions that need a repository that supports renders (e.g., rendering into a repository) versus functions that render into a plain directory. Rework `RenderType` so the method that renders into a hardlink doesn't need to worry about getting a `RenderType::Copy` value. Signed-off-by: J Robert Ray <[email protected]>
1 parent 086c6b9 commit 7ec8e8b

File tree

5 files changed

+406
-353
lines changed

5 files changed

+406
-353
lines changed

crates/spfs-cli/cmd-render/src/cmd_render.rs

+7-5
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@ pub struct CmdRender {
3131
/// using a local directory and `HardLink` for the repository.
3232
#[clap(
3333
long,
34-
value_parser = clap::builder::PossibleValuesParser::new(spfs::storage::fs::RenderType::VARIANTS)
35-
.map(|s| s.parse::<spfs::storage::fs::RenderType>().unwrap())
34+
value_parser = clap::builder::PossibleValuesParser::new(spfs::storage::fs::CliRenderType::VARIANTS)
35+
.map(|s| s.parse::<spfs::storage::fs::CliRenderType>().unwrap())
3636
)]
37-
strategy: Option<spfs::storage::fs::RenderType>,
37+
strategy: Option<spfs::storage::fs::CliRenderType>,
3838

3939
/// The tag or digest of what to render, use a '+' to join multiple layers
4040
reference: String,
@@ -121,7 +121,9 @@ impl CmdRender {
121121
.render_into_directory(
122122
env_spec,
123123
&target_dir,
124-
self.strategy.unwrap_or(spfs::storage::fs::RenderType::Copy),
124+
self.strategy
125+
.map(Into::into)
126+
.unwrap_or(spfs::storage::fs::RenderType::Copy),
125127
)
126128
.await?;
127129
Ok(RenderResult {
@@ -165,7 +167,7 @@ impl CmdRender {
165167
.collect();
166168
tracing::trace!("stack: {:?}", stack);
167169
renderer
168-
.render(&stack, self.strategy)
170+
.render(&stack, self.strategy.map(Into::into))
169171
.await
170172
.map(|paths_rendered| RenderResult {
171173
paths_rendered,

crates/spfs/src/resolve.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use serde::{Deserialize, Serialize};
1313
use super::config::get_config;
1414
use crate::prelude::*;
1515
use crate::storage::fallback::FallbackProxy;
16-
use crate::storage::fs::{ManifestRenderPath, RenderSummary};
16+
use crate::storage::fs::{CliRenderType, ManifestRenderPath, RenderSummary};
1717
use crate::{graph, runtime, storage, tracking, Error, Result};
1818

1919
#[cfg(test)]
@@ -53,7 +53,7 @@ async fn render_via_subcommand(
5353
// of overlayfs. To avoid any issues editing files and
5454
// hardlinks the rendering for them switches to Copy.
5555
cmd.arg("--strategy");
56-
cmd.arg::<&str>(crate::storage::fs::RenderType::Copy.into());
56+
cmd.arg::<&str>(CliRenderType::Copy.into());
5757
}
5858
cmd.arg(spec.to_string());
5959
tracing::debug!("{:?}", cmd);

crates/spfs/src/storage/fs/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ pub use render_reporter::{
2626
};
2727
pub use render_summary::{RenderSummary, RenderSummaryReporter};
2828
pub use renderer::{
29+
CliRenderType,
30+
HardLinkRenderType,
2931
RenderType,
3032
Renderer,
3133
DEFAULT_MAX_CONCURRENT_BLOBS,

crates/spfs/src/storage/fs/renderer.rs

+28-2
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,39 @@ pub const DEFAULT_MAX_CONCURRENT_BLOBS: usize = 100;
4040
/// See: [`Renderer::with_max_concurrent_branches`]
4141
pub const DEFAULT_MAX_CONCURRENT_BRANCHES: usize = 5;
4242

43+
/// Render type options available to command line commands.
4344
#[derive(Debug, Copy, Clone, strum::EnumString, strum::VariantNames, strum::IntoStaticStr)]
44-
pub enum RenderType {
45+
pub enum CliRenderType {
4546
HardLink,
4647
HardLinkNoProxy,
4748
Copy,
4849
}
4950

51+
#[derive(Debug, Default, Copy, Clone)]
52+
pub enum HardLinkRenderType {
53+
#[default]
54+
WithProxy,
55+
WithoutProxy,
56+
}
57+
58+
#[derive(Debug, Copy, Clone)]
59+
pub enum RenderType {
60+
HardLink(HardLinkRenderType),
61+
Copy,
62+
}
63+
64+
impl From<CliRenderType> for RenderType {
65+
fn from(cli_render_type: CliRenderType) -> Self {
66+
match cli_render_type {
67+
CliRenderType::HardLink => RenderType::HardLink(HardLinkRenderType::WithProxy),
68+
CliRenderType::HardLinkNoProxy => {
69+
RenderType::HardLink(HardLinkRenderType::WithoutProxy)
70+
}
71+
CliRenderType::Copy => RenderType::Copy,
72+
}
73+
}
74+
}
75+
5076
impl OpenFsRepository {
5177
fn get_render_storage(&self) -> Result<&crate::storage::fs::FsHashStore> {
5278
match &self.fs_impl.renders {
@@ -338,7 +364,7 @@ where
338364
self.render_manifest_into_dir(
339365
manifest,
340366
&working_dir,
341-
render_type.unwrap_or(RenderType::HardLink),
367+
render_type.unwrap_or(RenderType::HardLink(HardLinkRenderType::default())),
342368
)
343369
.await
344370
.map_err(|err| {

0 commit comments

Comments
 (0)