Merge pull request #1158 from spkenv/no-hard-links-for-dir-renders

Refactor rendering logic into separate methods

Author: jrray

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

@@ -31,10 +31,10 @@ pub struct CmdRender {
     /// using a local directory and `HardLink` for the repository.
     #[clap(
         long,
-        value_parser = clap::builder::PossibleValuesParser::new(spfs::storage::fs::RenderType::VARIANTS)
-            .map(|s| s.parse::<spfs::storage::fs::RenderType>().unwrap())
+        value_parser = clap::builder::PossibleValuesParser::new(spfs::storage::fs::CliRenderType::VARIANTS)
+            .map(|s| s.parse::<spfs::storage::fs::CliRenderType>().unwrap())
     )]
-    strategy: Option<spfs::storage::fs::RenderType>,
+    strategy: Option<spfs::storage::fs::CliRenderType>,
 
     /// The tag or digest of what to render, use a '+' to join multiple layers
     reference: String,
@@ -121,7 +121,9 @@ impl CmdRender {
             .render_into_directory(
                 env_spec,
                 &target_dir,
-                self.strategy.unwrap_or(spfs::storage::fs::RenderType::Copy),
+                self.strategy
+                    .map(Into::into)
+                    .unwrap_or(spfs::storage::fs::RenderType::Copy),
             )
             .await?;
         Ok(RenderResult {
@@ -165,7 +167,7 @@ impl CmdRender {
             .collect();
         tracing::trace!("stack: {:?}", stack);
         renderer
-            .render(&stack, self.strategy)
+            .render(&stack, self.strategy.map(Into::into))
             .await
             .map(|paths_rendered| RenderResult {
                 paths_rendered,

crates/spfs/src/resolve.rs

@@ -13,7 +13,7 @@ use serde::{Deserialize, Serialize};
 use super::config::get_config;
 use crate::prelude::*;
 use crate::storage::fallback::FallbackProxy;
-use crate::storage::fs::{ManifestRenderPath, RenderSummary};
+use crate::storage::fs::{CliRenderType, ManifestRenderPath, RenderSummary};
 use crate::{Error, Result, graph, runtime, storage, tracking};
 
 #[cfg(test)]
@@ -52,7 +52,7 @@ async fn render_via_subcommand(
         // of overlayfs. To avoid any issues editing files and
         // hardlinks the rendering for them switches to Copy.
         cmd.arg("--strategy");
-        cmd.arg::<&str>(crate::storage::fs::RenderType::Copy.into());
+        cmd.arg::<&str>(CliRenderType::Copy.into());
     }
     cmd.arg(spec.to_string());
     tracing::debug!("{:?}", cmd);

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

@@ -26,8 +26,10 @@ pub use render_reporter::{
 };
 pub use render_summary::{RenderSummary, RenderSummaryReporter};
 pub use renderer::{
+    CliRenderType,
     DEFAULT_MAX_CONCURRENT_BLOBS,
     DEFAULT_MAX_CONCURRENT_BRANCHES,
+    HardLinkRenderType,
     RenderType,
     Renderer,
 };

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

@@ -40,13 +40,39 @@ pub const DEFAULT_MAX_CONCURRENT_BLOBS: usize = 100;
 /// See: [`Renderer::with_max_concurrent_branches`]
 pub const DEFAULT_MAX_CONCURRENT_BRANCHES: usize = 5;
 
+/// Render type options available to command line commands.
 #[derive(Debug, Copy, Clone, strum::EnumString, strum::VariantNames, strum::IntoStaticStr)]
-pub enum RenderType {
+pub enum CliRenderType {
     HardLink,
     HardLinkNoProxy,
     Copy,
 }
 
+#[derive(Debug, Default, Copy, Clone)]
+pub enum HardLinkRenderType {
+    #[default]
+    WithProxy,
+    WithoutProxy,
+}
+
+#[derive(Debug, Copy, Clone)]
+pub enum RenderType {
+    HardLink(HardLinkRenderType),
+    Copy,
+}
+
+impl From<CliRenderType> for RenderType {
+    fn from(cli_render_type: CliRenderType) -> Self {
+        match cli_render_type {
+            CliRenderType::HardLink => RenderType::HardLink(HardLinkRenderType::WithProxy),
+            CliRenderType::HardLinkNoProxy => {
+                RenderType::HardLink(HardLinkRenderType::WithoutProxy)
+            }
+            CliRenderType::Copy => RenderType::Copy,
+        }
+    }
+}
+
 impl OpenFsRepository {
     fn get_render_storage(&self) -> Result<&crate::storage::fs::FsHashStore> {
         match &self.renders {
@@ -338,7 +364,7 @@ where
         self.render_manifest_into_dir(
             manifest,
             &working_dir,
-            render_type.unwrap_or(RenderType::HardLink),
+            render_type.unwrap_or(RenderType::HardLink(HardLinkRenderType::default())),
         )
         .await
         .map_err(|err| {