diff --git a/.dagger/modules/e2e/generate.dang b/.dagger/modules/e2e/generate.dang index ea71c02..cd55b74 100644 --- a/.dagger/modules/e2e/generate.dang +++ b/.dagger/modules/e2e/generate.dang @@ -20,6 +20,65 @@ type GenerateChecks { null } + """ + A CLI 1.0 module is generated by this SDK, and what it writes has to satisfy + the engine runtime by itself: that runtime does no codegen for a + dagger-module.toml module, it mounts sdk/ as @dagger.io/dagger and runs the + committed entrypoint. + + So this asserts the whole contract rather than a single marker file — the + bundled library the bindings import, the bindings themselves, the dispatch + entrypoint, and the config the runtime reads. A missing piece here is a module + that fails to load at `dagger call` time, long after generate. + """ + generateWorkspaceModuleCheck(ws: Workspace!): Void @check { + let path = fixtures.depAppModule + let changes = typescriptSdk.mod(ws, path: path).generate(ws) + let tree = changes.after.directory(path) + + Asserts.generated(changes, path + "/sdk/client.gen.ts") + Asserts.generated(changes, path + "/sdk/core.js") + Asserts.generated(changes, path + "/sdk/core.d.ts") + Asserts.generated(changes, path + "/sdk/index.ts") + Asserts.generated(changes, path + "/sdk/telemetry.ts") + Asserts.generated(changes, path + "/__dagger.entrypoint.ts") + + # package.json and tsconfig.json are asserted on the resulting tree rather + # than the changeset: this fixture already carries correct ones, so a + # correctly-behaving generate leaves them untouched and out of the diff. + + # The bindings must reach the runtime through the bundle sitting beside + # them, not through the npm package: sdk/ *is* @dagger.io/dagger here. + Asserts.stringContains( + tree.file("sdk/client.gen.ts").contents, + "from \"./core.js\"", + "module bindings should import the bundled runtime", + ) + + # The entrypoint dispatches by importing the user's classes from where they + # were declared, which only the source scan knows. + Asserts.stringContains( + tree.file("__dagger.entrypoint.ts").contents, + "from \"./src/index\"", + "entrypoint should import the module's own source", + ) + + # Without the pin the runtime installs typescript on every call instead of + # mounting its prebuilt copy. + Asserts.stringContains( + tree.file("package.json").contents, + "\"typescript\"", + "package.json should pin typescript", + ) + Asserts.stringContains( + tree.file("tsconfig.json").contents, + "./sdk/index.ts", + "tsconfig should alias @dagger.io/dagger to the generated sdk", + ) + + null + } + """ Generating a module with a local dependency must resolve that dependency and emit its typed bindings alongside the module's own — otherwise the module's diff --git a/design/module-gen.md b/design/module-gen.md index 7541627..89c17ba 100644 --- a/design/module-gen.md +++ b/design/module-gen.md @@ -543,15 +543,40 @@ Add an e2e check that generates the same fixture **both ways** and asserts the trees are identical (modulo the known drops: lockfile, re-shipped sources). This is the cutover's safety net and it is cheap while both paths exist. -**Phase 3 — cutover, for `dagger-module.toml` modules only.** Flip -`Mod.generate` to the local path when the module's config is -`dagger-module.toml`; keep delegating to `polyfill.moduleSource(...).generate` -for `dagger.json` ones (§8). Keep `generateLocalDependencies` staging (still -required: a dependent's schema can only be built if its local deps' generated -files exist, and dep generation may cross SDKs). Update the e2e assertions from -`sdk/index.ts` to the full expected tree, and add `__dagger.entrypoint.ts` + -`sdk/core.js` assertions. Verify `dagger call` on a generated fixture actually -runs — the runtime contract in §2.2 is only really proven by executing a module. +**Phase 3 — cutover, for `dagger-module.toml` modules only.** ✅ Done. +`Mod.generate` takes the local path when the module's config is +`dagger-module.toml` and keeps delegating to +`polyfill.moduleSource(...).generate` for `dagger.json` ones (§8). +`generateLocalDependencies` staging is kept for both: a dependent's schema can +only be loaded once its local deps' generated files exist, and dep generation +may cross SDKs. + +Validated three ways rather than one: + +- **Byte-for-byte against the engine.** Generating the same fixture both ways + produces identical `client.gen.ts`, `__dagger.entrypoint.ts`, `index.ts`, + `telemetry.ts`, `core.d.ts`, `package.json` and `tsconfig.json`. The one + exception is `core.js`, which is *ours* by construction: built from the + vendored lockfile at 4.3 MB against the engine's unpinned 5.4 MB (§4.2). + Getting there required three generator fixes the diff surfaced (§ the + generator was ported from an older upstream commit than the engine we pin). +- **By running a module.** A `dagger call` on a generated fixture returns its + value, exercising our bindings, bundle and entrypoint through the engine + runtime, dependency bindings included. The runtime contract in §2.2 is only + really proven by executing a module. +- **By an e2e check** asserting the whole tree contract, not one marker file. + +**VCS files are not written** (decided): no `.gitignore`, no `.gitattributes`. +The engine appends to both around codegen, but for a workspace module the +ignore list is reduced to `node_modules`/`.pnpm-store` anyway, which is the +user's business rather than codegen's. + +Worth knowing, since it looks like our bug when it happens: a module whose +`.gitignore` still ignores `sdk/` — a legacy `dagger.json` module migrated to +`.toml` — fails to load with *"committed generated file sdk/client.gen.ts is +missing"* even though the file is on disk, because the ignore keeps it out of +the module context. The engine has the same problem: it only avoids *adding* +generated paths for toml modules, it never removes one already there. **Phase 4 — upstream cleanup** (separate `dagger/dagger` PR, §9). @@ -634,15 +659,16 @@ late): Bundled without `--compile`, with the trimmed compiler at `node_modules/typescript`, it scans a fixture module and emits a `typedef.json` with the `location` data the entrypoint needs. -2. **Does our `module` mode reproduce the engine's `sdk/client.gen.ts` - byte-for-byte** for a fixture, given `ModuleSource.introspectionSchemaJSON`? - This is the differential check of §7 Phase 2, run by hand once, first. +2. ~~**Does our `module` mode reproduce the engine's `sdk/client.gen.ts` + byte-for-byte**~~ — **yes**, once the generator was synced with the engine + it targets (§7 Phase 3). It did not before: enum members were miscased, + `arguments` went unescaped, and the entrypoint carried no source maps. 3. ~~**Is `bun build` output reproducible enough**~~ — **yes**, with the image pinned by digest and the vendored lockfile in place: a second packager run over an unchanged tree reports no changes. Dropping the lockfile is what breaks it (§4.2). -Only the differential check is left, and it belongs to Phase 2 anyway. +All three are now answered; the work they were guarding is done. (For the record on the fetch alternative in §4.1: dang does support `@cache(policy:, ttl:)` → `withCachePolicy`, but a plain container exec is already content-addressed by the engine, so the decorator would mostly buy a TTL diff --git a/mod.dang b/mod.dang index c4e5d25..7419f83 100644 --- a/mod.dang +++ b/mod.dang @@ -93,9 +93,43 @@ type Mod { polyfill.workspace(ws).fork.changes } else { # Stage the local dependency closure so this module's codegen sees - # up-to-date dependency bindings before generating it. + # up-to-date dependency bindings before generating it. A dependency's + # schema can only be loaded once its own generated files exist, so this + # has to happen before the schema is read either way. let stagedWs = ws.withChanges(polyfill.workspace(ws).moduleSource("/" + rootPath).core.generateLocalDependencies(ws)) - polyfill.workspace(stagedWs).moduleSource("/" + rootPath).generate.changes + + generateStaged(stagedWs, ws) + } + } + + """ + Generate this module against a workspace that already has its dependency + closure staged, choosing the generator by config format. + + Split from `generate` so generate-all can reuse the routing without repeating + the staging: the engine dispatches the SDK's @generate rollup for each local + dependency, so generate-all already runs inside another module's staging. + """ + let generateStaged(ws: Workspace!, base: Workspace!): Changeset! { + if (isWorkspaceManaged(ws)) { + TypescriptSdk().moduleFiles(ws, rootPath, sourcePath, base) + } else { + # A dagger.json module stays with the engine's built-in TypeScript + # runtime end to end: it regenerates everything at call time anyway, so + # generating it here would only put a second, differently-versioned copy + # of the same files on disk. + polyfill.workspace(ws).moduleSource("/" + rootPath).generate.changes } } + + """ + Whether this module uses the CLI 1.0 config format, whose modules build from + their committed generated files rather than regenerating at call time. That + makes what `generate` writes the artifact the runtime executes — and makes + this SDK, not the engine, the thing that has to write it. + """ + let isWorkspaceManaged(ws: Workspace!): Boolean! { + let path = if (rootPath == ".") { "dagger-module.toml" } else { rootPath + "/dagger-module.toml" } + ws.directory("/", include: [path]).exists(path) + } } diff --git a/typescript-sdk.dang b/typescript-sdk.dang index 12ca451..32dfd82 100644 --- a/typescript-sdk.dang +++ b/typescript-sdk.dang @@ -601,26 +601,60 @@ type TypescriptSdk { } """ - Generate a module's SDK files in this SDK rather than through the engine's - built-in TypeScript runtime. + Generate a module's files and stage them at its source directory. - Staged behind `Mod.generate` while the two are compared: see the - generate:local-matches-engine check, which generates a fixture both ways and - diffs the result. + Takes the module's paths rather than a Mod so it can be called with a + workspace that already has the module's dependency closure staged — resolving + the module afresh from that workspace is what makes its schema loadable. """ - generateModuleLocal(ws: Workspace!, path: String!): Changeset! { - let mod = mod(ws, path: path, findUp: false) - let modSrc = polyfill.workspace(ws).moduleSource("/" + mod.rootPath).core - let sourcePath = mod.sourcePath + let moduleFiles(ws: Workspace!, rootPath: String!, sourcePath: String!, base: Workspace!): Changeset! { + let modSrc = polyfill.workspace(ws).moduleSource("/" + rootPath).core let sourcePrefix = if (sourcePath == ".") { "" } else { sourcePath + "/" } - polyfill.workspace(ws).fork.withDirectory(sourcePath, moduleDirectory( + # Everything the module is generated *from* reads the staged workspace, so + # dependency bindings are current. Everything the changeset is measured + # *against* reads the unstaged one: the engine may have just generated this + # module while staging it for a dependent, and diffing against that would + # report its own output as already present and emit nothing. + let existing = if (sourcePath == ".") { base.directory("/") } else { base.directory("/" + sourcePath) } + + let generated = moduleDirectory( modSrc.introspectionSchemaJSON.contents, modSrc.moduleOriginalName, - ws.directory("/" + sourcePrefix + "src"), - existingModuleConfig(ws, sourcePath), - mod.config.runtime, - )).changes + base.directory("/" + sourcePrefix + "src"), + existingModuleConfig(base, sourcePath), + moduleRuntime(base, sourcePath), + ) + + # Stage the generated files as a diff against the module's existing tree, + # not as its replacement. `withDirectory` swaps the whole directory, so + # staging only what we generate says everything else — the module's config + # and its source — was deleted. Applying to disk hides that, but staging + # into a workspace does not: the engine generates a dependency by running + # this against a workspace it then reuses, and the next module's dependency + # resolution finds the dependency's config gone. + polyfill.workspace(base).fork + .withDirectoryDiff(sourcePath, existing, existing.withDirectory(".", generated)) + .changes + } + + """ + Which TypeScript runtime a module's source directory is configured for, + detected from the files it carries — the same rule the engine applies when it + loads the module. + """ + let moduleRuntime(ws: Workspace!, sourcePath: String!): Runtime! { + ModConfig(sourcePath: sourcePath, ws: ws).runtime + } + + """ + Generate a module's files through this SDK, bypassing `Mod.generate`'s choice + of path. Exists so a check can generate the same module both ways and compare + the results; `Mod.generate` is what callers should use. + """ + generateModuleLocal(ws: Workspace!, path: String!): Changeset! { + let mod = mod(ws, path: path, findUp: false) + moduleFiles(ws, mod.rootPath, mod.sourcePath, ws) } """ @@ -779,7 +813,12 @@ type TypescriptSdk { # codegen is ephemeral: taking the changeset against the staged workspace # cancels it out, leaving only each module's own changes. let stagedWs = ws.withChanges(pws.moduleSource("/" + mod.rootPath).core.generateLocalDependencies(ws)) - polyfill.workspace(stagedWs).moduleSource("/" + mod.rootPath).generate.changes + + # Generate through Mod so this and `mod generate` cannot pick different + # generators — but only its staged half: Mod.generate would stage a + # second time, and the engine already re-enters this function under each + # dependency's own staging. + mod.generateStaged(stagedWs, ws) } # Force the per-module codegen to evaluate concurrently: selecting a field on