go: fix Go self-call schema names and cover dependency use#2
Open
grouville wants to merge 2 commits into
Open
Conversation
Problem: When a Go module has SELF_CALLS enabled, the Go SDK writes the module schema that is then used to generate the module's own Go client. For exported fields, that schema was using the Go name directly. So a field called Message was exposed as Message. The engine exposes that same field as message. Because of that mismatch, the generated Go client could be wrong. In the case we found, it could generate a field and a method with the same Go name, which does not compile. Fix: Emit exported fields with the same name the engine uses. Message now becomes message. Test: Add a Message field to the existing Go self-call fixture and query message from the integration test. This proves the generated Go code compiles and the field is available under the expected name. Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>
Problem: The review raised a possible failure where a Go module with SELF_CALLS works when called directly, but breaks when another module depends on it. If that happened, the caller could still generate Go code for dag.Dep(), but the running engine would not expose dep at runtime. The user would only see the failure when calling the function. We did not reproduce that exact failure, but this is still a useful area to cover because it is easy to miss. A module dependency is loaded through a different path than the module being called directly. Solution: Add two small Go fixtures: - self-calls-as-dep: caller -> dep - self-calls-transitive: caller -> middle -> dep Only dep has SELF_CALLS enabled. That keeps the tests focused on the dependency case from the review. Tests: viaDep calls a dependency function that calls itself through dag.Dep(). viaDepContainer returns a container created by that self-call and then reads stdout from the caller. This proves the returned object is still usable. viaDepWorker calls back into dag.Dep() from a secondary object. This proves the case is not limited to methods on the root object. viaTransitiveDep calls through caller -> middle -> dep. This proves the self-call still works when the module with SELF_CALLS is a dependency of a dependency. Signed-off-by: Guillaume de Rouville <guillaume@dagger.io>
6 tasks
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.
This PR is a follow-up against eunomie's
workspace-go-no-codegen-at-runtimebranch.That branch changes how Go modules with SELF_CALLS provide the schema used to generate their own Go client. My manual review led to those findings:
The actual bug
A Go module can expose fields on its root object:
The engine exposes that field as message, but the new self-call schema emitter was exposing it as Message.
That mismatch can break the generated Go client. In the failing case, generated code could end up with a field and a method using the same Go name, so the code would not compile.
What changed:
Messagenow becomesmessage.Dependency coverage added:
caller->depcaller->middle->depThe dep module is the only module with SELF_CALLS enabled. That keeps the tests focused on the case from the review.
The new tests verify:
Tested with:
Result: 17 passed.