Skip to content
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

tinygo: revise and simplify wasmtime argument handling #4555

Merged
merged 3 commits into from
Oct 28, 2024

Conversation

ydnar
Copy link
Contributor

@ydnar ydnar commented Oct 27, 2024

This PR implements 3 related changes to how TinyGo processes arguments for the wasmtime emulator. I ran across the issue related to trying to use wasmtime serve in a custom target file for my WasmCon talk next month.

It adds support for alternative wasmtime subcommands like serve, simplifies the Wasmtime-specific code paths, and fixes PWD handling for tinygo run when not running tests. I recommend reviewing by commit to see the individual changes.

Detailed changes:

  • Simplify buildAndRun, condensing wasmtime handling into a single section handling --env and --dir args.
  • Use wasmtime run subcommand instead of bare wasmtime. This enables use of wasmtime serve or other subcommands. The key change is that emulator arguments are inserted after the subcommand, e.g. run.
  • Set PWD to the package dir only when running tests, which allows tinygo run --target wasip1 to run with PWD=..
  • Fixed a related bug with -x (PrintCommands) where it repeated the full path and name of the command.

- Use wasmtime run subcommand instead of bare wasmtime. This enables use of wasmtime serve or other subcommands.
- Simplify buildAndRun, condensing wasmtime handling into a single section handling env vars and dir args.
- Only set PWD to the package dir if running tests, but not with tinygo run.
@ydnar ydnar self-assigned this Oct 27, 2024
@@ -925,7 +927,7 @@ func buildAndRun(pkgName string, config *compileopts.Config, stdout io.Writer, c

// Run binary.
if config.Options.PrintCommands != nil {
config.Options.PrintCommands(cmd.Path, cmd.Args...)
config.Options.PrintCommands(cmd.Path, cmd.Args[1:]...)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running tinygo run -x ... repeated the full path to wasmtime and wasmtime.

ydnar added a commit to ydnar/wasi-http-go that referenced this pull request Oct 27, 2024
@dgryski
Copy link
Member

dgryski commented Oct 28, 2024

The isSingleFile logic was for cases where we did tinygo run some/dir/file.go to make sure the directories were set correctly. If that still works then these cleanups LGTM.

@ydnar
Copy link
Contributor Author

ydnar commented Oct 28, 2024

Yes that still works.

The equivalent tinygo run ./cmd/foo also works now (without specifying main.go).

Copy link
Member

@dgryski dgryski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ydnar ydnar merged commit 0edeaf6 into tinygo-org:dev Oct 28, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants