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

sharing module specifier paths with xs #57

Closed
dckc opened this issue Sep 14, 2019 · 16 comments
Closed

sharing module specifier paths with xs #57

dckc opened this issue Sep 14, 2019 · 16 comments
Assignees
Labels
enhancement New feature or request SwingSet package: SwingSet

Comments

@dckc
Copy link
Member

dckc commented Sep 14, 2019

I'm struggling with

# Exception: (host): module "../../insist" not found!

Unfortunately, mcconfig doesn't say which occurrence is the problem. I thought ../.. would never work, but then it seemed to for a while. Now I'm struggling with it again.

There are many occurrences:
https://gist.github.com/dckc/c5f7b8a1c5834199cd36bf29f7f085e3

Aug 20: struggling
Moddable-OpenSource/moddable#251

Sep 2: solved some parts of it
https://github.com/Agoric/SwingSet/issues/126#issuecomment-527226803

cc @dtribble

@dckc
Copy link
Member Author

dckc commented Sep 21, 2019

Turns out the default implementation of fxFindModule in xs had hard-coded support for ./ and ../. So I hard-coded support for ../../! So there!
Moddable-OpenSource/moddable#267
5956f57

I wonder when is the last time I used gdb. Mid '90s?

@dtribble you had strong opinions about this ../../ stuff; do you want to keep this open to discuss alternatives? Or shall I just close this?

Oh... the moddable guys might not like my PR, so I guess we should stand by to see what they think. We might want to define our own platform anyway, so we might not need them to take this fix upstream. But it would be convenient.

@warner
Copy link
Member

warner commented Sep 22, 2019

Hm. Does ./sibling.js cause fewer problems than ../parent.js? Or is it specifically the climbing-upwards-out-of-cwd that is the trouble?

I haven't had a chance to talk (argue?) with @dtribble about this yet. The goal is to share code within the SwingSet library. I'm worried that the alternative to ../insist is publishing a dozen one-line modules (mostly useless outside of the swingset context) to NPM for everything that needs to be shared to more than one swingset subdirectory.

@dckc
Copy link
Member Author

dckc commented Sep 23, 2019

@dtribble wrote elsewhere:

no need. my issue is with the overall module includes architecture, which is driven by web-compat issues. it needs to be addressed there.

So I'm just standing by to hear from moddable now.

@warner warner transferred this issue from Agoric/SwingSet Dec 1, 2019
@warner warner added the SwingSet package: SwingSet label Dec 1, 2019
dckc pushed a commit to dckc/agoric-sdk that referenced this issue Dec 5, 2019
dckc pushed a commit to dckc/agoric-sdk that referenced this issue Dec 5, 2019
@dckc
Copy link
Member Author

dckc commented Dec 14, 2019

