diff --git a/CHANGELOG.md b/CHANGELOG.md index 1509f5c1..325d7319 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ### Fixed +- [#281](https://github.com/bytecodealliance/go-modules/issues/281): errors from internal `wasm-tools` calls are no longer silently ignored. This required fixing a number of related issues, including synthetic world packages for Component Model metadata generation, WIT generation, and WIT keyword escaping in WIT package or interface names. - [#284](https://github.com/bytecodealliance/go-modules/issues/284): do not use `bool` for `variant` or `result` GC shapes. TinyGo returns `result` and `variant` values with `bool` as 0 or 1, which breaks the memory representation of tagged unions (variants). ## [v0.5.0] — 2024-12-14 diff --git a/testdata/issues/issue281.wit b/testdata/issues/issue281.wit new file mode 100644 index 00000000..bee7fc76 --- /dev/null +++ b/testdata/issues/issue281.wit @@ -0,0 +1,11 @@ +package issues:issue281@0.1.0; + +world imports { + import wasi:http/types@0.2.1; +} + +package wasi:http@0.2.1 { + interface types { + type t = u8; + } +} diff --git a/testdata/issues/issue281.wit.json b/testdata/issues/issue281.wit.json new file mode 100644 index 00000000..e6bb6a57 --- /dev/null +++ b/testdata/issues/issue281.wit.json @@ -0,0 +1,53 @@ +{ + "worlds": [ + { + "name": "imports", + "imports": { + "interface-0": { + "interface": { + "id": 0 + } + } + }, + "exports": {}, + "package": 1 + } + ], + "interfaces": [ + { + "name": "types", + "types": { + "t": 0 + }, + "functions": {}, + "package": 0 + } + ], + "types": [ + { + "name": "t", + "kind": { + "type": "u8" + }, + "owner": { + "interface": 0 + } + } + ], + "packages": [ + { + "name": "wasi:http@0.2.1", + "interfaces": { + "types": 0 + }, + "worlds": {} + }, + { + "name": "issues:issue281@0.1.0", + "interfaces": {}, + "worlds": { + "imports": 0 + } + } + ] +} \ No newline at end of file diff --git a/testdata/issues/issue281.wit.json.golden.wit b/testdata/issues/issue281.wit.json.golden.wit new file mode 100644 index 00000000..dab07157 --- /dev/null +++ b/testdata/issues/issue281.wit.json.golden.wit @@ -0,0 +1,11 @@ +package issues:issue281@0.1.0; + +world imports { + import wasi:http/types@0.2.1; +} + +package wasi:http@0.2.1 { + interface types { + type t = u8; + } +} diff --git a/wit/bindgen/generator.go b/wit/bindgen/generator.go index 553b42f4..19d659f6 100644 --- a/wit/bindgen/generator.go +++ b/wit/bindgen/generator.go @@ -20,6 +20,7 @@ import ( "testing/fstest" "time" + "github.com/tetratelabs/wazero/sys" "go.bytecodealliance.org/cm" "go.bytecodealliance.org/internal/codec" "go.bytecodealliance.org/internal/go/gen" @@ -2364,13 +2365,15 @@ func (g *generator) newPackage(w *wit.World, i *wit.Interface, name string) (*ge // Generate wasm file res, world := synthesizeWorld(g.res, w, worldName) witText := res.WIT(wit.Filter(world, i), "") + world.Package.Worlds.Delete(worldName) // Undo mutation if g.opts.generateWIT { witFile := g.witFileFor(owner) witFile.WriteString(witText) } content, err := g.componentEmbed(witText) if err != nil { - // return nil, err + g.opts.logger.Errorf("WIT:\n%s\n\n", witText) + return nil, err } componentType := &wasm.CustomSection{ Name: "component-type:" + worldName, @@ -2390,7 +2393,7 @@ func (g *generator) newPackage(w *wit.World, i *wit.Interface, name string) (*ge return pkg, nil } -var replacer = strings.NewReplacer("/", "-", ":", "-", "@", "-v", ".", "") +var replacer = strings.NewReplacer("/", "-", ":", "-", "@", "-v", ".", "", "%", "") // componentEmbed runs generated WIT through wasm-tools to generate a wasm file with a component-type custom section. func (g *generator) componentEmbed(witData string) ([]byte, error) { @@ -2398,35 +2401,33 @@ func (g *generator) componentEmbed(witData string) ([]byte, error) { defer cancel() // TODO: --all-features? - filename := "./component.wit" + filename := "component.wit" args := []string{"component", "embed", "--only-custom", filename} fsMap := map[string]fs.FS{ - ".": fstest.MapFS{ + "": fstest.MapFS{ filename: &fstest.MapFile{Data: []byte(witData)}, }, } stdout := &bytes.Buffer{} - err := g.wasmTools.Run(ctx, nil, stdout, nil, fsMap, args...) + stderr := &bytes.Buffer{} + err := g.wasmTools.Run(ctx, nil, stdout, stderr, fsMap, args...) if err != nil { + if _, ok := err.(*sys.ExitError); ok { + return nil, fmt.Errorf("wasm-tools: %s", stderr.String()) + } return nil, fmt.Errorf("wasm-tools: %w", err) } return stdout.Bytes(), err } func synthesizeWorld(r *wit.Resolve, w *wit.World, name string) (*wit.Resolve, *wit.World) { - p := &wit.Package{} - p.Name.Namespace = "go" - p.Name.Package = "bindgen" - w = w.Clone() w.Name = name w.Docs = wit.Docs{} - w.Package = p - p.Worlds.Set(name, w) + w.Package.Worlds.Set(name, w) r = r.Clone() r.Worlds = append(r.Worlds, w) - r.Packages = append(r.Packages, p) return r, w } diff --git a/wit/bindgen/testdata_test.go b/wit/bindgen/testdata_test.go index 9cb53774..05c89471 100644 --- a/wit/bindgen/testdata_test.go +++ b/wit/bindgen/testdata_test.go @@ -21,6 +21,7 @@ import ( "go.bytecodealliance.org/internal/go/gen" "go.bytecodealliance.org/internal/relpath" "go.bytecodealliance.org/wit" + "go.bytecodealliance.org/wit/logging" ) var writeGoFiles = flag.Bool("write", false, "write generated Go files") @@ -108,6 +109,7 @@ func validateGeneratedGo(t *testing.T, res *wit.Resolve, origin string) { GeneratedBy("test"), PackageRoot(pkgPath), Versioned(true), + Logger(logging.NewLogger(os.Stderr, logging.LevelWarn)), ) if err != nil { t.Error(err) diff --git a/wit/ident.go b/wit/ident.go index 1a7a902c..958d01de 100644 --- a/wit/ident.go +++ b/wit/ident.go @@ -2,6 +2,7 @@ package wit import ( "errors" + "strconv" "strings" "github.com/coreos/go-semver/semver" @@ -36,8 +37,6 @@ type Ident struct { func ParseIdent(s string) (Ident, error) { var id Ident name, ver, hasVer := strings.Cut(s, "@") - base, ext, hasExt := strings.Cut(name, "/") - id.Namespace, id.Package, _ = strings.Cut(base, ":") if hasVer { var err error id.Version, err = semver.NewVersion(ver) @@ -45,12 +44,23 @@ func ParseIdent(s string) (Ident, error) { return id, err } } + base, ext, hasExt := strings.Cut(name, "/") + ns, pkg, _ := strings.Cut(base, ":") + id.Namespace = trimPercent(ns) + id.Package = trimPercent(pkg) if hasExt { - id.Extension = ext + id.Extension = trimPercent(ext) } return id, id.Validate() } +func trimPercent(s string) string { + if len(s) > 0 && s[0] == '%' { + return s[1:] + } + return s +} + // Validate validates id, returning any errors. func (id *Ident) Validate() error { switch { @@ -59,6 +69,59 @@ func (id *Ident) Validate() error { case id.Package == "": return errors.New("missing package name") } + if err := validateName(id.Namespace); err != nil { + return err + } + if err := validateName(id.Package); err != nil { + return err + } + return validateName(id.Extension) +} + +func validateName(s string) error { + if len(s) == 0 { + return nil + } + var prev rune + for _, c := range s { + switch { + case c >= 'a' && c <= 'z': + switch { + case prev >= 'A' && prev <= 'Z': + return errors.New("invalid character " + strconv.Quote(string(c))) + } + case c >= 'A' && c <= 'Z': + switch { + case prev == 0: // start of string + case prev >= 'A' && prev <= 'Z': + case prev >= '0' && prev <= '9': + case prev == '-': + default: + return errors.New("invalid character " + strconv.Quote(string(c))) + } + case c >= '0' && c <= '9': + switch { + case prev >= 'a' && prev <= 'z': + case prev >= 'A' && prev <= 'Z': + case prev >= '0' && prev <= '9': + default: + return errors.New("invalid character " + strconv.Quote(string(c))) + } + case c == '-': + switch prev { + case 0: // start of string + return errors.New("invalid leading -") + case '-': + return errors.New("invalid double --") + } + default: + return errors.New("invalid character " + strconv.Quote(string(c))) + } + prev = c + } + if prev == '-' { + return errors.New("invalid trailing -") + } return nil } @@ -68,15 +131,15 @@ func (id *Ident) String() string { return id.UnversionedString() } if id.Extension == "" { - return id.Namespace + ":" + id.Package + "@" + id.Version.String() + return escape(id.Namespace) + ":" + escape(id.Package) + "@" + id.Version.String() } - return id.Namespace + ":" + id.Package + "/" + id.Extension + "@" + id.Version.String() + return escape(id.Namespace) + ":" + escape(id.Package) + "/" + escape(id.Extension) + "@" + id.Version.String() } // UnversionedString returns a string representation of an [Ident] without version information. func (id *Ident) UnversionedString() string { if id.Extension == "" { - return id.Namespace + ":" + id.Package + return escape(id.Namespace) + ":" + escape(id.Package) } - return id.Namespace + ":" + id.Package + "/" + id.Extension + return escape(id.Namespace) + ":" + escape(id.Package) + "/" + escape(id.Extension) } diff --git a/wit/ident_test.go b/wit/ident_test.go index 7eff822d..4173eb2b 100644 --- a/wit/ident_test.go +++ b/wit/ident_test.go @@ -18,6 +18,16 @@ func TestIdent(t *testing.T) { {"wasi:io/streams", Ident{Namespace: "wasi", Package: "io", Extension: "streams"}, false}, {"wasi:io/streams@0.2.0", Ident{Namespace: "wasi", Package: "io", Extension: "streams", Version: semver.New("0.2.0")}, false}, + // Escaping + {"%use:%own", Ident{Namespace: "use", Package: "own"}, false}, + {"%use:%own@0.2.0", Ident{Namespace: "use", Package: "own", Version: semver.New("0.2.0")}, false}, + {"%use:%own/%type", Ident{Namespace: "use", Package: "own", Extension: "type"}, false}, + {"%use:%own/%type@0.2.0", Ident{Namespace: "use", Package: "own", Extension: "type", Version: semver.New("0.2.0")}, false}, + + // Mixed-case + {"ABC:def-GHI", Ident{Namespace: "ABC", Package: "def-GHI"}, false}, + {"ABC1:def2-GHI3", Ident{Namespace: "ABC1", Package: "def2-GHI3"}, false}, + // Errors {"", Ident{}, true}, {":", Ident{}, true}, @@ -28,6 +38,15 @@ func TestIdent(t *testing.T) { {"wasi:/", Ident{}, true}, {"wasi:clocks@", Ident{}, true}, {"wasi:clocks/wall-clock@", Ident{}, true}, + {"foo%:bar%baz", Ident{Namespace: "foo%", Package: "bar%baz"}, true}, + {"-foo:bar", Ident{Namespace: "-foo", Package: "bar"}, true}, + {"foo-:bar", Ident{Namespace: "foo-", Package: "bar"}, true}, + {"foo--foo:bar", Ident{Namespace: "foo--foo", Package: "bar"}, true}, + {"aBc:bar", Ident{Namespace: "aBc", Package: "bar"}, true}, + {"1:2", Ident{Namespace: "1", Package: "2"}, true}, + {"1a:2b", Ident{Namespace: "1a", Package: "2b"}, true}, + {"foo-1:bar", Ident{Namespace: "foo-1", Package: "bar"}, true}, + {"foo:bar-1", Ident{Namespace: "foo", Package: "bar-2"}, true}, } for _, tt := range tests { t.Run(tt.s, func(t *testing.T) { diff --git a/wit/wit.go b/wit/wit.go index b9687b3b..f1657e52 100644 --- a/wit/wit.go +++ b/wit/wit.go @@ -578,7 +578,7 @@ func relativeName(o TypeOwner, p *Package) string { return "" } qualifiedName := op.Name - qualifiedName.Package += "/" + name + qualifiedName.Package += "/" + escape(name) return qualifiedName.String() }