scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-08-28 05:27:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Simplify clone depth plumbing

Browse source 
Make RunCloneSettings::DEFAULT_DEPTH the single owner of the default
depth, and interpret the "0 = full history" sentinel in one place via
RunCloneSettings::depth_limit(). Docker's clone_depth becomes
Option<usize> to match Daytona's encoding, with a shared
depth_argument() helper for both git command builders. Drop the
unreachable Option on the resolved depth field, the hand-written
DaytonaSettings::Default, and the pure-forwarding
daytona_git_clone_options helper. The blob-import test helper reuses
the pool's own connect options instead of rebuilding a partial copy.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019bgXj5J218RXfiT72qhbLV
This commit is contained in:
Bryan Helmkamp 2026-08-21 18:37:39 -04:00
parent fe1d9dc691
commit 09f5bb0f84
No known key found for this signature in database
10 changed files with 67 additions and 124 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
lib/components/fabro-sandbox/src/clone_source.rs
View file

@ -91,13 +91,9 @@ pub(crate) fn exact_fetch_command(
checkout_path: &str,
fetch_source: &str,
commit_sha: &str,
depth: usize,
depth: Option<usize>,
) -> String {
let depth_arg = if depth == 0 {
String::new()
} else {
format!(" --depth {depth}")
};
let depth_arg = depth_argument(depth);
format!(
"{git} -C {} fetch{depth_arg} --no-tags {} -- {}",
sandbox::shell_quote(checkout_path),
@ -107,6 +103,12 @@ pub(crate) fn exact_fetch_command(
)
}
/// Leading-space ` --depth N` fragment for a Git command, or empty when
/// `depth` is `None` to fetch full history.
pub(crate) fn depth_argument(depth: Option<usize>) -> String {
depth.map_or_else(String::new, |depth| format!(" --depth {depth}"))
}
/// Point the admitted branch at `revision` and attach HEAD to it.
///
/// The checkout attaches to a real branch instead of detaching so callers that
@ -479,7 +481,7 @@ mod tests {
"/repos/acme's widgets",
"https://token@example.com/acme/widgets.git?x=a b",
sha,
10,
Some(10),
);
let checkout =
exact_checkout_verify_command("/repos/acme's widgets", "feature/a b", "FETCH_HEAD");
@ -505,7 +507,7 @@ mod tests {
"/repos/acme/widgets",
"origin",
"0123456789abcdef0123456789abcdef01234567",
0,
None,
),
"git -c maintenance.auto=0 -c gc.auto=0 -C /repos/acme/widgets fetch --no-tags origin -- 0123456789abcdef0123456789abcdef01234567"
);
@ -572,7 +574,7 @@ mod tests {
);
run_shell(
temp.path(),
&exact_fetch_command(checkout_path, remote_path, &admitted_sha, 10),
&exact_fetch_command(checkout_path, remote_path, &admitted_sha, Some(10)),
);
let checked_out_sha = run_shell(
temp.path(),

17
lib/components/fabro-sandbox/src/config.rs
View file

@ -12,30 +12,19 @@ use std::collections::HashMap;
use serde::de::{self, MapAccess, Visitor};
use serde::{Deserialize, Serialize};
#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
#[derive(Clone, Debug, Default, Deserialize, PartialEq, Serialize)]
pub struct DaytonaSettings {
pub auto_stop_interval: Option<i32>,
pub labels: Option<HashMap<String, String>>,
pub snapshot: Option<DaytonaSnapshotSettings>,
pub network: Option<DaytonaNetwork>,
/// Git history depth for the repository clone; `None` clones full
/// history.
pub clone_depth: Option<i32>,
#[serde(default)]
pub skip_clone: bool,
}
impl Default for DaytonaSettings {
fn default() -> Self {
Self {
auto_stop_interval: None,
labels: None,
snapshot: None,
network: None,
clone_depth: Some(100),
skip_clone: false,
}
}
}
#[derive(Clone, Debug, PartialEq)]
pub enum DaytonaNetwork {
Block,

54
lib/components/fabro-sandbox/src/daytona/mod.rs
View file

@ -101,23 +101,6 @@ const DAYTONA_STATE_CHANGE_POLL_INTERVAL: Duration = Duration::from_secs(1);
/// leaked by a dead worker. An explicit `0` disables auto-stop entirely.
const DEFAULT_AUTO_STOP_INTERVAL_MINUTES: i32 = 120;
fn daytona_git_clone_options(
branch: Option<String>,
commit_id: Option<String>,
username: Option<String>,
password: Option<String>,
depth: Option<i32>,
) -> GitCloneOptions {
GitCloneOptions {
branch,
commit_id,
username,
password,
depth,
..GitCloneOptions::default()
}
}
pub(crate) fn daytona_not_found(err: &DaytonaError) -> bool {
matches!(err, DaytonaError::NotFound { .. }) || err.status_code() == Some(404)
}
@ -1623,13 +1606,14 @@ impl Sandbox for DaytonaSandbox {
let git_svc = &git_svc;
let origin = origin_url.as_str();
let target = layout.primary_repo_path.as_str();
let options = daytona_git_clone_options(
branch.clone(),
commit_sha.clone(),
username.clone(),
password.clone(),
self.config.clone_depth,
);
let options = GitCloneOptions {
branch: branch.clone(),
commit_id: commit_sha.clone(),
username: username.clone(),
password: password.clone(),
depth: self.config.clone_depth,
..GitCloneOptions::default()
};
async move { git_svc.clone(origin, target, options).await }
},
|err: &DaytonaError| classify_clone_failure(err, clone_credential_context),
@ -3142,26 +3126,6 @@ mod tests {
assert!(!error.to_string().contains("Daytona client"));
}
#[test]
fn exact_checkout_uses_daytona_branch_and_commit_options() {
let options = daytona_git_clone_options(
Some("feature/work".to_string()),
Some("0123456789abcdef0123456789abcdef01234567".to_string()),
Some("x-access-token".to_string()),
Some("secret".to_string()),
Some(1),
);
assert_eq!(options.branch.as_deref(), Some("feature/work"));
assert_eq!(
options.commit_id.as_deref(),
Some("0123456789abcdef0123456789abcdef01234567")
);
assert_eq!(options.username.as_deref(), Some("x-access-token"));
assert_eq!(options.password.as_deref(), Some("secret"));
assert_eq!(options.depth, Some(1));
}
fn mock_sandbox_body(sandbox_id: &str) -> serde_json::Value {
serde_json::json!({
"id": sandbox_id,
@ -3528,7 +3492,7 @@ mod tests {
assert!(config.snapshot.is_none());
assert!(config.auto_stop_interval.is_none());
assert!(config.labels.is_none());
assert_eq!(config.clone_depth, Some(100));
assert!(config.clone_depth.is_none());
}
#[test]

24
lib/components/fabro-sandbox/src/docker.rs
View file

@ -18,6 +18,7 @@ use bollard::image::CreateImageOptions;
use bollard::models::{ContainerInspectResponse, HostConfig};
use fabro_github::GitHubCredentials;
use fabro_github::token_source::InstallationTokenSource;
use fabro_types::settings::run::RunCloneSettings;
use fabro_types::{CommandOutputStream, CommandTermination, RunId, SandboxProviderKind};
use fabro_util::time::elapsed_ms;
use futures::StreamExt;
@ -48,7 +49,7 @@ const DOCKER_BASH_REQUIREMENT: &str = "Docker sandboxes require /bin/bash for ev
pub(crate) const WORKING_DIRECTORY: &str = "/workspace";
pub(crate) const REPOS_ROOT: &str = "/repos";
const DEFAULT_GIT_CLONE_DEPTH: usize = 100;
const DEFAULT_GIT_CLONE_DEPTH: usize = RunCloneSettings::DEFAULT_DEPTH.unsigned_abs() as usize;
const GIT_CLONE_TIMEOUT: Duration = Duration::from_mins(5);
#[cfg(test)]
const EXEC_STOP_POLL_SLEEP_SECONDS: &str = "0.005";
@ -121,9 +122,9 @@ pub struct DockerSandboxOptions {
pub auto_pull: bool,
/// Additional `KEY=VALUE` environment variables for the container.
pub env_vars: Vec<String>,
/// Maximum Git history depth fetched during clone. Zero fetches full
/// Maximum Git history depth fetched during clone; `None` fetches full
/// history.
pub clone_depth: usize,
pub clone_depth: Option<usize>,
/// Create an empty workspace instead of cloning even when an origin exists.
pub skip_clone: bool,
}
@ -137,7 +138,7 @@ impl Default for DockerSandboxOptions {
cpu_quota: None,
auto_pull: true,
env_vars: Vec::new(),
clone_depth: DEFAULT_GIT_CLONE_DEPTH,
clone_depth: Some(DEFAULT_GIT_CLONE_DEPTH),
skip_clone: false,
}
}
@ -1541,7 +1542,7 @@ fn git_clone_command(
clone_url: &str,
branch: Option<&str>,
checkout_path: &str,
depth: usize,
depth: Option<usize>,
) -> String {
let mut command = format!("{} clone", sandbox::GIT);
if let Some(branch) = branch {
@ -1549,10 +1550,7 @@ fn git_clone_command(
command.push_str(&shell_quote(branch));
command.push_str(" --single-branch");
}
if depth > 0 {
command.push_str(" --depth ");
command.push_str(&depth.to_string());
}
command.push_str(&clone_source::depth_argument(depth));
command.push_str(" --no-tags");
command.push_str(" -- ");
command.push_str(&shell_quote(clone_url));
@ -2571,7 +2569,7 @@ mod tests {
let options = DockerSandboxOptions::default();
assert_eq!(options.image, "buildpack-deps:noble");
assert_eq!(options.network_mode.as_deref(), Some("bridge"));
assert_eq!(options.clone_depth, DEFAULT_GIT_CLONE_DEPTH);
assert_eq!(options.clone_depth, Some(DEFAULT_GIT_CLONE_DEPTH));
assert!(!options.skip_clone);
}
@ -2581,7 +2579,7 @@ mod tests {
"https://github.com/fabro-sh/fabro",
Some("main"),
"/repos/fabro-sh/fabro",
1,
Some(1),
);
assert_eq!(
command,
@ -2595,7 +2593,7 @@ mod tests {
"https://github.com/fabro-sh/fabro",
None,
"/repos/fabro-sh/fabro",
DEFAULT_GIT_CLONE_DEPTH,
Some(DEFAULT_GIT_CLONE_DEPTH),
);
assert_eq!(
command,
@ -2609,7 +2607,7 @@ mod tests {
"https://github.com/fabro-sh/fabro",
Some("main"),
"/repos/fabro-sh/fabro",
0,
None,
);
assert_eq!(
command,

7
lib/components/fabro-sandbox/src/from_environment.rs
View file

@ -61,7 +61,7 @@ pub fn daytona_config_from_environment(
DaytonaNetwork::AllowList(settings.network.allow.clone())
}
}),
clone_depth: clone.depth.filter(|depth| *depth > 0),
clone_depth: clone.depth_limit(),
skip_clone: !clone.enabled,
}
}
@ -133,9 +133,8 @@ fn docker_config_from_environment_env(
.map(|cpu| i64::from(cpu).saturating_mul(100_000)),
env_vars,
clone_depth: clone
.depth
.and_then(|depth| usize::try_from(depth).ok())
.unwrap_or(default_options.clone_depth),
.depth_limit()
.and_then(|depth| usize::try_from(depth).ok()),
skip_clone: !clone.enabled,
..DockerSandboxOptions::default()
}

9
lib/components/fabro-store/src/legacy_blob_import.rs
View file

@ -790,7 +790,7 @@ mod tests {
type TestResult<T> = std::result::Result<T, Box<dyn std::error::Error>>;
struct TestContext {
dir: tempfile::TempDir,
_dir: tempfile::TempDir,
source: Database,
source_db: slatedb::Db,
sqlite: sqlx::SqlitePool,
@ -812,7 +812,7 @@ mod tests {
let sqlite = sqlite.clone_pool();
let target = BlobStore::new(sqlite.clone());
Ok(Self {
dir,
_dir: dir,
source,
source_db,
sqlite,
@ -822,11 +822,8 @@ mod tests {
async fn new_with_single_sqlite_connection() -> TestResult<Self> {
let mut context = Self::new().await?;
let options = context.sqlite.connect_options().as_ref().clone();
context.sqlite.close().await;
let options = SqliteConnectOptions::new()
.filename(context.dir.path().join("fabro.sqlite3"))
.foreign_keys(true)
.journal_mode(SqliteJournalMode::Wal);
let sqlite = SqlitePoolOptions::new()
.max_connections(1)
.connect_with(options)

6
lib/components/fabro-workflow/src/operations/start.rs
View file

@ -1301,7 +1301,7 @@ reasoning = false
resolve_docker_config(&settings.run, |_| None)
.unwrap()
.clone_depth,
1
Some(1)
);
}
@ -1320,7 +1320,7 @@ reasoning = false
resolve_docker_config(&settings.run, |_| None)
.unwrap()
.clone_depth,
0
None
);
}
@ -1333,7 +1333,7 @@ reasoning = false
resolve_docker_config(&settings.run, |_| None)
.unwrap()
.clone_depth,
100
Some(100)
);
}

26
lib/foundation/fabro-config/src/resolve/run.rs
View file

@ -248,24 +248,20 @@ fn resolve_clone(
clone: Option<&RunCloneLayer>,
errors: &mut Vec<ResolveError>,
) -> RunCloneSettings {
let depth =
clone
.and_then(|clone| clone.depth)
.map_or(RunCloneSettings::DEFAULT_DEPTH, |depth| {
if depth < 0 {
errors.push(ResolveError::Invalid {
path: "run.clone.depth".to_string(),
reason: "depth must be at least 0".to_string(),
});
RunCloneSettings::DEFAULT_DEPTH
} else {
depth
}
});
let mut depth = clone
.and_then(|clone| clone.depth)
.unwrap_or(RunCloneSettings::DEFAULT_DEPTH);
if depth < 0 {
errors.push(ResolveError::Invalid {
path: "run.clone.depth".to_string(),
reason: "depth must be at least 0".to_string(),
});
depth = RunCloneSettings::DEFAULT_DEPTH;
}
RunCloneSettings {
enabled: clone.and_then(|clone| clone.enabled).unwrap_or(true),
depth: Some(depth),
depth,
}
}

6
lib/foundation/fabro-config/src/tests/resolve_run.rs
View file

@ -126,7 +126,7 @@ fn resolves_run_defaults_from_empty_settings() {
assert!(!settings.environment.lifecycle.preserve);
assert!(settings.environment.lifecycle.stop_on_terminal);
assert!(settings.clone.enabled);
assert_eq!(settings.clone.depth, Some(100));
assert_eq!(settings.clone.depth, 100);
assert!(settings.run_branch.enabled);
assert!(settings.run_branch.push);
assert!(settings.meta_branch.enabled);
@ -312,7 +312,7 @@ push = false
.run;
assert!(!settings.clone.enabled);
assert_eq!(settings.clone.depth, Some(1));
assert_eq!(settings.clone.depth, 1);
assert!(settings.run_branch.enabled);
assert!(!settings.run_branch.push);
assert!(settings.meta_branch.enabled);
@ -332,7 +332,7 @@ depth = 0
.expect("zero clone depth should resolve")
.run;
assert_eq!(settings.clone.depth, Some(0));
assert_eq!(settings.clone.depth, 0);
}
#[test]

22
lib/foundation/fabro-types/src/settings/run.rs
View file

@ -961,32 +961,30 @@ impl Default for RunCheckpointSettings {
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
pub struct RunCloneSettings {
pub enabled: bool,
#[serde(
default = "default_clone_depth",
skip_serializing_if = "Option::is_none"
)]
pub depth: Option<i32>,
#[serde(default = "default_clone_depth")]
pub depth: i32,
}
impl RunCloneSettings {
pub const DEFAULT_DEPTH: i32 = 100;
/// Git history depth to fetch, or `None` to fetch full history.
pub fn depth_limit(&self) -> Option<i32> {
(self.depth > 0).then_some(self.depth)
}
}
impl Default for RunCloneSettings {
fn default() -> Self {
Self {
enabled: true,
depth: Some(Self::DEFAULT_DEPTH),
depth: Self::DEFAULT_DEPTH,
}
}
}
#[expect(
clippy::unnecessary_wraps,
reason = "serde default provider must return the field's Option<i32> type"
)]
fn default_clone_depth() -> Option<i32> {
Some(RunCloneSettings::DEFAULT_DEPTH)
fn default_clone_depth() -> i32 {
RunCloneSettings::DEFAULT_DEPTH
}
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]