From 4e503f8293241dc738dd3dbddb4220a73c437beb Mon Sep 17 00:00:00 2001 From: Toni Alatalo Date: Mon, 13 Apr 2026 15:52:08 +0300 Subject: [PATCH 1/3] Add failing test for docker-compose dockerfile path resolution The test uses `context: ..` and `dockerfile: .devcontainer/Dockerfile` in the compose build section. The dockerfile path should be resolved relative to the build context (the project root), but is currently resolved relative to the compose file's directory (.devcontainer/), producing a doubled path. Test output: assertion `left == right` failed: dockerfile should be resolved relative to build context (../.devcontainer/Dockerfile = /.devcontainer/Dockerfile), not relative to compose file directory which would produce /.devcontainer/.devcontainer/Dockerfile left: Some("/path/to/local/project/.devcontainer/.devcontainer/Dockerfile") right: Some("/path/to/local/project/.devcontainer/Dockerfile") Closes #53473 Co-Authored-By: Claude Opus 4.6 --- .../src/devcontainer_manifest.rs | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/crates/dev_container/src/devcontainer_manifest.rs b/crates/dev_container/src/devcontainer_manifest.rs index 0ab80a9d2ad901..ff5a8f519bd53b 100644 --- a/crates/dev_container/src/devcontainer_manifest.rs +++ b/crates/dev_container/src/devcontainer_manifest.rs @@ -3674,6 +3674,40 @@ ENV DOCKER_BUILDKIT=1 ) } + #[gpui::test] + async fn test_docker_compose_dockerfile_resolved_relative_to_context( + 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 dockerfile_location = devcontainer_manifest.dockerfile_location().await; + let expected_path = + PathBuf::from(TEST_PROJECT_PATH).join(".devcontainer/Dockerfile"); + assert_eq!( + dockerfile_location, + Some(expected_path), + "dockerfile should be resolved relative to build context (../.devcontainer/Dockerfile \ + = /.devcontainer/Dockerfile), not relative to compose file directory \ + which would produce /.devcontainer/.devcontainer/Dockerfile" + ); + } + #[gpui::test] async fn test_spawns_devcontainer_with_docker_compose_and_no_update_uid( cx: &mut TestAppContext, @@ -4931,6 +4965,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( From a9478647d269a07a84c4016289ccae4bcd827c37 Mon Sep 17 00:00:00 2001 From: Toni Alatalo Date: Tue, 14 Apr 2026 07:58:25 +0300 Subject: [PATCH 2/3] dev_container: Resolve compose dockerfile path relative to build context Per the Docker Compose spec, the `dockerfile` field is relative to the build `context` directory, not the compose file's directory. When `context: ..` and `dockerfile: .devcontainer/Dockerfile` are used, the path was incorrectly resolved as `/.devcontainer/.devcontainer/Dockerfile` instead of `/.devcontainer/Dockerfile`. Co-Authored-By: Claude Opus 4.6 --- .../src/devcontainer_manifest.rs | 100 ++++++++++++++---- 1 file changed, 80 insertions(+), 20 deletions(-) diff --git a/crates/dev_container/src/devcontainer_manifest.rs b/crates/dev_container/src/devcontainer_manifest.rs index ff5a8f519bd53b..612fce7a999013 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, } @@ -2164,6 +2168,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"; @@ -2405,7 +2436,7 @@ mod test { use std::{ collections::HashMap, ffi::OsStr, - path::PathBuf, + path::{Path, PathBuf}, process::{ExitStatus, Output}, sync::{Arc, Mutex}, }; @@ -2431,7 +2462,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, @@ -3674,10 +3705,47 @@ 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_docker_compose_dockerfile_resolved_relative_to_context( - cx: &mut TestAppContext, - ) { + async fn test_dockerfile_location_with_compose_context_parent(cx: &mut TestAppContext) { cx.executor().allow_parking(); env_logger::try_init().ok(); @@ -3696,16 +3764,8 @@ ENV DOCKER_BUILDKIT=1 devcontainer_manifest.parse_nonremote_vars().unwrap(); - let dockerfile_location = devcontainer_manifest.dockerfile_location().await; - let expected_path = - PathBuf::from(TEST_PROJECT_PATH).join(".devcontainer/Dockerfile"); - assert_eq!( - dockerfile_location, - Some(expected_path), - "dockerfile should be resolved relative to build context (../.devcontainer/Dockerfile \ - = /.devcontainer/Dockerfile), not relative to compose file directory \ - which would produce /.devcontainer/.devcontainer/Dockerfile" - ); + let expected = PathBuf::from(TEST_PROJECT_PATH).join(".devcontainer/Dockerfile"); + assert_eq!(devcontainer_manifest.dockerfile_location().await, Some(expected)); } #[gpui::test] From ec413bc410ae976f5e1ae184acc4f3efbc6e5b1a Mon Sep 17 00:00:00 2001 From: Toni Alatalo Date: Thu, 16 Apr 2026 19:20:17 +0300 Subject: [PATCH 3/3] Run cargo fmt Co-Authored-By: Claude Opus 4.6 --- crates/dev_container/src/devcontainer_manifest.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/dev_container/src/devcontainer_manifest.rs b/crates/dev_container/src/devcontainer_manifest.rs index 612fce7a999013..2940b1d56ea042 100644 --- a/crates/dev_container/src/devcontainer_manifest.rs +++ b/crates/dev_container/src/devcontainer_manifest.rs @@ -3765,7 +3765,10 @@ ENV DOCKER_BUILDKIT=1 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)); + assert_eq!( + devcontainer_manifest.dockerfile_location().await, + Some(expected) + ); } #[gpui::test]