Skip to content

Drop the polyfill for the engine's findRoots - #3

Merged
marcosnils merged 3 commits into
mainfrom
docker-pr-2-polyfill-removal-rework-lead-19fb5865
Aug 26, 2026
Merged

Drop the polyfill for the engine's findRoots#3
marcosnils merged 3 commits into
mainfrom
docker-pr-2-polyfill-removal-rework-lead-19fb5865

Conversation

@eunomie

@eunomie eunomie commented Aug 25, 2026

Copy link
Copy Markdown
Member

Supersedes #2, which called Workspace.findConfigDirs — an API the engine does not have. On v1.0.0-beta.10 that fails to load with field "findConfigDirs" not found in Dagger.Workspace. The native discovery API is Workspace.findRoots (dagger/dagger#13854), so this branch redoes the migration against the real API and verifies it on a beta.10 engine.

The Docker module used dagger/polyfill to find Dockerfile and Compose projects around the directory Dagger was started from. Workspace.findRoots preserves that behavior exactly: it returns the nearest enclosing project first when the cwd holds no config file of its own, then every project at or below the cwd, all as cwd-relative paths, de-duplicated. That is the polyfill's findConfigDirs contract one for one, so the call sites swap directly.

No Docker or Compose behavior is intended to change.

Notes for reviewers

  • No path-resolution helper here, unlike dagger/go#42. That module wants workspace-root-relative module roots, so it converts findRoots' cwd-relative answers with a workspaceRootPath helper. This module already speaks cwd-relative paths end to end — DockerProject.path / ComposeProject.path document a ".." prefix as the enclosing-project marker, and findRoots results are usable as-is with the other workspace APIs. So this follows dagger/ruff#2's direct shape instead.
  • Docker.relativeToCwd is a different concern and stays. It converts the workspace-root-absolute path ws.findUp returns into a cwd-relative one, for Docker.project. It is unrelated to findRoots' output. It does move from the removed Workspace.path field to Workspace.cwd; cwd spells the same location with a leading /, which the helper already strips.
  • No exclude:. The old polyfill call passed none, so none is passed now. (Ruff's .venv/node_modules exclusions are specific to Python trees, not a campaign-wide default.)
  • Both proxy entries are load-bearing. dagger.toml installs proxy into the workspace; dagger.json declares it as a module dependency, which is what puts proxy in compose.dang's namespace. Removing the dagger.json entry breaks Compose.startServices. The old entry pointed at a local ./proxy directory that does not exist in this repository, so it is repointed at the published module and pinned.
  • dagger.json stays; no dagger-module.toml migration. dagger setup --auto-apply on a copy of this tree reports "No migration needed" on beta.10, and sibling dagger/go's root module is also still on dagger.json. Out of scope here.
  • The generated dagger.lock is deliberately not committed. Siblings dagger/go and dagger/ruff do track theirs, so this repo probably should too — but the lock is produced from dagger.toml, which this branch does not touch, so it belongs in its own change rather than here.

Test

dagger -m . check

Not bare dagger check: this repo is a plain module and is not installed into its own workspace, so at the repo root dagger check runs only dang-sdk:generate and never reaches this module's checks.

Against a real v1.0.0-beta.10 engine, dagger -m . check passes all four checks (docker:build-all, docker:build-unref-stages, docker:compose:lint, docker:lint-all).

Discovery was also verified to be cwd-anchored, and the checks still pass from a nested cwd:

cwd docker.projects.path
repo root fixtures/multistage
fixtures/ multistage
fixtures/multistage/ .
fixtures/multistage/sub/ ..

grouville and others added 3 commits August 25, 2026 17:03
The dependency named a local ./proxy directory that does not exist in
this repository, so the module could not be loaded at all: the proxy
module lives in github.com/dagger/proxy and the workspace config
already installs it from there. Name the same published source in the
module config and pin it, so a fresh clone resolves.

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>
Signed-off-by: Yves Brissaud <yves@dagger.io>
Compose.depth and Compose.relativeToCwd have no call sites: the Compose
type never calls findUp, so it never has a workspace-root-absolute path
to convert. Docker.relativeToCwd, which Docker.project does use, stays.

Removing them here keeps the following engine bump from having to port
dead code onto the v1 spelling of Workspace.cwd.

Signed-off-by: Yves Brissaud <yves@dagger.io>
The engine now ships Workspace.findRoots (dagger/dagger#13854): the
same cwd-aware project discovery this module got from
github.com/dagger/polyfill. Its contract matches the polyfill's
findConfigDirs one for one -- the nearest enclosing project first when
the cwd holds no config of its own, then every project at or below the
cwd, all as cwd-relative paths -- so the call site swaps directly and
no path resolution is needed: this module speaks cwd-relative paths
throughout, and findRoots results are usable as-is with the other
workspace APIs.

This module was still pinned to v0.20.6 and read the cwd through the
legacy Workspace.path field, which the v1 view removed. So the version
bump also moves Docker.relativeToCwd onto Workspace.cwd, which spells
the same location with a leading "/" -- the helper already strips it
before comparing segments, so the conversion is unchanged.

The engineVersion bump and the dependency removal have to land
together: findRoots only exists on the v1 view of Workspace.

Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>
Signed-off-by: Yves Brissaud <yves@dagger.io>
@eunomie
eunomie requested a review from marcosnils August 25, 2026 15:27
@eunomie
eunomie marked this pull request as ready for review August 25, 2026 15:27
@marcosnils
marcosnils merged commit d28077f into main Aug 26, 2026
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.

3 participants