The switch to a monorepo (#267) seems to conflict with my earlier approach to this issue.

The earlier approach involved putting the xs code at the top level directory. That'll be awkward for several packages at once.

@dckc dckc mentioned this issue Dec 14, 2019
@dckc
Copy link
Member Author

dckc commented Dec 25, 2019

ugh. this module search path thing has got me down again.

I found an algorithm for generating manifests that worked well enough to test eventual-send and (with one kludge) marshal, but when I try it out on cosmic-swingset... the generated manifest resolves @agoric/swingset-vat to .../src/index ; that imports ./controller which xs thinks is @agoric/controller.

Looking at how I dealt with this before, I totally hacked up the import site: https://github.com/dckc/agoric-sdk/blob/xs-platform/packages/cosmic-swingset/lib/ag-solo/start.js#L9-L19

I might try tweaking module lookup in our xs platform, but I'm afraid that by that point the info is lost. The xsc compiler might need to cooperate. And then I'm not sure how to handle the makefile stuff.

for reference: moddable folks Aug 21

The module specifier in the Moddable SDK is not a path. Trying to make it behave as one isn't going to be easy.

@dckc dckc changed the title struggling with ../.. in import specifier paths sharing module specifier paths with xs Dec 25, 2019
@dtribble
Copy link
Member

dtribble commented Dec 26, 2019

@erights @michaelfig In the changes above, Dan had to basically change the imports to avoid the common pattern of an index.js files importing local definitions and re-exporting them (compare with the version in master). The "naming path" for a file in nodejs is some mangling of the directory structure of the files and the "mounting" of packages. That mangling is not the same in the XS view of the world, so the swingset-vats name in import ... from '@agoric/singset-vats is only present in package.json files and not a directory tree.

How do we harmonize the structure so that the module import patterns work in both XS and node?

@michaelfig
Copy link
Member

It's much uglier and somewhat violates the package abstraction, but we can change the import sites to:

import ... from '@agoric/swingset-vats/src/index.js';

@dckc
Copy link
Member Author

dckc commented Jan 2, 2020

chatted with @warner about this today; he reminded me about the Jetpack Modules project he worked on at Mozilla. Here's hoping to study it to see if it provides / inspires a solution.

p.s. He adds:

Looks like https://github.com/warner/addon-sdk/blob/master/python-lib/cuddlefish/manifest.py is the code that built the manifest

@dckc
Copy link
Member Author

dckc commented Jan 4, 2020

idea: use Compartment map to reconcile paths

  1. Use node to resolve module specifiers to absolute .js file paths
    • esp. detective package as used in tape-xs to automatically build manifests
  2. Choose a somewhat arbitrary name for each file for use in the XS compiler and build tools; symlink the arbitrary name to the full path
  3. Make a wrapper main module (again, as done in tape-xs) that makes a Compartment whose map connects the absolute paths to the arbitrary names.

@dckc
Copy link
Member Author

dckc commented Jan 7, 2020

This compartment map approach is going reasonably well.

https://github.com/dckc/tape-xs/blob/master/bin/modlinks.js is a refinement of tape-xs-build that generates a manifest include file with a modules section with "external" names for each module along with compartment sections that map "internal" names to "external" names.

As shown in https://gist.github.com/dckc/f3ca2ed672174eadeb17de489adc9df2 , we then make a ~25 line manifest that includes the above as well as a ~25 line main that uses the compartment info to make compartments.

It's starting to be able to run the SwingSet/test/test*.js scripts, though there are various bundling issues to re-integrate.

I also moved stuff around a little bit here in my branch of agoric-sdk 714fecf .. 05dd667

@dckc
Copy link
Member Author

dckc commented Jan 10, 2020

I got bin/ag-solo-xs to build using the Compartment approach to module specifiers.
https://github.com/dckc/agoric-sdk/runs/382790593

I was disappointed to learn that relative specifier handling is done relative to the Symbol(/node_modules/@agoric/swingset-vat/src/controller.xsb) module id and not relative to the @agoric/swingset-vat/src/controller compartment map key strings. IOU more context in source code comments so that makes sense.

bin/ag-solo-xs.js ~50 lines
lib/ag-solo/manifest.json ~50 lines
lib/ag-solo/xs-compartments.json 186 lines, generated using modlinks.js, save for1 tweak (3444cfe) for the kernel source bundle.

xs-build-lite branch 3444cfe

@dckc
Copy link
Member Author

dckc commented Jun 3, 2020

@kriskowal writes in keybase:

I’m writing a tool that generates compartmap.json file for a package just by digging up all the node_modules and reconstructing a description of one way to weave them together with compartments.

I've done overlapping work. output:
https://github.com/dckc/agoric-sdk/blob/xs-build-lite/packages/cosmic-swingset/lib/ag-solo/xs-compartments.json

tool (messy):
https://github.com/agoric-labs/tape-xs/blob/master/bin/modlinks.js

@dckc
Copy link
Member Author

dckc commented Aug 25, 2020

The endo compartmap approach is working pretty well in the context of #1407 , but before we close this... I wrote:

I'm hesitant to rely on the findmods.js tool that builds it.

I'd like to see that it shares a typescript type for the compartmap with endo.

@warner writes in #1407 (comment) :

... we need to think through when exactly it must be rebuilt (if/when swingset files get moved around, or liveslots.js acquires a new import?), and figure out some rules for rebuilding it.

cc @kriskowal

@dckc
Copy link
Member Author

dckc commented Sep 1, 2020

I'd like to see that it shares a typescript type for the compartmap with endo.

The present design does not share a type, though the difference looks arbitrary and is probably straightforward to reconcile.

https://github.com/dckc/agoric-sdk/commits/endo-types

@dckc
Copy link
Member Author

dckc commented Jan 15, 2021

The plan we worked out in #2107 is to use rollup, for now. It seems to be working: #2194

Meanwhile,

But to address this fully, we'll need

  • a Compartment constructor on XS that has a more complete module loading API.

@dckc
Copy link
Member Author

dckc commented Jan 25, 2021

Let's track the Compartment constructor with module loading as endojs/endo#536

@dckc dckc closed this as completed Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request SwingSet package: SwingSet
Projects
None yet
Development

No branches or pull requests

4 participants