dev_container: Resolve compose dockerfile path relative to build context - #53860
Conversation
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 =
<project>/.devcontainer/Dockerfile), not relative to compose file
directory which would produce
<project>/.devcontainer/.devcontainer/Dockerfile
left: Some("/path/to/local/project/.devcontainer/.devcontainer/Dockerfile")
right: Some("/path/to/local/project/.devcontainer/Dockerfile")
Closes zed-industries#53473
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
02cdf7e to
c33bcf2
Compare
85be5db to
3c05715
Compare
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 `<project>/.devcontainer/.devcontainer/Dockerfile` instead of `<project>/.devcontainer/Dockerfile`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3c05715 to
a947864
Compare
|
@antont thanks for this! Generally looks good - can you run |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@KyleBarton oops, sorry I forgot that chore again, it's done now! BTW this was not enough to get my work system working with the new code but I encountered a new error, apparently because the dev container includes several containers (Postgres and localstack in addition to our project) so the docker ps output parsing failed ( With that additional fix my work project's dev container launches fine: #54068 I had to resort to using an old official release in the meantime, but now am back using main + patches for work. |
…ext (zed-industries#53860) ## 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 zed-industries#53473 ## Prior art This fix is extracted from zed-industries#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 zed-industries#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 <noreply@anthropic.com>
…ext (zed-industries#53860) ## 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 zed-industries#53473 ## Prior art This fix is extracted from zed-industries#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 zed-industries#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 <noreply@anthropic.com>
…ext (zed-industries#53860) ## 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 zed-industries#53473 ## Prior art This fix is extracted from zed-industries#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 zed-industries#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 <noreply@anthropic.com>
…ext (zed-industries#53860) ## 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 zed-industries#53473 ## Prior art This fix is extracted from zed-industries#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 zed-industries#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 <noreply@anthropic.com>
…ext (zed-industries#53860) ## 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 zed-industries#53473 ## Prior art This fix is extracted from zed-industries#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 zed-industries#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 <noreply@anthropic.com>
Context
When a Docker Compose service specifies
context: ..anddockerfile: .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/Dockerfilewhich doesn't exist.Per the Docker Compose spec, the
dockerfilefield 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:
dockerfile_location()rather than separate helper methods.devcontainer/withcontext: .., and compose file at project root withcontext: .How to Review
Single file change in
crates/dev_container/src/devcontainer_manifest.rs:config_directorydirectly. Usesnormalize_pathto resolve..components.FakeDockercompose config entries and corresponding tests asserting correct resolved paths.Self-Review Checklist
Release Notes:
dockerfilepath being resolved relative to the compose file instead of the buildcontextdirectory.