Skip to content
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

Native Windows urbit binaries using MingW #4675

Merged
merged 77 commits into from Sep 9, 2021

Conversation

locpyl-tidnyd
Copy link
Contributor

@locpyl-tidnyd locpyl-tidnyd commented Mar 27, 2021

This PR enables C urbit binaries to build and run natively on Windows using MingW.

Remaining work:

  • GitHub action to build the binaries
  • graceful exit (|exit etc) does not work
  • binaries are not completely standalone
  • CLI terminal is not available
  • daemon mode is disabled

To try it out, install the MSYS2 environment, check out the source or download a tarball, open a MingW-64 shell (not a MSYS shell, they are not interchangable), change to the pkg/urbit directory, and run first ./configure to prepare the build environment and then make build/urbit build/urbit-worker to build. If you don't have the binary pill file checked out, ./configure will detect that and download it. The resulting binaries are standalone and run as-is (without copying dlls) both from the MingW shell and from Windows.

./configure can optionally use Cachix to store and fetch pre-built dependencies in a manner similar to Nix. To enable fetching pre-built dependencies, set the CACHIX_CACHE environment variable to the name of the cache you want to use. The default cache used by Urbit is ares, but it is not write-accessible from external PRs. This PR uses uses the cache locpyl-tidnyd-test1. To store the dependencies you build, put your access token into the CACHIX_AUTH_TOKEN environment variable.

export CACHIX_CACHE=cache-name
export CACHIX_AUTH_TOKEN=eyJh... # if you have write access
./configure

The C runtime MingW relies on, msvcrt.dll, has the notion of accessing files in text and binary mode.
In text mode, it does CRLF conversion and handles some control characters, which breaks binary files.
This change adds O_BINARY in every fopen/open call and #defines it to 0 on other platforms.
Most C files have multiple #includes that are duplicated in include/c/portable.h.
Removing them helps keep include-related #ifdefs in include/c/portable.h.
#including library files after all.h avoids clashes on MingW and allows portable.h
to add compatibility #defines where necessary.
Vere writes the embedded CA store to a tempfile and directs libcurl and libh2o to pick it up from there,
but on MingW this is inconvenient because temp paths are different and mkstemp(3) is not available.
Loading CA store certificates directly from memory is tidier.
In some places, _ce_patch_delete is invoked before _ce_patch_free. This works on Unix,
where an open file can be unlinked, but on Windows one must open files with an extra
flag FILE_SHARE_DELETE for this to work, and there is no way to pass this flag to open().
gcc 9 and earlier default to -fcommon, i.e. linker merges identical symbols. gcc 10 defaults
to -fno-common, and the build breaks because c3_global is not defined anywhere.
@Fang-
Copy link
Member

Fang- commented Mar 28, 2021

albeit for now without CLI terminal functionality

Is this running up against Window's lack of ansi terminal emulation and/or general compatibility in that area? Curious to hear what you think it would take to support this.

In any case: very cool stuff, thank you!

To avoid allocating a buffer on MingW, mdb_strerror uses a horrible
stack hack which explodes in vere. Source patch replaces it with mdb_logerror
on MingW. This change adds the default implementation for other platforms.
@ohAitch
Copy link
Contributor

ohAitch commented Mar 28, 2021

I think in that case you could run it in PuTTY? (Or VSCode or for that matter Windows Terminal)

@locpyl-tidnyd
Copy link
Contributor Author

In more recent Windows 10, the default console actually supports VT escape characters as an opt-in, but the good old Win32 console functions should be enough to implement the terminal. In fact, I suspect that one could even get by with just \r and \b and turning off echo. The real answer is that I did not consider the terminal a high priority item. Everyone, myself included, uses herb/http via lens port to talk to ships running in the background anyway. Also, the code in term.c is rather hairy and I've never used termios/termcaps before, so I concentrated on getting ships off the ground first.

This commit adds code changes, compatibility functions, stubs and a build script
to build urbit binaries on MingW64. Some functionality is limited or missing:
terminal input and daemon mode is not available, graceful exit does not work,
and the binaries are not completely static and use (portable) MingW dlls.

To build the binaries, install the MSYS2 environment, check out or copy the urbit
repo and pill binaries, open a MingW64 shell and `cd pkg/urbit && ./build-mingw`.
@Fang-
Copy link
Member

Fang- commented Mar 29, 2021

the code in term.c is rather hairy

#4463 intends to make it a bit less hairy (and also refactors the terminal protocol a little bit). You may or may not want to develop/test against that if you end up putting in a lot of term.c work. Also happy to hop on a call or email thread or w/e and walk you through the codebase, if that's useful to you!

If you want to fully support all functionality, you'll need to get fancier than just \r \b. You can imagine getting smart about down-converting output that's too complex for Win32 consoles to properly render, but at that point you have to ask yourself what you want the terminal for in the first place.

Copy link
Member

@joemfb joemfb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My apologies for the long delay. This is excellent work.

I've tested this branch pretty extensively now, and both the build scripts and resulting binary are working quite well.

I've reviewed the changes above, requesting clarification around some of the more complicated pieces, suggested some small tweaks, and noted potential future improvements. Some further notes:

