From 3e2a425e88b774e6ebcc98019f15d20ed6e34f23 Mon Sep 17 00:00:00 2001 From: Guillaume de Rouville Date: Fri, 21 Aug 2026 10:33:03 +0200 Subject: [PATCH 1/3] Drop the polyfill for native workspace APIs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The behaviors this module leaned on `dagger/polyfill` for now live in the engine: dagger/dagger#13854 added the native workspace and module-source APIs, and #13855 gave `Workspace.changes` an explicit baseline. Module discovery goes through `currentModule.asSDK(workspace: ws).modules`, which returns the modules registered to this SDK that are relevant to the client's cwd. The engine owns both membership and the cwd policy, so the SDK no longer scans for config files or rebases discovery results by hand — the `findConfigDirs` walk and its `workspacePath` helper go away with it. Changesets follow the baseline-diff shape the native APIs were designed for: stage onto the workspace value already in hand, then diff against it. No workspace fork is involved, because none is needed. `generateAll` folds the per-module changesets together with `changeset.withChangesets`. Generation would read as `ModuleSource.generate(ws).changes(ws)`, which is what #13855 built the pair for. On v1.0.0-beta.10 that pair does not hold its baseline: once a module's generated files already exist, it reports the whole generated context as added rather than only what generation changed — deleting one file and regenerating returned 279 paths instead of 1, including the engine-owned dagger-module.toml. So generation stages the local dependency closure itself, takes the module's `generatedContextChangeset`, and re-roots that at the caller's cwd explicitly, which is the shape the polyfill produced. `Mod.atCwd` carries the reason and the engine version, so it can go once the engine holds the baseline. Staging paths anchor at "/" because the native workspace resolves a relative path from the cwd, where the polyfill's took workspace-root paths. `Workspace.withNewDirectory` is not a drop-in for the polyfill fork's `withDirectory`: on a host-backed workspace the edit lands on an empty delta base and is then diffed against the host tree, so whatever already sat at that path is reported as removed. The polyfill's merged instead. init has to layer the template onto the existing contents itself, or `dagger module init` into a directory a user already has files in deletes them. sdk-sdk's `contract:does-not-remove-existing-files` only inits into an empty path and cannot catch that, so `e-2-e:init-layering-check` covers it here. The engineVersion bump and the dependency removal land together: the version gate scopes the engine's new changeset rooting to migrated modules. Signed-off-by: Guillaume de Rouville Signed-off-by: Yves Brissaud --- .../e2e/fixtures/layering/app/keep.txt | 1 + .../e2e/fixtures/layering/app/nested/deep.txt | 1 + .dagger/modules/e2e/main.dang | 33 +++++-- README.md | 12 ++- dagger.json | 11 +-- dagger.lock | 6 +- docs/cwd-aware-discovery.md | 20 +++-- mod.dang | 44 +++++++++- php-sdk.dang | 85 +++++++++---------- 9 files changed, 133 insertions(+), 80 deletions(-) create mode 100644 .dagger/modules/e2e/fixtures/layering/app/keep.txt create mode 100644 .dagger/modules/e2e/fixtures/layering/app/nested/deep.txt diff --git a/.dagger/modules/e2e/fixtures/layering/app/keep.txt b/.dagger/modules/e2e/fixtures/layering/app/keep.txt new file mode 100644 index 0000000..50400f1 --- /dev/null +++ b/.dagger/modules/e2e/fixtures/layering/app/keep.txt @@ -0,0 +1 @@ +user data diff --git a/.dagger/modules/e2e/fixtures/layering/app/nested/deep.txt b/.dagger/modules/e2e/fixtures/layering/app/nested/deep.txt new file mode 100644 index 0000000..ce22f91 --- /dev/null +++ b/.dagger/modules/e2e/fixtures/layering/app/nested/deep.txt @@ -0,0 +1 @@ +more user data diff --git a/.dagger/modules/e2e/main.dang b/.dagger/modules/e2e/main.dang index 38c0fff..2dae361 100644 --- a/.dagger/modules/e2e/main.dang +++ b/.dagger/modules/e2e/main.dang @@ -10,6 +10,7 @@ type E2e { let mixedModulePath: String! = fixtureRoot + "/mixed-discovery/ancestor/work/app" let mixedNestedPath: String! = mixedModulePath + "/nested/deeper" let generateModulePath: String! = fixtureRoot + "/generate/app" + let layeringModulePath: String! = fixtureRoot + "/layering/app" let assert(condition: Boolean!, message: String!): Void { if (condition == false) { raise message } @@ -36,14 +37,13 @@ type E2e { } """ - Managed discovery is cwd-aware, resolves against the passed workspace, and - supports both legacy and CLI 1.0 configs. + Module selection is cwd-aware and resolves against the passed workspace. """ pub moduleDiscoveryCheck(ws: Workspace!): Void @check { let roots = phpSdk.modules(ws).{{rootPath}}.map { mod => mod.rootPath } - assert(contains(roots, lookupModulePath), "root discovery missed a legacy PHP module") - assert(contains(roots, tomlModulePath), "root discovery missed a TOML PHP module") - assert(contains(roots, nonPhpModulePath) == false, "discovery included a non-PHP module") + assert(contains(roots, lookupModulePath), "root scope missed the module registered with a legacy dagger.json") + assert(contains(roots, tomlModulePath), "root scope missed the module registered with a dagger-module.toml") + assert(contains(roots, nonPhpModulePath) == false, "scope included a module this SDK does not manage") # Keep dagger.toml in this full snapshot: asSDK(workspace:) reads the # managed-module list from the workspace it is explicitly passed. @@ -54,15 +54,34 @@ type E2e { assert(contains(scoped.map { mod => mod.path }, ".."), "nested module path should be cwd-relative") # Reverse the config ordering: the farther config is TOML and the nearest - # managed config is JSON. The polyfill must still choose the nearest root. + # managed config is JSON. Registration, not config filename, decides what + # the cwd scope selects. let mixedWs = ws.directory("/").asWorkspace(cwd: mixedNestedPath) let mixed = phpSdk.modules(mixedWs).{{rootPath, path}} assert(mixed.length == 1, "mixed-config cwd should see only its nearest managed module") - assert(contains(mixed.map { mod => mod.rootPath }, mixedModulePath), "mixed discovery chose the farther TOML config") + assert(contains(mixed.map { mod => mod.rootPath }, mixedModulePath), "mixed-config cwd resolved the wrong root") assert(contains(mixed.map { mod => mod.path }, "../.."), "mixed module path should be cwd-relative") null } + """ + init layers onto the target directory instead of replacing it. + + `Workspace.withNewDirectory` writes a fresh directory rather than merging + into the existing one, so seeding a module into a path that already holds + files has to combine the two by hand or it deletes them. sdk-sdk's + `contract:does-not-remove-existing-files` only ever inits into an empty path, + so it cannot see this. + """ + pub initLayeringCheck(ws: Workspace!): Void @check { + let removed = phpSdk + .initModule(ws, name: "layering", path: layeringModulePath) + .removedPaths + + assert(removed.length == 0, "init removed files that already existed in the target directory") + null + } + """ Generation from below the workspace root must resolve the stable rootPath from `/`, rather than prefixing the caller cwd a second time. diff --git a/README.md b/README.md index ae4d2a5..6edd94c 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,7 @@ PHP codegen) still lives in [`github.com/dagger/dagger/sdk/php`](https://github.com/dagger/dagger/tree/main/sdk/php); this module wraps the init/scaffolding ergonomics on top of it. -Backed by [`github.com/dagger/polyfill`](https://github.com/dagger/polyfill). +It uses the engine's native `Workspace` and `ModuleSource` APIs directly. ## Install @@ -74,11 +74,15 @@ dagger call php-sdk generate-all dagger call php-sdk modules root-path path ``` -Discovery supports both CLI 1.0 `dagger-module.toml` and legacy `dagger.json`. -It returns the nearest enclosing managed module plus managed modules beneath -the current directory. `rootPath` is the stable workspace-root-relative +The list comes from the modules registered to this SDK in the workspace +config: it returns the nearest enclosing managed module plus managed modules +beneath the current directory. `rootPath` is the stable workspace-root-relative identity; `path` is relative to the caller's current directory. +`dagger call php-sdk mod --path ` is the path-driven lookup instead: it +walks up to the nearest module config, supporting both CLI 1.0 +`dagger-module.toml` and legacy `dagger.json`. + See [`php-sdk.dang`](./php-sdk.dang) for the full type surface. ## Skipping generation diff --git a/dagger.json b/dagger.json index 29862d1..1c14561 100644 --- a/dagger.json +++ b/dagger.json @@ -1,14 +1,7 @@ { "name": "php-sdk", - "engineVersion": "v1.0.0-0", + "engineVersion": "v1.0.0-beta.10", "sdk": { "source": "dang" - }, - "dependencies": [ - { - "name": "polyfill", - "source": "github.com/dagger/polyfill@main", - "pin": "e90bbfc4843258a877a3a95b8db1571e7981e65f" - } - ] + } } diff --git a/dagger.lock b/dagger.lock index 7e5723d..5d1e96f 100644 --- a/dagger.lock +++ b/dagger.lock @@ -1,3 +1,3 @@ -[["version","1"]] -["","git.head",["https://github.com/dagger/sdk-sdk"],"e1747f4b6221fa24da080701e027243e0cc5fa33","float"] -["","git.ref",["https://github.com/dagger/polyfill","main"],"ec3ea84a2351b4beb06ecece951f2e5ef66509ff","float"] \ No newline at end of file +[["version","2"]] +["","container.from",["docker.io/library/golang:1.25-alpine","linux/amd64"],"sha256:1ae0735f00daffa3aaf1363a5184c0d2dc55c78e3db4ec70241cdac97bf84b59"] +["","git.ref",["https://github.com/dagger/sdk-sdk","HEAD"],{"sha":"e1747f4b6221fa24da080701e027243e0cc5fa33"}] \ No newline at end of file diff --git a/docs/cwd-aware-discovery.md b/docs/cwd-aware-discovery.md index 1807ba6..16592ff 100644 --- a/docs/cwd-aware-discovery.md +++ b/docs/cwd-aware-discovery.md @@ -1,15 +1,13 @@ # CWD-aware module discovery -The PHP SDK delegates config discovery to -[`github.com/dagger/polyfill`](https://github.com/dagger/polyfill), then -intersects the discovered directories with the PHP SDK modules registered on -the `Workspace` passed to `modules`. +The PHP SDK asks `currentModule.asSDK(workspace: ws).modules` for the modules +registered to it that are relevant to the caller's current directory. The engine +owns both membership and scope selection, so the SDK neither scans config files +nor reconstructs the cwd policy. -Discovery returns managed modules at or below the client's current directory -and, when the current directory has no module config, its nearest enclosing -managed module. Both `dagger-module.toml` and legacy `dagger.json` participate -in discovery, so the nearest config wins regardless of filename. Composer -`vendor` directories are excluded. +Selection returns managed modules at or below the client's current directory +and, when the current directory itself is not registered, its nearest enclosing +managed module. Each `Mod` exposes two coordinates: @@ -19,3 +17,7 @@ Each `Mod` exposes two coordinates: Generation anchors `rootPath` at `/` before resolving the module source. This keeps generation correct when invoked from a module root or another nested directory. + +`mod` is the separate, path-driven lookup: it walks up from an arbitrary +workspace path to the nearest module config, so it does read `dagger.json` and +`dagger-module.toml` and does not require the module to be registered. diff --git a/mod.dang b/mod.dang index 45f95ea..7022b4f 100644 --- a/mod.dang +++ b/mod.dang @@ -50,11 +50,47 @@ type Mod { """ pub generate: Changeset! { if (skipGenerate) { - polyfill.workspace(ws).fork.changes + changeset } else { - # rootPath is workspace-root-relative, so anchor it before passing it - # through the cwd-aware polyfill. - polyfill.workspace(ws).moduleSource("/" + rootPath).generate.changes + # rootPath is workspace-root-relative: Workspace.moduleSource resolves a + # relative path from ws.cwd and would prefix it a second time. + let ref = if (rootPath == ".") { "/" } else { "/" + rootPath } + + # Stage the local dependency closure so this module's codegen sees + # up-to-date dependency bindings. The staged workspace is only an input: + # the engine diffs the generated context against the same base it + # generated from, so the dependencies' codegen does not ride along. + let stagedWs = ws.withChanges(ws.moduleSource(ref).generateLocalDependencies(ws)) + + atCwd(stagedWs.moduleSource(ref).generatedContextChangeset) + } + } + + """ + Re-root a workspace-rooted changeset at the client's cwd. + + The engine roots a generated context at the workspace, but a returned + changeset is applied relative to the caller's cwd, so without this every path + lands nested under the cwd a second time. `ModuleSource.generate` followed by + `Workspace.changes(from:)` is the native API for this, but on + v1.0.0-beta.10 that pair reports a module's whole generated context as added + rather than only what generation changed — including engine-owned files the + SDK must not touch — so the rooting stays explicit here. + """ + let atCwd(changes: Changeset!): Changeset! { + let cwd = ws.cwd.trimPrefix("/").trimSuffix("/") + if (cwd == "" or cwd == ".") { + changes + } else { + # A cwd-relative changeset cannot express paths outside the cwd — e.g. a + # module discovered above it — and re-rooting would silently drop them, + # leaving that module never regenerated. Fail loudly instead. + let outside = (changes.addedPaths + changes.modifiedPaths + changes.removedPaths) + .filter { path => path != cwd and path.hasPrefix(cwd + "/") == false } + if (outside.length > 0) { + raise "generated changes fall outside the current directory " + cwd + ": " + outside.join(", ") + } + changes.after.directory(cwd).changes(changes.before.directory(cwd)) } } } diff --git a/php-sdk.dang b/php-sdk.dang index 32462e2..e5317ac 100644 --- a/php-sdk.dang +++ b/php-sdk.dang @@ -22,40 +22,19 @@ type PhpSdk { pub targetRuntime: String! { "php" } """ - Return every managed PHP SDK module visible from the client's cwd: the - nearest enclosing module plus modules at or below the cwd. Filesystem - discovery is intersected with the SDK list on the passed workspace. - """ - pub modules(ws: Workspace!): [Mod!]! { - # Resolve the managed list against the workspace supplied by the caller. - # Module dependencies no longer inherit an ambient current workspace. - let managed = currentModule.asSDK(workspace: ws).modules.{{path}} - let cwd = normalizePath(ws.cwd) - polyfill.workspace(ws) - .findConfigDirs(moduleConfigFilenames, exclude: ["**/vendor/**"]) - .map { dir => workspacePath(cwd, dir) } - .uniq - .filter { path => - managed.filter { module => normalizePath(module.path) == path }.length > 0 - } - .map { path => Mod(rootPath: path, ws: ws, skipGenerateFilename: skipGenerateFilename) } - } + Return every managed PHP SDK module visible from the client's cwd: every + module at or below the cwd, plus the nearest enclosing module when the cwd + itself is not managed. + The engine owns both the managed list and the cwd policy, so the SDK does no + filesystem discovery of its own. Paths come back workspace-root-relative, the + currency Mod.rootPath uses. """ - Resolve a cwd-relative discovery result to a workspace-root-relative path. - """ - let workspacePath(cwd: String!, path: String!): String! { - let base = if (cwd == ".") { [] } else { cwd.split("/") } - let segments = path.split("/").reduce(base) { acc, segment => - if (segment == "..") { - acc.dropLast(1) - } else if (segment == "." or segment == "") { - acc - } else { - acc + [segment] - } - } - if (segments.length == 0) { "." } else { segments.join("/") } + pub modules(ws: Workspace!): [Mod!]! { + currentModule + .asSDK(workspace: ws) + .modules.{{path}} + .map { module => Mod(rootPath: module.path, ws: ws, skipGenerateFilename: skipGenerateFilename) } } let normalizePath(path: String!): String! { @@ -183,9 +162,18 @@ type PhpSdk { if (currentModule.source.exists("templates/" + selectedTemplate) == false) { raise "unknown init template: " + template } else { - polyfill.workspace(ws).fork - .withDirectory(modPath, renderedTemplate(name, selectedTemplate)) - .changes + # modPath is workspace-root-relative: a relative path resolves from + # ws.cwd and would be prefixed a second time on a direct call from a + # subdirectory. + let target = if (modPath == ".") { "/" } else { "/" + modPath } + + # withNewDirectory writes a fresh directory rather than merging into the + # one already there, so layer the template onto the existing contents: + # init must never remove a user's files. + ws.withNewDirectory( + target, + existingDir(ws, modPath).withDirectory(".", renderedTemplate(name, selectedTemplate)), + ).changes(ws) } } @@ -196,7 +184,19 @@ type PhpSdk { this function. The engine owns materializing the generated client files. """ pub initClient(ws: Workspace!, path: String!, module: String!, dev: Boolean! = false): Changeset! { - polyfill.workspace(ws).fork.changes + changeset + } + + """ + Existing contents of a workspace directory, empty when it does not exist yet. + """ + let existingDir(ws: Workspace!, path: String!): Directory! { + if (path == ".") { + ws.directory("/") + } else { + let filtered = ws.directory("/", include: [path + "/**"]) + if (filtered.exists(path)) { filtered.directory(path) } else { directory } + } } """ @@ -222,13 +222,10 @@ type PhpSdk { Modules with the generate skip marker are skipped. """ pub generateAll(ws: Workspace!): Changeset! @generate { - let pws = polyfill.workspace(ws) - - modules(ws) - .filter { mod => mod.skipGenerate == false } - .reduce(pws.fork) { fork, mod => - fork.merge(pws.moduleSource("/" + mod.rootPath).generate) - } - .changes + changeset.withChangesets( + modules(ws) + .filter { mod => mod.skipGenerate == false } + .map { mod => mod.generate }, + ) } } From a665463ef8750e4ca8f89327f84b718188e73679 Mon Sep 17 00:00:00 2001 From: Yves Brissaud Date: Fri, 21 Aug 2026 10:33:20 +0200 Subject: [PATCH 2/3] Check that generated changesets stay rooted at the caller's cwd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing in the suite noticed where a generated client actually lands. Every generation fixture runs the Dang runtime, which emits no files, so the existing cwd check compares two empty changesets and passes either way. That leaves a real trap uncovered. `ModuleSource.generatedContextChangeset` measures its paths from the module source's context directory, not from the client's cwd — reaching for it directly instead of going through `ModuleSource.generate` writes the generated client one module path deeper than it belongs, e.g. `mymod/mymod/sdk/generated/`. The engine says as much where the field is defined: a caller that needs cwd-relative paths applies the generated files to a workspace and diffs it against the workspace it started from. Catching that needs a module whose codegen emits something, so this fixture runs the PHP runtime. It carries only a module config: codegen produces the composer scaffolding and the client itself, which is exactly what the check inspects, so there is nothing to commit alongside it. Signed-off-by: Yves Brissaud --- .../fixtures/rooting/app/dagger-module.toml | 5 ++++ .dagger/modules/e2e/main.dang | 23 +++++++++++++++++++ dagger.toml | 3 +++ 3 files changed, 31 insertions(+) create mode 100644 .dagger/modules/e2e/fixtures/rooting/app/dagger-module.toml diff --git a/.dagger/modules/e2e/fixtures/rooting/app/dagger-module.toml b/.dagger/modules/e2e/fixtures/rooting/app/dagger-module.toml new file mode 100644 index 0000000..ea7a212 --- /dev/null +++ b/.dagger/modules/e2e/fixtures/rooting/app/dagger-module.toml @@ -0,0 +1,5 @@ +name = "rooting-app" +engineVersion = "v1.0.0-0" + +[runtime] +source = "php" diff --git a/.dagger/modules/e2e/main.dang b/.dagger/modules/e2e/main.dang index 2dae361..ff2e928 100644 --- a/.dagger/modules/e2e/main.dang +++ b/.dagger/modules/e2e/main.dang @@ -11,6 +11,7 @@ type E2e { let mixedNestedPath: String! = mixedModulePath + "/nested/deeper" let generateModulePath: String! = fixtureRoot + "/generate/app" let layeringModulePath: String! = fixtureRoot + "/layering/app" + let rootingModulePath: String! = fixtureRoot + "/rooting/app" let assert(condition: Boolean!, message: String!): Void { if (condition == false) { raise message } @@ -101,4 +102,26 @@ type E2e { assert(fromParent.isEmpty == fromModule.isEmpty, "generation resolved different modules from different cwds") null } + + """ + A generated changeset is rooted at the caller's cwd, not at the workspace + root. + + This needs a module whose codegen actually emits files, so it runs the PHP + runtime rather than the lightweight Dang fixtures the other checks use. A + changeset built straight from ModuleSource.generatedContextChangeset is + measured from the module source's context directory instead, which lands the + generated client one module path deeper than it belongs. + """ + pub generateRootingCheck(ws: Workspace!): Void @check { + let modWs = ws.directory("/").asWorkspace(cwd: rootingModulePath) + let added = phpSdk.generateAll(modWs).addedPaths + + assert(added.length > 0, "generating a PHP module should add files") + assert( + added.filter { path => path.trimPrefix(fixtureRoot) != path }.length == 0, + "generated paths must be relative to the caller's cwd, not the workspace root", + ) + null + } } diff --git a/dagger.toml b/dagger.toml index 4f1eb3b..6215447 100644 --- a/dagger.toml +++ b/dagger.toml @@ -22,6 +22,9 @@ name = "php" [[modules.php-sdk.as-sdk.modules]] path = ".dagger/modules/e2e/fixtures/generate/app" +[[modules.php-sdk.as-sdk.modules]] +path = ".dagger/modules/e2e/fixtures/rooting/app" + [[modules.php-sdk.as-sdk.modules]] path = ".dagger/modules/e2e/fixtures/lookup/app" From 9f3ed90684a4d3709d0d76636770e2db11c08a60 Mon Sep 17 00:00:00 2001 From: Yves Brissaud Date: Fri, 21 Aug 2026 10:33:30 +0200 Subject: [PATCH 3/3] chore: pin sdk-sdk to 3344489 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The committed lock was still in the v1 format, whose `float` entry the current CLI no longer honors: every run silently re-resolved sdk-sdk to whatever main pointed at and rewrote the file. Recording it in v2 makes the revision the checks actually run against explicit again. Pinning it here also matters for what it contains. The old entry named `e1747f4`, which still declared `daggerCliVersion = "1.0.0-beta.9"`, and a beta.9 CLI cannot drive a beta.10 engine — beta.10 moved registering a function's required `Workspace` argument as a CLI flag out of the engine and into the CLI, so the older CLI never sees `initModule` and the contract checks fail with `unknown command "init-module"`. dagger/sdk-sdk#14 fixed that pin, and dagger/sdk-sdk#17 then added the `monorepo` group, which drives a workspace whose dagger.toml sits in a git subdirectory — the layout where module paths cross the engine/SDK boundary root-relative while the selected config reads them relative to itself. That is exactly what this module's path anchoring has to get right, so it is worth being on. Signed-off-by: Yves Brissaud --- dagger.lock | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dagger.lock b/dagger.lock index 5d1e96f..036e362 100644 --- a/dagger.lock +++ b/dagger.lock @@ -1,3 +1,2 @@ [["version","2"]] -["","container.from",["docker.io/library/golang:1.25-alpine","linux/amd64"],"sha256:1ae0735f00daffa3aaf1363a5184c0d2dc55c78e3db4ec70241cdac97bf84b59"] -["","git.ref",["https://github.com/dagger/sdk-sdk","HEAD"],{"sha":"e1747f4b6221fa24da080701e027243e0cc5fa33"}] \ No newline at end of file +["","git.ref",["https://github.com/dagger/sdk-sdk","HEAD"],{"ref":"refs/heads/main","sha":"334448911a8292fba0d677e5f31926c79ad80ad3"}] \ No newline at end of file