Drop the polyfill for the engine's findRoots - #3
Merged
marcosnils merged 3 commits intoAug 26, 2026
Conversation
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>
This was referenced Aug 25, 2026
eunomie
marked this pull request as ready for review
August 25, 2026 15:27
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #2, which called
Workspace.findConfigDirs— an API the engine does not have. Onv1.0.0-beta.10that fails to load withfield "findConfigDirs" not found in Dagger.Workspace. The native discovery API isWorkspace.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/polyfillto find Dockerfile and Compose projects around the directory Dagger was started from.Workspace.findRootspreserves 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'sfindConfigDirscontract one for one, so the call sites swap directly.No Docker or Compose behavior is intended to change.
Notes for reviewers
findRoots' cwd-relative answers with aworkspaceRootPathhelper. This module already speaks cwd-relative paths end to end —DockerProject.path/ComposeProject.pathdocument a".."prefix as the enclosing-project marker, andfindRootsresults are usable as-is with the other workspace APIs. So this follows dagger/ruff#2's direct shape instead.Docker.relativeToCwdis a different concern and stays. It converts the workspace-root-absolute pathws.findUpreturns into a cwd-relative one, forDocker.project. It is unrelated tofindRoots' output. It does move from the removedWorkspace.pathfield toWorkspace.cwd;cwdspells the same location with a leading/, which the helper already strips.exclude:. The old polyfill call passed none, so none is passed now. (Ruff's.venv/node_modulesexclusions are specific to Python trees, not a campaign-wide default.)dagger.tomlinstallsproxyinto the workspace;dagger.jsondeclares it as a module dependency, which is what putsproxyincompose.dang's namespace. Removing thedagger.jsonentry breaksCompose.startServices. The old entry pointed at a local./proxydirectory that does not exist in this repository, so it is repointed at the published module and pinned.dagger.jsonstays; nodagger-module.tomlmigration.dagger setup --auto-applyon a copy of this tree reports "No migration needed" on beta.10, and siblingdagger/go's root module is also still ondagger.json. Out of scope here.dagger.lockis deliberately not committed. Siblingsdagger/goanddagger/ruffdo track theirs, so this repo probably should too — but the lock is produced fromdagger.toml, which this branch does not touch, so it belongs in its own change rather than here.Test
dagger -m . checkNot bare
dagger check: this repo is a plain module and is not installed into its own workspace, so at the repo rootdagger checkruns onlydang-sdk:generateand never reaches this module's checks.Against a real
v1.0.0-beta.10engine,dagger -m . checkpasses 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:
docker.projects.pathfixtures/multistagefixtures/multistagefixtures/multistage/.fixtures/multistage/sub/..