mirror of
https://github.com/zed-industries/zed.git
synced 2026-05-25 06:24:56 +00:00
f939ec6049
> **Draft / open question for maintainers.** The failure mode this fixes is narrow — a new-Zed-created container exists under the `name`-field project while a CLI-derivation tool (`@devcontainers/cli`, VS Code) operates on the same folder (the container persists in Docker, so the originating Zed session doesn't need to still be open). See issue #54255 failure mode 3 and the fixture's step 6. > > I'd like to pose this as a question rather than a claim: is matching `@devcontainers/cli`'s `getProjectName` precedence something the project wants to take on, given the narrowness of the bug? I wrote this implementation mostly as a way to explore what parity would actually cost — happy to close it if you'd rather leave it as-is, or pare it down (e.g. just rule 4) if a partial match is preferable. > > The broader value beyond this specific bug: devcontainer impls agreeing on the same project name means containers created by Zed, the devcontainer CLI, and VS Code are interchangeable for the same folder, which feels worth it to me — but you know the project's priorities better. > > Folds in #54068 (detection) — closing that PR unmerged; its `MultipleMatchingContainers` error lands here. 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 #54255 ## Summary **Match `@devcontainers/cli`'s full `getProjectName` precedence.** Replaces `safe_id_lower(devcontainer.json's name)` with the five-step chain the reference CLI walks (see [`src/spec-node/dockerCompose.ts` in devcontainers/cli](https://github.com/devcontainers/cli/blob/main/src/spec-node/dockerCompose.ts)): 1. `COMPOSE_PROJECT_NAME` from the local environment. 2. `COMPOSE_PROJECT_NAME=` in the workspace `.env` file. 3. Top-level `name:` on the merged compose config, when at least one fragment declared it explicitly. 4. `${workspaceFolderBasename}_devcontainer` — only when the first compose file's directory is `<workspace>/.devcontainer/`. 5. Otherwise, the plain basename of the first compose file's directory (no suffix). The old Zed implementation diverged at every one of those inputs: any user setting `COMPOSE_PROJECT_NAME`, shipping a `.env` with one, declaring a top-level compose `name:`, or pointing `dockerComposeFile` outside `.devcontainer/` (e.g. `"../docker-compose.yml"`) got a different project namespace than the CLI and VS Code, producing two compose projects for the same folder. Adds a small `sanitize_compose_project_name()` helper implementing the CLI's rules (lowercase + strip `[^-_a-z0-9]`) — notably preserving hyphens, which `safe_id_lower` would have replaced with underscores. Adds two helpers used by the precedence walk: - `parse_dotenv_compose_project_name` — line scan extracting `COMPOSE_PROJECT_NAME=…` from the workspace `.env`, matching the subset the CLI's regex dotenv reader recognizes. - `compose_fragment_declares_name` — parses each compose fragment with `yaml-rust2` (already a transitive workspace dep; slated to become a direct dep via #53922) and checks for a `name` key on the root mapping (block, quoted, or flow style all work), matching the CLI's own `yaml.load`. `docker compose config` always injects `name: devcontainer` into its merged output when no fragment declared one, so rule 3 needs to distinguish the user-provided case from the injected default — this helper supplies that signal. On YAML parse failure it returns "not declared" (rule 4 applies), matching the CLI's fallback. `project_name()` becomes async and fallible (`async fn project_name(&self) -> Result<String, DevContainerError>`) so it can load the `.env` file and each compose fragment via `self.fs.load`. Four call sites now `.await?` the derivation. Real I/O errors on the `.env` read propagate as `FilesystemError` (matching the CLI's narrow `ENOENT`/`EISDIR` swallow); fragment-rescan read errors are logged and skipped (matching the CLI's broader try/catch over its fragment read + parse). The `name` field is still used as the features image-tag prefix (`generate_features_image_tag`); only the compose project namespace is decoupled from it. **Duplicate-container detection (from #54068).** When `check_for_existing_container`'s label-based lookup returns more than one match, propagate `MultipleMatchingContainers(ids)` with instructions to clean up the stale one(s). This covers the mixed-version upgrade edge case where a pre-fix Zed left a container under the legacy project name alongside a CLI-style one — transparent to users in the common case (one tool, one container), explicit error when two legacy siblings need manual cleanup. ## Why Full write-up with verified fixtures and captured output: #54255. Three failure modes from the same root cause, all resolved by this change: 1. **Interop** — opening a folder in both Zed and `devcontainer up` (or Zed and VS Code) creates two compose projects with identical `devcontainer.local_folder` + `devcontainer.config_file` labels, breaking the spec's uniqueness invariant. 2. **Cross-worktree silent db/volume reuse** — if multiple git worktrees share a `devcontainer.json` with the same `name`, Zed uses the same compose project for all of them; Compose reuses stateful siblings (db, cache, localstack) by config-hash, so worktree B silently inherits worktree A's database. Fixture + captured output: [antont/zed-devcontainer-db-share-repro](https://github.com/antont/zed-devcontainer-db-share-repro). 3. **Mixed-version Zed sessions** — the Rust impl landed in stable v0.232.2 (2026-04-15, #52338). Older Zed (≤v0.231.x) shelled out to `@devcontainers/cli` so it used the reference derivation. The collision shows up when a new-Zed-created container exists under the name-field project while a CLI-derivation tool (old Zed, `devcontainer up`, VS Code) operates on the same folder. ## Migration / compatibility Existing Zed-created containers (under the old `safe_id_lower(name)` project) continue to be found via `check_for_existing_container`'s label-based lookup — they're looked up by `devcontainer.local_folder` + `devcontainer.config_file`, not by project name. A user with duplicate legacy containers from a prior Zed session sees `MultipleMatchingContainers` with cleanup instructions. ## Revision — 2026-04-22 Revised per @KyleBarton review on the prior version: - Swapped the YAML parser from `serde_yaml_ng` to `yaml-rust2` (already transitive via `tree-sitter-yaml`; net reduction of one direct workspace dep; also what #53922 will pull in). - Dropped the mixed-version tiebreak (`pick_canonical_container`) and its `com.docker.compose.project` serde label. The edge case it covered is transient enough to address via the explicit `MultipleMatchingContainers` error rather than permanent tiebreaking code. - Folded #54068's detection commit into this PR; #54068 closed unmerged. - Rebased onto `main`. ## Test plan - [x] `cargo test -p dev_container --lib` — 89 passed, including: - `sanitize_compose_project_name_matches_cli_rules` - `--project-name` assertion added to `test_spawns_devcontainer_with_docker_compose` - `check_for_existing_container_errors_when_multiple_match` - `derive_project_name_env_wins_over_everything` - `derive_project_name_dotenv_wins_over_compose_and_fallback` - `derive_project_name_compose_name_wins_over_fallback` - `derive_project_name_skips_compose_name_when_not_explicitly_declared` - `derive_project_name_omits_suffix_when_compose_file_outside_devcontainer_dir` - `derive_project_name_normalizes_compose_path_for_rule_4` - `compose_fragment_declares_name_detects_top_level_name_key` (covers block, quoted-key, and flow-style roots, plus parse failure → not-declared) - `is_missing_file_error_only_accepts_notfound_and_isadirectory` - [x] `cargo fmt --all` — clean - [x] `./script/clippy -p dev_container` — clean - [x] **End-to-end with fixture** [antont/zed-devcontainer-compose-test](https://github.com/antont/zed-devcontainer-compose-test): - Build `zed` from this branch. - Clean slate: `docker ps -a --filter "label=devcontainer.local_folder=$PWD" -q | xargs -r docker rm -f` - `zed --dev-container /path/to/devcontainer-compose-test` → Zed creates container under project `devcontainer-compose-test_devcontainer` (was `compose_duplicate_repro` before the fix). - `devcontainer up --workspace-folder $PWD` → CLI reports the same `containerId` Zed created; no second compose project is introduced. - Captured: `devcontainer-compose-test_devcontainer-app-1`, `composeProjectName: "devcontainer-compose-test_devcontainer"` reported by both tools. Release Notes: - Fixed dev container Docker Compose project name now matches the full `getProjectName` precedence from the reference devcontainer CLI (`COMPOSE_PROJECT_NAME` in the environment, then in the workspace `.env`, then an explicit top-level `name:` on the merged compose config, then the basename of the first compose file's directory — with the `_devcontainer` suffix only when that directory is `<workspace>/.devcontainer`). This prevents duplicate containers when the same folder is opened with both Zed and the devcontainer CLI / VS Code. --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
47 lines
1.2 KiB
TOML
47 lines
1.2 KiB
TOML
[package]
|
|
name = "dev_container"
|
|
version = "0.1.0"
|
|
publish.workspace = true
|
|
edition.workspace = true
|
|
|
|
[dependencies]
|
|
anyhow.workspace = true
|
|
async-tar.workspace = true
|
|
async-trait.workspace = true
|
|
serde.workspace = true
|
|
serde_json.workspace = true
|
|
serde_json_lenient.workspace = true
|
|
yaml-rust2.workspace = true
|
|
shlex.workspace = true
|
|
http_client.workspace = true
|
|
http.workspace = true
|
|
gpui.workspace = true
|
|
fs.workspace = true
|
|
futures.workspace = true
|
|
log.workspace = true
|
|
menu.workspace = true
|
|
paths.workspace = true
|
|
regex.workspace = true
|
|
picker.workspace = true
|
|
project.workspace = true
|
|
settings.workspace = true
|
|
ui.workspace = true
|
|
util.workspace = true
|
|
walkdir.workspace = true
|
|
worktree.workspace = true
|
|
workspace.workspace = true
|
|
|
|
[dev-dependencies]
|
|
fs = { workspace = true, features = ["test-support"] }
|
|
gpui = { workspace = true, features = ["test-support"] }
|
|
project = { workspace = true, features = ["test-support"] }
|
|
serde_json.workspace = true
|
|
settings = { workspace = true, features = ["test-support"] }
|
|
|
|
workspace = { workspace = true, features = ["test-support"] }
|
|
worktree = { workspace = true, features = ["test-support"] }
|
|
util = { workspace = true, features = ["test-support"] }
|
|
env_logger.workspace = true
|
|
|
|
[lints]
|
|
workspace = true
|