From bc88fe49315352ebc362b0bc544fc69dd0784e39 Mon Sep 17 00:00:00 2001 From: Toni Alatalo Date: Thu, 16 Apr 2026 19:56:32 +0300 Subject: [PATCH] dev_container: Resolve compose dockerfile path relative to build context (#53860) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Context When a Docker Compose service specifies `context: ..` and `dockerfile: .devcontainer/Dockerfile`, Zed resolves the dockerfile path relative to the compose file's directory instead of the build context. This produces a doubled path like `.devcontainer/.devcontainer/Dockerfile` which doesn't exist. Per the [Docker Compose spec](https://docs.docker.com/reference/compose-file/build/#dockerfile), the `dockerfile` field is relative to the build context directory. The fix resolves the context directory first (relative to the compose file), then joins the dockerfile path to that. Closes #53473 ## Prior art This fix is extracted from #53170 by @zdeneklapes, which addresses this bug among several other dev container startup issues. This PR isolates the dockerfile path resolution fix into a focused change to make it easier to review and merge independently. Differences from #53170: - **Scope**: Only the dockerfile-relative-to-context fix, not the other fixes (compose build args preservation, remote user fallback, Docker inspect labels, etc.) - **Implementation**: Inline resolution in `dockerfile_location()` rather than separate helper methods - **Absolute path handling**: Handles absolute dockerfile and context paths - **Tests**: Two test cases — compose file inside `.devcontainer/` with `context: ..`, and compose file at project root with `context: .` ## How to Review Single file change in `crates/dev_container/src/devcontainer_manifest.rs`: - **Fix** (line 234-252): Resolve build context relative to compose file directory, then join dockerfile to that, instead of joining dockerfile to `config_directory` directly. Uses `normalize_path` to resolve `..` components. - **Tests**: Two new `FakeDocker` compose config entries and corresponding tests asserting correct resolved paths. ## Self-Review Checklist - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed docker-compose `dockerfile` path being resolved relative to the compose file instead of the build `context` directory. --------- Co-authored-by: Claude Opus 4.6 --- .../src/devcontainer_manifest.rs | 135 +++++++++++++++++- 1 file changed, 128 insertions(+), 7 deletions(-) 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(