core/schema: don't prefix main workspace module checks#12951
core/schema: don't prefix main workspace module checks#12951eunomie wants to merge 1 commit intodagger:mainfrom
Conversation
`reparentWorkspaceTreeRoot` was unconditionally wrapping every primary workspace module's tree root with a synthetic parent named after the module, so every check/generator/service path was rendered as `<module>:<item>` — including the workspace's own root module. As a result, `dagger check -l` on this repo showed `dagger-dev:generated` instead of just `generated`, contradicting the documented behavior (docs/current_docs/introduction/core-concepts/checks.mdx) where only toolchain items are namespaced. The integration contract is already asserted strictly for toolchains by `TestChecksAsToolchain` but only loosely for main modules by `TestChecksDirectSDK` / `TestChecksAsBlueprint` (via `Contains`, which accepts either form). Fix: have `SchemaBuilder.PrimaryMods` also return the set of "main" (entrypoint-proxied) module names, and skip the reparent for those modules inside `reparentWorkspaceTreeRoot`. Main modules are the ones installed with `Entrypoint` — the workspace root module or a blueprint — whose main-object methods are proxied onto the Query root. Toolchain modules remain prefixed with their module name. Also strengthen `TestChecksDirectSDK` and `TestChecksAsBlueprint` with `NotContains` assertions on the prefixed form, so a future regression surfaces immediately. Signed-off-by: Yves Brissaud <yves@dagger.io>
9c91497 to
4cb4275
Compare
|
I think maybe we shouldn't merge this (or any other attempt to patch the prefix change). Rationale:
TLDR: big risk, small reward. I recommend just leaving it as is for now. NOTE: this assumes I'm correct that nothing breaks. If I am wrong please tell me! |
|
This PR is stale because it has been open 14 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
reparentWorkspaceTreeRootwas unconditionally wrapping every primary workspace module's tree root with a synthetic parent named after the module, so every check/generator/service path was rendered as<module>:<item>— including the workspace's own root module.As a result,
dagger check -lon this repo showeddagger-dev:generatedinstead of justgenerated, contradicting the documented behavior (docs/current_docs/introduction/core-concepts/checks.mdx) where only toolchain items are namespaced. The integration contract is already asserted strictly for toolchains byTestChecksAsToolchainbut only loosely for main modules byTestChecksDirectSDK/TestChecksAsBlueprint(viaContains, which accepts either form).Fix: have
SchemaBuilder.PrimaryModsalso return the set of "main" (entrypoint-proxied) module names, and skip the reparent for those modules insidereparentWorkspaceTreeRoot. Main modules are the ones installed withEntrypoint— the workspace root module or a blueprint — whose main-object methods are proxied onto the Query root. Toolchain modules remain prefixed with their module name.Also strengthen
TestChecksDirectSDKandTestChecksAsBlueprintwithNotContainsassertions on the prefixed form, so a future regression surfaces immediately.