Skip to content

compileopts: move {root} replacement to compileopts.Emulator() #2454

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

Merged
merged 2 commits into from
Jan 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions compileopts/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,15 @@ func (c *Config) WasmAbi() string {
return c.Target.WasmAbi
}

// Emulator returns the emulator target config
func (c *Config) Emulator() []string {
var emulator []string
for _, s := range c.Target.Emulator {
emulator = append(emulator, strings.ReplaceAll(s, "{root}", goenv.Get("TINYGOROOT")))
}
Comment on lines +468 to +471
Copy link
Member

@aykevl aykevl Jan 14, 2022

Choose a reason for hiding this comment

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

This works, but a more efficient version is the following:

Suggested change
var emulator []string
for _, s := range c.Target.Emulator {
emulator = append(emulator, strings.ReplaceAll(s, "{root}", goenv.Get("TINYGOROOT")))
}
emulator = make([]string, len(c.Target.Emulator))
for i, s := range c.Target.Emulator {
emulator[i] = strings.ReplaceAll(s, "{root}", goenv.Get("TINYGOROOT"))
}

...probably not worth the effort though. Just an idea. It's even possible the Go compiler recognizes the idiom and optimizes it.

return emulator
}

type TestConfig struct {
CompileTestBinary bool
// TODO: Filter the test functions to run, include verbose flag, etc
Expand Down
7 changes: 0 additions & 7 deletions compileopts/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,13 +232,6 @@ func LoadTarget(options *Options) (*TargetSpec, error) {
spec.ExtraFiles = append(spec.ExtraFiles, "src/internal/task/task_asyncify_wasm.S")
}

// TODO(dgryski): handle CFLAGS and LDFLAGS here too?
var emu []string
for _, s := range spec.Emulator {
emu = append(emu, strings.ReplaceAll(s, "{root}", goenv.Get("TINYGOROOT")))
}
spec.Emulator = emu

return spec, nil
}

