vugu / vugu

Vugu: A modern UI library for Go+WebAssembly (experimental)
https://www.vugu.org
MIT License
4.8k stars 175 forks source link

Lint code with golangci-lint #252

Closed Lord-Y closed 6 months ago

Lord-Y commented 7 months ago

As said in my last comment in PR https://github.com/vugu/vugu/pull/248, here is the 1st of many series of PR. This PR goes with https://github.com/vugu/vugu/issues/246 requirements.

I usually remove every unused functions/variables for a cleaner code. I didn't do it this time so I add //nolint:golint,unused tag. Tell me if you want me to delete them or not.

I'm not sure what you wanted to do https://github.com/Lord-Y/vugu/blob/057f77f040a6752b26d472deff9d9023b379c82e/tinygo-dev/testpgm.go#L19 with jr param.

owenwaller commented 6 months ago

Hi @Lord-Y

Thanks for this and for splitting the original larger PR.

I've just synced this locally and am seeing a number of test failures.

In devutil the tinygo test is failing with:

$ go test -v ./devutil/
=== RUN   TestWasmCompiler
    compiler_test.go:19: Using temporary dir: /tmp/TestWasmCompiler3622694087
WasmCompiler: Successful build
WasmCompiler: build error: exit status 1; full output:
# TestWasmCompiler
./main.go:2:20: syntax error: unexpected valid at end of statement

    compiler_test.go:47: we correctly got a build error here: WasmCompiler: build error: exit status 1; full output:
        # TestWasmCompiler
        ./main.go:2:20: syntax error: unexpected valid at end of statement
WasmCompiler: Successful generate
WasmCompiler: Successful build
    compiler_test.go:74: Using temporary dir: /tmp/TestWasmCompiler1447538440
WasmCompiler: Successful build
WasmCompiler: build error: exit status 1; full output:
# TestWasmCompiler
./main.go:2:19: syntax error: unexpected valid at end of statement

2023/11/24 13:04:19 MainWasmHandler: Execute error:
WasmCompiler: build error: exit status 1; full output:
# TestWasmCompiler
./main.go:2:19: syntax error: unexpected valid at end of statement
--- PASS: TestWasmCompiler (0.46s)
=== RUN   TestFileServer
    file-server_test.go:19: Using temporary dir: /tmp/TestFileServer581452147
