From 5e6e41155e6100c2b5bdbe121e4c91eb6974d370 Mon Sep 17 00:00:00 2001 From: KyleBarton Date: Thu, 16 Apr 2026 11:50:08 -0700 Subject: [PATCH] Simplify remote workspace folder calculation for Dev Containers (#53829) 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 Closes #53635 Release Notes: - Fixed issues with Windows pathing when calculating target workspace directory for dev containers --- crates/dev_container/src/devcontainer_api.rs | 2 + crates/dev_container/src/devcontainer_json.rs | 132 ++++++++ .../src/devcontainer_manifest.rs | 90 ++++-- crates/dev_container/src/docker.rs | 285 +----------------- 4 files changed, 202 insertions(+), 307 deletions(-) diff --git a/crates/dev_container/src/devcontainer_api.rs b/crates/dev_container/src/devcontainer_api.rs index f9f0136fcfc5ffe29c643acb0371b89107ab3d47..385185aacc1fca3b9eb1489436412b8599cee853 100644 --- a/crates/dev_container/src/devcontainer_api.rs +++ b/crates/dev_container/src/devcontainer_api.rs @@ -75,6 +75,7 @@ pub enum DevContainerError { DevContainerUpFailed(String), DevContainerNotFound, DevContainerParseFailed, + DevContainerValidationFailed(String), FilesystemError, ResourceFetchFailed, NotInValidProject, @@ -110,6 +111,7 @@ impl Display for DevContainerError { "Error downloading resources locally".to_string(), DevContainerError::ResourceFetchFailed => "Failed to fetch resources from template or feature repository".to_string(), + DevContainerError::DevContainerValidationFailed(failure) => failure.to_string(), } ) } diff --git a/crates/dev_container/src/devcontainer_json.rs b/crates/dev_container/src/devcontainer_json.rs index 7326d9bfb1f9acd3145ab5b74b47ca056dbeb1ab..752dcfc037cc75b97fcf5d70b80f6d218769624c 100644 --- a/crates/dev_container/src/devcontainer_json.rs +++ b/crates/dev_container/src/devcontainer_json.rs @@ -259,6 +259,32 @@ impl DevContainer { DevContainerBuildType::None } } + + pub(crate) fn validate_devcontainer_contents(&self) -> Result<(), DevContainerError> { + match self.build_type() { + DevContainerBuildType::Image(_) => Ok(()), + DevContainerBuildType::Dockerfile(_) => { + if (self.workspace_folder.is_some() && self.workspace_mount.is_none()) + || (self.workspace_folder.is_none() && self.workspace_mount.is_some()) + { + return Err(DevContainerError::DevContainerValidationFailed( + "workspaceMount and workspaceFolder must both be defined, or neither defined" + .to_string(), + )); + } + Ok(()) + } + DevContainerBuildType::DockerCompose => { + if self.service.is_none() { + return Err(DevContainerError::DevContainerValidationFailed( + "must specify a connecting service for docker-compose".to_string(), + )); + } + Ok(()) + } + DevContainerBuildType::None => Ok(()), + } + } } // Custom deserializer that parses the entire customizations object as a @@ -1477,4 +1503,110 @@ mod test { assert_eq!(rendered, "type=tmpfs,target=/tmp,consistency=cached"); } + + #[test] + fn should_fail_validation_with_workspace_mount_only() { + let given_image_container_json = r#" + // These are some external comments. serde_lenient should handle them + { + // These are some internal comments + "build": { + "dockerfile": "Dockerfile", + }, + "name": "myDevContainer", + "workspaceMount": "source=/app,target=/workspaces/app,type=bind,consistency=cached", + "customizations": { + "vscode": { + // Just confirm that this can be included and ignored + }, + "zed": { + "extensions": [ + "html" + ] + } + } + } + "#; + + let result = deserialize_devcontainer_json(given_image_container_json); + + assert!(result.is_ok()); + let devcontainer = result.expect("ok"); + + assert_eq!( + devcontainer.validate_devcontainer_contents(), + Err(DevContainerError::DevContainerValidationFailed( + "workspaceMount and workspaceFolder must both be defined, or neither defined" + .to_string() + )) + ); + } + + #[test] + fn should_fail_validation_with_workspace_folder_only() { + let given_image_container_json = r#" + // These are some external comments. serde_lenient should handle them + { + // These are some internal comments + "build": { + "dockerfile": "Dockerfile", + }, + "name": "myDevContainer", + "workspaceFolder": "/workspaces", + "customizations": { + "vscode": { + // Just confirm that this can be included and ignored + }, + "zed": { + "extensions": [ + "html" + ] + } + } + } + "#; + + let result = deserialize_devcontainer_json(given_image_container_json); + + assert!(result.is_ok()); + let devcontainer = result.expect("ok"); + + assert_eq!( + devcontainer.validate_devcontainer_contents(), + Err(DevContainerError::DevContainerValidationFailed( + "workspaceMount and workspaceFolder must both be defined, or neither defined" + .to_string() + )) + ); + } + #[test] + fn should_pass_validation_with_workspace_folder_for_docker_compose() { + let given_image_container_json = r#" + // These are some external comments. serde_lenient should handle them + { + // These are some internal comments + "dockerComposeFile": "docker-compose-plain.yml", + "service": "app", + "name": "myDevContainer", + "workspaceFolder": "/workspaces", + "customizations": { + "vscode": { + // Just confirm that this can be included and ignored + }, + "zed": { + "extensions": [ + "html" + ] + } + } + } + "#; + + let result = deserialize_devcontainer_json(given_image_container_json); + + assert!(result.is_ok()); + let devcontainer = result.expect("ok"); + + assert!(devcontainer.validate_devcontainer_contents().is_ok()); + } } diff --git a/crates/dev_container/src/devcontainer_manifest.rs b/crates/dev_container/src/devcontainer_manifest.rs index b44822eedbfc426a53b5a6f32afbfe9b1b45376e..9b1783e821c8d15b69ca50a8d8264ca98230eb8e 100644 --- a/crates/dev_container/src/devcontainer_manifest.rs +++ b/crates/dev_container/src/devcontainer_manifest.rs @@ -23,7 +23,6 @@ use crate::{ docker::{ Docker, DockerClient, DockerComposeConfig, DockerComposeService, DockerComposeServiceBuild, DockerComposeServicePort, DockerComposeVolume, DockerInspect, DockerPs, - get_remote_dir_from_config, }, features::{DevContainerFeatureJson, FeatureManifest, parse_oci_feature_ref}, get_oci_token, @@ -57,7 +56,7 @@ struct DevContainerManifest { features_build_info: Option, features: Vec, } -const DEFAULT_REMOTE_PROJECT_DIR: &str = "/workspaces/"; +const DEFAULT_REMOTE_PROJECT_DIR: &str = "/workspaces"; impl DevContainerManifest { async fn new( context: &DevContainerContext, @@ -772,17 +771,14 @@ RUN sed -i -E 's/((^|\s)PATH=)([^\$]*)$/\1\${{PATH:-\3}}/g' /etc/profile || true }; let remote_user = get_remote_user_from_config(&running_container, self)?; - let remote_workspace_folder = get_remote_dir_from_config( - &running_container, - (&self.local_project_directory.display()).to_string(), - )?; + let remote_workspace_folder = self.remote_workspace_folder()?; let remote_env = self.runtime_remote_env(&running_container.config.env_as_map()?)?; Ok(DevContainerUp { container_id: running_container.id, remote_user, - remote_workspace_folder, + remote_workspace_folder: remote_workspace_folder.display().to_string(), extension_ids: self.extension_ids(), remote_env, }) @@ -1716,7 +1712,14 @@ RUN sed -i -E 's/((^|\s)PATH=)([^\$]*)$/\1\${PATH:-\3}/g' /etc/profile || true .as_ref() .map(|folder| PathBuf::from(folder)) .or(Some( - PathBuf::from(DEFAULT_REMOTE_PROJECT_DIR).join(self.local_workspace_base_name()?), + // We explicitly use "/" here, instead of PathBuf::join + // because we want remote targets to use unix-style filepaths, + // even on a Windows host + PathBuf::from(format!( + "{}/{}", + DEFAULT_REMOTE_PROJECT_DIR, + self.local_workspace_base_name()? + )), )) .ok_or(DevContainerError::DevContainerParseFailed) } @@ -1738,7 +1741,14 @@ RUN sed -i -E 's/((^|\s)PATH=)([^\$]*)$/\1\${PATH:-\3}/g' /etc/profile || true Ok(MountDefinition { source: Some(self.local_workspace_folder()), - target: format!("/workspaces/{}", project_directory_name.display()), + // We explicitly use "/" here, instead of PathBuf::join + // because we want the remote target to use unix-style filepaths, + // even on a Windows host + target: format!( + "{}/{}", + PathBuf::from(DEFAULT_REMOTE_PROJECT_DIR).display(), + project_directory_name.display() + ), mount_type: None, }) } @@ -1847,6 +1857,8 @@ RUN sed -i -E 's/((^|\s)PATH=)([^\$]*)$/\1\${PATH:-\3}/g' /etc/profile || true } async fn build_and_run(&mut self) -> Result { + self.dev_container().validate_devcontainer_contents()?; + self.run_initialize_commands().await?; self.download_feature_and_dockerfile_resources().await?; @@ -1980,17 +1992,14 @@ RUN sed -i -E 's/((^|\s)PATH=)([^\$]*)$/\1\${PATH:-\3}/g' /etc/profile || true let remote_user = get_remote_user_from_config(&docker_inspect, self)?; - let remote_folder = get_remote_dir_from_config( - &docker_inspect, - (&self.local_project_directory.display()).to_string(), - )?; + let remote_folder = self.remote_workspace_folder()?; let remote_env = self.runtime_remote_env(&docker_inspect.config.env_as_map()?)?; let dev_container_up = DevContainerUp { container_id: docker_ps.id, remote_user: remote_user, - remote_workspace_folder: remote_folder, + remote_workspace_folder: remote_folder.display().to_string(), extension_ids: self.extension_ids(), remote_env, }; @@ -2494,7 +2503,10 @@ mod test { }, oci::TokenResponse, }; + #[cfg(not(target_os = "windows"))] const TEST_PROJECT_PATH: &str = "/path/to/local/project"; + #[cfg(target_os = "windows")] + const TEST_PROJECT_PATH: &str = r#"C:\\path\to\local\project"#; async fn build_tarball(content: Vec<(&str, &str)>) -> Vec { let buffer = futures::io::Cursor::new(Vec::new()); @@ -2755,11 +2767,11 @@ mod test { OsStr::new("--sig-proxy=false"), OsStr::new("-d"), OsStr::new("--mount"), - OsStr::new( - "type=bind,source=/path/to/local/project,target=/workspaces/project,consistency=cached" - ), + OsStr::new(&format!( + "type=bind,source={TEST_PROJECT_PATH},target=/workspaces/project,consistency=cached" + )), OsStr::new("-l"), - OsStr::new("devcontainer.local_folder=/path/to/local/project"), + OsStr::new(&format!("devcontainer.local_folder={TEST_PROJECT_PATH}")), OsStr::new("-l"), OsStr::new(&format!( "devcontainer.config_file={expected_config_file_label}" @@ -2935,7 +2947,8 @@ mod test { .remote_env .as_ref() .and_then(|env| env.get("LOCAL_WORKSPACE_FOLDER")), - Some(&TEST_PROJECT_PATH.to_string()) + // We replace backslashes with forward slashes during variable replacement for JSON safety + Some(&TEST_PROJECT_PATH.replace("\\", "/")) ); // ${localEnv:VARIABLE_NAME} @@ -3038,7 +3051,8 @@ mod test { .remote_env .as_ref() .and_then(|env| env.get("LOCAL_WORKSPACE_FOLDER")), - Some(&TEST_PROJECT_PATH.to_string()) + // We replace backslashes with forward slashes during variable replacement for JSON safety + Some(&TEST_PROJECT_PATH.replace("\\", "/")) ); } @@ -4569,6 +4583,33 @@ RUN sed -i -E 's/((^|\s)PATH=)([^\$]*)$/\1\${PATH:-\3}/g' /etc/profile || true ); } + #[cfg(target_os = "windows")] + #[gpui::test] + async fn test_spawns_devcontainer_with_plain_image(cx: &mut TestAppContext) { + cx.executor().allow_parking(); + env_logger::try_init().ok(); + let given_devcontainer_contents = r#" + { + "name": "cli-${devcontainerId}", + "image": "test_image:latest", + } + "#; + + let (_, mut devcontainer_manifest) = + init_default_devcontainer_manifest(cx, given_devcontainer_contents) + .await + .unwrap(); + + devcontainer_manifest.parse_nonremote_vars().unwrap(); + + let devcontainer_up = devcontainer_manifest.build_and_run().await.unwrap(); + + assert_eq!( + devcontainer_up.remote_workspace_folder, + "/workspaces/project" + ); + } + #[cfg(not(target_os = "windows"))] #[gpui::test] async fn test_spawns_devcontainer_with_docker_compose_and_plain_image(cx: &mut TestAppContext) { @@ -5017,11 +5058,14 @@ FROM docker.io/hexpm/elixir:1.21-erlang-28.4.1-debian-trixie-20260316-slim AS de &self, config_files: &Vec, ) -> Result, DevContainerError> { + let project_path = PathBuf::from(TEST_PROJECT_PATH); if config_files.len() == 1 && config_files.get(0) - == Some(&PathBuf::from( - "/path/to/local/project/.devcontainer/docker-compose.yml", - )) + == Some( + &project_path + .join(".devcontainer") + .join("docker-compose.yml"), + ) { return Ok(Some(DockerComposeConfig { name: None, diff --git a/crates/dev_container/src/docker.rs b/crates/dev_container/src/docker.rs index faa2e7e364b00f05b6da248852bda2ea3d18cc37..60c28e289671fb19138bac2c0b790fab2e232752 100644 --- a/crates/dev_container/src/docker.rs +++ b/crates/dev_container/src/docker.rs @@ -522,36 +522,6 @@ where } } -pub(crate) fn get_remote_dir_from_config( - config: &DockerInspect, - local_dir: String, -) -> Result { - let local_path = PathBuf::from(&local_dir); - - let Some(mounts) = &config.mounts else { - log::error!("No mounts defined for container"); - return Err(DevContainerError::ContainerNotValid(config.id.clone())); - }; - - for mount in mounts { - // Sometimes docker will mount the local filesystem on host_mnt for system isolation - let mount_source = PathBuf::from(&mount.source.trim_start_matches("/host_mnt")); - if let Ok(relative_path_to_project) = local_path.strip_prefix(&mount_source) { - let remote_dir = format!( - "{}/{}", - &mount.destination, - relative_path_to_project.display() - ); - return Ok(remote_dir); - } - if mount.source == local_dir { - return Ok(mount.destination.clone()); - } - } - log::error!("No mounts to local folder"); - Err(DevContainerError::ContainerNotValid(config.id.clone())) -} - #[cfg(test)] mod test { use std::{ @@ -565,7 +535,7 @@ mod test { devcontainer_json::MountDefinition, docker::{ Docker, DockerComposeConfig, DockerComposeService, DockerComposeServicePort, - DockerComposeVolume, DockerInspect, DockerPs, get_remote_dir_from_config, + DockerComposeVolume, DockerInspect, DockerPs, }, }; @@ -710,259 +680,6 @@ mod test { assert_eq!(result.id, "abdb6ab59573".to_string()); } - #[test] - fn should_get_target_dir_from_docker_inspect() { - let given_config = r#" - { - "Id": "abdb6ab59573659b11dac9f4973796741be35b642c9b48960709304ce46dbf85", - "Created": "2026-02-04T23:44:21.802688084Z", - "Path": "/bin/sh", - "Args": [ - "-c", - "echo Container started\ntrap \"exit 0\" 15\n\nexec \"$@\"\nwhile sleep 1 & wait $!; do :; done", - "-" - ], - "State": { - "Status": "running", - "Running": true, - "Paused": false, - "Restarting": false, - "OOMKilled": false, - "Dead": false, - "Pid": 23087, - "ExitCode": 0, - "Error": "", - "StartedAt": "2026-02-04T23:44:21.954875084Z", - "FinishedAt": "0001-01-01T00:00:00Z" - }, - "Image": "sha256:3dcb059253b2ebb44de3936620e1cff3dadcd2c1c982d579081ca8128c1eb319", - "ResolvConfPath": "/var/lib/docker/containers/abdb6ab59573659b11dac9f4973796741be35b642c9b48960709304ce46dbf85/resolv.conf", - "HostnamePath": "/var/lib/docker/containers/abdb6ab59573659b11dac9f4973796741be35b642c9b48960709304ce46dbf85/hostname", - "HostsPath": "/var/lib/docker/containers/abdb6ab59573659b11dac9f4973796741be35b642c9b48960709304ce46dbf85/hosts", - "LogPath": "/var/lib/docker/containers/abdb6ab59573659b11dac9f4973796741be35b642c9b48960709304ce46dbf85/abdb6ab59573659b11dac9f4973796741be35b642c9b48960709304ce46dbf85-json.log", - "Name": "/objective_haslett", - "RestartCount": 0, - "Driver": "overlayfs", - "Platform": "linux", - "MountLabel": "", - "ProcessLabel": "", - "AppArmorProfile": "", - "ExecIDs": [ - "008019d93df4107fcbba78bcc6e1ed7e121844f36c26aca1a56284655a6adb53" - ], - "HostConfig": { - "Binds": null, - "ContainerIDFile": "", - "LogConfig": { - "Type": "json-file", - "Config": {} - }, - "NetworkMode": "bridge", - "PortBindings": {}, - "RestartPolicy": { - "Name": "no", - "MaximumRetryCount": 0 - }, - "AutoRemove": false, - "VolumeDriver": "", - "VolumesFrom": null, - "ConsoleSize": [ - 0, - 0 - ], - "CapAdd": null, - "CapDrop": null, - "CgroupnsMode": "private", - "Dns": [], - "DnsOptions": [], - "DnsSearch": [], - "ExtraHosts": null, - "GroupAdd": null, - "IpcMode": "private", - "Cgroup": "", - "Links": null, - "OomScoreAdj": 0, - "PidMode": "", - "Privileged": false, - "PublishAllPorts": false, - "ReadonlyRootfs": false, - "SecurityOpt": null, - "UTSMode": "", - "UsernsMode": "", - "ShmSize": 67108864, - "Runtime": "runc", - "Isolation": "", - "CpuShares": 0, - "Memory": 0, - "NanoCpus": 0, - "CgroupParent": "", - "BlkioWeight": 0, - "BlkioWeightDevice": [], - "BlkioDeviceReadBps": [], - "BlkioDeviceWriteBps": [], - "BlkioDeviceReadIOps": [], - "BlkioDeviceWriteIOps": [], - "CpuPeriod": 0, - "CpuQuota": 0, - "CpuRealtimePeriod": 0, - "CpuRealtimeRuntime": 0, - "CpusetCpus": "", - "CpusetMems": "", - "Devices": [], - "DeviceCgroupRules": null, - "DeviceRequests": null, - "MemoryReservation": 0, - "MemorySwap": 0, - "MemorySwappiness": null, - "OomKillDisable": null, - "PidsLimit": null, - "Ulimits": [], - "CpuCount": 0, - "CpuPercent": 0, - "IOMaximumIOps": 0, - "IOMaximumBandwidth": 0, - "Mounts": [ - { - "Type": "bind", - "Source": "/somepath/cli", - "Target": "/workspaces/cli", - "Consistency": "cached" - } - ], - "MaskedPaths": [ - "/proc/asound", - "/proc/acpi", - "/proc/interrupts", - "/proc/kcore", - "/proc/keys", - "/proc/latency_stats", - "/proc/timer_list", - "/proc/timer_stats", - "/proc/sched_debug", - "/proc/scsi", - "/sys/firmware", - "/sys/devices/virtual/powercap" - ], - "ReadonlyPaths": [ - "/proc/bus", - "/proc/fs", - "/proc/irq", - "/proc/sys", - "/proc/sysrq-trigger" - ] - }, - "GraphDriver": { - "Data": null, - "Name": "overlayfs" - }, - "Mounts": [ - { - "Type": "bind", - "Source": "/somepath/cli", - "Destination": "/workspaces/cli", - "Mode": "", - "RW": true, - "Propagation": "rprivate" - } - ], - "Config": { - "Hostname": "abdb6ab59573", - "Domainname": "", - "User": "root", - "AttachStdin": false, - "AttachStdout": true, - "AttachStderr": true, - "Tty": false, - "OpenStdin": false, - "StdinOnce": false, - "Env": [ - "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" - ], - "Cmd": [ - "-c", - "echo Container started\ntrap \"exit 0\" 15\n\nexec \"$@\"\nwhile sleep 1 & wait $!; do :; done", - "-" - ], - "Image": "mcr.microsoft.com/devcontainers/base:ubuntu", - "Volumes": null, - "WorkingDir": "", - "Entrypoint": [ - "/bin/sh" - ], - "OnBuild": null, - "Labels": { - "dev.containers.features": "common", - "dev.containers.id": "base-ubuntu", - "dev.containers.release": "v0.4.24", - "dev.containers.source": "https://github.com/devcontainers/images", - "dev.containers.timestamp": "Fri, 30 Jan 2026 16:52:34 GMT", - "dev.containers.variant": "noble", - "devcontainer.config_file": "/somepath/cli/.devcontainer/dev_container_2/devcontainer.json", - "devcontainer.local_folder": "/somepath/cli", - "devcontainer.metadata": "[{\"id\":\"ghcr.io/devcontainers/features/common-utils:2\"},{\"id\":\"ghcr.io/devcontainers/features/git:1\",\"customizations\":{\"vscode\":{\"settings\":{\"github.copilot.chat.codeGeneration.instructions\":[{\"text\":\"This dev container includes an up-to-date version of Git, built from source as needed, pre-installed and available on the `PATH`.\"}]}}}},{\"remoteUser\":\"vscode\"}]", - "org.opencontainers.image.ref.name": "ubuntu", - "org.opencontainers.image.version": "24.04", - "version": "2.1.6" - }, - "StopTimeout": 1 - }, - "NetworkSettings": { - "Bridge": "", - "SandboxID": "2a94990d542fe532deb75f1cc67f761df2d669e3b41161f914079e88516cc54b", - "SandboxKey": "/var/run/docker/netns/2a94990d542f", - "Ports": {}, - "HairpinMode": false, - "LinkLocalIPv6Address": "", - "LinkLocalIPv6PrefixLen": 0, - "SecondaryIPAddresses": null, - "SecondaryIPv6Addresses": null, - "EndpointID": "ef5b35a8fbb145565853e1a1d960e737fcc18c20920e96494e4c0cfc55683570", - "Gateway": "172.17.0.1", - "GlobalIPv6Address": "", - "GlobalIPv6PrefixLen": 0, - "IPAddress": "172.17.0.3", - "IPPrefixLen": 16, - "IPv6Gateway": "", - "MacAddress": "", - "Networks": { - "bridge": { - "IPAMConfig": null, - "Links": null, - "Aliases": null, - "MacAddress": "9a:ec:af:8a:ac:81", - "DriverOpts": null, - "GwPriority": 0, - "NetworkID": "51bb8ccc4d1281db44f16d915963fc728619d4a68e2f90e5ea8f1cb94885063e", - "EndpointID": "ef5b35a8fbb145565853e1a1d960e737fcc18c20920e96494e4c0cfc55683570", - "Gateway": "172.17.0.1", - "IPAddress": "172.17.0.3", - "IPPrefixLen": 16, - "IPv6Gateway": "", - "GlobalIPv6Address": "", - "GlobalIPv6PrefixLen": 0, - "DNSNames": null - } - } - }, - "ImageManifestDescriptor": { - "mediaType": "application/vnd.oci.image.manifest.v1+json", - "digest": "sha256:39c3436527190561948236894c55b59fa58aa08d68d8867e703c8d5ab72a3593", - "size": 2195, - "platform": { - "architecture": "arm64", - "os": "linux" - } - } - } - "#; - let config = serde_json_lenient::from_str::(given_config).unwrap(); - - let target_dir = get_remote_dir_from_config(&config, "/somepath/cli".to_string()); - - assert!(target_dir.is_ok()); - assert_eq!(target_dir.unwrap(), "/workspaces/cli/".to_string()); - } - #[test] fn should_deserialize_object_metadata_from_docker_compose_container() { // The devcontainer CLI writes metadata as a bare JSON object (not an array)