python-sdk: report only what generation produced - #18
Merged
eunomie merged 2 commits intoAug 25, 2026
Conversation
Picks up dagger/sdk-sdk#17, a `monorepo` check group covering a workspace config in a subdirectory of the git root: the layout where the caller's cwd sits below the workspace root, so module paths cross the engine <-> SDK boundary root-relative while init changesets are applied at the root. sdk-sdk reported three of those checks red against the SDK it was written with; all six pass here on v1.0.0-beta.10, which carries the engine-side fix for dagger/dagger#13889. `dagger check`: 43/43, up from 37/37. The golang:1.26-alpine digest is a transitive pin the workspace already resolves at check time; recording it keeps a check run from dirtying the tree. Signed-off-by: Yves Brissaud <yves@dagger.io>
Generation threaded the workspace through ModuleSource.generate and diffed it back with Workspace.changes. On v1.0.0-beta.10 that loses the baseline: the module's whole generated context comes back as added rather than only what generation changed. Every generate and generateAll claimed to add the module's own dagger.json, which the engine owns and had not touched, and once a module has generated output on disk, files codegen rewrites byte for byte are reported as added too. The engine defect is in applying a changeset, not in comparing workspaces: withChanges projects the structural before/after diff, which a fresh mtime alone puts a path into, rather than the content-based paths the changeset reports. dagger/dagger#13947 fixes it. Write the generated context as a directory instead, the shape go-sdk settled on in dagger/go-sdk#30. A directory overlay diffs correctly, so Workspace.changes still does the cwd rooting and the outside-the-cwd guard, and nothing here has to be reverted once the engine fix lands. generatedContextDirectory does not resolve the local dependency closure the way generate(ws) did, so generation stages it explicitly, and generateAll merges each module's changeset rather than threading one workspace through all of them. The generate fixture now commits the .gitattributes that generation emits, so e-2-e:generate-skips-existing-files-check catches a generator returning its whole context: every other file there is genuinely new on a fresh module. Signed-off-by: Yves Brissaud <yves@dagger.io>
eunomie
force-pushed
the
python-sdk-generate-context-changeset
branch
from
August 25, 2026 14:15
842e489 to
c724b03
Compare
eunomie
marked this pull request as ready for review
August 25, 2026 14:26
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.
Follow-up to #14, which merged before this landed.
Generation used
moduleSource(...).generate(ws).changes(ws). Onv1.0.0-beta.10that loses the diff baseline: the module's whole generated context comes back as
added instead of only what generation changed. Every
generateandgenerateAllclaimed to add the module's owndagger.json, which the engineowns and had not touched — and once a module already has generated output on
disk, the files codegen rewrites byte for byte are reported as added too.
Root cause
Reproduced on
v1.0.0-beta.10here. It is notworkspaceChangesBetween, as anearlier revision of this PR said — that function applies the overlay to both
sides, identically on
beta.10and onmain.A
Changesetreports the paths whose content differs, butDirectory.withChanges(andChangeset.layer/Changeset.export) project thestructural before/after diff, which a fresh mtime alone puts a path into —
and codegen gives a fresh mtime to every file it rewrites. A host-backed
workspace then sizes the baseline it compares against from the reported paths,
so the surplus files diff against nothing and read as newly added.
On a re-run, with generated output already on disk and one generated file
drifted, the old idiom reports
.gitattributes,.gitignoreanddagger.jsonas added while the correct answer is "one file modified".
Fixed by dagger/dagger#13947, which is open and unmerged: not in
v1.0.0-beta.10, and not ondagger/dagger@main.This is the same defect for both shapes python-sdk sees — the engine-owned
dagger.jsonon a first generate, and already-on-disk generated files on are-generate. Both leak through
Changeset.layer/Directory.withChangesprojecting the structural diff, so #13947's
TestWorkspaceChangesAgainstExistingBaselinecovers them; nothing here needs aseparate engine report. (
Changeset.exporthas the same gap and reproduceshere too: exporting the generated context rewrote the fixture's
dagger.jsonthough it was never a reported path.)
Fix
Write the generated context as a directory rather than threading a workspace
through
ModuleSource.generate— the shape go-sdk settled on indagger/go-sdk#30. A directory overlay diffs correctly, so
Workspace.changesstill does the cwd rooting and the outside-the-cwd guard, and nothing here has
to be reverted once dagger/dagger#13947 lands.
generatedContextDirectorydoes not resolve the local dependency closure the waygenerate(ws)did, so generation stages it explicitly, andgenerateAllmergeseach module's changeset rather than threading one workspace through all of them.
The generate fixture now commits the
.gitattributesgeneration emits, soe-2-e:generate-skips-existing-files-checkcatches a generator returning itswhole context; every other file there is genuinely new on a fresh module, so only
an already-present one can catch it. On the previous idiom it fails with:
sdk-sdk
Also bumps
sdk-sdkto3344489(dagger/sdk-sdk#17), which adds amonorepocheck group for a workspace config in a subdirectory of the git root. sdk-sdk
reported three of those red against the SDK they were written with; all six pass
here on
beta.10, which carries the engine-side fix for dagger/dagger#13889.dagger check: 43/43 (was 37/37).