Skip to content

Standardize on a single GOOS/GOARCH and deprecate node-syscall module. #1111

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 23 commits into from
Apr 20, 2022

Conversation

nevkontakte
Copy link
Member

@nevkontakte nevkontakte commented Mar 27, 2022

This is a large change, which changes the approach GopherJS takes to building standard library. The core objective is to use a single, fixed GOOS/GOARCH pair when building standard library, regardless of the host OS. The main benefit of this change is reducing a number of environments we need to support, thus reducing the effort required for new Go version adoption and diagnosing issues for users on operating systems other than Linux. As a side benefit, we can reuse a lot more code standard library has for wasm, and we no longer need the node-syscall extension for basic stuff like file system access.

  • Standard library code is now always built with GOOS=js and GOARCH=wasm, regardless of GOOS/GOARCH set by the user.
  • User code (i.e. everything outside of the standard library) is built by default with GOOS=js and GOARCH=ecmascript. This makes GOOS=js equal between GopherJS and Go WebAssembly, while GOARCH allows to distinguish the two. In future we may use different values of GOARCH like ecmascript6 if we decide to output code using the newer syntax.
    • If GOOS and/or GOARCH environment variables are explicitly set and not empty, GopherJS will build user code with them. This is a compatibility option with the previous behavior when GopherJS used GOOS from the host system. I hope to remove it after a couple of major releases.
    • In addition to that, gopherjs build tag is always set, allowing to target GopherJS regardless of the GOOS/GOARCH.
  • Instead of using node-syscall module, we now use the same code paths as Go WebAssembly for interaction with the os. In particular, we emulate file system syscalls using Node.js fs API if it is available. I expect that for most users node-syscall module will now become unnecessary.
    • It is possible to re-enable node-syscall code paths by building the code with legacy_syscall build tag, which provides old implementations for Syscall, Syscall6, RawSyscall and RawSyscall6 functions in the syscall package. However, the standard library will still be built with GOOS=js GOARCH=wasm, so this option is mostly left for the users who used the syscall package directly for operations that can't be emulated using Node.js APIs. I hope we will be able to remove this completely in future.
  • I fixed several bugs that were preventing GopherJS compiler from working natively on Windows. To prevent regressions I set up minimal CI workflows for Windows and Mac OS.
  • Output size for our reference TodoMVC app reduced by 2.17% minified, mostly thanks to throwing away unnecessary platform-specific code.
  • For users who only run GopherJS in a browser environment, nothing significant should have changed.

In the long term this change unlocks various improvements to the GopherJS build system, including adoption of golang.org/x/tools/go/packages instead of go/build. Additional notes and context on this change can be found here.

Fixes #693.

nevkontakte added a commit to nevkontakte/gopherjs that referenced this pull request Apr 19, 2022
See gopherjs#1111.

Co-authored-by: Jonathan Hall <flimzy@flimzy.com>
nevkontakte and others added 23 commits April 19, 2022 11:29
All relevant tests are passing and a stub for FS functions is provided
when running in an environment without Node `fs` module or compatible.
  - Fix skip condition in tests that should be run under normal Go.
  - TestNativesDontImportExtraPackages loads original packages with
    `js/wasm` build target.
  - Skip tests that rely on GC finalizers.
  - Skip tests that rely on exact function names in stack traces.
  - Tweak goroutine leak detection to work with GopherJS stack traces.
  - Use fake networking transport for http client tests under nodejs.
…sts.

The removed packages either actually pass all tests, or skip
incompatible tests for `js/wasm` by default, which generally suits us
well.
Use the wasm implementation from the standard library instead. It has
more features and supports browsers with streaming unavailable. We still
keep `XMLHttpRequest` transport in case GopherJS users rely on it.
Most GopherJS users target browser and likely don't want to pay the cost
of file system polyfill they can't use in a browser anyway, so
minimizing it is worth the effort.

In addition, I converted the polyfill into  ES5 syntax, since we haven't
formally abandoned it yet.
GopherJS will use this target going forward. The values where chosen by
the following logic:

  - GOOS describes the targeted API environment, which is generally the
    same as when building wasm: both execute under a browser or nodejs.
    Build tag `js` is shared by GopherJS and wasm and can be used to
    target the general environment.
  - GOARCH describes the low-level emitted code, which is currently
    EcmaScript 5. Using `ecmascript` here differentiates from `wasm`,
    while not referencing GopherJS explicitly. It also leaves us a
    different paths for future upgrades to ES6+ by either changing
    GOARCH or keeping it and making this change implicit.

At the same time, we use fixed js/wasm target when building standard
library packages, which resolves a lot of concerns discussed in
gopherjs#693. Using js/ecmascript is
not feasible because it'll require us to maintain a lot more overlays
and/or rewrite build tags, which leads to the exactly same outcome.

In addition, we always set `gopherjs` build tag to make it easy to
guard sources using GopherJS-specific features. This is similar to `gc`
and `gccgo` build tags respective compilers set.

This change is potentially breaking for users that may have relied on
GOOS=linux in their sources. To make transition easier for them, we
support using GOOS/GOARCH from environment. However, they will be only
applied to the user code. Standard library will still be built with
js/wasm.
GopherJS doesn't support CGo. This was originally enabled to address
gopherjs#215 by providing a better
error when attempting to build a CGo package (instead of a compiler
panic). However, this approach makes GopherJS refuse to build packages,
which have a CGo and purego versions guarded by the `cgo` build tag:
GopherJS woudl try to load the cgo version and error out.

