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 1

Simplify feature fetch functions: extract shared helpers

Browse source 
- Extract read_feature_metadata(), extract_tgz(), and create_feature_dir()
  helpers to eliminate copy-paste across fetch_feature_oci/local/https
- Move ensure_oras() out of per-feature calls into a once-per-resolve
  check via oras_checked flag
- Simplify dockerfile::generate() to take &HashMap instead of
  &Option<HashMap>, removing an unnecessary clone in the caller

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-03-03 00:59:41 -05:00
parent ba2b6faef3
commit eab81aee99
3 changed files with 84 additions and 134 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

42
crates/arc-devcontainer/src/dockerfile.rs
View file

@ -6,7 +6,7 @@ use crate::features::FeatureLayer;
pub fn generate(
base_dockerfile: &str,
feature_layers: &[FeatureLayer],
container_env: &Option<HashMap<String, String>>,
container_env: &HashMap<String, String>,
remote_user: Option<&str>,
) -> String {
let mut sections: Vec<String> = Vec::new();
@ -18,16 +18,14 @@ pub fn generate(
sections.push(layer.dockerfile_snippet.clone());
}
if let Some(env) = container_env {
if !env.is_empty() {
let mut keys: Vec<&String> = env.keys().collect();
keys.sort();
let env_lines: Vec<String> = keys
.iter()
.map(|k| format!("ENV {}={}", k, env[*k]))
.collect();
sections.push(env_lines.join("\n"));
}
if !container_env.is_empty() {
let mut keys: Vec<&String> = container_env.keys().collect();
keys.sort();
let env_lines: Vec<String> = keys
.iter()
.map(|k| format!("ENV {}={}", k, container_env[*k]))
.collect();
sections.push(env_lines.join("\n"));
}
if let Some(user) = remote_user {
@ -53,7 +51,7 @@ mod tests {
#[test]
fn base_image_only() {
let result = generate("FROM ubuntu:22.04", &[], &None, None);
let result = generate("FROM ubuntu:22.04", &[], &HashMap::new(), None);
assert_eq!(
result,
"# Generated by arc-devcontainer\n\nFROM ubuntu:22.04\n"
@ -63,7 +61,7 @@ mod tests {
#[test]
fn base_dockerfile_preserved_as_is() {
let base = "FROM ubuntu:22.04\nRUN apt-get update\nRUN apt-get install -y curl";
let result = generate(base, &[], &None, None);
let result = generate(base, &[], &HashMap::new(), None);
assert_eq!(
result,
"# Generated by arc-devcontainer\n\nFROM ubuntu:22.04\nRUN apt-get update\nRUN apt-get install -y curl\n"
@ -76,7 +74,7 @@ mod tests {
make_layer("node", "node-1", "RUN install-node.sh"),
make_layer("python", "python-1", "RUN install-python.sh"),
];
let result = generate("FROM ubuntu:22.04", &layers, &None, None);
let result = generate("FROM ubuntu:22.04", &layers, &HashMap::new(), None);
assert_eq!(
result,
"# Generated by arc-devcontainer\n\n\
@ -92,7 +90,7 @@ mod tests {
env.insert("ZEBRA".to_string(), "stripes".to_string());
env.insert("APPLE".to_string(), "red".to_string());
env.insert("MANGO".to_string(), "yellow".to_string());
let result = generate("FROM alpine", &[], &Some(env), None);
let result = generate("FROM alpine", &[], &env, None);
assert_eq!(
result,
"# Generated by arc-devcontainer\n\n\
@ -105,7 +103,7 @@ mod tests {
#[test]
fn with_remote_user() {
let result = generate("FROM alpine", &[], &None, Some("vscode"));
let result = generate("FROM alpine", &[], &HashMap::new(), Some("vscode"));
assert_eq!(
result,
"# Generated by arc-devcontainer\n\n\
@ -120,7 +118,7 @@ mod tests {
let mut env = HashMap::new();
env.insert("PATH".to_string(), "/usr/local/bin".to_string());
env.insert("HOME".to_string(), "/home/vscode".to_string());
let result = generate("FROM ubuntu:22.04", &layers, &Some(env), Some("vscode"));
let result = generate("FROM ubuntu:22.04", &layers, &env, Some("vscode"));
assert_eq!(
result,
"# Generated by arc-devcontainer\n\n\
@ -134,7 +132,7 @@ mod tests {
#[test]
fn empty_feature_layers_no_extra_blank_lines() {
let result = generate("FROM alpine", &[], &None, Some("dev"));
let result = generate("FROM alpine", &[], &HashMap::new(), Some("dev"));
assert_eq!(
result,
"# Generated by arc-devcontainer\n\n\
@ -146,7 +144,7 @@ mod tests {
#[test]
fn empty_env_map_treated_as_none() {
let env = HashMap::new();
let result = generate("FROM alpine", &[], &Some(env), None);
let result = generate("FROM alpine", &[], &env, None);
assert_eq!(
result,
"# Generated by arc-devcontainer\n\nFROM alpine\n"
@ -162,7 +160,7 @@ mod tests {
\n\
FROM ubuntu:22.04\n\
COPY --from=builder /app/bin /usr/local/bin";
let result = generate(base, &[], &None, None);
let result = generate(base, &[], &HashMap::new(), None);
assert!(result.starts_with("# Generated by arc-devcontainer\n\n"));
assert!(result.contains("FROM ubuntu:22.04 AS builder"));
assert!(result.contains("COPY --from=builder /app/bin /usr/local/bin"));
@ -173,7 +171,7 @@ mod tests {
fn container_env_only() {
let mut cenv = HashMap::new();
cenv.insert("DEBIAN_FRONTEND".to_string(), "noninteractive".to_string());
let result = generate("FROM alpine", &[], &Some(cenv), None);
let result = generate("FROM alpine", &[], &cenv, None);
assert_eq!(
result,
"# Generated by arc-devcontainer\n\n\
@ -188,7 +186,7 @@ mod tests {
cenv.insert("ALPHA".to_string(), "first".to_string());
cenv.insert("BETA".to_string(), "second".to_string());
cenv.insert("GAMMA".to_string(), "third".to_string());
let result = generate("FROM alpine", &[], &Some(cenv), None);
let result = generate("FROM alpine", &[], &cenv, None);
assert!(result.contains("ENV ALPHA=first"));
assert!(result.contains("ENV BETA=second"));
assert!(result.contains("ENV GAMMA=third"));

168
crates/arc-devcontainer/src/features.rs
View file

@ -132,14 +132,45 @@ async fn ensure_oras() -> crate::Result<()> {
Ok(())
}
/// Fetch a single OCI feature using `oras pull` and extract its contents.
/// Returns the parsed feature metadata.
async fn fetch_feature_oci(
feature_id: &str,
output_dir: &Path,
) -> crate::Result<FeatureMetadata> {
ensure_oras().await?;
/// Extract a tgz archive in the given directory.
async fn extract_tgz(feature_dir: &Path, feature_id: &str) -> crate::Result<()> {
let status = tokio::process::Command::new("tar")
.args(["xzf", "devcontainer-feature.tgz"])
.current_dir(feature_dir)
.status()
.await
.map_err(|e| DevcontainerError::Feature(format!("failed to extract tgz: {e}")))?;
if !status.success() {
return Err(DevcontainerError::Feature(format!(
"tar extraction failed for {feature_id}"
)));
}
Ok(())
}
/// Read and parse devcontainer-feature.json from a feature directory.
async fn read_feature_metadata(feature_dir: &Path) -> crate::Result<FeatureMetadata> {
let metadata_path = feature_dir.join("devcontainer-feature.json");
let metadata_str = tokio::fs::read_to_string(&metadata_path)
.await
.map_err(|e| {
DevcontainerError::Feature(format!(
"failed to read {}: {e}",
metadata_path.display()
))
})?;
serde_json::from_str(&metadata_str).map_err(|e| {
DevcontainerError::Feature(format!(
"failed to parse {}: {e}",
metadata_path.display()
))
})
}
/// Create a feature output directory under the temp dir.
async fn create_feature_dir(output_dir: &Path, feature_id: &str) -> crate::Result<std::path::PathBuf> {
let dir_name = dir_name_from_id(feature_id);
let feature_dir = output_dir.join(&dir_name);
tokio::fs::create_dir_all(&feature_dir)
@ -150,6 +181,15 @@ async fn fetch_feature_oci(
feature_dir.display()
))
})?;
Ok(feature_dir)
}
/// Fetch a single OCI feature using `oras pull` and extract its contents.
async fn fetch_feature_oci(
feature_id: &str,
output_dir: &Path,
) -> crate::Result<FeatureMetadata> {
let feature_dir = create_feature_dir(output_dir, feature_id).await?;
info!(feature_id, "pulling feature with oras");
@ -167,44 +207,11 @@ async fn fetch_feature_oci(
)));
}
// Extract any tgz files
let tgz_path = feature_dir.join("devcontainer-feature.tgz");
if tgz_path.exists() {
let status = tokio::process::Command::new("tar")
.args(["xzf", "devcontainer-feature.tgz"])
.current_dir(&feature_dir)
.status()
.await
.map_err(|e| {
DevcontainerError::Feature(format!("failed to extract tgz: {e}"))
})?;
if !status.success() {
return Err(DevcontainerError::Feature(format!(
"tar extraction failed for {feature_id}"
)));
}
if feature_dir.join("devcontainer-feature.tgz").exists() {
extract_tgz(&feature_dir, feature_id).await?;
}
// Read metadata
let metadata_path = feature_dir.join("devcontainer-feature.json");
let metadata_str = tokio::fs::read_to_string(&metadata_path)
.await
.map_err(|e| {
DevcontainerError::Feature(format!(
"failed to read {}: {e}",
metadata_path.display()
))
})?;
let metadata: FeatureMetadata = serde_json::from_str(&metadata_str).map_err(|e| {
DevcontainerError::Feature(format!(
"failed to parse {}: {e}",
metadata_path.display()
))
})?;
Ok(metadata)
read_feature_metadata(&feature_dir).await
}
/// Fetch a local feature by copying its directory.
@ -221,28 +228,10 @@ async fn fetch_feature_local(
)));
}
let dir_name = dir_name_from_id(feature_id);
let feature_dir = output_dir.join(&dir_name);
let feature_dir = create_feature_dir(output_dir, feature_id).await?;
copy_dir_recursive(&local_path, &feature_dir).await?;
let metadata_path = feature_dir.join("devcontainer-feature.json");
let metadata_str = tokio::fs::read_to_string(&metadata_path)
.await
.map_err(|e| {
DevcontainerError::Feature(format!(
"failed to read {}: {e}",
metadata_path.display()
))
})?;
let metadata: FeatureMetadata = serde_json::from_str(&metadata_str).map_err(|e| {
DevcontainerError::Feature(format!(
"failed to parse {}: {e}",
metadata_path.display()
))
})?;
Ok(metadata)
read_feature_metadata(&feature_dir).await
}
/// Fetch a feature from an HTTPS URL (tgz archive).
@ -250,16 +239,7 @@ async fn fetch_feature_https(
feature_id: &str,
output_dir: &Path,
) -> crate::Result<FeatureMetadata> {
let dir_name = dir_name_from_id(feature_id);
let feature_dir = output_dir.join(&dir_name);
tokio::fs::create_dir_all(&feature_dir)
.await
.map_err(|e| {
DevcontainerError::Feature(format!(
"failed to create dir {}: {e}",
feature_dir.display()
))
})?;
let feature_dir = create_feature_dir(output_dir, feature_id).await?;
info!(feature_id, "downloading feature from HTTPS");
@ -286,37 +266,9 @@ async fn fetch_feature_https(
))
})?;
let status = tokio::process::Command::new("tar")
.args(["xzf", "devcontainer-feature.tgz"])
.current_dir(&feature_dir)
.status()
.await
.map_err(|e| DevcontainerError::Feature(format!("failed to extract tgz: {e}")))?;
extract_tgz(&feature_dir, feature_id).await?;
if !status.success() {
return Err(DevcontainerError::Feature(format!(
"tar extraction failed for {feature_id}"
)));
}
let metadata_path = feature_dir.join("devcontainer-feature.json");
let metadata_str = tokio::fs::read_to_string(&metadata_path)
.await
.map_err(|e| {
DevcontainerError::Feature(format!(
"failed to read {}: {e}",
metadata_path.display()
))
})?;
let metadata: FeatureMetadata = serde_json::from_str(&metadata_str).map_err(|e| {
DevcontainerError::Feature(format!(
"failed to parse {}: {e}",
metadata_path.display()
))
})?;
Ok(metadata)
read_feature_metadata(&feature_dir).await
}
/// Dispatch feature fetch based on the feature ID prefix.
@ -324,12 +276,17 @@ async fn fetch_feature_dispatch(
feature_id: &str,
output_dir: &Path,
devcontainer_dir: &Path,
oras_checked: &mut bool,
) -> crate::Result<FeatureMetadata> {
if feature_id.starts_with("./") || feature_id.starts_with("../") {
fetch_feature_local(feature_id, output_dir, devcontainer_dir).await
} else if feature_id.starts_with("https://") {
fetch_feature_https(feature_id, output_dir).await
} else {
if !*oras_checked {
ensure_oras().await?;
*oras_checked = true;
}
fetch_feature_oci(feature_id, output_dir).await
}
}
@ -560,9 +517,10 @@ pub async fn resolve_features(
let mut all_options: HashMap<String, serde_json::Value> = features.clone();
// Fetch all features and collect metadata
let mut oras_checked = false;
let mut metadata_map: HashMap<String, FeatureMetadata> = HashMap::new();
for feature_id in &feature_ids {
let metadata = fetch_feature_dispatch(feature_id, &tmp_dir, devcontainer_dir).await?;
let metadata = fetch_feature_dispatch(feature_id, &tmp_dir, devcontainer_dir, &mut oras_checked).await?;
metadata_map.insert(feature_id.clone(), metadata);
}
@ -581,7 +539,7 @@ pub async fn resolve_features(
});
if !already_present {
info!(dep_id, "auto-injecting missing dependsOn target");
let dep_metadata = fetch_feature_dispatch(dep_id, &tmp_dir, devcontainer_dir).await?;
let dep_metadata = fetch_feature_dispatch(dep_id, &tmp_dir, devcontainer_dir, &mut oras_checked).await?;
metadata_map.insert(dep_id.clone(), dep_metadata);
feature_ids.push(dep_id.clone());
all_options.insert(dep_id.clone(), dep_options.clone());

8
crates/arc-devcontainer/src/lib.rs
View file

@ -262,17 +262,11 @@ impl DevcontainerResolver {
merged_container_env.insert(k.clone(), variables::substitute(v, &vars));
}
}
let container_env_option = if merged_container_env.is_empty() {
None
} else {
Some(merged_container_env.clone())
};
// Generate final Dockerfile
let dockerfile_content = dockerfile::generate(
&base_dockerfile,
&resolved_features.layers,
&container_env_option,
&merged_container_env,
devcontainer.remote_user.as_deref(),
);