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/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 38c0fff..ff2e928 100644 --- a/.dagger/modules/e2e/main.dang +++ b/.dagger/modules/e2e/main.dang @@ -10,6 +10,8 @@ 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 rootingModulePath: String! = fixtureRoot + "/rooting/app" let assert(condition: Boolean!, message: String!): Void { if (condition == false) { raise message } @@ -36,14 +38,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 +55,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. @@ -82,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/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..036e362 100644 --- a/dagger.lock +++ b/dagger.lock @@ -1,3 +1,2 @@ -[["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"]] +["","git.ref",["https://github.com/dagger/sdk-sdk","HEAD"],{"ref":"refs/heads/main","sha":"334448911a8292fba0d677e5f31926c79ad80ad3"}] \ No newline at end of file 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" 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 }, + ) } }