After this change GopherJS won't attempt to load Go sources with `import
"C"`, and it will load sources with `//go:build !cgo` instead. For
packages that require CGo one of the following errors will be returned:

  - "No buildable sources" if all Go sources import C.
  - "Undefined symbol" for symbols defined in the Go sources that import
    C and that are referenced by other Go source files.
  - "Unknown import path C" if you try to `gopherjs run` a source with
    import C directly.
To make transition somewhat less dramatic, we can provide the old
implementations for `syscall.Syscall` and its friends in case someone
actually wanted to make arbitrary syscalls from GopherJS running under
Node. I can't really think of any valid reason to, but who knows?

To enable this, a `legacy_syscall` build tag must be passed to GopherJS.
This shim is mostly intended for non-standard library code that was
using syscall package directly and it is more restrictive than
previously (e.g. the set of `syscall` functions still remains that of
`js/wasm`), but this seems like a reasonable compromise between
complexity and flexibility.

I am fairly confident that our support for file system access and a few
other syscalls, emulated via Node APIs is more than sufficient for all
our users, so after a release or two we will be able to delete this code
path entirely.
We could remove our overlay entirely, but our implementation seems to be
more advanced than the one of `js/wasm` in the standard library, so we
keep it for now.
We only build standard library for `js/wasm` now, which works for us
as-is, except for a couple of `go:linkname` directives we need to keep.
After we switched to building standard library for the `js/wasm` target,
some of the overlays we had to maintain for other GOOS values are no
longer necessary:

  - `crypto/x509`
  - `debug/elf`
  - `internal/syscall/linux`
  - `internal/testenv`
  - `net`
  - "noat" implementation of `os.RemoveAll`.
  - Reduce code duplication.
  - Ensure that it uses the same loading context as the compiler.
  - Ensure it uses the same test variants of packages as the compiler.
They were used as a side channel to extract GOROOT and GOPATH from
`build.Session`, which is an incorrect use of options. Eventually all
logic that requires these paths should be moved to the `build` package,
but for now we just provide an additional method to create a source
mapping callback.
Embedded file systems use unix-style slashes, which was causing errors
on Windows, when two styles were mixed. Converting all VFS paths to
unix-style slashes resolves the issue.

Fixes gopherjs#1049.
There are environments like AMD, which expose `require()` function, but
do not provide `fs` module. In such cases we should fall back to our
stub (see gopherjs#693 (comment)).
Ideally we'd do the complete set of tests, but there is a fair amount of
edge cases fixing which would require a significant effort. For now we
simply ensure that GopherJS can build, run tests and passes its own
tests.
See gopherjs#1111.

Co-authored-by: Jonathan Hall <flimzy@flimzy.com>
@flimzy flimzy mentioned this pull request Apr 19, 2022
@nevkontakte nevkontakte merged commit 2c06401 into gopherjs:master Apr 20, 2022
@nevkontakte nevkontakte deleted the syscall2 branch April 20, 2022 20:00
dmitshur added a commit that referenced this pull request Jan 3, 2023
GopherJS 1.18 has standardized on a single value for GOOS and GOARCH
in PR #1111. Update the README accordingly. (There has been agreement
in issue #1169 that it's okay to be updating the top-level README for
GopherJS 1.18-specific changes by now.)

Also reword advice in compatibility.md to be more general. The upstream
Go WebAssembly port (GOOS=js GOARCH=wasm) is also a 32-bit environment,
and and the 64-bit variant (i.e., GOARCH=wasm64) doesn't exist yet, so
it doesn't make for a good example. But a reminder that some types vary
in size by architecture is probably worth including anyway.
dmitshur added a commit that referenced this pull request Jan 3, 2023
GopherJS 1.18 has standardized on a single value for GOOS and GOARCH
in PR #1111. Update the README accordingly. (There has been agreement
in issue #1169 that it's okay to be updating the top-level README for
GopherJS 1.18-specific changes by now.)

Also reword advice in compatibility.md to be more general. The upstream
Go WebAssembly port (GOOS=js GOARCH=wasm) is also a 32-bit environment,
and the 64-bit variant (i.e., GOARCH=wasm64) doesn't exist yet, so it's
not a good example. But a reminder that some types vary in size based on
the architecture is probably worth including anyway.
dmitshur added a commit that referenced this pull request Jan 3, 2023
GopherJS 1.18 has standardized on a single value for GOOS and GOARCH
in PR #1111. Update the README accordingly. (There has been agreement
in issue #1169 that it's okay to be updating the top-level README for
GopherJS 1.18-specific changes by now.)

Also reword advice in compatibility.md to be more general. The upstream
Go WebAssembly port (i.e., GOOS=js GOARCH=wasm, and notably not wasm64)
is limited to 4 GiB of linear memory, it just happens to at this time
use 64-bit variables for int/uint/uintptr types. So maybe it's not a
great example to refer to. But a reminder that some types vary in size
based on the architecture or implementation is probably worth including
anyway.
dmitshur added a commit that referenced this pull request Jan 3, 2023
GopherJS 1.18 has standardized on a single value for GOOS and GOARCH
in PR #1111. Update the README accordingly. (There has been agreement
in issue #1169 that it's okay to be updating the top-level README for
GopherJS 1.18-specific changes by now.)

Also reword advice in compatibility.md to be more general. The upstream
Go WebAssembly port (i.e., GOOS=js GOARCH=wasm, and notably not wasm64)
is limited to 4 GiB of linear memory, it just happens to at this time
use 64-bit variables for int/uint/uintptr types. So maybe it's not a
great example to refer to. But a reminder that some types vary in size
based on the architecture or implementation is probably worth including
anyway.
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.

proposal: Limit support to one GOOS value (primarily for stdlib).
2 participants