Expand Down
51 changes: 28 additions & 23 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,8 @@ func dirsToModuleRoot(maindir, modroot string) []string {
// run the binary.
func runPackageTest(config *compileopts.Config, stdout, stderr io.Writer, result builder.BuildResult, testVerbose, testShort bool, testRunRegexp string, testBenchRegexp string, testBenchTime string) (bool, error) {
var cmd *exec.Cmd
if len(config.Target.Emulator) == 0 {
emulator := config.Emulator()
if len(emulator) == 0 {
// Run directly.
var flags []string
if testVerbose {
Expand All @@ -272,8 +273,8 @@ func runPackageTest(config *compileopts.Config, stdout, stderr io.Writer, result
cmd = executeCommand(config.Options, result.Binary, flags...)
} else {
// Run in an emulator.
args := append(config.Target.Emulator[1:], result.Binary)
if config.Target.Emulator[0] == "wasmtime" {
args := append(emulator[1:], result.Binary)
if emulator[0] == "wasmtime" {
// create a new temp directory just for this run, announce it to os.TempDir() via TMPDIR
tmpdir, err := ioutil.TempDir("", "tinygotmp")
if err != nil {
Expand All @@ -282,10 +283,12 @@ func runPackageTest(config *compileopts.Config, stdout, stderr io.Writer, result
args = append(args, "--dir="+tmpdir, "--env=TMPDIR="+tmpdir)
// TODO: add option to not delete temp dir for debugging?
defer os.RemoveAll(tmpdir)

// allow reading from directories up to module root
for _, d := range dirsToModuleRoot(result.MainDir, result.ModuleRoot) {
args = append(args, "--dir="+d)
}

// mark end of wasmtime arguments and start of program ones: --
args = append(args, "--")
if testVerbose {
Expand All @@ -301,7 +304,7 @@ func runPackageTest(config *compileopts.Config, stdout, stderr io.Writer, result
args = append(args, "-test.bench="+testBenchRegexp)
}
}
cmd = executeCommand(config.Options, config.Target.Emulator[0], args...)
cmd = executeCommand(config.Options, emulator[0], args...)
}
cmd.Dir = result.MainDir
cmd.Stdout = stdout
Expand Down Expand Up @@ -499,12 +502,13 @@ func Debug(debugger, pkgName string, ocdOutput bool, options *compileopts.Option
gdbInterface, openocdInterface := config.Programmer()
switch gdbInterface {
case "msd", "command", "":
if len(config.Target.Emulator) != 0 {
if config.Target.Emulator[0] == "mgba" {
emulator := config.Emulator()
if len(emulator) != 0 {
if emulator[0] == "mgba" {
gdbInterface = "mgba"
} else if config.Target.Emulator[0] == "simavr" {
} else if emulator[0] == "simavr" {
gdbInterface = "simavr"
} else if strings.HasPrefix(config.Target.Emulator[0], "qemu-system-") {
} else if strings.HasPrefix(emulator[0], "qemu-system-") {
gdbInterface = "qemu"
} else {
// Assume QEMU as an emulator.
Expand Down Expand Up @@ -572,34 +576,34 @@ func Debug(debugger, pkgName string, ocdOutput bool, options *compileopts.Option
}
case "qemu":
port = ":1234"

emulator := config.Emulator()
// Run in an emulator.
args := append(config.Target.Emulator[1:], result.Binary, "-s", "-S")
daemon = executeCommand(config.Options, config.Target.Emulator[0], args...)
args := append(emulator[1:], result.Binary, "-s", "-S")
daemon = executeCommand(config.Options, emulator[0], args...)
daemon.Stdout = os.Stdout
daemon.Stderr = os.Stderr
case "qemu-user":
port = ":1234"

emulator := config.Emulator()
// Run in an emulator.
args := append(config.Target.Emulator[1:], "-g", "1234", result.Binary)
daemon = executeCommand(config.Options, config.Target.Emulator[0], args...)
args := append(emulator[1:], "-g", "1234", result.Binary)
daemon = executeCommand(config.Options, emulator[0], args...)
daemon.Stdout = os.Stdout
daemon.Stderr = os.Stderr
case "mgba":
port = ":2345"

emulator := config.Emulator()
// Run in an emulator.
args := append(config.Target.Emulator[1:], result.Binary, "-g")
daemon = executeCommand(config.Options, config.Target.Emulator[0], args...)
args := append(emulator[1:], result.Binary, "-g")
daemon = executeCommand(config.Options, emulator[0], args...)
daemon.Stdout = os.Stdout
daemon.Stderr = os.Stderr
case "simavr":
port = ":1234"

emulator := config.Emulator()
// Run in an emulator.
args := append(config.Target.Emulator[1:], "-g", result.Binary)
daemon = executeCommand(config.Options, config.Target.Emulator[0], args...)
args := append(emulator[1:], "-g", result.Binary)
daemon = executeCommand(config.Options, emulator[0], args...)
daemon.Stdout = os.Stdout
daemon.Stderr = os.Stderr
case "msd":
Expand Down Expand Up @@ -694,7 +698,8 @@ func Run(pkgName string, options *compileopts.Options) error {
}

return builder.Build(pkgName, ".elf", config, func(result builder.BuildResult) error {
if len(config.Target.Emulator) == 0 {
emulator := config.Emulator()
if len(emulator) == 0 {
// Run directly.
cmd := executeCommand(config.Options, result.Binary)
cmd.Stdout = os.Stdout
Expand All @@ -710,8 +715,8 @@ func Run(pkgName string, options *compileopts.Options) error {
return nil
} else {
// Run in an emulator.
args := append(config.Target.Emulator[1:], result.Binary)
cmd := executeCommand(config.Options, config.Target.Emulator[0], args...)
args := append(emulator[1:], result.Binary)
cmd := executeCommand(config.Options, emulator[0], args...)
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
err := cmd.Run()
Expand Down
18 changes: 12 additions & 6 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,13 +377,19 @@ func runTestWithConfig(name string, t *testing.T, options compileopts.Options, c
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
var cmd *exec.Cmd
if len(spec.Emulator) == 0 {

// make sure any special vars in the emulator definition are rewritten
config := compileopts.Config{Target: spec}
emulator := config.Emulator()

if len(emulator) == 0 {
cmd = exec.CommandContext(ctx, binary)
} else {
args := append(spec.Emulator[1:], binary)
cmd = exec.CommandContext(ctx, spec.Emulator[0], args...)
args := append(emulator[1:], binary)
cmd = exec.CommandContext(ctx, emulator[0], args...)
}
if len(spec.Emulator) != 0 && spec.Emulator[0] == "wasmtime" {

if len(emulator) != 0 && emulator[0] == "wasmtime" {
// Allow reading from the current directory.
cmd.Args = append(cmd.Args, "--dir=.")
for _, v := range environmentVars {
Expand All @@ -399,7 +405,7 @@ func runTestWithConfig(name string, t *testing.T, options compileopts.Options, c

// Run the test.
stdout := &bytes.Buffer{}
if len(spec.Emulator) != 0 && spec.Emulator[0] == "simavr" {
if len(emulator) != 0 && emulator[0] == "simavr" {
cmd.Stdout = os.Stderr
cmd.Stderr = stdout
} else {
Expand All @@ -421,7 +427,7 @@ func runTestWithConfig(name string, t *testing.T, options compileopts.Options, c
actual := bytes.Replace(stdout.Bytes(), []byte{'\r', '\n'}, []byte{'\n'}, -1)
expected = bytes.Replace(expected, []byte{'\r', '\n'}, []byte{'\n'}, -1) // for Windows

if len(spec.Emulator) != 0 && spec.Emulator[0] == "simavr" {
if len(emulator) != 0 && emulator[0] == "simavr" {
// Strip simavr log formatting.
actual = bytes.Replace(actual, []byte{0x1b, '[', '3', '2', 'm'}, nil, -1)
actual = bytes.Replace(actual, []byte{0x1b, '[', '0', 'm'}, nil, -1)
Expand Down