-
Notifications
You must be signed in to change notification settings - Fork 11
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
wit: sort packages in topological order when rendering WIT #208
Conversation
That was a fun debugging journey! Thankful we had WIT → JSON → WIT round trip tests. Turns out we had an old JSON representation of |
@@ -58,7 +58,7 @@ func main() { | |||
|
|||
err := cmd.Run(context.Background(), os.Args) | |||
if err != nil { | |||
fmt.Printf("error: %v\n", err) | |||
fmt.Fprintf(os.Stderr, "error: %v\n", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unrelated to the PR, but a bug I caught when debugging the new logic.
@@ -26,7 +26,7 @@ func LoadWIT(ctx context.Context, forceWIT bool, path string) (*wit.Resolve, err | |||
return wit.ParseWIT(bytes) | |||
} | |||
} | |||
if forceWIT || !strings.HasSuffix(path, ".json") { | |||
if forceWIT || (path != "" && path != "-" && !strings.HasSuffix(path, ".json")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another logic bug that was forced WIT parsing of stdin.
Multiple packages in a single WIT file are now sorted in topological order of dependence, starting with the package with fewest dependents. In practice, this means that the "root" package, e.g.
wasi:http
would be listed first when serializingwasi:http/proxy
and its dependents.This means that other tools, like
wit-bindgen
can process generated WIT without additional arguments like--world
.