Skip to content

fix(build,compose): resolve dockerfile path per Docker semantics#30

Merged
us merged 2 commits into
us:mainfrom
jahirvidrio:fix/build-dockerfile-path-resolution
Jun 21, 2026
Merged

fix(build,compose): resolve dockerfile path per Docker semantics#30
us merged 2 commits into
us:mainfrom
jahirvidrio:fix/build-dockerfile-path-resolution

Conversation

@jahirvidrio

@jahirvidrio jahirvidrio commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

What problem does this solve?

mocker build -f <abs path> <abs context> fails with Dockerfile not found at <doubled-path> because ImageManager.build calls URL.appendingPathComponent on the dockerfile argument, which doesn't treat absolute paths as special — it concatenates them onto the build context. The fileExists guard then throws before container build is ever invoked.

Per docs.docker.com (docker buildx build --file): an absolute -f is used verbatim, a relative -f is resolved against the current working directory. mocker now matches both.

Naïvely fixing only mocker build would have shifted Compose (which reuses ImageManager.build) from context-relative dockerfile resolution to CWD-relative — a regression, because the Compose spec resolves build.dockerfile relative to the service's build.context. Both surfaces are corrected together so the Docker-compat contract holds on both:

  • absolute -f → used verbatim
  • relative -f → CWD-relative for mocker build, context-relative for Compose
  • omitted → <context>/Dockerfile

What changed

  • Sources/MockerKit/Image/ImageManager.swift: extracted resolveContextPath(context:cwd:) from the inline absolute/relative branch in build (de-duplicates the same logic that lived inline); added pure resolveDockerfilePath(context:dockerfile:cwd:) with cwd injected so it is testable without the filesystem; added public static composeDockerfilePath(context:dockerfile:cwd:) that chains the two so Compose can pre-resolve the dockerfile against its context — no new path-resolution policy added, just reuse. build(dockerfile:) parameter widened from String = "Dockerfile" to String? = nil so the three branches (nil / absolute / relative) stay distinguishable end-to-end.
  • Sources/Mocker/Commands/Build.swift: @Option var file is now String? so ArgumentParser surfaces "not provided" as nil instead of the literal "Dockerfile".
  • Sources/MockerKit/Compose/ComposeOrchestrator.swift, Sources/Mocker/Commands/Compose.swift: pre-resolve the dockerfile via composeDockerfilePath before calling imageManager.build, so a relative build.dockerfile stays context-relative.

How was it tested?

swift test: 151/151, 0 failures (14 new tests). 12 pure resolver tests in Tests/MockerKitTests/ImageManagerBuildArgsTests.swift cover all three branches, the CWD-vs-context divergence, the original doubled-path case, and Compose context-relative resolution; 2 parse tests in Tests/MockerTests/CLITests.swift assert -f omitted parses to nil.

Any breaking changes?

No. Widening dockerfile: String = "Dockerfile" to dockerfile: String? = nil on ImageManager.build is source-compatible: callers that omit dockerfile: keep using the default, and callers passing a String literal widen implicitly to String?. No existing call-site outside the two Compose sites updated above required modification.

Fixes absolute -f paths being doubled onto the context (devcontainer
regression) and relative -f paths being resolved against context instead
of CWD, matching Docker parity for docker buildx build --file.

@us us left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix 🙏 The absolute/relative split matches buildx semantics, and handling the Compose context-relative case in the same PR (instead of leaving it as a follow-up regression) is exactly right. The resolver tests cover the CWD-vs-context divergence cleanly.

One optional, non-blocking nit: composeDockerfilePath could take dockerfile: String? and forward nil straight to resolveDockerfilePath, dropping the ?? "Dockerfile" at both call sites and keeping the default in one place. Totally fine to leave as-is.

Approving — thanks for the thorough tests and write-up!

@us us merged commit cc4241a into us:main Jun 21, 2026
1 check passed
@jahirvidrio jahirvidrio deleted the fix/build-dockerfile-path-resolution branch June 21, 2026 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants