fix(polyfill): include local dir deps declared in dagger-module.toml#10
Draft
eunomie wants to merge 1 commit into
Draft
fix(polyfill): include local dir deps declared in dagger-module.toml#10eunomie wants to merge 1 commit into
eunomie wants to merge 1 commit into
Conversation
c3dbd35 to
0ff9be8
Compare
#7 added dagger-module.toml support to workspace-module-generate but, for toml modules, returned an empty source config on the assumption that the engine resolves includes and dependencies when the source is loaded. That holds for a module's own files (loaded via "**"), but not for local directory dependencies: their directories live outside the module and must be added to the loaded context explicitly, or the engine fails with "dir module source does not contain a dagger config file" during dagger generate. Parse the dagger-module.toml dependencies (and include) so moduleSourceInclude recurses into local dir dependencies and pulls their directories into the loaded context. Signed-off-by: Yves Brissaud <yves@dagger.io>
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.
What
dagger generateon a workspace whose module declares a local directory dependency indagger-module.tomlfails:Repro: a workspace with the Go SDK and two modules where one depends on the other via a relative path (e.g.
../my-deps), thendagger generate.Root cause
#7 added
dagger-module.tomlsupport toworkspace-module-generate, but for TOML modules it returned an empty source config, on the assumption that the engine resolves includes and dependencies when the source is loaded:That holds for a module's own files (loaded via
"**"), but not for local directory dependencies: their directories live outside the module directory.moduleSourceIncluderecurses overconfig.dependenciesto pull each local dep's directory into the loaded context — with an empty config it never does, so the dependency's directory is dropped. The engine then loads the module as a dir source over that reduced context, resolves the dependency from its owndagger-module.toml, and fails because the dependency's directory isn't present.Fix
Parse the
dagger-module.tomldependencies (and include) so the recursion visits local dir dependencies and adds their directories to the loaded context.parseSourceConfigTOML, with a note pointing at feat: add asTOML and TOML value type dagger#13379: onceFile.asTOMLships and the helper'sdagger.io/daggeris bumped, the local decode can be replaced by reading the fields through the engine API (dropping thepelletier/go-tomldependency).github.com/pelletier/go-toml v1.9.5(same lib/version the engine uses).Tests
TestParseSourceConfigTOMLReadsDependenciesAndInclude— TOML deps/include parse correctly.TestModuleSourceIncludeReadsTOMLDependencies— a module whose dependency is declared only indagger-module.tomlnow includes the dependency's directory.go build,go vet,gofmt,go testall clean.A
polyfill-e2eend-to-end.generatecheck (with adagger-module.tomlfixture) is a sensible follow-up but needs a CLI-1.0 dev engine to validate; left out of this PR.