Add native support for Windows #452

mrbbot · 2023-03-16T17:24:56Z

Hey! 👋 This PR adds native support for Windows to workerd. The primary aim here is to get Miniflare 3 and wrangler dev --experimental-local running on Windows natively, for local development and testing. This is one of the last few things we need to do before graduating --experimental-local to --local.

This PR is not intended to allow workerd to run in production on Windows servers. With that in mind, a few features haven’t been implemented, notably configuration watching and creating single executable binaries. We may consider adding these later.

Windows setup instructions have been added to the README.

We use LLVM's MSVC-compatible compiler driver clang-cl to compile on Windows, as opposed to using MSVC directly. This enables us to use the "same" compiler frontend on Linux, macOS, and Windows, massively reducing the effort required to compile workerd on Windows. Notably, this provides proper support for #pragma once when using symlinked virtual includes, __atomic_* functions, a standards-compliant preprocessor, support for GNU statement expressions used by some KJ macros, and understands the .c++ extension by default.

TODO:

Make sure GitHub actions can build/test workerd on Windows
Internal PR to make sure we haven't broken anything
Switch back to http_archive for capnp-cpp in WORKSPACE once Fix Bazel builds on Windows capnproto/capnproto#1640 merged
Separate PR to enable Windows binaries to be published to npm (Publish native Windows builds to npm #502)

WORKSPACE

patches/v8/0009-Allow-Windows-builds-under-Bazel.patch

mrbbot · 2023-03-16T17:30:59Z

src/workerd/api/kv.c++

+  auto request = client->request(kj::HttpMethod::GET, urlStr, headers);
  return context.awaitIo(js,
-      client->request(kj::HttpMethod::GET, urlStr, headers).response,
+      kj::mv(request.response),


Without this change, and similar changes for KV list and WebSocket fetch, we were seeing the following error:

workerd/jsg/util.c++:423: error: e = kj/_virtual_includes/kj\kj/memory.h:258: failed: expected ptr != nullptr; null Own<> dereference stack: 7ff74084ca28 7ff74084b33a 7ff7406c84ad 7ff74156873a 7ff74156a07d 7ff741567ec2 7ff741567ddb 7ff740b06b46 7ff740b066c6 7ff740b0626a 7ff740b061e4 7ff740b061a0 7ff73ce30d82 7ff73ce2e936 7ff73ce2b727 7ff73ce2ad0b 7ff741dffafb 7ff741d77feb 7ff741d77feb 7ff741d7675b 7ff741d7635a 7ff73d208d2b 7ff73d203ff0 7ff73d20242f 7ff73ccfba9f 7ff7409dbb80 7ff7409db9a2 7ff741492de8 7ff74148b5b6 7ff7415319c2 7ff740f9a3ce 7ff740f9a216; sentryErrorContext = jsgInternalError workerd/io/worker.c++:1686: info: uncaught exception; source = Uncaught (in promise); exception = Error: internal error workerd/io/io-context.c++:381: info: uncaught exception; exception = workerd/jsg/_virtual_includes/jsg\workerd/jsg/value.h:1347: failed: jsg.Error: internal error stack: 0 7ff7408529b6 7ff74153ee1d 7ff74153ed55 7ff74153e996 7ff7408156bf 7ff74080b48f 7ff74080b39b 7ff74080b63c 7ff740f9b670 7ff7408156bf 7ff74080b48f 7ff74080b39b 7ff74080d746 7ff74080d604 7ff74080b63c 7ff740f9c370 7ff7408156bf 7ff74080b48f 7ff74080b39b 7ff74080c8be 7ff740807b1d 7ff740814c42 7ff7408091ca 7ff740808ddf 7ff73ca1b750 7ff73ca259b1 7ff73ca2561c 7ff73ca255f7 7ff73ca255cc 7ff73ca2554d 7ff73ca25513 workerd/server/server.c++:2244: error: Uncaught exception: workerd/jsg/_virtual_includes/jsg\workerd/jsg/value.h:1347: failed: remote.jsg.Error: internal error stack: 0 7ff7408529b6 7ff74153ee1d 7ff74153ed55 7ff74153e996 7ff7408156bf 7ff74080b48f 7ff74080b39b 7ff74080b63c 7ff740f9b670 7ff7408156bf 7ff74080b48f 7ff74080b39b 7ff74080d746 7ff74080d604 7ff74080b63c 7ff740f9c370 7ff7408156bf 7ff74080b48f 7ff74080b39b 7ff74080c8be 7ff740807b1d 7ff740814c42 7ff7408091ca 7ff740808ddf 7ff73ca1b750 7ff73ca259b1 7ff73ca2561c 7ff73ca255f7 7ff73ca255cc 7ff73ca2554d 7ff73ca25513

I'm not too sure what's going on here.

This is probably because client is kj::mved as part of the same statement (see next line of code). It is unspecified in what order these happen, and compilers can differ. It seems when compiling for Windows, Clang is doing the kj::mv before it attempts to dereference client, and so by the time it dereferences, it is moved away.

I'm surprised that Clang's behavior differs between Linux and Windows, though. I would normally expect a particular compiler to be consistent across platforms. Maybe they felt they needed to match MSVC on Windows? I wonder if there's a flag we can use to control this. It's going to be pretty annoying for those of us developing on Linux to have to deal with these problems, which won't be noticed until CI time...

Hmmm, looking at the flags, I can't see anything that would control this. I'm guessing it would be a /Zc: option, but there's only:

/Zc:alignedNew- Disable C++17 aligned allocation functions /Zc:alignedNew Enable C++17 aligned allocation functions /Zc:char8_t- Disable char8_t from c++2a /Zc:char8_t Enable char8_t from C++2a /Zc:dllexportInlines- Do not dllexport/dllimport inline member functions of dllexport/import classes /Zc:dllexportInlines dllexport/dllimport inline member functions of dllexport/import classes (default) /Zc:sizedDealloc- Disable C++14 sized global deallocation functions /Zc:sizedDealloc Enable C++14 sized global deallocation functions /Zc:strictStrings Treat string literals as const /Zc:threadSafeInit- Disable thread-safe initialization of static variables /Zc:threadSafeInit Enable thread-safe initialization of static variables /Zc:trigraphs- Disable trigraphs (default) /Zc:trigraphs Enable trigraphs /Zc:twoPhase- Disable two-phase name lookup in templates (default) /Zc:twoPhase Enable two-phase name lookup in templates /Zc:wchar_t- Disable C++ builtin type wchar_t /Zc:wchar_t Enable C++ builtin type wchar_t (default)

There's also these, but they aren't enabled by default:

-fms-compatibility-version=<value> Dot-separated value representing the Microsoft compiler version number to report in _MSC_VER (0 = don't define it (default)) -fms-compatibility Enable full Microsoft Visual C++ compatibility -fms-extensions Accept some non-standard constructs supported by the Microsoft compiler

src/workerd/io/BUILD.bazel

src/workerd/jsg/jsg.c++

src/workerd/server/server-test.c++

src/workerd/tools/BUILD.bazel

src/workerd/util/sqlite.c++

README.md

src/workerd/api/streams/common.h

src/workerd/io/compatibility-date.c++

src/workerd/jsg/setup.c++

src/workerd/server/server.c++

src/workerd/server/workerd.c++

src/workerd/util/sqlite.c++

.bazelrc

mrbbot · 2023-03-17T22:00:00Z

190a151 should've fixed the flaky blob test. See the commit description for rationale.

.bazelversion

src/workerd/api/crypto.c++

kentonv · 2023-03-28T00:43:56Z

src/workerd/io/BUILD.bazel

-        exclude = [
-            "trace.h",
-        ],
+        # `trace.h` should really be excluded here, but doing so causes the build to fail on


This is weird, is it because there's a relative (quoted) include that needs to be converted to an absolute (angle-bracketed) include somewhere?

FWIW, I tried replacing all the relative includes in src/workerd/io with absolutes (and vice-versa), and neither seemed to help. 😕

Looks kinda similar to bazelbuild/bazel#6337, but that should've been fixed...

src/workerd/io/compatibility-date.c++

src/workerd/server/workerd.c++

src/workerd/util/sqlite.c++

mrbbot · 2023-03-29T14:49:47Z

Last force-push is a rebase. Should be ready for another review assuming CI passes. Will squash all these fixups before merging. 👍

mrbbot · 2023-03-30T10:33:50Z

Rebasing again 👍

.bazelversion

kentonv

Setting this to "request changes" because fixups need to be squashed, otherwise it looks good to merge.

mrbbot · 2023-03-31T21:25:37Z

Awesome, squashed 😀

We don't need a separate PR to update bazel.

mrbbot · 2023-04-03T08:13:03Z

Rebased before merging 👍

As of cloudflare/workerd#452, `workerd` supports Windows' natively. This change upgrades `workerd`, then removes the WSL and Docker runtime shells.

As of cloudflare/workerd#452, `workerd` supports Windows' natively. This change upgrades `workerd`, then removes the WSL and Docker runtime shells. It also `Cherry`picks #509, to allow Miniflare development on Windows too.

mrbbot force-pushed the bcoll/windows-clang branch from 34e9abf to f5ba4ff Compare March 16, 2023 17:43

mrbbot commented Mar 16, 2023

View reviewed changes

mrbbot mentioned this pull request Mar 16, 2023

Fix Bazel builds on Windows capnproto/capnproto#1640

Merged

mrbbot requested a review from mikea March 16, 2023 17:44

mrbbot marked this pull request as ready for review March 16, 2023 17:44

mrbbot commented Mar 16, 2023

View reviewed changes

README.md Show resolved Hide resolved