--- PASS: TestFileServer (0.00s)
=== RUN   TestMux
--- PASS: TestMux (0.00s)
=== RUN   TestTinygoCompiler
TinygoCompiler: build error: exit status 1; cmd.args: [docker run --rm -v /:/src -e HOME=/tmp --user=1000 -w /src/tmp/TestTinygoCompiler2139571437 tinygo/tinygo:0.22.0 tinygo build -o /src//tmp/WasmCompiler3762543416 -target=wasm .], full output:
go: errors parsing go.mod:
/src/tmp/TestTinygoCompiler2139571437/go.mod:3: invalid go version '1.21.4': must match format 1.23

    tinygo-compiler_test.go:41: TinygoCompiler: build error: exit status 1; cmd.args: [docker run --rm -v /:/src -e HOME=/tmp --user=1000 -w /src/tmp/TestTinygoCompiler2139571437 tinygo/tinygo:0.22.0 tinygo build -o /src//tmp/WasmCompiler3762543416 -target=wasm .], full output:
        go: errors parsing go.mod:
        /src/tmp/TestTinygoCompiler2139571437/go.mod:3: invalid go version '1.21.4': must match format 1.23

--- FAIL: TestTinygoCompiler (0.84s)
FAIL
FAIL    github.com/vugu/vugu/devutil    1.304s
FAIL

Two things are strange about this:

I have manually installed tinygo v0.33.0

$ tinygo version
tinygo version 0.30.0 linux/amd64 (using go version go1.21.4 and LLVM version 16.0.1)

but the failure is trying use a docker image for tinygo v0.22. So far I've not worked out why or how this is occurring. I saw the same failure with tinygo v0.22 FWIW.

Also I am on the current version of go (v1.21.4) so I have the changes to the format to the version in the go.mod line to support a versioned toolchain, which AFAIK needs the form v.X.Y.Z.

The wasm-test-suite is failing with:

$ go test -v ./wasm-test-suite/
=== RUN   Test001Simple
=== RUN   Test001Simple/go
2023/11/24 13:14:24 URL: http://localhost:8846/fs774006153/
=== RUN   Test001Simple/go/t0
=== RUN   Test001Simple/go/t1
=== RUN   Test001Simple/go/t2
=== RUN   Test001Simple/go/t3
=== RUN   Test001Simple/go/t4
=== RUN   Test001Simple/go/t5
=== RUN   Test001Simple/go/t6
=== RUN   Test001Simple/go/t7
=== RUN   Test001Simple/go/t8
=== RUN   Test001Simple/go/t9
=== RUN   Test001Simple/tinygo/NoDocker
TinygoCompiler: Successful generate
TinygoCompiler: successful build
2023/11/24 13:14:38 URL: http://localhost:8846/fs690380628/
2023/11/24 13:14:39 ERROR: encountered exception 'Uncaught (in promise)' (136:64478)

Which then leads to a test timeout, due to the uncaught exception failure.

So ATM none of the tinygo tests are passing.

The simplehttp package is failing with:

$ go test ./simplehttp/
--- FAIL: TestSimpleHandlerDev (0.95s)
    simple-handler_test.go:47: 
            Error Trace:    simple-handler_test.go:47
            Error:          "// Copyright 2018 The Go Authors. All rights reserved.
                            // Use of this source code is governed by a BSD-style
                            // license that can be found in the LICENSE file.

                            "use strict";

                            (() => {
                                const enosys = () => {
                                    const err = new Error("not implemented");
                                    err.code = "ENOSYS";
                                    return err;
                                };

                                if (!globalThis.fs) {
                                    let outputBuf = "";
                                    globalThis.fs = {
                                        constants: { O_WRONLY: -1, O_RDWR: -1, O_CREAT: -1, O_TRUNC: -1, O_APPEND: -1, O_EXCL: -1 }, // unused
                                        writeSync(fd, buf) {
                                            outputBuf += decoder.decode(buf);
                                            const nl = outputBuf.lastIndexOf("\n");
                                            if (nl != -1) {
                                                console.log(outputBuf.substring(0, nl));
                                                outputBuf = outputBuf.substring(nl + 1);
                                            }
                                            return buf.length;
                                        },
                                        write(fd, buf, offset, length, position, callback) {
                                            if (offset !== 0 || length !== buf.length || position !== null) {
                                                callback(enosys());
                                                return;
                                            }
                                            const n = this.writeSync(fd, buf);
                                            callback(null, n);
                                        },
                                        chmod(path, mode, callback) { callback(enosys()); },
                                        chown(path, uid, gid, callback) { callback(enosys()); },
                                        close(fd, callback) { callback(enosys()); },
                                        fchmod(fd, mode, callback) { callback(enosys()); },
                                        fchown(fd, uid, gid, callback) { callback(enosys()); },
                                        fstat(fd, callback) { callback(enosys()); },
                                        fsync(fd, callback) { callback(null); },
                                        ftruncate(fd, length, callback) { callback(enosys()); },
                                        lchown(path, uid, gid, callback) { callback(enosys()); },
                                        link(path, link, callback) { callback(enosys()); },
                                        lstat(path, callback) { callback(enosys()); },
                                        mkdir(path, perm, callback) { callback(enosys()); },
                                        open(path, flags, mode, callback) { callback(enosys()); },
                                        read(fd, buffer, offset, length, position, callback) { callback(enosys()); },
                                        readdir(path, callback) { callback(enosys()); },
                                        readlink(path, callback) { callback(enosys()); },
                                        rename(from, to, callback) { callback(enosys()); },
                                        rmdir(path, callback) { callback(enosys()); },
                                        stat(path, callback) { callback(enosys()); },
                                        symlink(path, link, callback) { callback(enosys()); },
                                        truncate(path, length, callback) { callback(enosys()); },
                                        unlink(path, callback) { callback(enosys()); },
                                        utimes(path, atime, mtime, callback) { callback(enosys()); },
                                    };
                                }

                                if (!globalThis.process) {
                                    globalThis.process = {
                                        getuid() { return -1; },
                                        getgid() { return -1; },
                                        geteuid() { return -1; },
                                        getegid() { return -1; },
                                        getgroups() { throw enosys(); },
                                        pid: -1,
                                        ppid: -1,
                                        umask() { throw enosys(); },
                                        cwd() { throw enosys(); },
                                        chdir() { throw enosys(); },
                                    }
                                }

                                if (!globalThis.crypto) {
                                    throw new Error("globalThis.crypto is not available, polyfill required (crypto.getRandomValues only)");
                                }

                                if (!globalThis.performance) {
                                    throw new Error("globalThis.performance is not available, polyfill required (performance.now only)");
                                }

                                if (!globalThis.TextEncoder) {
                                    throw new Error("globalThis.TextEncoder is not available, polyfill required");
                                }

                                if (!globalThis.TextDecoder) {
                                    throw new Error("globalThis.TextDecoder is not available, polyfill required");
                                }

                                const encoder = new TextEncoder("utf-8");
                                const decoder = new TextDecoder("utf-8");

                                globalThis.Go = class {
                                    constructor() {
                                        this.argv = ["js"];
                                        this.env = {};
                                        this.exit = (code) => {
                                            if (code !== 0) {
                                                console.warn("exit code:", code);
                                            }
                                        };
                                        this._exitPromise = new Promise((resolve) => {
                                            this._resolveExitPromise = resolve;
                                        });
                                        this._pendingEvent = null;
                                        this._scheduledTimeouts = new Map();
                                        this._nextCallbackTimeoutID = 1;

                                        const setInt64 = (addr, v) => {
                                            this.mem.setUint32(addr + 0, v, true);
                                            this.mem.setUint32(addr + 4, Math.floor(v / 4294967296), true);
                                        }

                                        const setInt32 = (addr, v) => {
                                            this.mem.setUint32(addr + 0, v, true);
                                        }

                                        const getInt64 = (addr) => {
                                            const low = this.mem.getUint32(addr + 0, true);
                                            const high = this.mem.getInt32(addr + 4, true);
                                            return low + high * 4294967296;
                                        }

                                        const loadValue = (addr) => {
                                            const f = this.mem.getFloat64(addr, true);
                                            if (f === 0) {
                                                return undefined;
                                            }
                                            if (!isNaN(f)) {
                                                return f;
                                            }

                                            const id = this.mem.getUint32(addr, true);
                                            return this._values[id];
                                        }

                                        const storeValue = (addr, v) => {
                                            const nanHead = 0x7FF80000;

                                            if (typeof v === "number" && v !== 0) {
                                                if (isNaN(v)) {
                                                    this.mem.setUint32(addr + 4, nanHead, true);
                                                    this.mem.setUint32(addr, 0, true);
                                                    return;
                                                }
                                                this.mem.setFloat64(addr, v, true);
                                                return;
                                            }

                                            if (v === undefined) {
                                                this.mem.setFloat64(addr, 0, true);
                                                return;
                                            }

                                            let id = this._ids.get(v);
                                            if (id === undefined) {
                                                id = this._idPool.pop();
                                                if (id === undefined) {
                                                    id = this._values.length;
                                                }
                                                this._values[id] = v;
                                                this._goRefCounts[id] = 0;
                                                this._ids.set(v, id);
                                            }
                                            this._goRefCounts[id]++;
                                            let typeFlag = 0;
                                            switch (typeof v) {
                                                case "object":
                                                    if (v !== null) {
                                                        typeFlag = 1;
                                                    }
                                                    break;
                                                case "string":
                                                    typeFlag = 2;
                                                    break;
                                                case "symbol":
                                                    typeFlag = 3;
                                                    break;
                                                case "function":
                                                    typeFlag = 4;
                                                    break;
                                            }
                                            this.mem.setUint32(addr + 4, nanHead | typeFlag, true);
                                            this.mem.setUint32(addr, id, true);
                                        }

                                        const loadSlice = (addr) => {
                                            const array = getInt64(addr + 0);
                                            const len = getInt64(addr + 8);
                                            return new Uint8Array(this._inst.exports.mem.buffer, array, len);
                                        }

                                        const loadSliceOfValues = (addr) => {
                                            const array = getInt64(addr + 0);
                                            const len = getInt64(addr + 8);
                                            const a = new Array(len);
                                            for (let i = 0; i < len; i++) {
                                                a[i] = loadValue(array + i * 8);
                                            }
                                            return a;
                                        }

                                        const loadString = (addr) => {
                                            const saddr = getInt64(addr + 0);
                                            const len = getInt64(addr + 8);
                                            return decoder.decode(new DataView(this._inst.exports.mem.buffer, saddr, len));
                                        }

                                        const timeOrigin = Date.now() - performance.now();
                                        this.importObject = {
                                            _gotest: {
                                                add: (a, b) => a + b,
                                            },
                                            gojs: {
                                                // Go's SP does not change as long as no Go code is running. Some operations (e.g. calls, getters and setters)
                                                // may synchronously trigger a Go event handler. This makes Go code get executed in the middle of the imported
                                                // function. A goroutine can switch to a new stack if the current stack is too small (see morestack function).
                                                // This changes the SP, thus we have to update the SP used by the imported function.

                                                // func wasmExit(code int32)
                                                "runtime.wasmExit": (sp) => {
                                                    sp >>>= 0;
                                                    const code = this.mem.getInt32(sp + 8, true);
                                                    this.exited = true;
                                                    delete this._inst;
                                                    delete this._values;
                                                    delete this._goRefCounts;
                                                    delete this._ids;
                                                    delete this._idPool;
                                                    this.exit(code);
                                                },

                                                // func wasmWrite(fd uintptr, p unsafe.Pointer, n int32)
                                                "runtime.wasmWrite": (sp) => {
                                                    sp >>>= 0;
                                                    const fd = getInt64(sp + 8);
                                                    const p = getInt64(sp + 16);
                                                    const n = this.mem.getInt32(sp + 24, true);
                                                    fs.writeSync(fd, new Uint8Array(this._inst.exports.mem.buffer, p, n));
                                                },

                                                // func resetMemoryDataView()
                                                "runtime.resetMemoryDataView": (sp) => {
                                                    sp >>>= 0;
                                                    this.mem = new DataView(this._inst.exports.mem.buffer);
                                                },

                                                // func nanotime1() int64
                                                "runtime.nanotime1": (sp) => {
                                                    sp >>>= 0;
                                                    setInt64(sp + 8, (timeOrigin + performance.now()) * 1000000);
                                                },

                                                // func walltime() (sec int64, nsec int32)
                                                "runtime.walltime": (sp) => {
                                                    sp >>>= 0;
                                                    const msec = (new Date).getTime();
                                                    setInt64(sp + 8, msec / 1000);
                                                    this.mem.setInt32(sp + 16, (msec % 1000) * 1000000, true);
                                                },

                                                // func scheduleTimeoutEvent(delay int64) int32
                                                "runtime.scheduleTimeoutEvent": (sp) => {
                                                    sp >>>= 0;
                                                    const id = this._nextCallbackTimeoutID;
                                                    this._nextCallbackTimeoutID++;
                                                    this._scheduledTimeouts.set(id, setTimeout(
                                                        () => {
                                                            this._resume();
                                                            while (this._scheduledTimeouts.has(id)) {
                                                                // for some reason Go failed to register the timeout event, log and try again
                                                                // (temporary workaround for https://github.com/golang/go/issues/28975)
                                                                console.warn("scheduleTimeoutEvent: missed timeout event");
                                                                this._resume();
                                                            }
                                                        },
                                                        getInt64(sp + 8),
                                                    ));
                                                    this.mem.setInt32(sp + 16, id, true);
                                                },

                                                // func clearTimeoutEvent(id int32)
                                                "runtime.clearTimeoutEvent": (sp) => {
                                                    sp >>>= 0;
                                                    const id = this.mem.getInt32(sp + 8, true);
                                                    clearTimeout(this._scheduledTimeouts.get(id));
                                                    this._scheduledTimeouts.delete(id);
                                                },

                                                // func getRandomData(r []byte)
                                                "runtime.getRandomData": (sp) => {
                                                    sp >>>= 0;
                                                    crypto.getRandomValues(loadSlice(sp + 8));
                                                },

                                                // func finalizeRef(v ref)
                                                "syscall/js.finalizeRef": (sp) => {
                                                    sp >>>= 0;
                                                    const id = this.mem.getUint32(sp + 8, true);
                                                    this._goRefCounts[id]--;
                                                    if (this._goRefCounts[id] === 0) {
                                                        const v = this._values[id];
                                                        this._values[id] = null;
                                                        this._ids.delete(v);
                                                        this._idPool.push(id);
                                                    }
                                                },

                                                // func stringVal(value string) ref
                                                "syscall/js.stringVal": (sp) => {
                                                    sp >>>= 0;
                                                    storeValue(sp + 24, loadString(sp + 8));
                                                },

                                                // func valueGet(v ref, p string) ref
                                                "syscall/js.valueGet": (sp) => {
                                                    sp >>>= 0;
                                                    const result = Reflect.get(loadValue(sp + 8), loadString(sp + 16));
                                                    sp = this._inst.exports.getsp() >>> 0; // see comment above
                                                    storeValue(sp + 32, result);
                                                },

                                                // func valueSet(v ref, p string, x ref)
                                                "syscall/js.valueSet": (sp) => {
                                                    sp >>>= 0;
                                                    Reflect.set(loadValue(sp + 8), loadString(sp + 16), loadValue(sp + 32));
                                                },

                                                // func valueDelete(v ref, p string)
                                                "syscall/js.valueDelete": (sp) => {
                                                    sp >>>= 0;
                                                    Reflect.deleteProperty(loadValue(sp + 8), loadString(sp + 16));
                                                },

                                                // func valueIndex(v ref, i int) ref
                                                "syscall/js.valueIndex": (sp) => {
                                                    sp >>>= 0;
                                                    storeValue(sp + 24, Reflect.get(loadValue(sp + 8), getInt64(sp + 16)));
                                                },

                                                // valueSetIndex(v ref, i int, x ref)
                                                "syscall/js.valueSetIndex": (sp) => {
                                                    sp >>>= 0;
                                                    Reflect.set(loadValue(sp + 8), getInt64(sp + 16), loadValue(sp + 24));
                                                },

                                                // func valueCall(v ref, m string, args []ref) (ref, bool)
                                                "syscall/js.valueCall": (sp) => {
                                                    sp >>>= 0;
                                                    try {
                                                        const v = loadValue(sp + 8);
                                                        const m = Reflect.get(v, loadString(sp + 16));
                                                        const args = loadSliceOfValues(sp + 32);
                                                        const result = Reflect.apply(m, v, args);
                                                        sp = this._inst.exports.getsp() >>> 0; // see comment above
                                                        storeValue(sp + 56, result);
                                                        this.mem.setUint8(sp + 64, 1);
                                                    } catch (err) {
                                                        sp = this._inst.exports.getsp() >>> 0; // see comment above
                                                        storeValue(sp + 56, err);
                                                        this.mem.setUint8(sp + 64, 0);
                                                    }
                                                },

                                                // func valueInvoke(v ref, args []ref) (ref, bool)
                                                "syscall/js.valueInvoke": (sp) => {
                                                    sp >>>= 0;
                                                    try {
                                                        const v = loadValue(sp + 8);
                                                        const args = loadSliceOfValues(sp + 16);
                                                        const result = Reflect.apply(v, undefined, args);
                                                        sp = this._inst.exports.getsp() >>> 0; // see comment above
                                                        storeValue(sp + 40, result);
                                                        this.mem.setUint8(sp + 48, 1);
                                                    } catch (err) {
                                                        sp = this._inst.exports.getsp() >>> 0; // see comment above
                                                        storeValue(sp + 40, err);
                                                        this.mem.setUint8(sp + 48, 0);
                                                    }
                                                },

                                                // func valueNew(v ref, args []ref) (ref, bool)
                                                "syscall/js.valueNew": (sp) => {
                                                    sp >>>= 0;
                                                    try {
                                                        const v = loadValue(sp + 8);
                                                        const args = loadSliceOfValues(sp + 16);
                                                        const result = Reflect.construct(v, args);
                                                        sp = this._inst.exports.getsp() >>> 0; // see comment above
                                                        storeValue(sp + 40, result);
                                                        this.mem.setUint8(sp + 48, 1);
                                                    } catch (err) {
                                                        sp = this._inst.exports.getsp() >>> 0; // see comment above
                                                        storeValue(sp + 40, err);
                                                        this.mem.setUint8(sp + 48, 0);
                                                    }
                                                },

                                                // func valueLength(v ref) int
                                                "syscall/js.valueLength": (sp) => {
                                                    sp >>>= 0;
                                                    setInt64(sp + 16, parseInt(loadValue(sp + 8).length));
                                                },

                                                // valuePrepareString(v ref) (ref, int)
                                                "syscall/js.valuePrepareString": (sp) => {
                                                    sp >>>= 0;
                                                    const str = encoder.encode(String(loadValue(sp + 8)));
                                                    storeValue(sp + 16, str);
                                                    setInt64(sp + 24, str.length);
                                                },

                                                // valueLoadString(v ref, b []byte)
                                                "syscall/js.valueLoadString": (sp) => {
                                                    sp >>>= 0;
                                                    const str = loadValue(sp + 8);
                                                    loadSlice(sp + 16).set(str);
                                                },

                                                // func valueInstanceOf(v ref, t ref) bool
                                                "syscall/js.valueInstanceOf": (sp) => {
                                                    sp >>>= 0;
                                                    this.mem.setUint8(sp + 24, (loadValue(sp + 8) instanceof loadValue(sp + 16)) ? 1 : 0);
                                                },

                                                // func copyBytesToGo(dst []byte, src ref) (int, bool)
                                                "syscall/js.copyBytesToGo": (sp) => {
                                                    sp >>>= 0;
                                                    const dst = loadSlice(sp + 8);
                                                    const src = loadValue(sp + 32);
                                                    if (!(src instanceof Uint8Array || src instanceof Uint8ClampedArray)) {
                                                        this.mem.setUint8(sp + 48, 0);
                                                        return;
                                                    }
                                                    const toCopy = src.subarray(0, dst.length);
                                                    dst.set(toCopy);
                                                    setInt64(sp + 40, toCopy.length);
                                                    this.mem.setUint8(sp + 48, 1);
                                                },

                                                // func copyBytesToJS(dst ref, src []byte) (int, bool)
                                                "syscall/js.copyBytesToJS": (sp) => {
                                                    sp >>>= 0;
                                                    const dst = loadValue(sp + 8);
                                                    const src = loadSlice(sp + 16);
                                                    if (!(dst instanceof Uint8Array || dst instanceof Uint8ClampedArray)) {
                                                        this.mem.setUint8(sp + 48, 0);
                                                        return;
                                                    }
                                                    const toCopy = src.subarray(0, dst.length);
                                                    dst.set(toCopy);
                                                    setInt64(sp + 40, toCopy.length);
                                                    this.mem.setUint8(sp + 48, 1);
                                                },

                                                "debug": (value) => {
                                                    console.log(value);
                                                },
                                            }
                                        };
                                    }

                                    async run(instance) {
                                        if (!(instance instanceof WebAssembly.Instance)) {
                                            throw new Error("Go.run: WebAssembly.Instance expected");
                                        }
                                        this._inst = instance;
                                        this.mem = new DataView(this._inst.exports.mem.buffer);
                                        this._values = [ // JS values that Go currently has references to, indexed by reference id
                                            NaN,
                                            0,
                                            null,
                                            true,
                                            false,
                                            globalThis,
                                            this,
                                        ];
                                        this._goRefCounts = new Array(this._values.length).fill(Infinity); // number of references that Go has to a JS value, indexed by reference id
                                        this._ids = new Map([ // mapping from JS values to reference ids
                                            [0, 1],
                                            [null, 2],
                                            [true, 3],
                                            [false, 4],
                                            [globalThis, 5],
                                            [this, 6],
                                        ]);
                                        this._idPool = [];   // unused ids that have been garbage collected
                                        this.exited = false; // whether the Go program has exited

                                        // Pass command line arguments and environment variables to WebAssembly by writing them to the linear memory.
                                        let offset = 4096;

                                        const strPtr = (str) => {
                                            const ptr = offset;
                                            const bytes = encoder.encode(str + "\0");
                                            new Uint8Array(this.mem.buffer, offset, bytes.length).set(bytes);
                                            offset += bytes.length;
                                            if (offset % 8 !== 0) {
                                                offset += 8 - (offset % 8);
                                            }
                                            return ptr;
                                        };

                                        const argc = this.argv.length;

                                        const argvPtrs = [];
                                        this.argv.forEach((arg) => {
                                            argvPtrs.push(strPtr(arg));
                                        });
                                        argvPtrs.push(0);

                                        const keys = Object.keys(this.env).sort();
                                        keys.forEach((key) => {
                                            argvPtrs.push(strPtr(`${key}=${this.env[key]}`));
                                        });
                                        argvPtrs.push(0);

                                        const argv = offset;
                                        argvPtrs.forEach((ptr) => {
                                            this.mem.setUint32(offset, ptr, true);
                                            this.mem.setUint32(offset + 4, 0, true);
                                            offset += 8;
                                        });

                                        // The linker guarantees global data starts from at least wasmMinDataAddr.
                                        // Keep in sync with cmd/link/internal/ld/data.go:wasmMinDataAddr.
                                        const wasmMinDataAddr = 4096 + 8192;
                                        if (offset >= wasmMinDataAddr) {
                                            throw new Error("total length of command line and environment variables exceeds limit");
                                        }

                                        this._inst.exports.run(argc, argv);
                                        if (this.exited) {
                                            this._resolveExitPromise();
                                        }
                                        await this._exitPromise;
                                    }

                                    _resume() {
                                        if (this.exited) {
                                            throw new Error("Go program has already exited");
                                        }
                                        this._inst.exports.resume();
                                        if (this.exited) {
                                            this._resolveExitPromise();
                                        }
                                    }

                                    _makeFuncWrapper(id) {
                                        const go = this;
                                        return function () {
                                            const event = { id: id, this: this, args: arguments };
                                            go._pendingEvent = event;
                                            go._resume();
                                            return event.result;
                                        };
                                    }
                                }
                            })();
                            " does not contain "global.Go"
            Test:           TestSimpleHandlerDev
FAIL
FAIL    github.com/vugu/vugu/simplehttp 0.954s
FAIL

And a failure in vugufmt

=== RUN   TestGoFmtNoError
    gofmt_test.go:36: 
            Error Trace:    gofmt_test.go:36
                                        gofmt_test.go:40
                                        path.go:477
                                        path.go:501
                                        path.go:572
                                        gofmt_test.go:39
            Error:          Not equal: 
                            expected: "//Package vugufmt provides gofmt-like functionality for vugu files.\npackage vugufmt\n"
                            actual  : "// Package vugufmt provides gofmt-like functionality for vugu files.\npackage vugufmt\n"

                            Diff:
                            --- Expected
                            +++ Actual
                            @@ -1,2 +1,2 @@
                            -//Package vugufmt provides gofmt-like functionality for vugu files.
                            +// Package vugufmt provides gofmt-like functionality for vugu files.
                             package vugufmt
            Test:           TestGoFmtNoError
            Messages:       doc.go

But this one is trivial to fix.

Are you also seeing these?

We want to keep all of the tests building at each PR stage.

Owen

owenwaller commented 6 months ago

Sorry I forgot to say....

In general the PR itself looks to be fine. :smile:

We will need to have a bit of a debate about returning errors vs panic'ing - in general I think it's ok if a function in a tool package panics, but not in package used by a framework or server, which seems to be the case here.

Lord-Y commented 6 months ago

@owenwaller Instead of the bulk PR that I previously did, this is a step by step PR. So you need to use go1.17 and tinygo 0.22. All tests will then be successful.

Lord-Y commented 6 months ago

So @owenwaller you need to do:

go install golang.org/dl/go1.17@latest
go1.17 download
go1.17 test XXX
owenwaller commented 6 months ago

Hi @Lord-Y

Ah ha, yes downgrading to go1.17 from g1.21.4 does indeed make the tests pass (also with tinygo v0.22).

This is a question for @bradleypeabody

Should we:

  1. Accept this PR as is and merge it in. or
  2. Extend this PR so that it also includes any changes we need to bump the maximal go version needed to build vugu from v1.17.0 to v.1.20.X (as a minimum) (ideally v1.21.X). Vugu isn't currently building with go v1.21.4 or
  3. Create an 2nd PR that just contains the changes needed to bump the maximal version of go.

Personally I prefer option (3), we can then combine the PRs locally and push the merged version back as a new release.

@Lord-Y @bradleypeabody is on thanksgiving so he might be slow to respond. But have you any idea how much work it would be to do option (3)? Personally, I think the quicker we get vugu building with a current version of Go the better, as it is unreasonable to ask folks to downgrade their go tool chain to do this.

Owen

Lord-Y commented 6 months ago

@owenwaller as said in https://github.com/vugu/vugu/issues/246#issuecomment-1817900990 , my plan was to have PRs 252 to 256 merged. And only then make a PR for the upgrade to golang 1.21.x. I almost finished the requirements last sunday but didn't have the time to continue this week. So I'll continue this week-end.

Lord-Y commented 6 months ago

@owenwaller So all my tests are passing with go 1.21.4. I'll wait for the PRs 252 to 256 merged before creating the new one.

owenwaller commented 6 months ago

Hi @Lord-Y

OK thanks. So if we can get PR #252 and #256 merged in first, can PR's #253, #254 and #255 can all be merged after the new PR that updates go to v1.21.4?

@bradleypeabody I think both of these PR's 252 and 256 are fine. If you agree can you merge these in, so they don't block the new PR that will update go to v1.21.4 (which we can't build vugu with ATM). We can then merge in the other changes.

@Lord-Y if you have the new PR that does the go upgrade finished, can you open a new PR for that so we can review it?

bradleypeabody commented 6 months ago

Okay, I'm going to go ahead and merge these in just to get the flow going. There are some test failures, please have a look at those and submit another PR to fix.

owenwaller commented 6 months ago

@bradleypeabody @Lord-Y

Given that this change is merged, I am going to close this PR.

Can you please be specific in the new PR what tests have failed?

Thanks