feat(xsnap): Add xsnap #2168
feat(xsnap): Add xsnap #2168
Conversation
packages/xsnap/src/xsnap.c
Outdated
| * @param dest: where to store address of newly allocated buffer, | ||
| * which is null-terminated for compatibility with C strings | ||
| * @returns count of bytes read or -1 for error; | ||
| * caller takes ownership of *dest unless an error occurs |
There was a problem hiding this comment.
I need to update this. I changed the protocol to something more C idiomatic.
There was a problem hiding this comment.
fine. I wonder why the "resolve conversation" button isn't showing up for me.
There was a problem hiding this comment.
Let's punt on the function comment.
The norm in XS code is no comments. This is static (not exported) and the implementation is entirely straightforward. And we don't comment fxWriteNetString
| char command = *nsbuf; | ||
| // fprintf(stderr, "command: len %d %c arg: %s\n", nslen, command, nsbuf + 1); | ||
| switch(command) { | ||
| case '?': |
There was a problem hiding this comment.
For symmetry with outbound syscalls, I changed the 'd' code to '?'.
There was a problem hiding this comment.
I don't really see the symmetry. I wonder about > for "send". But I don't feel strongly.
|
I don't see any automated tests; it seems odd to approve a feature without automated tests. I suppose it doesn't matter much since a follow-on PR will have tests, but still... I'd like to hear more about norms in this area before I approve. |
| -fno-common \ | ||
| -DINCLUDE_XSPLATFORM \ | ||
| -DXSPLATFORM=\"xsnap.h\" \ | ||
| -DmxDebug=1 \ |
There was a problem hiding this comment.
if mxDebug is the flag that either makes normal startup impossible (because intentional exceptions wait for an xsbug that isn't running) or makes debugging impossible (because exceptions have no stack traces), maybe we should paint an arrow/comment on it here.
There was a problem hiding this comment.
This #define causes some additional functions to be incorporated in the binary, but the DEBUG environment variable is also necessary and links in additional static libraries and reduces the optimization latitude given to gcc. I’m not sure why mxDebug is always set, but I assume for good reason.
| $(MACOS_VERSION_MIN) \ | ||
| -DINCLUDE_XSPLATFORM \ | ||
| -DXSPLATFORM=\"xsnap.h\" \ | ||
| -DmxDebug=1 \ |
There was a problem hiding this comment.
OTOH painting an arrow on it in multiple files would be a drag
|
|
||
| const bin = debug ? xsnapDebugBin : xsnapBin; | ||
|
|
||
| const xsnapProcess = spawn(bin, args, { |
There was a problem hiding this comment.
do we get an actionable error message if the binary wasn't compiled first? or is that not even a thing, since it will always get compiled in the process of installing this file? I'm not sure how install-time scripts will work in the monorepo symlinking step.
There was a problem hiding this comment.
I think so.
I got ENOENT aka file not found once before I fixed a typo in the lin makefiles.
There was a problem hiding this comment.
Yes, it fails but not mysteriously. The npm postinstall script should ensure that nobody sees this error. We should keep an eye open to be sure that any wrappers for yarn install are triggering the postinstall script.
kriskowal
force-pushed
the
xsnap
branch
2 times, most recently
from
f927f5d to
d8b92ae
Compare
Co-Authored-By: Kris Kowal <kris@agoric.com>
This change brings the XS binary
xsnapinto a new package of Agoric SDK. Thexsnapbinary can start a virtual machine from scratch or from a snapshot, and opens file descriptors 3 and 4 from the parent process to send and receive instructions, including “syscalls”, using a netstring envelope protocol. Within the netstrings, we use a single character prefix control code to evaluate, import modules, execute scripts, take snapshots, and send and receive binary messages synchronously (“syscalls”).Co-authored-by: Dan Connolly dckc@madmode.com
Co-authored-by: Michael Kellner mkellner@moddable.tech
Co-authored-by: Moddable Open Source 32498429+Moddable-OpenSource@users.noreply