build:

  • the "poor man's nix shell" extends *-sources.json with a mingw object to configure the build. this probably precludes future use of nim to manage those sources (i've been updating them manually anyway, /cc @brendanhay)

patches:

  • libh2o is patched to workaround windows opposite iovec member order with a new STRLIT macro, and to disable functionality around AF_UNIX sockets
  • lmdb is patched to fix it's error-logging string allocation

known issues:

libsigsegv replacement:

  • this PR includes a custom SEH handler to replace our use of libsigsegv, resulting in massive performance improvements

before:

> :time =|(i=@ud |-(?:(=(i 0xff.ffff) ~ $(i +(i)))))
>=
[%took ~s5..c229]
> :time =|(i=@ud |-(?:(=(i 0xff.ffff) ~ $(i +(i)))))
>=
[%took ~s2..d5c8]

> :time (gulf 1 0xf.ffff)
>=
[%took ~s5..0cac]
> :time (gulf 1 0xf.ffff)
>=
[%took ~s0..8a08]

> :time (jam (gulf 1 0xf.ffff))
>=
[%took ~s29..9843]
> :time (jam (gulf 1 0xf.ffff))
>=
[%took ~s2..d245]

> :time (cue (jam (gulf 1 0xf.ffff)))
>=
[%took ~s38..d209]
> :time (cue (jam (gulf 1 0xf.ffff)))
>=
[%took ~s5..7cdf]

after:

> :time =|(i=@ud |-(?:(=(i 0xff.ffff) ~ $(i +(i)))))
>=
[%took ~s3..0489]
> :time =|(i=@ud |-(?:(=(i 0xff.ffff) ~ $(i +(i)))))
>=
[%took ~s2..fb4a]

> :time (gulf 1 0xf.ffff)
>=
[%took ~s0..af85]
> :time (gulf 1 0xf.ffff)
>=
[%took ~s0..9e9e]

> :time (jam (gulf 1 0xf.ffff))
>=
[%took ~s2..fbbc]
> :time (jam (gulf 1 0xf.ffff))
>=
[%took ~s2..d823]

> :time (cue (jam (gulf 1 0xf.ffff)))
>=
[%took ~s5..c382]
> :time (cue (jam (gulf 1 0xf.ffff)))
>=
[%took ~s5..a927]

(each expression was timed twice in a new process, the first time includes the cost of page-faults. note also that :time is a low-resolution benchmark of wall-clock time ...)


Barring the minor improvements requested above, I think this PR is ready to be merged. The biggest obstacle to integration will be the coming terminal rewrite (#4463). I tried a merge locally, and the conflicts look quite manageable. But it's possible that there will be issues with new functionality (2-dimensional output / screen updates, log/slog handling). /cc @Fang-

.github/workflows/build.yml Show resolved Hide resolved
pkg/ent/configure Show resolved Hide resolved
pkg/urbit/Makefile Show resolved Hide resolved
pkg/urbit/compat/mingw/argon2u.patch Outdated Show resolved Hide resolved
pkg/urbit/compat/mingw/poor-mans-nix-shell.sh Outdated Show resolved Hide resolved
pkg/urbit/daemon/main.c Show resolved Hide resolved
@@ -780,7 +722,7 @@ _term_io_suck_char(u3_utty* uty_u, c3_y cay_y)
else if ( 8 == cay_y || 127 == cay_y ) {
_term_io_belt(uty_u, u3nc(c3__bac, u3_nul));
}
else if ( 13 == cay_y ) {
else if ( 13 == cay_y || 10 == cay_y ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we've discussed elsewhere, it's really strange that the terminal worked before this change. I suspect it's an artifact of "raw mode", and it may not be worth investigating further ...

/cc @Fang-

#if defined(U3_OS_mingw)
sprintf(cev_c, "%u", u3_Host.cev_u);
arg_c[7] = cev_c;
arg_c[8] = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A note to my future self: having a platform-dependent number of positional arguments like this is a little unfortunate, it'd be much better to use descriptive args with getopt_long().

nix/sources-mingw.json Outdated Show resolved Hide resolved
pkg/urbit/compat/mingw/compat.h Outdated Show resolved Hide resolved
@locpyl-tidnyd
Copy link
Contributor Author

locpyl-tidnyd commented Jun 4, 2021

Ctrl-Break is the "ungraceful exit" key in Windows console. Windows uses ^Z as the end-of-input character where Unix uses ^D.

@locpyl-tidnyd locpyl-tidnyd requested a review from joemfb June 4, 2021 14:43
Copy link
Member

@joemfb joemfb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor Author

@locpyl-tidnyd locpyl-tidnyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit message fail: should have been "put downloaded dependencies under $NIX_STORE (defaults to ...)"

move SEH handler to compat/mingw/seh_handler.c
(default to pkg/build so that git ignores them)
@joemfb joemfb mentioned this pull request Aug 12, 2021
6 tasks
@joemfb joemfb changed the base branch from master to release/next-vere September 2, 2021 16:37
@joemfb joemfb merged commit 07e2a6b into urbit:release/next-vere Sep 9, 2021
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.

None yet

4 participants