diff --git a/TODO_ARGS.md b/TODO_ARGS.md index 4af657d0d..e9274020f 100644 --- a/TODO_ARGS.md +++ b/TODO_ARGS.md @@ -2,6 +2,6 @@ Unsupported CLI args for the current pinned upstream command surface. -- Upstream commit: `39685cf1aa58b5b11e90085bd32562fad61f4103` +- Upstream commit: `2d81ee3c9ed96a7312c18c7513a17933f8f66d41` - Source: `upstream/src/spec-node/devContainersSpecCLI.ts` diff --git a/cmd/devcontainer/src/cli_metadata.json b/cmd/devcontainer/src/cli_metadata.json index a31b02178..3cf08055b 100644 --- a/cmd/devcontainer/src/cli_metadata.json +++ b/cmd/devcontainer/src/cli_metadata.json @@ -1,5 +1,5 @@ { - "upstreamCommit": "39685cf1aa58b5b11e90085bd32562fad61f4103", + "upstreamCommit": "2d81ee3c9ed96a7312c18c7513a17933f8f66d41", "sourcePath": "upstream/src/spec-node/devContainersSpecCLI.ts", "root": { "lines": [ diff --git a/cmd/devcontainer/src/commands/common.rs b/cmd/devcontainer/src/commands/common.rs index be0075d29..d622cc3cb 100644 --- a/cmd/devcontainer/src/commands/common.rs +++ b/cmd/devcontainer/src/commands/common.rs @@ -3,6 +3,7 @@ mod args; mod config_resolution; mod fs; +mod labels; mod manifest; pub(crate) use args::{ @@ -12,7 +13,13 @@ pub(crate) use args::{ validate_paired_options, }; pub(crate) use config_resolution::{ - load_resolved_config, resolve_override_config_path, resolve_read_configuration_path, + load_resolved_config, load_resolved_config_with_id_labels, resolve_override_config_path, + resolve_read_configuration_path, }; pub(crate) use fs::{copy_directory_recursive, package_collection_target}; +pub(crate) use labels::{ + default_devcontainer_id_label_pairs, default_devcontainer_id_labels, + normalize_devcontainer_label_path, normalize_devcontainer_label_path_for_platform, + DEVCONTAINER_CONFIG_FILE_LABEL, DEVCONTAINER_LOCAL_FOLDER_LABEL, +}; pub(crate) use manifest::{generate_manifest_docs, parse_manifest, ManifestDocOptions}; diff --git a/cmd/devcontainer/src/commands/common/config_resolution.rs b/cmd/devcontainer/src/commands/common/config_resolution.rs index 654c9491a..00a30cc62 100644 --- a/cmd/devcontainer/src/commands/common/config_resolution.rs +++ b/cmd/devcontainer/src/commands/common/config_resolution.rs @@ -10,7 +10,8 @@ use serde_json::Value; use crate::config::{self, ConfigContext}; use crate::runtime::mounts::mount_option_target; -use super::args::{parse_option_value, parse_option_values, validate_option_values}; +use super::args::{parse_option_value, validate_option_values}; +use super::labels::id_label_map; pub(crate) fn resolve_read_configuration_path( args: &[String], @@ -70,15 +71,31 @@ fn infer_workspace_folder_from_config(config_path: &Path) -> PathBuf { } pub(crate) fn load_resolved_config(args: &[String]) -> Result<(PathBuf, PathBuf, Value), String> { + load_resolved_config_with_label_override(args, None) +} + +pub(crate) fn load_resolved_config_with_id_labels( + args: &[String], + id_labels: HashMap, +) -> Result<(PathBuf, PathBuf, Value), String> { + load_resolved_config_with_label_override(args, Some(id_labels)) +} + +fn load_resolved_config_with_label_override( + args: &[String], + id_labels: Option>, +) -> Result<(PathBuf, PathBuf, Value), String> { let (workspace_folder, config_file) = resolve_read_configuration_path(args)?; let config_source = resolve_override_config_path(args)?.unwrap_or_else(|| config_file.clone()); let raw = fs::read_to_string(&config_source).map_err(|error| error.to_string())?; let parsed = config::parse_jsonc_value(&raw)?; + let id_labels = + id_labels.unwrap_or_else(|| id_label_map(args, &workspace_folder, &config_file)); let base_context = ConfigContext { workspace_folder: workspace_folder.clone(), env: env::vars().collect(), container_workspace_folder: None, - id_labels: id_label_map(args, &workspace_folder, &config_file), + id_labels: id_labels.clone(), }; let container_workspace_folder = parsed .get("workspaceFolder") @@ -123,7 +140,7 @@ pub(crate) fn load_resolved_config(args: &[String]) -> Result<(PathBuf, PathBuf, workspace_folder: base_context.workspace_folder.clone(), env: base_context.env, container_workspace_folder, - id_labels: base_context.id_labels, + id_labels, }, ); Ok((workspace_folder, config_file, substituted)) @@ -150,28 +167,48 @@ pub(crate) fn resolve_override_config_path(args: &[String]) -> Result HashMap { - let mut labels = parse_option_values(args, "--id-label") - .into_iter() - .filter_map(|entry| { - entry - .split_once('=') - .map(|(key, value)| (key.to_string(), value.to_string())) - }) - .collect::>(); - if labels.is_empty() { - labels.insert( - "devcontainer.local_folder".to_string(), - workspace_folder.display().to_string(), - ); - labels.insert( - "devcontainer.config_file".to_string(), - config_file.display().to_string(), +#[cfg(test)] +mod tests { + use std::collections::HashMap; + use std::fs; + + use crate::commands::common::DEVCONTAINER_LOCAL_FOLDER_LABEL; + use crate::test_support::unique_temp_dir; + + use super::{load_resolved_config, load_resolved_config_with_id_labels}; + + #[test] + fn load_resolved_config_with_id_labels_recomputes_devcontainer_id_from_override_labels() { + let workspace = unique_temp_dir("devcontainer-config-resolution"); + let config_dir = workspace.join(".devcontainer"); + let config_file = config_dir.join("devcontainer.json"); + fs::create_dir_all(&config_dir).expect("config dir"); + fs::write( + &config_file, + "{\n \"mounts\": [{\n \"source\": \"cache-${devcontainerId}\",\n \"target\": \"/cache\",\n \"type\": \"volume\"\n }],\n \"postAttachCommand\": \"echo ${devcontainerId}\"\n}\n", + ) + .expect("config write"); + + let args = vec![ + "--workspace-folder".to_string(), + workspace.display().to_string(), + ]; + let (_, _, current) = load_resolved_config(&args).expect("current config"); + let (_, _, legacy) = load_resolved_config_with_id_labels( + &args, + HashMap::from([( + DEVCONTAINER_LOCAL_FOLDER_LABEL.to_string(), + workspace.display().to_string(), + )]), + ) + .expect("legacy config"); + + assert_ne!( + current["mounts"][0]["source"], + legacy["mounts"][0]["source"] ); + assert_ne!(current["postAttachCommand"], legacy["postAttachCommand"]); + + let _ = fs::remove_dir_all(workspace); } - labels } diff --git a/cmd/devcontainer/src/commands/common/labels.rs b/cmd/devcontainer/src/commands/common/labels.rs new file mode 100644 index 000000000..f3dc333be --- /dev/null +++ b/cmd/devcontainer/src/commands/common/labels.rs @@ -0,0 +1,179 @@ +//! Shared helpers for default devcontainer id-label generation and normalization. + +use std::collections::HashMap; +use std::path::Path; + +use super::args::parse_option_values; + +pub(crate) const DEVCONTAINER_LOCAL_FOLDER_LABEL: &str = "devcontainer.local_folder"; +pub(crate) const DEVCONTAINER_CONFIG_FILE_LABEL: &str = "devcontainer.config_file"; + +pub(crate) fn id_label_map( + args: &[String], + workspace_folder: &Path, + config_file: &Path, +) -> HashMap { + let mut labels = parse_option_values(args, "--id-label") + .into_iter() + .filter_map(|entry| { + entry + .split_once('=') + .map(|(key, value)| (key.to_string(), value.to_string())) + }) + .collect::>(); + if labels.is_empty() { + labels.extend(default_devcontainer_id_label_pairs( + workspace_folder, + config_file, + )); + } + labels +} + +pub(crate) fn default_devcontainer_id_labels( + workspace_folder: &Path, + config_file: &Path, +) -> Vec { + default_devcontainer_id_label_pairs(workspace_folder, config_file) + .into_iter() + .map(|(key, value)| format!("{key}={value}")) + .collect() +} + +pub(crate) fn default_devcontainer_id_label_pairs( + workspace_folder: &Path, + config_file: &Path, +) -> [(String, String); 2] { + default_devcontainer_id_label_pairs_for_platform( + std::env::consts::OS, + workspace_folder, + config_file, + ) +} + +pub(crate) fn default_devcontainer_id_label_pairs_for_platform( + platform: &str, + workspace_folder: &Path, + config_file: &Path, +) -> [(String, String); 2] { + [ + ( + DEVCONTAINER_LOCAL_FOLDER_LABEL.to_string(), + normalize_devcontainer_label_path_for_platform( + platform, + &workspace_folder.display().to_string(), + ), + ), + ( + DEVCONTAINER_CONFIG_FILE_LABEL.to_string(), + normalize_devcontainer_label_path_for_platform( + platform, + &config_file.display().to_string(), + ), + ), + ] +} + +pub(crate) fn normalize_devcontainer_label_path(value: &str) -> String { + normalize_devcontainer_label_path_for_platform(std::env::consts::OS, value) +} + +pub(crate) fn normalize_devcontainer_label_path_for_platform( + platform: &str, + value: &str, +) -> String { + if platform != "windows" { + return value.to_string(); + } + + normalize_windows_label_path(value) +} + +fn normalize_windows_label_path(value: &str) -> String { + let value = value.replace('/', "\\"); + let bytes = value.as_bytes(); + let (prefix, rest, absolute) = if let Some(rest) = value.strip_prefix("\\\\") { + ("\\\\".to_string(), rest, true) + } else if bytes.len() >= 2 && bytes[1] == b':' { + let drive = value[..1].to_ascii_lowercase(); + let rest = &value[2..]; + (format!("{drive}:"), rest, rest.starts_with('\\')) + } else { + (String::new(), value.as_str(), value.starts_with('\\')) + }; + + let mut segments = Vec::new(); + for segment in rest.split('\\') { + if segment.is_empty() || segment == "." { + continue; + } + if segment == ".." { + if segments.last().is_some_and(|last| last != "..") { + segments.pop(); + } else if !absolute { + segments.push(segment.to_string()); + } + continue; + } + segments.push(segment.to_string()); + } + + let mut normalized = prefix; + if absolute && !normalized.ends_with('\\') { + normalized.push('\\'); + } + normalized.push_str(&segments.join("\\")); + if normalized.is_empty() { + ".".to_string() + } else { + normalized + } +} + +#[cfg(test)] +mod tests { + use std::path::Path; + + use super::{ + default_devcontainer_id_label_pairs_for_platform, + normalize_devcontainer_label_path_for_platform, DEVCONTAINER_CONFIG_FILE_LABEL, + DEVCONTAINER_LOCAL_FOLDER_LABEL, + }; + + #[test] + fn normalize_devcontainer_label_path_lowercases_windows_drive_letters() { + assert_eq!( + normalize_devcontainer_label_path_for_platform("windows", "C:\\CodeBlocks\\remill"), + "c:\\CodeBlocks\\remill" + ); + } + + #[test] + fn normalize_devcontainer_label_path_normalizes_windows_separators_and_segments() { + assert_eq!( + normalize_devcontainer_label_path_for_platform( + "windows", + "C:/CodeBlocks/remill/.devcontainer/../devcontainer.json" + ), + "c:\\CodeBlocks\\remill\\devcontainer.json" + ); + } + + #[test] + fn default_devcontainer_id_labels_use_normalized_windows_paths() { + let [(workspace_key, workspace_value), (config_key, config_value)] = + default_devcontainer_id_label_pairs_for_platform( + "windows", + Path::new("C:/CodeBlocks/remill"), + Path::new("C:/CodeBlocks/remill/.devcontainer/devcontainer.json"), + ); + + assert_eq!(workspace_key, DEVCONTAINER_LOCAL_FOLDER_LABEL); + assert_eq!(workspace_value, "c:\\CodeBlocks\\remill"); + assert_eq!(config_key, DEVCONTAINER_CONFIG_FILE_LABEL); + assert_eq!( + config_value, + "c:\\CodeBlocks\\remill\\.devcontainer\\devcontainer.json" + ); + } +} diff --git a/cmd/devcontainer/src/commands/configuration/tests/upgrade.rs b/cmd/devcontainer/src/commands/configuration/tests/upgrade.rs index 73b2d9787..dcfe6b093 100644 --- a/cmd/devcontainer/src/commands/configuration/tests/upgrade.rs +++ b/cmd/devcontainer/src/commands/configuration/tests/upgrade.rs @@ -7,6 +7,7 @@ use serde_json::json; use sha2::{Digest, Sha256}; use super::support::unique_temp_dir; +use crate::commands::configuration::ensure_native_lockfile; use crate::commands::configuration::upgrade::{ build_outdated_payload, feature_id_without_version, lockfile_path, run_upgrade_lockfile, }; @@ -188,6 +189,122 @@ fn upgrade_lockfile_reads_workspace_oci_layout_digests() { let _ = fs::remove_dir_all(root); } +#[test] +fn ensure_native_lockfile_uses_shared_lockfile_format() { + let root = unique_temp_dir(); + fs::create_dir_all(&root).expect("failed to create root"); + let config_file = root.join(".devcontainer.json"); + + ensure_native_lockfile( + &[ + "--workspace-folder".to_string(), + root.display().to_string(), + "--experimental-lockfile".to_string(), + ], + &config_file, + &json!({ + "image": "debian:bookworm", + "features": { + "ghcr.io/devcontainers/features/github-cli": {} + } + }), + ) + .expect("lockfile write"); + + let lockfile = fs::read_to_string(root.join(".devcontainer-lock.json")).expect("lockfile"); + assert!(!lockfile.ends_with('\n')); + let _ = fs::remove_dir_all(root); +} + +#[test] +fn upgrade_lockfile_uses_shared_lockfile_format() { + let root = unique_temp_dir(); + fs::create_dir_all(&root).expect("failed to create root"); + fs::write( + root.join(".devcontainer.json"), + "{\n \"image\": \"debian:bookworm\",\n \"features\": {\n \"ghcr.io/devcontainers/features/github-cli\": {}\n }\n}\n", + ) + .expect("failed to write config"); + + run_upgrade_lockfile(&["--workspace-folder".to_string(), root.display().to_string()]) + .expect("lockfile payload"); + + let lockfile = fs::read_to_string(root.join(".devcontainer-lock.json")).expect("lockfile"); + assert!(!lockfile.ends_with('\n')); + + let _ = fs::remove_dir_all(root); +} + +#[test] +fn ensure_native_lockfile_reports_missing_frozen_lockfile() { + let root = unique_temp_dir(); + fs::create_dir_all(&root).expect("failed to create root"); + let config_file = root.join(".devcontainer.json"); + + let error = ensure_native_lockfile( + &[ + "--workspace-folder".to_string(), + root.display().to_string(), + "--experimental-frozen-lockfile".to_string(), + ], + &config_file, + &json!({ + "image": "debian:bookworm", + "features": { + "ghcr.io/devcontainers/features/github-cli": {} + } + }), + ) + .expect_err("missing frozen lockfile error"); + + assert_eq!(error, "Lockfile does not exist."); + let _ = fs::remove_dir_all(root); +} + +#[test] +fn ensure_native_lockfile_accepts_semantically_identical_existing_json() { + let root = unique_temp_dir(); + fs::create_dir_all(&root).expect("failed to create root"); + let config_file = root.join(".devcontainer.json"); + ensure_native_lockfile( + &[ + "--workspace-folder".to_string(), + root.display().to_string(), + "--experimental-lockfile".to_string(), + ], + &config_file, + &json!({ + "image": "debian:bookworm", + "features": { + "ghcr.io/devcontainers/features/github-cli": {} + } + }), + ) + .expect("lockfile seed"); + let lockfile_path = root.join(".devcontainer-lock.json"); + let lockfile = fs::read_to_string(&lockfile_path).expect("lockfile"); + let reformatted = lockfile.trim_end_matches('\n').to_string(); + fs::write(&lockfile_path, reformatted).expect("lockfile rewrite"); + + ensure_native_lockfile( + &[ + "--workspace-folder".to_string(), + root.display().to_string(), + "--experimental-frozen-lockfile".to_string(), + ], + &config_file, + &json!({ + "image": "debian:bookworm", + "features": { + "ghcr.io/devcontainers/features/github-cli": {} + } + }), + ) + .expect("lockfile match"); + + let _ = fs::remove_dir_all(root); +} + fn write_workspace_layout_version( workspace_root: &Path, base: &str, diff --git a/cmd/devcontainer/src/commands/configuration/upgrade.rs b/cmd/devcontainer/src/commands/configuration/upgrade.rs index 82711217c..50ad75cea 100644 --- a/cmd/devcontainer/src/commands/configuration/upgrade.rs +++ b/cmd/devcontainer/src/commands/configuration/upgrade.rs @@ -85,7 +85,10 @@ pub(super) fn ensure_native_lockfile( let path = lockfile_path(config_file); if common::has_flag(args, "--experimental-frozen-lockfile") { let existing = read_lockfile(path.clone())?; - if existing.as_ref() != Some(&generated) { + let Some(existing) = existing else { + return Err("Lockfile does not exist.".to_string()); + }; + if existing != generated { return Err(format!( "Lockfile at {} is out of date for the current feature configuration", path.display() @@ -93,15 +96,16 @@ pub(super) fn ensure_native_lockfile( } } if common::has_flag(args, "--experimental-lockfile") { - fs::write( - &path, - serde_json::to_string_pretty(&generated).map_err(|error| error.to_string())?, - ) - .map_err(|error| error.to_string())?; + let lockfile = serialized_lockfile(&generated)?; + fs::write(&path, lockfile).map_err(|error| error.to_string())?; } Ok(()) } +fn serialized_lockfile(lockfile: &Lockfile) -> Result { + serde_json::to_string_pretty(lockfile).map_err(|error| error.to_string()) +} + #[cfg(test)] pub(super) fn build_outdated_payload(args: &[String]) -> Result { build_outdated_payload_with_logger(args, None) @@ -238,11 +242,8 @@ fn run_upgrade_lockfile_with_logger( if let Some(logger) = logger { logger.info(format!("Writing lockfile: '{}'", lockfile_path.display())); } - fs::write( - &lockfile_path, - serde_json::to_string_pretty(&generated).map_err(|error| error.to_string())?, - ) - .map_err(|error| error.to_string())?; + fs::write(&lockfile_path, serialized_lockfile(&generated)?) + .map_err(|error| error.to_string())?; if let Some(logger) = logger { logger.debug(format!( "Lockfile write complete: '{}'", diff --git a/cmd/devcontainer/src/runtime/compose/override_file.rs b/cmd/devcontainer/src/runtime/compose/override_file.rs index e7a865e0f..24547ba3c 100644 --- a/cmd/devcontainer/src/runtime/compose/override_file.rs +++ b/cmd/devcontainer/src/runtime/compose/override_file.rs @@ -65,17 +65,9 @@ pub(super) fn compose_metadata_override_file( remote_workspace_folder, common::runtime_options(args).omit_config_remote_env_from_metadata, )?; - let mut labels = vec![ - format!( - "devcontainer.local_folder={}", - resolved.workspace_folder.display() - ), - format!( - "devcontainer.config_file={}", - resolved.config_file.display() - ), - format!("devcontainer.metadata={metadata}"), - ]; + let mut labels = + common::default_devcontainer_id_labels(&resolved.workspace_folder, &resolved.config_file); + labels.push(format!("devcontainer.metadata={metadata}")); labels.extend(common::parse_option_values(args, "--id-label")); if labels.is_empty() { return Ok(None); diff --git a/cmd/devcontainer/src/runtime/compose/tests.rs b/cmd/devcontainer/src/runtime/compose/tests.rs index b51f34914..f50046eaf 100644 --- a/cmd/devcontainer/src/runtime/compose/tests.rs +++ b/cmd/devcontainer/src/runtime/compose/tests.rs @@ -723,8 +723,7 @@ fn compose_feature_build_enforces_frozen_lockfile() { ) .expect_err("expected frozen lockfile enforcement"); - assert!(error.contains("Lockfile at")); - assert!(error.contains("is out of date for the current feature configuration")); + assert_eq!(error, "Lockfile does not exist."); let _ = fs::remove_dir_all(root); } diff --git a/cmd/devcontainer/src/runtime/container.rs b/cmd/devcontainer/src/runtime/container.rs index f2ab34f0e..84ff7036e 100644 --- a/cmd/devcontainer/src/runtime/container.rs +++ b/cmd/devcontainer/src/runtime/container.rs @@ -4,13 +4,19 @@ mod discovery; mod engine_run; mod uid_update; +use std::collections::HashMap; + use super::lifecycle::LifecycleMode; -pub(crate) use discovery::{ensure_up_container, resolve_target_container}; +pub(crate) use discovery::{ + ensure_up_container, probe_up_container_id_labels, resolve_target_container_match, + ResolvedTargetContainer, +}; pub(crate) use engine_run::should_add_gpu_capability; pub(crate) use uid_update::prepare_up_image; pub(crate) struct UpContainer { pub(crate) container_id: String, pub(crate) lifecycle_mode: LifecycleMode, + pub(crate) matched_id_labels: Option>, } diff --git a/cmd/devcontainer/src/runtime/container/discovery.rs b/cmd/devcontainer/src/runtime/container/discovery.rs index 9fb057aab..abf5928ae 100644 --- a/cmd/devcontainer/src/runtime/container/discovery.rs +++ b/cmd/devcontainer/src/runtime/container/discovery.rs @@ -1,7 +1,10 @@ //! Container discovery, reuse, and creation orchestration for native runtime flows. +use std::collections::HashMap; use std::path::Path; +use serde_json::Value; + use crate::commands::common; use super::super::compose; @@ -11,6 +14,12 @@ use super::super::lifecycle::LifecycleMode; use super::engine_run::{remove_container, start_container, start_existing_container}; use super::UpContainer; +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct ResolvedTargetContainer { + pub(crate) container_id: String, + pub(crate) id_labels: Option>, +} + pub(crate) fn ensure_up_container( resolved: &ResolvedConfig, args: &[String], @@ -24,6 +33,54 @@ pub(crate) fn ensure_up_container( ensure_engine_up_container(resolved, args, image_name, remote_workspace_folder) } +pub(crate) fn probe_up_container_id_labels( + resolved: &ResolvedConfig, + args: &[String], +) -> Result>, String> { + if common::has_flag(args, "--remove-existing-container") { + return Ok(None); + } + + if compose::uses_compose_config(&resolved.configuration) { + if let Some(container_id) = compose::resolve_container_id(resolved, args)? { + return inspect_matched_default_id_labels( + args, + &container_id, + Some(resolved.workspace_folder.as_path()), + Some(resolved.config_file.as_path()), + ); + } + if let Some(container_id) = compose::resolve_container_id_including_stopped(resolved, args)? + { + return inspect_matched_default_id_labels( + args, + &container_id, + Some(resolved.workspace_folder.as_path()), + Some(resolved.config_file.as_path()), + ); + } + return Ok(None); + } + + if let Some(target) = find_target_container( + args, + Some(resolved.workspace_folder.as_path()), + Some(resolved.config_file.as_path()), + false, + )? { + return Ok(target.id_labels); + } + if let Some(target) = find_target_container( + args, + Some(resolved.workspace_folder.as_path()), + Some(resolved.config_file.as_path()), + true, + )? { + return Ok(target.id_labels); + } + Ok(None) +} + fn ensure_compose_up_container( resolved: &ResolvedConfig, args: &[String], @@ -82,12 +139,13 @@ fn ensure_engine_up_container( )?; let remove_existing = common::has_flag(args, "--remove-existing-container"); match running { - Some(container_id) if remove_existing => { - remove_container(args, &container_id)?; + Some(target) if remove_existing => { + remove_container(args, &target.container_id)?; create_engine_container(resolved, args, image_name, remote_workspace_folder) } - Some(container_id) => Ok(UpContainer { - container_id, + Some(target) => Ok(UpContainer { + container_id: target.container_id, + matched_id_labels: target.id_labels, lifecycle_mode: LifecycleMode::UpReused, }), None => match find_target_container( @@ -96,14 +154,15 @@ fn ensure_engine_up_container( Some(resolved.config_file.as_path()), true, )? { - Some(container_id) if remove_existing => { - remove_container(args, &container_id)?; + Some(target) if remove_existing => { + remove_container(args, &target.container_id)?; create_engine_container(resolved, args, image_name, remote_workspace_folder) } - Some(container_id) => { - start_existing_container(args, &container_id)?; + Some(target) => { + start_existing_container(args, &target.container_id)?; Ok(UpContainer { - container_id, + container_id: target.container_id, + matched_id_labels: target.id_labels, lifecycle_mode: LifecycleMode::UpStarted, }) } @@ -126,6 +185,7 @@ fn create_compose_container( .ok_or_else(|| "Dev container not found.".to_string())?; Ok(UpContainer { container_id, + matched_id_labels: None, lifecycle_mode: LifecycleMode::UpCreated, }) } @@ -141,6 +201,16 @@ fn refresh_compose_container( compose::up_service(resolved, args, remote_workspace_folder, image_name, true)?; let updated_container_id = compose::resolve_container_id(resolved, args)? .ok_or_else(|| "Dev container not found.".to_string())?; + let matched_id_labels = if updated_container_id == previous_container_id { + inspect_matched_default_id_labels( + args, + &updated_container_id, + Some(resolved.workspace_folder.as_path()), + Some(resolved.config_file.as_path()), + )? + } else { + None + }; Ok(UpContainer { lifecycle_mode: if updated_container_id == previous_container_id { unchanged_mode @@ -148,6 +218,7 @@ fn refresh_compose_container( LifecycleMode::UpCreated }, container_id: updated_container_id, + matched_id_labels, }) } @@ -160,22 +231,28 @@ fn create_engine_container( start_container(resolved, args, image_name, remote_workspace_folder).map(|container_id| { UpContainer { container_id, + matched_id_labels: None, lifecycle_mode: LifecycleMode::UpCreated, } }) } -pub(crate) fn resolve_target_container( +pub(crate) fn resolve_target_container_match( args: &[String], workspace_folder: Option<&Path>, config_file: Option<&Path>, -) -> Result { +) -> Result { if let Some(container_id) = common::parse_option_value(args, "--container-id") { - return Ok(container_id); + let id_labels = + inspect_matched_default_id_labels(args, &container_id, workspace_folder, config_file)?; + return Ok(ResolvedTargetContainer { + container_id, + id_labels, + }); } match find_target_container(args, workspace_folder, config_file, false)? { - Some(container_id) => Ok(container_id), + Some(target) => Ok(target), None => Err("Dev container not found.".to_string()), } } @@ -185,7 +262,8 @@ fn find_target_container( workspace_folder: Option<&Path>, config_file: Option<&Path>, include_stopped: bool, -) -> Result, String> { +) -> Result, String> { + let has_explicit_id_labels = !common::parse_option_values(args, "--id-label").is_empty(); let labels = target_container_labels(args, workspace_folder, config_file); if labels.is_empty() { return Err( @@ -194,6 +272,45 @@ fn find_target_container( ); } + if let Some(mut target) = query_target_container(args, &labels, include_stopped)? { + if !has_explicit_id_labels { + target.id_labels = inspect_matched_default_id_labels( + args, + &target.container_id, + workspace_folder, + config_file, + )?; + } + return Ok(Some(target)); + } + + if has_explicit_id_labels || std::env::consts::OS != "windows" { + return Ok(None); + } + + find_normalized_default_label_match(args, workspace_folder, config_file, include_stopped) +} + +fn query_target_container( + args: &[String], + labels: &[String], + include_stopped: bool, +) -> Result, String> { + let result = engine::run_engine(args, ps_engine_args(labels, include_stopped))?; + if result.status_code != 0 { + return Err(engine::stderr_or_stdout(&result)); + } + + Ok(parse_container_ids(&result.stdout) + .into_iter() + .next() + .map(|container_id| ResolvedTargetContainer { + container_id, + id_labels: None, + })) +} + +fn ps_engine_args(labels: &[String], include_stopped: bool) -> Vec { let mut engine_args = vec!["ps".to_string(), "-q".to_string()]; if include_stopped { engine_args.push("-a".to_string()); @@ -202,18 +319,229 @@ fn find_target_container( engine_args.push("--filter".to_string()); engine_args.push(format!("label={label}")); } + engine_args +} - let result = engine::run_engine(args, engine_args)?; +fn find_normalized_default_label_match( + args: &[String], + workspace_folder: Option<&Path>, + config_file: Option<&Path>, + include_stopped: bool, +) -> Result, String> { + let Some(workspace_folder) = workspace_folder else { + return Ok(None); + }; + let [(_, normalized_workspace), (_, normalized_config)] = + common::default_devcontainer_id_label_pairs( + workspace_folder, + config_file.unwrap_or(workspace_folder), + ); + let candidate_ids = list_container_ids_by_label_name( + args, + common::DEVCONTAINER_LOCAL_FOLDER_LABEL, + include_stopped, + )?; + let mut legacy_match = None; + for container_id in candidate_ids { + let Some(labels) = inspect_container_labels(args, &container_id)? else { + continue; + }; + match normalized_default_label_match( + &labels, + normalized_workspace.as_str(), + config_file.map(|_| normalized_config.as_str()), + common::DEVCONTAINER_LOCAL_FOLDER_LABEL, + common::DEVCONTAINER_CONFIG_FILE_LABEL, + ) { + Some(DefaultLabelMatch::Current) => { + return Ok(Some(ResolvedTargetContainer { + container_id, + id_labels: None, + })) + } + Some(DefaultLabelMatch::Legacy) if legacy_match.is_none() => { + legacy_match = Some(ResolvedTargetContainer { + container_id, + id_labels: Some(legacy_default_id_labels( + &labels, + common::DEVCONTAINER_LOCAL_FOLDER_LABEL, + common::DEVCONTAINER_CONFIG_FILE_LABEL, + )), + }); + } + _ => {} + } + } + Ok(legacy_match) +} + +fn list_container_ids_by_label_name( + args: &[String], + label_name: &str, + include_stopped: bool, +) -> Result, String> { + let result = engine::run_engine( + args, + ps_engine_args(&[label_name.to_string()], include_stopped), + )?; if result.status_code != 0 { return Err(engine::stderr_or_stdout(&result)); } + Ok(parse_container_ids(&result.stdout)) +} - Ok(result - .stdout +fn parse_container_ids(stdout: &str) -> Vec { + stdout .lines() .map(str::trim) - .find(|line| !line.is_empty() && !line.chars().any(char::is_whitespace)) - .map(str::to_string)) + .filter(|line| !line.is_empty() && !line.chars().any(char::is_whitespace)) + .map(str::to_string) + .collect() +} + +fn inspect_container_labels( + args: &[String], + container_id: &str, +) -> Result>, String> { + let result = engine::run_engine(args, vec!["inspect".to_string(), container_id.to_string()])?; + if result.status_code != 0 { + return Err(engine::stderr_or_stdout(&result)); + } + let inspected: Value = serde_json::from_str(&result.stdout) + .map_err(|error| format!("Invalid inspect JSON: {error}"))?; + Ok(inspected + .as_array() + .and_then(|entries| entries.first()) + .and_then(|details| details.get("Config")) + .and_then(|config| config.get("Labels")) + .and_then(Value::as_object) + .map(|labels| { + labels + .iter() + .filter_map(|(key, value)| { + value.as_str().map(|value| (key.clone(), value.to_string())) + }) + .collect() + })) +} + +fn inspect_matched_default_id_labels( + args: &[String], + container_id: &str, + workspace_folder: Option<&Path>, + config_file: Option<&Path>, +) -> Result>, String> { + let Some(workspace_folder) = workspace_folder else { + return Ok(None); + }; + let Some(labels) = inspect_container_labels(args, container_id)? else { + return Ok(None); + }; + Ok(matched_default_id_labels_for_platform( + std::env::consts::OS, + &labels, + workspace_folder, + config_file, + )) +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum DefaultLabelMatch { + Current, + Legacy, +} + +fn normalized_default_label_match( + labels: &HashMap, + normalized_workspace: &str, + normalized_config: Option<&str>, + workspace_key: &str, + config_key: &str, +) -> Option { + default_label_match_for_platform( + "windows", + labels, + normalized_workspace, + normalized_config, + workspace_key, + config_key, + ) +} + +fn default_label_match_for_platform( + platform: &str, + labels: &HashMap, + normalized_workspace: &str, + normalized_config: Option<&str>, + workspace_key: &str, + config_key: &str, +) -> Option { + let workspace_value = labels + .get(workspace_key) + .map(|value| common::normalize_devcontainer_label_path_for_platform(platform, value))?; + if workspace_value != normalized_workspace { + return None; + } + + match ( + normalized_config, + labels + .get(config_key) + .map(|value| common::normalize_devcontainer_label_path_for_platform(platform, value)), + ) { + (Some(target_config), Some(container_config)) if container_config == target_config => { + Some(DefaultLabelMatch::Current) + } + (Some(_), None) => Some(DefaultLabelMatch::Legacy), + (None, _) => Some(DefaultLabelMatch::Current), + _ => None, + } +} + +fn legacy_default_id_labels( + labels: &HashMap, + workspace_key: &str, + _config_key: &str, +) -> HashMap { + labels + .get(workspace_key) + .map(|workspace_value| { + HashMap::from([(workspace_key.to_string(), workspace_value.to_string())]) + }) + .unwrap_or_default() +} + +fn matched_default_id_labels_for_platform( + platform: &str, + labels: &HashMap, + workspace_folder: &Path, + config_file: Option<&Path>, +) -> Option> { + let normalized_workspace = common::normalize_devcontainer_label_path_for_platform( + platform, + &workspace_folder.display().to_string(), + ); + let normalized_config = config_file.map(|config_file| { + common::normalize_devcontainer_label_path_for_platform( + platform, + &config_file.display().to_string(), + ) + }); + match default_label_match_for_platform( + platform, + labels, + normalized_workspace.as_str(), + normalized_config.as_deref(), + common::DEVCONTAINER_LOCAL_FOLDER_LABEL, + common::DEVCONTAINER_CONFIG_FILE_LABEL, + ) { + Some(DefaultLabelMatch::Legacy) => Some(legacy_default_id_labels( + labels, + common::DEVCONTAINER_LOCAL_FOLDER_LABEL, + common::DEVCONTAINER_CONFIG_FILE_LABEL, + )), + _ => None, + } } fn target_container_labels( @@ -223,18 +551,163 @@ fn target_container_labels( ) -> Vec { let mut labels = common::parse_option_values(args, "--id-label"); if labels.is_empty() { - if let Some(workspace_folder) = workspace_folder { - labels.push(format!( - "devcontainer.local_folder={}", - workspace_folder.display() - )); - } - if let Some(config_file) = config_file { - labels.push(format!( - "devcontainer.config_file={}", - config_file.display() + if let (Some(workspace_folder), Some(config_file)) = (workspace_folder, config_file) { + labels.extend(common::default_devcontainer_id_labels( + workspace_folder, + config_file, )); + } else if let Some(workspace_folder) = workspace_folder { + let [(workspace_key, workspace_value), _] = + common::default_devcontainer_id_label_pairs(workspace_folder, workspace_folder); + labels.push(format!("{workspace_key}={workspace_value}")); } } labels } + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + use std::path::Path; + + use super::{ + legacy_default_id_labels, matched_default_id_labels_for_platform, + normalized_default_label_match, DefaultLabelMatch, + }; + use crate::commands::common; + + #[test] + fn normalized_default_label_match_accepts_windows_path_casing_changes() { + let mut labels = HashMap::new(); + labels.insert( + common::DEVCONTAINER_LOCAL_FOLDER_LABEL.to_string(), + "C:\\CodeBlocks\\remill".to_string(), + ); + labels.insert( + common::DEVCONTAINER_CONFIG_FILE_LABEL.to_string(), + "C:/CodeBlocks/remill/.devcontainer/devcontainer.json".to_string(), + ); + + let label_match = normalized_default_label_match( + &labels, + "c:\\CodeBlocks\\remill", + Some("c:\\CodeBlocks\\remill\\.devcontainer\\devcontainer.json"), + common::DEVCONTAINER_LOCAL_FOLDER_LABEL, + common::DEVCONTAINER_CONFIG_FILE_LABEL, + ); + + assert_eq!(label_match, Some(DefaultLabelMatch::Current)); + } + + #[test] + fn normalized_default_label_match_keeps_legacy_workspace_only_matches() { + let mut labels = HashMap::new(); + labels.insert( + common::DEVCONTAINER_LOCAL_FOLDER_LABEL.to_string(), + "C:\\CodeBlocks\\remill".to_string(), + ); + + let label_match = normalized_default_label_match( + &labels, + "c:\\CodeBlocks\\remill", + Some("c:\\CodeBlocks\\remill\\.devcontainer\\devcontainer.json"), + common::DEVCONTAINER_LOCAL_FOLDER_LABEL, + common::DEVCONTAINER_CONFIG_FILE_LABEL, + ); + + assert_eq!(label_match, Some(DefaultLabelMatch::Legacy)); + } + + #[test] + fn legacy_default_id_labels_preserve_workspace_only_label_set() { + let mut labels = HashMap::new(); + labels.insert( + common::DEVCONTAINER_LOCAL_FOLDER_LABEL.to_string(), + "C:\\CodeBlocks\\remill".to_string(), + ); + labels.insert("unrelated".to_string(), "ignored".to_string()); + + assert_eq!( + legacy_default_id_labels( + &labels, + common::DEVCONTAINER_LOCAL_FOLDER_LABEL, + common::DEVCONTAINER_CONFIG_FILE_LABEL, + ), + HashMap::from([( + common::DEVCONTAINER_LOCAL_FOLDER_LABEL.to_string(), + "C:\\CodeBlocks\\remill".to_string(), + )]) + ); + } + + #[test] + fn matched_default_id_labels_for_platform_preserves_legacy_windows_workspace_only_labels() { + let mut labels = HashMap::new(); + labels.insert( + common::DEVCONTAINER_LOCAL_FOLDER_LABEL.to_string(), + "C:\\CodeBlocks\\remill".to_string(), + ); + + assert_eq!( + matched_default_id_labels_for_platform( + "windows", + &labels, + Path::new("C:/CodeBlocks/remill"), + Some(Path::new( + "C:/CodeBlocks/remill/.devcontainer/devcontainer.json" + )), + ), + Some(HashMap::from([( + common::DEVCONTAINER_LOCAL_FOLDER_LABEL.to_string(), + "C:\\CodeBlocks\\remill".to_string(), + )])) + ); + } + + #[test] + fn matched_default_id_labels_for_platform_ignores_current_label_sets() { + let mut labels = HashMap::new(); + labels.insert( + common::DEVCONTAINER_LOCAL_FOLDER_LABEL.to_string(), + "c:\\CodeBlocks\\remill".to_string(), + ); + labels.insert( + common::DEVCONTAINER_CONFIG_FILE_LABEL.to_string(), + "c:\\CodeBlocks\\remill\\.devcontainer\\devcontainer.json".to_string(), + ); + + assert_eq!( + matched_default_id_labels_for_platform( + "windows", + &labels, + Path::new("C:/CodeBlocks/remill"), + Some(Path::new( + "C:/CodeBlocks/remill/.devcontainer/devcontainer.json" + )), + ), + None + ); + } + + #[test] + fn matched_default_id_labels_for_platform_preserves_legacy_posix_workspace_only_labels() { + let mut labels = HashMap::new(); + labels.insert( + common::DEVCONTAINER_LOCAL_FOLDER_LABEL.to_string(), + "/tmp/remill".to_string(), + ); + + assert_eq!( + matched_default_id_labels_for_platform( + "macos", + &labels, + Path::new("/tmp/remill"), + Some(Path::new("/tmp/remill/.devcontainer/devcontainer.json")), + ), + Some(HashMap::from([( + common::DEVCONTAINER_LOCAL_FOLDER_LABEL.to_string(), + "/tmp/remill".to_string(), + )])) + ); + } +} diff --git a/cmd/devcontainer/src/runtime/container/engine_run.rs b/cmd/devcontainer/src/runtime/container/engine_run.rs index 4ec74bfdd..0b4910024 100644 --- a/cmd/devcontainer/src/runtime/container/engine_run.rs +++ b/cmd/devcontainer/src/runtime/container/engine_run.rs @@ -17,19 +17,15 @@ pub(super) fn start_container( image_name: &str, remote_workspace_folder: &str, ) -> Result { + let default_labels = + common::default_devcontainer_id_labels(&resolved.workspace_folder, &resolved.config_file); let mut engine_args = vec![ "run".to_string(), "-d".to_string(), "--label".to_string(), - format!( - "devcontainer.local_folder={}", - resolved.workspace_folder.display() - ), + default_labels[0].clone(), "--label".to_string(), - format!( - "devcontainer.config_file={}", - resolved.config_file.display() - ), + default_labels[1].clone(), "--label".to_string(), format!( "devcontainer.metadata={}", diff --git a/cmd/devcontainer/src/runtime/context.rs b/cmd/devcontainer/src/runtime/context.rs index 00c2ffac3..c7ca66d10 100644 --- a/cmd/devcontainer/src/runtime/context.rs +++ b/cmd/devcontainer/src/runtime/context.rs @@ -3,6 +3,7 @@ mod inspection; mod workspace; +use std::collections::HashMap; use std::path::PathBuf; use serde_json::Value; @@ -47,6 +48,19 @@ pub(crate) fn load_required_config(args: &[String]) -> Result, +) -> Result { + let (workspace_folder, config_file, configuration) = + common::load_resolved_config_with_id_labels(args, id_labels)?; + Ok(ResolvedConfig { + workspace_folder, + config_file, + configuration, + }) +} + pub(crate) fn load_optional_config(args: &[String]) -> Result, String> { let explicit_config = common::parse_option_value(args, "--config"); match load_required_config(args) { @@ -83,7 +97,10 @@ pub(crate) fn resolve_existing_container_context( } else { workspace_folder_from_args(args)? }; - let container_id = container::resolve_target_container( + let container::ResolvedTargetContainer { + container_id, + id_labels, + } = container::resolve_target_container_match( args, resolved .as_ref() @@ -91,6 +108,10 @@ pub(crate) fn resolve_existing_container_context( .or(workspace_folder.as_deref()), resolved.as_ref().map(|value| value.config_file.as_path()), )?; + let resolved = match (resolved, id_labels) { + (Some(_), Some(id_labels)) => Some(load_required_config_with_id_labels(args, id_labels)?), + (resolved, _) => resolved, + }; let inspected = if resolved.is_none() { Some(inspect_container_context(args, &container_id)?) } else { diff --git a/cmd/devcontainer/src/runtime/context/inspection.rs b/cmd/devcontainer/src/runtime/context/inspection.rs index 3c11593b9..b7cf4721c 100644 --- a/cmd/devcontainer/src/runtime/context/inspection.rs +++ b/cmd/devcontainer/src/runtime/context/inspection.rs @@ -88,9 +88,15 @@ fn inspect_workspace_mount( ) -> Option { let mounts = details.get("Mounts").and_then(Value::as_array)?; if let Some(local_workspace_folder) = local_workspace_folder { - let local_workspace_folder = local_workspace_folder.display().to_string(); + let local_workspace_folder = common::normalize_devcontainer_label_path( + &local_workspace_folder.display().to_string(), + ); if let Some(destination) = mounts.iter().find_map(|mount| { - (mount.get("Source").and_then(Value::as_str) == Some(local_workspace_folder.as_str())) + let source = mount + .get("Source") + .and_then(Value::as_str) + .map(common::normalize_devcontainer_label_path); + (source.as_deref() == Some(local_workspace_folder.as_str())) .then(|| mount.get("Destination").and_then(Value::as_str)) .flatten() }) { diff --git a/cmd/devcontainer/src/runtime/metadata.rs b/cmd/devcontainer/src/runtime/metadata.rs index 13e401305..85f001c20 100644 --- a/cmd/devcontainer/src/runtime/metadata.rs +++ b/cmd/devcontainer/src/runtime/metadata.rs @@ -72,7 +72,7 @@ pub(crate) fn serialized_container_metadata( metadata .entry("workspaceFolder".to_string()) .or_insert_with(|| Value::String(remote_workspace_folder.to_string())); - serde_json::to_string(&Value::Object(metadata)) + serde_json::to_string(&Value::Array(vec![Value::Object(metadata)])) .map_err(|error| format!("Failed to serialize container metadata: {error}")) } @@ -186,8 +186,27 @@ mod tests { ) .expect("metadata"); let parsed: Value = serde_json::from_str(&metadata).expect("metadata json"); + let entries = parsed.as_array().expect("metadata array"); - assert_eq!(parsed["remoteUser"], "vscode"); - assert!(parsed.get("remoteEnv").is_none(), "{parsed}"); + assert_eq!(entries[0]["remoteUser"], "vscode"); + assert!(entries[0].get("remoteEnv").is_none(), "{parsed}"); + } + + #[test] + fn serialized_container_metadata_wraps_single_metadata_entry_in_array() { + let metadata = serialized_container_metadata( + &json!({ + "remoteUser": "vscode" + }), + "/workspace", + false, + ) + .expect("metadata"); + let parsed: Value = serde_json::from_str(&metadata).expect("metadata json"); + let entries = parsed.as_array().expect("metadata array"); + + assert_eq!(entries.len(), 1); + assert_eq!(entries[0]["remoteUser"], "vscode"); + assert_eq!(entries[0]["workspaceFolder"], "/workspace"); } } diff --git a/cmd/devcontainer/src/runtime/mod.rs b/cmd/devcontainer/src/runtime/mod.rs index c37243609..f0038a9e4 100644 --- a/cmd/devcontainer/src/runtime/mod.rs +++ b/cmd/devcontainer/src/runtime/mod.rs @@ -22,6 +22,32 @@ pub enum ExecResult { Streaming(i32), } +fn effective_up_resolved_config( + args: &[String], + resolved: context::ResolvedConfig, +) -> Result { + let feature_support = configuration::resolve_feature_support( + args, + &resolved.workspace_folder, + &resolved.config_file, + &resolved.configuration, + )?; + let effective_configuration = feature_support + .as_ref() + .map(|resolved_features| { + configuration::apply_feature_metadata( + &resolved.configuration, + &resolved_features.metadata_entries, + ) + }) + .unwrap_or_else(|| resolved.configuration.clone()); + Ok(context::ResolvedConfig { + workspace_folder: resolved.workspace_folder, + config_file: resolved.config_file, + configuration: effective_configuration, + }) +} + pub fn run_build(args: &[String]) -> Result { let resolved = context::load_required_config(args)?; let feature_support = configuration::resolve_feature_support( @@ -56,27 +82,16 @@ pub fn run_build(args: &[String]) -> Result { pub fn run_up(args: &[String]) -> Result { let _ = mounts::cli_mount_values(args)?; - let resolved = context::load_required_config(args)?; - let feature_support = configuration::resolve_feature_support( - args, - &resolved.workspace_folder, - &resolved.config_file, - &resolved.configuration, - )?; - let effective_configuration = feature_support - .as_ref() - .map(|resolved_features| { - configuration::apply_feature_metadata( - &resolved.configuration, - &resolved_features.metadata_entries, - ) - }) - .unwrap_or_else(|| resolved.configuration.clone()); - let effective_resolved = context::ResolvedConfig { - workspace_folder: resolved.workspace_folder.clone(), - config_file: resolved.config_file.clone(), - configuration: effective_configuration.clone(), - }; + let effective_resolved = + effective_up_resolved_config(args, context::load_required_config(args)?)?; + let effective_resolved = + match container::probe_up_container_id_labels(&effective_resolved, args)? { + Some(id_labels) => effective_up_resolved_config( + args, + context::load_required_config_with_id_labels(args, id_labels)?, + )?, + None => effective_resolved, + }; lifecycle::run_initialize_command( args, &effective_resolved.configuration, @@ -94,10 +109,19 @@ pub fn run_up(args: &[String]) -> Result { &image_name, &remote_workspace_folder, )?; + let lifecycle_resolved = match up_container.matched_id_labels.clone() { + Some(id_labels) => effective_up_resolved_config( + args, + context::load_required_config_with_id_labels(args, id_labels)?, + )?, + None => effective_resolved, + }; + let remote_workspace_folder = + context::remote_workspace_folder_for_args(&lifecycle_resolved, args); lifecycle::run_lifecycle_commands( &up_container.container_id, args, - &effective_resolved.configuration, + &lifecycle_resolved.configuration, &remote_workspace_folder, up_container.lifecycle_mode, )?; @@ -107,12 +131,12 @@ pub fn run_up(args: &[String]) -> Result { "command": "up", "containerId": up_container.container_id, "composeProjectName": compose_project_name, - "remoteUser": context::remote_user(&effective_resolved.configuration), + "remoteUser": context::remote_user(&lifecycle_resolved.configuration), "remoteWorkspaceFolder": remote_workspace_folder, - "configuration": if common::has_flag(args, "--include-configuration") { effective_resolved.configuration.clone() } else { Value::Null }, - "mergedConfiguration": if common::has_flag(args, "--include-merged-configuration") { effective_resolved.configuration.clone() } else { Value::Null }, - "workspaceFolder": effective_resolved.workspace_folder, - "configFile": effective_resolved.config_file, + "configuration": if common::has_flag(args, "--include-configuration") { lifecycle_resolved.configuration.clone() } else { Value::Null }, + "mergedConfiguration": if common::has_flag(args, "--include-merged-configuration") { lifecycle_resolved.configuration.clone() } else { Value::Null }, + "workspaceFolder": lifecycle_resolved.workspace_folder, + "configFile": lifecycle_resolved.config_file, })) } diff --git a/cmd/devcontainer/tests/runtime_container_smoke/compose_flow.rs b/cmd/devcontainer/tests/runtime_container_smoke/compose_flow.rs index fb3fb987c..75f6fc1a8 100644 --- a/cmd/devcontainer/tests/runtime_container_smoke/compose_flow.rs +++ b/cmd/devcontainer/tests/runtime_container_smoke/compose_flow.rs @@ -1,9 +1,31 @@ //! Runtime container smoke tests for compose-backed up flows. +use std::collections::HashMap; use std::fs; +use std::path::Path; + +use devcontainer::config::{substitute_local_context, ConfigContext}; +use serde_json::json; use crate::support::runtime_harness::{write_devcontainer_config, RuntimeHarness}; +const DEVCONTAINER_LOCAL_FOLDER_LABEL: &str = "devcontainer.local_folder"; + +fn devcontainer_id_for_labels(workspace: &Path, labels: HashMap) -> String { + substitute_local_context( + &json!("${devcontainerId}"), + &ConfigContext { + workspace_folder: workspace.to_path_buf(), + env: HashMap::new(), + container_workspace_folder: None, + id_labels: labels, + }, + ) + .as_str() + .expect("devcontainer id") + .to_string() +} + fn generated_override_contents(harness: &RuntimeHarness) -> String { let log = harness.read_compose_file_log(); let mut capture = false; @@ -298,6 +320,68 @@ fn exec_accepts_custom_compose_binary_for_compose_workspaces() { )); } +#[test] +fn up_reused_compose_service_preserves_legacy_devcontainer_id() { + let harness = RuntimeHarness::new(); + let workspace = harness.workspace(); + fs::create_dir_all(workspace.join(".devcontainer")).expect("workspace config dir"); + fs::write( + workspace.join(".devcontainer").join("docker-compose.yml"), + "services:\n app:\n image: alpine:3.20\n", + ) + .expect("compose"); + write_devcontainer_config( + &workspace, + "{\n \"dockerComposeFile\": \"docker-compose.yml\",\n \"service\": \"app\",\n \"workspaceFolder\": \"/workspace\",\n \"postAttachCommand\": \"echo ${devcontainerId}\"\n}\n", + ); + let workspace = workspace.canonicalize().expect("workspace path"); + let legacy_id = devcontainer_id_for_labels( + &workspace, + HashMap::from([( + DEVCONTAINER_LOCAL_FOLDER_LABEL.to_string(), + workspace.display().to_string(), + )]), + ); + let inspect_path = harness.root.join("inspect.json"); + fs::write( + &inspect_path, + json!([{ + "Config": { + "Labels": { + DEVCONTAINER_LOCAL_FOLDER_LABEL: workspace.display().to_string() + } + }, + "Mounts": [] + }]) + .to_string(), + ) + .expect("inspect json"); + + let fake_podman = harness.fake_podman.to_string_lossy().to_string(); + let output = harness.run( + &[ + "up", + "--docker-path", + fake_podman.as_str(), + "--workspace-folder", + workspace.to_string_lossy().as_ref(), + ], + &[ + ("FAKE_PODMAN_COMPOSE_PS_OUTPUT", "existing-compose-id"), + ( + "FAKE_PODMAN_INSPECT_FILE", + inspect_path.to_string_lossy().as_ref(), + ), + ], + ); + + assert!(output.status.success(), "{output:?}"); + let payload = harness.parse_stdout_json(&output); + assert_eq!(payload["containerId"], "existing-compose-id"); + let exec_log = harness.read_exec_log(); + assert!(exec_log.contains(&format!("/bin/sh -lc echo {legacy_id}"))); +} + #[test] fn up_expect_existing_compose_container_fails_when_missing() { let harness = RuntimeHarness::new(); diff --git a/cmd/devcontainer/tests/runtime_container_smoke/reuse.rs b/cmd/devcontainer/tests/runtime_container_smoke/reuse.rs index 43d2f3484..61aa7d75e 100644 --- a/cmd/devcontainer/tests/runtime_container_smoke/reuse.rs +++ b/cmd/devcontainer/tests/runtime_container_smoke/reuse.rs @@ -1,9 +1,32 @@ //! Runtime container smoke tests for container reuse and restart behavior. +use std::collections::HashMap; use std::fs; +use std::path::Path; + +use devcontainer::config::{substitute_local_context, ConfigContext}; +use serde_json::json; use crate::support::runtime_harness::{write_devcontainer_config, RuntimeHarness}; +const DEVCONTAINER_LOCAL_FOLDER_LABEL: &str = "devcontainer.local_folder"; +const DEVCONTAINER_CONFIG_FILE_LABEL: &str = "devcontainer.config_file"; + +fn devcontainer_id_for_labels(workspace: &Path, labels: HashMap) -> String { + substitute_local_context( + &json!("${devcontainerId}"), + &ConfigContext { + workspace_folder: workspace.to_path_buf(), + env: HashMap::new(), + container_workspace_folder: None, + id_labels: labels, + }, + ) + .as_str() + .expect("devcontainer id") + .to_string() +} + #[test] fn up_preserves_custom_id_labels_for_followup_exec() { let harness = RuntimeHarness::new(); @@ -207,3 +230,81 @@ fn up_expect_existing_container_fails_when_missing() { "Dev container not found." ); } + +#[test] +fn up_reuse_applies_legacy_devcontainer_id_before_initialize_command() { + let harness = RuntimeHarness::new(); + let workspace = harness.workspace(); + fs::create_dir_all(&workspace).expect("workspace dir"); + let init_id_path = workspace.join("initialize-id.txt"); + let config_path = write_devcontainer_config( + &workspace, + &format!( + "{{\n \"image\": \"alpine:3.20\",\n \"initializeCommand\": \"printf %s ${{devcontainerId}} > {}\",\n \"postAttachCommand\": \"echo ${{devcontainerId}}\"\n}}\n", + init_id_path.display() + ), + ); + let workspace = workspace.canonicalize().expect("workspace path"); + let config_path = config_path.canonicalize().expect("config path"); + let legacy_id = devcontainer_id_for_labels( + &workspace, + HashMap::from([( + DEVCONTAINER_LOCAL_FOLDER_LABEL.to_string(), + workspace.display().to_string(), + )]), + ); + let current_id = devcontainer_id_for_labels( + &workspace, + HashMap::from([ + ( + DEVCONTAINER_LOCAL_FOLDER_LABEL.to_string(), + workspace.display().to_string(), + ), + ( + DEVCONTAINER_CONFIG_FILE_LABEL.to_string(), + config_path.display().to_string(), + ), + ]), + ); + let inspect_path = harness.root.join("inspect.json"); + fs::write( + &inspect_path, + json!([{ + "Config": { + "Labels": { + DEVCONTAINER_LOCAL_FOLDER_LABEL: workspace.display().to_string() + } + }, + "Mounts": [] + }]) + .to_string(), + ) + .expect("inspect json"); + + let fake_podman = harness.fake_podman.to_string_lossy().to_string(); + let output = harness.run( + &[ + "up", + "--docker-path", + fake_podman.as_str(), + "--workspace-folder", + workspace.to_string_lossy().as_ref(), + ], + &[ + ("FAKE_PODMAN_PS_OUTPUT", "existing-container-id"), + ( + "FAKE_PODMAN_INSPECT_FILE", + inspect_path.to_string_lossy().as_ref(), + ), + ], + ); + + assert!(output.status.success(), "{output:?}"); + assert_eq!( + fs::read_to_string(&init_id_path).expect("initialize id"), + legacy_id + ); + let exec_log = harness.read_exec_log(); + assert!(exec_log.contains(&format!("/bin/sh -lc echo {legacy_id}"))); + assert!(!exec_log.contains(&format!("/bin/sh -lc echo {current_id}"))); +} diff --git a/cmd/devcontainer/tests/runtime_lifecycle_smoke/commands.rs b/cmd/devcontainer/tests/runtime_lifecycle_smoke/commands.rs index 49b1bee73..3f3cf2e3e 100644 --- a/cmd/devcontainer/tests/runtime_lifecycle_smoke/commands.rs +++ b/cmd/devcontainer/tests/runtime_lifecycle_smoke/commands.rs @@ -1,10 +1,31 @@ //! Smoke tests for lifecycle command execution across up, set-up, and run-user-commands flows. use serde_json::json; +use std::collections::HashMap; use std::fs; +use std::path::Path; + +use devcontainer::config::{substitute_local_context, ConfigContext}; use crate::support::runtime_harness::{write_devcontainer_config, RuntimeHarness}; +const DEVCONTAINER_LOCAL_FOLDER_LABEL: &str = "devcontainer.local_folder"; + +fn devcontainer_id_for_labels(workspace: &Path, labels: HashMap) -> String { + substitute_local_context( + &json!("${devcontainerId}"), + &ConfigContext { + workspace_folder: workspace.to_path_buf(), + env: HashMap::new(), + container_workspace_folder: None, + id_labels: labels, + }, + ) + .as_str() + .expect("devcontainer id") + .to_string() +} + #[test] fn run_user_commands_resolves_container_ids_from_headered_ps_output() { let harness = RuntimeHarness::new(); @@ -326,3 +347,57 @@ fn object_lifecycle_commands_are_executed() { assert!(invocations .contains("exec --workdir /workspaces/workspace fake-container-id printf %s second value")); } + +#[test] +fn run_user_commands_with_container_id_preserves_legacy_devcontainer_id_labels() { + let harness = RuntimeHarness::new(); + let workspace = harness.workspace(); + fs::create_dir_all(&workspace).expect("workspace dir"); + let config_path = write_devcontainer_config( + &workspace, + "{\n \"image\": \"alpine:3.20\",\n \"postAttachCommand\": \"echo ${devcontainerId}\"\n}\n", + ); + let workspace = workspace.canonicalize().expect("workspace path"); + let legacy_id = devcontainer_id_for_labels( + &workspace, + HashMap::from([( + DEVCONTAINER_LOCAL_FOLDER_LABEL.to_string(), + workspace.display().to_string(), + )]), + ); + let inspect_path = harness.root.join("inspect.json"); + fs::write( + &inspect_path, + json!([{ + "Config": { + "Labels": { + DEVCONTAINER_LOCAL_FOLDER_LABEL: workspace.display().to_string() + } + }, + "Mounts": [] + }]) + .to_string(), + ) + .expect("inspect json"); + + let fake_podman = harness.fake_podman.to_string_lossy().to_string(); + let output = harness.run( + &[ + "run-user-commands", + "--docker-path", + fake_podman.as_str(), + "--container-id", + "fake-container-id", + "--config", + config_path.to_string_lossy().as_ref(), + ], + &[( + "FAKE_PODMAN_INSPECT_FILE", + inspect_path.to_string_lossy().as_ref(), + )], + ); + + assert!(output.status.success(), "{output:?}"); + let exec_log = harness.read_exec_log(); + assert!(exec_log.contains(&format!("/bin/sh -lc echo {legacy_id}"))); +} diff --git a/docs/cli/command-reference.md b/docs/cli/command-reference.md index dd4addcd0..cc8ae352f 100644 --- a/docs/cli/command-reference.md +++ b/docs/cli/command-reference.md @@ -2,7 +2,7 @@ Generated from the pinned upstream CLI command matrix. -- Upstream commit: `39685cf1aa58b5b11e90085bd32562fad61f4103` +- Upstream commit: `2d81ee3c9ed96a7312c18c7513a17933f8f66d41` - Source: `upstream/src/spec-node/devContainersSpecCLI.ts` ## Top-Level Commands diff --git a/docs/upstream/command-matrix.json b/docs/upstream/command-matrix.json index 67742638a..3dee14450 100644 --- a/docs/upstream/command-matrix.json +++ b/docs/upstream/command-matrix.json @@ -1,5 +1,5 @@ { - "upstreamCommit": "39685cf1aa58b5b11e90085bd32562fad61f4103", + "upstreamCommit": "2d81ee3c9ed96a7312c18c7513a17933f8f66d41", "sourcePath": "upstream/src/spec-node/devContainersSpecCLI.ts", "topLevel": [ "up", diff --git a/docs/upstream/compatibility-baseline.json b/docs/upstream/compatibility-baseline.json index 8032e6afe..a3eb562e4 100644 --- a/docs/upstream/compatibility-baseline.json +++ b/docs/upstream/compatibility-baseline.json @@ -1,5 +1,5 @@ { "submodulePath": "upstream", - "pinnedCommit": "39685cf1aa58b5b11e90085bd32562fad61f4103", - "compatibilityContract": "This repository targets upstream/ at commit 39685cf1aa58b5b11e90085bd32562fad61f4103." + "pinnedCommit": "2d81ee3c9ed96a7312c18c7513a17933f8f66d41", + "compatibilityContract": "This repository targets upstream/ at commit 2d81ee3c9ed96a7312c18c7513a17933f8f66d41." } diff --git a/docs/upstream/compatibility-dashboard.md b/docs/upstream/compatibility-dashboard.md index 39c40a87a..29480b802 100644 --- a/docs/upstream/compatibility-dashboard.md +++ b/docs/upstream/compatibility-dashboard.md @@ -1,6 +1,6 @@ # Native Compatibility Dashboard -- Pinned upstream commit: `39685cf1aa58b5b11e90085bd32562fad61f4103` +- Pinned upstream commit: `2d81ee3c9ed96a7312c18c7513a17933f8f66d41` - Pinned spec commit: `c95ffeed1d059abfe9ffbe79762dc2fa4e7c2421` - Command matrix source: `docs/upstream/command-matrix.json` - Native parity inventory: `docs/upstream/parity-inventory.md` diff --git a/docs/upstream/compose-parity.md b/docs/upstream/compose-parity.md index 41b397c6b..59a44ec08 100644 --- a/docs/upstream/compose-parity.md +++ b/docs/upstream/compose-parity.md @@ -1,6 +1,6 @@ # Compose Parity Inventory -Pinned upstream CLI commit: `39685cf1aa58b5b11e90085bd32562fad61f4103` +Pinned upstream CLI commit: `2d81ee3c9ed96a7312c18c7513a17933f8f66d41` This is a semantic parity note for the native Rust Compose path. It complements the generated command-matrix inventory in `docs/upstream/parity-inventory.md`, which only records static source references. diff --git a/docs/upstream/parity-inventory.json b/docs/upstream/parity-inventory.json index 9e2940df2..c97ac18c5 100644 --- a/docs/upstream/parity-inventory.json +++ b/docs/upstream/parity-inventory.json @@ -1,5 +1,5 @@ { - "upstreamCommit": "39685cf1aa58b5b11e90085bd32562fad61f4103", + "upstreamCommit": "2d81ee3c9ed96a7312c18c7513a17933f8f66d41", "sourcePath": "upstream/src/spec-node/devContainersSpecCLI.ts", "summary": { "commandPathsTotal": 20, diff --git a/docs/upstream/parity-inventory.md b/docs/upstream/parity-inventory.md index cd88b3049..8fdcd904d 100644 --- a/docs/upstream/parity-inventory.md +++ b/docs/upstream/parity-inventory.md @@ -2,7 +2,7 @@ Generated from the pinned upstream CLI command matrix and static source evidence in the Rust implementation. -- Upstream commit: `39685cf1aa58b5b11e90085bd32562fad61f4103` +- Upstream commit: `2d81ee3c9ed96a7312c18c7513a17933f8f66d41` - Source: `upstream/src/spec-node/devContainersSpecCLI.ts` - Declared upstream command paths present natively: `20/20` - Upstream options with a native source reference in mapped files: `200/200` diff --git a/docs/upstream/test-coverage-map.json b/docs/upstream/test-coverage-map.json index 4b1afaeff..70465b181 100644 --- a/docs/upstream/test-coverage-map.json +++ b/docs/upstream/test-coverage-map.json @@ -1,5 +1,5 @@ { - "upstreamCommit": "39685cf1aa58b5b11e90085bd32562fad61f4103", + "upstreamCommit": "2d81ee3c9ed96a7312c18c7513a17933f8f66d41", "suites": [ { "upstreamTest": "upstream/src/test/cli.build.test.ts", @@ -182,7 +182,7 @@ "cmd/devcontainer/tests/cli_smoke/lockfile.rs", "cmd/devcontainer/src/commands/configuration/tests/upgrade.rs" ], - "notes": "Native lockfile coverage includes outdated, upgrade, dry-run, root-relative path handling, and workspace-local OCI layout mirrors for published Feature version and digest resolution." + "notes": "Native lockfile coverage includes outdated, upgrade, dry-run, root-relative path handling, trailing-newline writes, missing frozen-lockfile errors, and workspace-local OCI layout mirrors for published Feature version and digest resolution." }, { "upstreamTest": "upstream/src/test/container-features/registryCompatibilityOCI.test.ts", @@ -276,7 +276,16 @@ "cmd/devcontainer/tests/runtime_build_smoke/features.rs", "cmd/devcontainer/src/runtime/metadata.rs" ], - "notes": "Metadata persistence and merge behavior are covered, but upstream image metadata matrices are broader." + "notes": "Metadata persistence and merge behavior are covered, including array-only label serialization for single metadata entries, but upstream image metadata matrices are broader." + }, + { + "upstreamTest": "upstream/src/test/labelPathNormalization.test.ts", + "status": "covered", + "nativeTests": [ + "cmd/devcontainer/src/commands/common/labels.rs", + "cmd/devcontainer/src/runtime/container/discovery.rs" + ], + "notes": "Native unit coverage now exercises Windows label normalization plus legacy workspace-only matching for default devcontainer labels." }, { "upstreamTest": "upstream/src/test/updateUID.test.ts", diff --git a/docs/upstream/test-coverage-map.md b/docs/upstream/test-coverage-map.md index e49e87a62..cc44cde63 100644 --- a/docs/upstream/test-coverage-map.md +++ b/docs/upstream/test-coverage-map.md @@ -2,9 +2,9 @@ Machine-readable upstream test coverage inventory for the native Rust CLI. -- Upstream commit: `39685cf1aa58b5b11e90085bd32562fad61f4103` -- Upstream tests inventoried: `34` -- Covered: `5` +- Upstream commit: `2d81ee3c9ed96a7312c18c7513a17933f8f66d41` +- Upstream tests inventoried: `35` +- Covered: `6` - Partial: `25` - Missing: `4` @@ -30,7 +30,7 @@ Machine-readable upstream test coverage inventory for the native Rust CLI. | `upstream/src/test/container-features/featuresCLICommands.test.ts` | partial | `cmd/devcontainer/tests/cli_smoke/collections.rs`
`cmd/devcontainer/src/commands/collections/tests/features.rs`
`cmd/devcontainer/src/commands/collections/tests/feature_tests.rs`
`cmd/devcontainer/src/commands/collections/tests/publish.rs` | CLI coverage exists for Features commands, but published flows remain substitute-based. | | `upstream/src/test/container-features/generateFeaturesConfig.test.ts` | partial | `cmd/devcontainer/src/commands/configuration/tests/read.rs`
`cmd/devcontainer/tests/runtime_build_smoke/features.rs` | Generated feature configuration is exercised indirectly through read/build paths. | | `upstream/src/test/container-features/lifecycleHooks.test.ts` | partial | `cmd/devcontainer/tests/runtime_lifecycle_smoke.rs`
`cmd/devcontainer/tests/runtime_lifecycle_smoke/commands.rs`
`cmd/devcontainer/tests/runtime_lifecycle_smoke/selection.rs` | Lifecycle behavior is tested natively, but not specifically as upstream Feature-contributed hook cases. | -| `upstream/src/test/container-features/lockfile.test.ts` | covered | `cmd/devcontainer/tests/cli_smoke/lockfile.rs`
`cmd/devcontainer/src/commands/configuration/tests/upgrade.rs` | Native lockfile coverage includes outdated, upgrade, dry-run, root-relative path handling, and workspace-local OCI layout mirrors for published Feature version and digest resolution. | +| `upstream/src/test/container-features/lockfile.test.ts` | covered | `cmd/devcontainer/tests/cli_smoke/lockfile.rs`
`cmd/devcontainer/src/commands/configuration/tests/upgrade.rs` | Native lockfile coverage includes outdated, upgrade, dry-run, root-relative path handling, trailing-newline writes, missing frozen-lockfile errors, and workspace-local OCI layout mirrors for published Feature version and digest resolution. | | `upstream/src/test/container-features/registryCompatibilityOCI.test.ts` | partial | `cmd/devcontainer/src/commands/collections/tests/features.rs`
`cmd/devcontainer/tests/network_smoke/ghcr.rs` | Native coverage now includes OCI-manifest-shaped `features info manifest`, canonical ids, `publishedTags` output, and a dedicated anonymous GHCR manifest smoke check, but registry auth and broader live OCI pull flows are still missing. | | `upstream/src/test/container-templates/containerTemplatesOCI.test.ts` | partial | `cmd/devcontainer/src/commands/collections/tests/templates.rs`
`cmd/devcontainer/tests/cli_smoke/collections.rs` | Native coverage now includes workspace-local OCI metadata lookup and archive-backed template apply flows, but live registry template fetch behavior is still missing. | | `upstream/src/test/container-templates/templatesCLICommands.test.ts` | partial | `cmd/devcontainer/tests/cli_smoke/collections.rs`
`cmd/devcontainer/src/commands/collections/tests/templates.rs`
`cmd/devcontainer/src/commands/collections/tests/publish.rs` | Template CLI commands now cover workspace-local OCI metadata and apply flows, but published-template behavior is still partial without live registry fetches. | @@ -41,7 +41,8 @@ Machine-readable upstream test coverage inventory for the native Rust CLI. | `upstream/src/test/dotfiles.test.ts` | covered | `cmd/devcontainer/tests/runtime_lifecycle_smoke/dotfiles.rs` | Native dotfiles coverage includes ordering, reinstall markers, and personalization stop behavior. | | `upstream/src/test/getEntPasswd.test.ts` | missing | none | No native passwd-entry parsing or lookup tests exist. | | `upstream/src/test/getHomeFolder.test.ts` | missing | none | No native home-folder resolution test suite exists. | -| `upstream/src/test/imageMetadata.test.ts` | partial | `cmd/devcontainer/tests/runtime_exec_smoke.rs`
`cmd/devcontainer/tests/runtime_configuration_smoke.rs`
`cmd/devcontainer/tests/runtime_container_smoke/basic.rs`
`cmd/devcontainer/tests/runtime_build_smoke/features.rs`
`cmd/devcontainer/src/runtime/metadata.rs` | Metadata persistence and merge behavior are covered, but upstream image metadata matrices are broader. | +| `upstream/src/test/imageMetadata.test.ts` | partial | `cmd/devcontainer/tests/runtime_exec_smoke.rs`
`cmd/devcontainer/tests/runtime_configuration_smoke.rs`
`cmd/devcontainer/tests/runtime_container_smoke/basic.rs`
`cmd/devcontainer/tests/runtime_build_smoke/features.rs`
`cmd/devcontainer/src/runtime/metadata.rs` | Metadata persistence and merge behavior are covered, including array-only label serialization for single metadata entries, but upstream image metadata matrices are broader. | +| `upstream/src/test/labelPathNormalization.test.ts` | covered | `cmd/devcontainer/src/commands/common/labels.rs`
`cmd/devcontainer/src/runtime/container/discovery.rs` | Native unit coverage now exercises Windows label normalization plus legacy workspace-only matching for default devcontainer labels. | | `upstream/src/test/updateUID.test.ts` | covered | `cmd/devcontainer/src/runtime/container/uid_update/tests.rs` | Native UID-update coverage includes image inspection, platform preservation, local tags, and podman behavior. | | `upstream/src/test/utils.test.ts` | missing | none | No single native utility suite maps to upstream spec utility coverage. | | `upstream/src/test/variableSubstitution.test.ts` | partial | `cmd/devcontainer/src/config.rs`
`cmd/devcontainer/tests/cli_smoke/read_configuration.rs` | Native substitution coverage exists, but the supported substitution surface is still narrower than upstream. | diff --git a/upstream b/upstream index 39685cf1a..2d81ee3c9 160000 --- a/upstream +++ b/upstream @@ -1 +1 @@ -Subproject commit 39685cf1aa58b5b11e90085bd32562fad61f4103 +Subproject commit 2d81ee3c9ed96a7312c18c7513a17933f8f66d41