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

migrate xsnap to endo #681

Open
dckc opened this issue Apr 20, 2021 · 10 comments
Open

migrate xsnap to endo #681

dckc opened this issue Apr 20, 2021 · 10 comments
Assignees
Labels
kriskowal-reviewed-2024-01 Issues that kriskowal is satisfied do not need attention from team bug review as of January, 2024 xsnap

Comments

@dckc
Copy link
Contributor

dckc commented Apr 20, 2021

@kriskowal @warner and everybody,

The folks at moddable just refactored xsnap between platform and application:
agoric-labs/xsnap-pub@c2cebe7

and they provided xsnap-ava.c, which has our pipe I/O and event loop (it's a bit of a misnomer, but no matter for now).
https://github.com/agoric-labs/xsnap-pub/blob/master/xsnap/sources/xsnap-ava.c

This was somewhat prompted by me wishing that our collaboration could use less copy-and-paste and more sharing.

I'm already in the process of copying / moving xsnap from agoric-sdk to endo; maybe now is a good time to reconcile things?
https://github.com/Agoric/agoric-sdk/blob/master/packages/xsnap/src/xsnap.c
https://github.com/endojs/endo/blob/535-ava-xs/packages/xsnap/src/xsnap.c

I tried to look at diffs between xsnap/src/xsnap.c and xsnap-ava.c and while I didn't see anything wrong, it looks like a lot of work to be sure that nothing was lost in translation.

Other questions that come to mind:

  • submodules are tedious; is this as good as it gets? do we want another one?
  • how should binary artifacts be packaged?
@dckc dckc self-assigned this Apr 20, 2021
@kriskowal
Copy link
Member

kriskowal commented Apr 20, 2021 via email

@dckc
Copy link
Contributor Author

dckc commented Apr 20, 2021

What if we move xsnap to endojs/endo ...

I'm leaning that direction, yes...

... and give Peter and Patrick collaborator access instead of syncing another repository?

I wonder if they would see access as a "gift". It would put a burden on them to yarn test any contributions and such. yarn and the whole npm world is outside their normal workflow.

@dckc
Copy link
Contributor Author

dckc commented Apr 20, 2021

When xsnap is moved out of agoric-sdk, how should the binary get built? postinstall?

@kriskowal
Copy link
Member

kriskowal commented Apr 21, 2021 via email

@dckc
Copy link
Contributor Author

dckc commented May 4, 2021

For testnet monitoring, we made some tweaks to the moddable SDK: agoric-labs/moddable#5

Are we going to maintain a branch? ugh.

@dckc
Copy link
Contributor Author

dckc commented May 5, 2021

Using agoric-labs required some fix-up in the deployment mode of build.js:

Agoric/agoric-sdk@a1a2693

as @warner notes,

we need to make sure to not add another git submodule which happens to be hosted at a matching url

@dckc
Copy link
Contributor Author

dckc commented May 12, 2021

Moddable refined metering to cover stack operations. But now that we've forked again, integrating it is non-trivial. Our agoric-labs ag-testnet branch is "5 commits ahead, 19 commits behind Moddable-OpenSource:public"

p.s. escalated to its own issue: Agoric/agoric-sdk#3139

@dckc
Copy link
Contributor Author

dckc commented May 20, 2021

@kriskowal @dtribble @warner and I just discussed this

  1. Agoric keeps xsnap.c and such in agoric-sdk. As @phoddie pointed out, on Apr 19, moddable provided a xsnap: platform vs application refactor c2cebe7; we have not yet integrated it. We should reconcile these. (But not urgently; we're inclined to accept the technical debt in favor of other priorities just now.)
  2. Once we reconcile them, we should figure out how to manage things going forward

All this is separate from the issue of keeping the main XS sources in sync, which see Agoric/agoric-sdk#3139

@dckc
Copy link
Contributor Author

dckc commented Jun 25, 2021

Having makeSnapstore in the @agoric/xsnap package is a bit awkward, as @warner just noted in Agoric/agoric-sdk#2848 (comment)

It has moved back and forth between SwingSet and xsnap, but it probably doesn't belong in either.

@dckc
Copy link
Contributor Author

dckc commented Aug 27, 2021

The technical debt around the xsnap: platform vs application refactor is addressed as of agoric-sdk/pull/3607 Aug 13 85b252c .

So it's more straightforward to migrate the package to endo.

This would probably be a good time to move ava-xs to its own package.

This is not urgent, AFAIK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kriskowal-reviewed-2024-01 Issues that kriskowal is satisfied do not need attention from team bug review as of January, 2024 xsnap
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants