From f5fde71b1b33c12e811f5a07b5b811a5edb82920 Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Sat, 12 Oct 2024 10:33:24 +0200 Subject: [PATCH 1/4] loader: don't panic when main package is not named 'main' This can in fact happen in practice, so return an actual error message instead. --- errors_test.go | 1 + loader/loader.go | 8 ++++++-- testdata/errors/invalidmain.go | 6 ++++++ 3 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 testdata/errors/invalidmain.go diff --git a/errors_test.go b/errors_test.go index 62d5af2cbb..5045840ff5 100644 --- a/errors_test.go +++ b/errors_test.go @@ -24,6 +24,7 @@ func TestErrors(t *testing.T) { {name: "cgo"}, {name: "compiler"}, {name: "interp"}, + {name: "invalidmain"}, {name: "linker-flashoverflow", target: "cortex-m-qemu"}, {name: "linker-ramoverflow", target: "cortex-m-qemu"}, {name: "linker-undefined", target: "darwin/arm64"}, diff --git a/loader/loader.go b/loader/loader.go index d10485707f..6786ee5335 100644 --- a/loader/loader.go +++ b/loader/loader.go @@ -418,8 +418,12 @@ func (p *Package) Check() error { packageName := p.ImportPath if p == p.program.MainPkg() { if p.Name != "main" { - // Sanity check. Should not ever trigger. - panic("expected main package to have name 'main'") + return Errors{p, []error{ + scanner.Error{ + Pos: p.program.fset.Position(p.Files[0].Name.Pos()), + Msg: fmt.Sprintf("expected main package to have name \"main\", not %#v", p.Name), + }, + }} } packageName = "main" } diff --git a/testdata/errors/invalidmain.go b/testdata/errors/invalidmain.go new file mode 100644 index 0000000000..a86e32c8dd --- /dev/null +++ b/testdata/errors/invalidmain.go @@ -0,0 +1,6 @@ +// some comment to move the first line + +package foobar + +// ERROR: # command-line-arguments +// ERROR: invalidmain.go:3:9: expected main package to have name "main", not "foobar" From 415307e4c1196df835c61b4c2384cd8b76d8f40d Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Sat, 12 Oct 2024 10:50:34 +0200 Subject: [PATCH 2/4] main: make sure typecheck errors are correctly reported --- errors_test.go | 1 + testdata/errors/invalidname.go | 6 ++++++ testdata/errors/invalidname/invalidname.go | 3 +++ 3 files changed, 10 insertions(+) create mode 100644 testdata/errors/invalidname.go create mode 100644 testdata/errors/invalidname/invalidname.go diff --git a/errors_test.go b/errors_test.go index 5045840ff5..fe118616e3 100644 --- a/errors_test.go +++ b/errors_test.go @@ -25,6 +25,7 @@ func TestErrors(t *testing.T) { {name: "compiler"}, {name: "interp"}, {name: "invalidmain"}, + {name: "invalidname"}, {name: "linker-flashoverflow", target: "cortex-m-qemu"}, {name: "linker-ramoverflow", target: "cortex-m-qemu"}, {name: "linker-undefined", target: "darwin/arm64"}, diff --git a/testdata/errors/invalidname.go b/testdata/errors/invalidname.go new file mode 100644 index 0000000000..9a470b0d8d --- /dev/null +++ b/testdata/errors/invalidname.go @@ -0,0 +1,6 @@ +package main + +import _ "github.com/tinygo-org/tinygo/testdata/errors/invalidname" + +// ERROR: # github.com/tinygo-org/tinygo/testdata/errors/invalidname +// ERROR: invalidname{{[\\/]}}invalidname.go:3:9: invalid package name _ diff --git a/testdata/errors/invalidname/invalidname.go b/testdata/errors/invalidname/invalidname.go new file mode 100644 index 0000000000..c75242244e --- /dev/null +++ b/testdata/errors/invalidname/invalidname.go @@ -0,0 +1,3 @@ +// some comment to move the 'package' line + +package _ From 01fb6508b9cdd2782c9b90d35580332cce926aae Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Sat, 12 Oct 2024 10:54:22 +0200 Subject: [PATCH 3/4] loader: make sure we always return an error even without type errors This issue was originally reported here: https://github.com/NixOS/nixpkgs/pull/341170#issuecomment-2359237471 The fix here isn't a great fix, it turns the error message from this: # runtime/interrupt into this: # runtime/interrupt package requires newer Go version go1.23 ...so not great, because it doesn't show the real error message (which is that TinyGo wasn't compiled with the right Go version). But at least it gives a hint in the right direction. It's difficult to test for this specific case, so I've left out testing in this case (boo!) --- loader/loader.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/loader/loader.go b/loader/loader.go index 6786ee5335..f3ffa86144 100644 --- a/loader/loader.go +++ b/loader/loader.go @@ -432,7 +432,15 @@ func (p *Package) Check() error { if err, ok := err.(Errors); ok { return err } - return Errors{p, typeErrors} + if len(typeErrors) != 0 { + // Got type errors, so return them. + return Errors{p, typeErrors} + } + // This can happen in some weird cases. + // The only case I know is when compiling a Go 1.23 program, with a + // TinyGo version that supports Go 1.23 but is compiled using Go 1.22. + // So this should be pretty rare. + return Errors{p, []error{err}} } p.Pkg = typesPkg From e06f03baa33554010c9bbe4aaeb22bf945b29225 Mon Sep 17 00:00:00 2001 From: Ayke van Laethem Date: Sat, 12 Oct 2024 11:10:21 +0200 Subject: [PATCH 4/4] builder: check for Go toolchain version used to compile TinyGo This shows a much better error message for issues like this one: https://github.com/NixOS/nixpkgs/pull/341170#issuecomment-2359237471 The new error message would be: cannot compile with Go toolchain version go1.23 (TinyGo was built using toolchain version go1.21.4) --- builder/config.go | 26 ++++++++++++++++++++++---- goenv/version.go | 12 +++++++++--- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/builder/config.go b/builder/config.go index 40c6f0c9a7..8db8ff8fe7 100644 --- a/builder/config.go +++ b/builder/config.go @@ -2,6 +2,7 @@ package builder import ( "fmt" + "runtime" "github.com/tinygo-org/tinygo/compileopts" "github.com/tinygo-org/tinygo/goenv" @@ -23,20 +24,37 @@ func NewConfig(options *compileopts.Options) (*compileopts.Config, error) { spec.OpenOCDCommands = options.OpenOCDCommands } - major, minor, err := goenv.GetGorootVersion() + // Version range supported by TinyGo. + const minorMin = 19 + const minorMax = 23 + + // Check that we support this Go toolchain version. + gorootMajor, gorootMinor, err := goenv.GetGorootVersion() if err != nil { return nil, err } - if major != 1 || minor < 19 || minor > 23 { + if gorootMajor != 1 || gorootMinor < minorMin || gorootMinor > minorMax { // Note: when this gets updated, also update the Go compatibility matrix: // https://github.com/tinygo-org/tinygo-site/blob/dev/content/docs/reference/go-compat-matrix.md - return nil, fmt.Errorf("requires go version 1.19 through 1.23, got go%d.%d", major, minor) + return nil, fmt.Errorf("requires go version 1.19 through 1.23, got go%d.%d", gorootMajor, gorootMinor) + } + + // Check that the Go toolchain version isn't too new, if we haven't been + // compiled with the latest Go version. + // This may be a bit too aggressive: if the newer version doesn't change the + // Go language we will most likely be able to compile it. + buildMajor, buildMinor, err := goenv.Parse(runtime.Version()) + if err != nil { + return nil, err + } + if buildMajor != 1 || buildMinor < gorootMinor { + return nil, fmt.Errorf("cannot compile with Go toolchain version go%d.%d (TinyGo was built using toolchain version %s)", gorootMajor, gorootMinor, runtime.Version()) } return &compileopts.Config{ Options: options, Target: spec, - GoMinorVersion: minor, + GoMinorVersion: gorootMinor, TestConfig: options.TestConfig, }, nil } diff --git a/goenv/version.go b/goenv/version.go index 1db5a630af..cdfa278bd5 100644 --- a/goenv/version.go +++ b/goenv/version.go @@ -34,19 +34,25 @@ func GetGorootVersion() (major, minor int, err error) { if err != nil { return 0, 0, err } + return Parse(s) +} - if s == "" || s[:2] != "go" { +// Parse parses the Go version (like "go1.3.2") in the parameter and return the +// major and minor version: 1 and 3 in this example. If there is an error, (0, +// 0) and an error will be returned. +func Parse(version string) (major, minor int, err error) { + if version == "" || version[:2] != "go" { return 0, 0, errors.New("could not parse Go version: version does not start with 'go' prefix") } - parts := strings.Split(s[2:], ".") + parts := strings.Split(version[2:], ".") if len(parts) < 2 { return 0, 0, errors.New("could not parse Go version: version has less than two parts") } // Ignore the errors, we don't really handle errors here anyway. var trailing string - n, err := fmt.Sscanf(s, "go%d.%d%s", &major, &minor, &trailing) + n, err := fmt.Sscanf(version, "go%d.%d%s", &major, &minor, &trailing) if n == 2 && err == io.EOF { // Means there were no trailing characters (i.e., not an alpha/beta) err = nil