diff --git a/crates/dev_container/src/devcontainer_manifest.rs b/crates/dev_container/src/devcontainer_manifest.rs index 04815f5406f695be39ac919b35fcf430a2fbd623..b44822eedbfc426a53b5a6f32afbfe9b1b45376e 100644 --- a/crates/dev_container/src/devcontainer_manifest.rs +++ b/crates/dev_container/src/devcontainer_manifest.rs @@ -10,7 +10,7 @@ use regex::Regex; use fs::Fs; use http_client::HttpClient; -use util::{ResultExt, command::Command}; +use util::{ResultExt, command::Command, normalize_path}; use crate::{ DevContainerConfig, DevContainerContext, @@ -231,10 +231,14 @@ impl DevContainerManifest { else { return None; }; - main_service - .build - .and_then(|b| b.dockerfile) - .map(|dockerfile| self.config_directory.join(dockerfile)) + main_service.build.and_then(|b| { + let compose_file = docker_compose_manifest.files.first()?; + resolve_compose_dockerfile( + compose_file, + b.context.as_deref(), + b.dockerfile.as_deref()?, + ) + }) } DevContainerBuildType::None => None, } @@ -2189,6 +2193,33 @@ fn find_primary_service( } } +/// Resolves a compose service's dockerfile path according to the Docker Compose spec: +/// `dockerfile` is relative to the build `context`, and `context` is relative to +/// the compose file's directory. +fn resolve_compose_dockerfile( + compose_file: &Path, + context: Option<&str>, + dockerfile: &str, +) -> Option { + let dockerfile = PathBuf::from(dockerfile); + if dockerfile.is_absolute() { + return Some(dockerfile); + } + let compose_dir = compose_file.parent()?; + let context_dir = match context { + Some(ctx) => { + let ctx = PathBuf::from(ctx); + if ctx.is_absolute() { + ctx + } else { + normalize_path(&compose_dir.join(ctx)) + } + } + None => compose_dir.to_path_buf(), + }; + Some(context_dir.join(dockerfile)) +} + /// Destination folder inside the container where feature content is staged during build. /// Mirrors the CLI's `FEATURES_CONTAINER_TEMP_DEST_FOLDER`. const FEATURES_CONTAINER_TEMP_DEST_FOLDER: &str = "/tmp/dev-container-features"; @@ -2428,7 +2459,7 @@ mod test { use std::{ collections::HashMap, ffi::OsStr, - path::PathBuf, + path::{Path, PathBuf}, process::{ExitStatus, Output}, sync::{Arc, Mutex}, }; @@ -2454,7 +2485,7 @@ mod test { devcontainer_manifest::{ ConfigStatus, DevContainerManifest, DockerBuildResources, DockerComposeResources, DockerInspect, extract_feature_id, find_primary_service, get_remote_user_from_config, - image_from_dockerfile, + image_from_dockerfile, resolve_compose_dockerfile, }, docker::{ DockerClient, DockerComposeConfig, DockerComposeService, DockerComposeServiceBuild, @@ -3709,6 +3740,72 @@ ENV DOCKER_BUILDKIT=1 ) } + #[test] + fn test_resolve_compose_dockerfile() { + let compose = Path::new("/project/.devcontainer/docker-compose.yml"); + + // Bug case (#53473): context ".." with relative dockerfile + assert_eq!( + resolve_compose_dockerfile(compose, Some(".."), ".devcontainer/Dockerfile"), + Some(PathBuf::from("/project/.devcontainer/Dockerfile")), + ); + + // Compose path containing ".." (as docker_compose_manifest() produces) + assert_eq!( + resolve_compose_dockerfile( + Path::new("/project/.devcontainer/../docker-compose.yml"), + Some("."), + "docker/Dockerfile", + ), + Some(PathBuf::from("/project/docker/Dockerfile")), + ); + + // Absolute dockerfile returned as-is + assert_eq!( + resolve_compose_dockerfile(compose, Some("."), "/absolute/Dockerfile"), + Some(PathBuf::from("/absolute/Dockerfile")), + ); + + // Absolute context used directly + assert_eq!( + resolve_compose_dockerfile(compose, Some("/abs/context"), "Dockerfile"), + Some(PathBuf::from("/abs/context/Dockerfile")), + ); + + // No context defaults to compose file's directory + assert_eq!( + resolve_compose_dockerfile(compose, None, "Dockerfile"), + Some(PathBuf::from("/project/.devcontainer/Dockerfile")), + ); + } + + #[gpui::test] + async fn test_dockerfile_location_with_compose_context_parent(cx: &mut TestAppContext) { + cx.executor().allow_parking(); + env_logger::try_init().ok(); + + let given_devcontainer_contents = r#" + { + "name": "Test", + "dockerComposeFile": "docker-compose-context-parent.yml", + "service": "app", + "workspaceFolder": "/workspaces/${localWorkspaceFolderBasename}" + } + "#; + let (_, mut devcontainer_manifest) = + init_default_devcontainer_manifest(cx, given_devcontainer_contents) + .await + .unwrap(); + + devcontainer_manifest.parse_nonremote_vars().unwrap(); + + let expected = PathBuf::from(TEST_PROJECT_PATH).join(".devcontainer/Dockerfile"); + assert_eq!( + devcontainer_manifest.dockerfile_location().await, + Some(expected) + ); + } + #[gpui::test] async fn test_spawns_devcontainer_with_docker_compose_and_no_update_uid( cx: &mut TestAppContext, @@ -4968,6 +5065,30 @@ FROM docker.io/hexpm/elixir:1.21-erlang-28.4.1-debian-trixie-20260316-slim AS de )]), })); } + if config_files.len() == 1 + && config_files.get(0) + == Some(&PathBuf::from( + "/path/to/local/project/.devcontainer/docker-compose-context-parent.yml", + )) + { + return Ok(Some(DockerComposeConfig { + name: None, + services: HashMap::from([( + "app".to_string(), + DockerComposeService { + build: Some(DockerComposeServiceBuild { + context: Some("..".to_string()), + dockerfile: Some(".devcontainer/Dockerfile".to_string()), + args: None, + additional_contexts: None, + target: None, + }), + ..Default::default() + }, + )]), + volumes: HashMap::new(), + })); + } if config_files.len() == 1 && config_files.get(0) == Some(&PathBuf::from(