-
Notifications
You must be signed in to change notification settings - Fork 263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move Go SDK to use wit-bindgen 0.9 #1630
Conversation
Signed-off-by: Ryan Levick <me@ryanlevick.com>
Thank you @rylev for using the the go-bindgen in Spin's Go SDK! These findings are great and insightful. It seems to me that the first two points are bugs, and the last two points are feature requests. I will raise each of them as an issue on the wit-bindgen repo and start working on fixing the bugs and improving the |
Thanks for getting this work going. I too have found the |
@Mossaka FYI: the other bug I'm seeing has been reproduced and logged as an issue here: bytecodealliance/wit-bindgen#607 |
Signed-off-by: Ryan Levick <me@ryanlevick.com>
Signed-off-by: Ryan Levick <me@ryanlevick.com>
Signed-off-by: Ryan Levick <me@ryanlevick.com>
This is blocked on one final issue: bytecodealliance/wit-bindgen#617. Once that issue is solved, this should be fine to merge. |
Thank you, @rylev !! |
Hey @rylev, could you please elaborate more about how this particular issue is blocking this PR? |
Concretely, bytecodealliance/wit-bindgen#617 is about adding wit information including the metadata (e.g., the module "producer") to the component model so that tooling knows what the wit would the component targets. In If the module is produced with newer tooling, we know that we can just use the normal One thing I want to try now is to see can we just try to use the |
Going to close this for now. We still want to resolve spinframework/spin-go-sdk#2, but things have evolved sufficiently from the wit-bindgen 0.9 days that this PR is not going to get merged without starting over. |
Resolves spinframework/spin-go-sdk#2 This is a not quite working move of the internals of the Go SDK to use wit-bindgen 0.8
Overall the move greatly simplifies things as we can stop using the C generator and move to direct tinygo support.
@Mossaka Here are a few bugs and usability issues I've found:
use
imports all the way through. When generating code for SDK I get some type errors because some definitions are not created:undefined: FermyonSpinHttpTypesTuple2StringStringT
I have to add type aliases for these for them to work.Result
andOption
types are not the most helpful. Our SDK usually has return types ofvalue, error
and having to translate theResult
andOption
types to the (arguably more idiomatic) version is tedious. What speaks against the generated code following thevalue, error
pattern more closely?Display
impls for error types?I don't have time today to investigate the errors today, but I'll try to do so tomorrow. Let me know if you'd like to take a look together!