refactor(compartment-mapper)!: Cleanly separate StaticModuleRecord dependency

[kriskowal]

Currently, the compartment mapper gets a lot of leverage from a shared assembler and import hook maker. Unfortunately, that means that import, archive, and importArchive all entrain a dependency on the fat StaticModuleRecord constructor and Babel. It should be possible for tools like @agoric/import-bundle to avoid that dependency by importing @endo/compartment-mapper/import-archive and so only support execution of precompiled archives.

This change begins the process of teasing apart these entry points. First, we consolidate some of the machinery in parse.js that is only used in assemble.js into the assembler directly. Instead of capturing the parserForLanguage mapping from lexical scope, we now thread it into assemble as a required field on the so-called options bag.

Then, import, archive, and import-archive each create their own parserForLanguage table with parsers that may be customized for their specific purpose. For example, archive uses parse-archive-mjs and parse-archive-cjs which pre-compile and pre-analyze modules respectively and enter a Syrup record into the archive instead of the source bytes. importArchive then expects to see only premjs and precjs files in archives, and has a parseForLanguage table that can produce static module records from the Syrup-encoded record.

Notably, import gains an efficiency from this factoring, since its parserForLanguage table consists of parsers that don't do any extra work for archiving.

A future change will need to create the API surface so these layers of compartment-mapper can be imported directly.

Refs #655

[erights]

Nothing calls app.agar-make. Shouldn't it be called during yarn build? If so, can we avoid checking in unreviewable binary?

[erights]

Oops. Still reviewing, but accidentally hit return while writing text for the eventual review message. Interesting github hazard.

[kriskowal]

Nothing calls app.agar-make. Shouldn't it be called during yarn build? If so, can we avoid checking in unreviewable binary?

Checking inapp.agar ensures backward compatibility version over version, that an archive generated with a prior version’s makeArchive will still be usable with a newer version’s parseArchive etc. We run app.agar-make.js manually and only when we are deliberately breaking backward compatibility, either by changing the nature of the test or the nature of the archive format. In this case, I’m breaking the old format. I expect that we’ll be committed to the format soon and keeping not just one app.agar, but a number of historical archives, will be useful.

[erights]

Nothing calls app.agar-make. Shouldn't it be called during yarn build? If so, can we avoid checking in unreviewable binary?

Checking inapp.agar ensures backward compatibility version over version, that an archive generated with a prior version’s makeArchive will still be usable with a newer version’s parseArchive etc. We run app.agar-make.js manually and only when we are deliberately breaking backward compatibility, either by changing the nature of the test or the nature of the archive format. In this case, I’m breaking the old format. I expect that we’ll be committed to the format soon and keeping not just one app.agar, but a number of historical archives, will be useful.

This all makes sense. Is it stated anywhere?

[kriskowal]

I’ve appended two commits to address feedback. The second change documents the process for updating the checked-in archive test fixture, including a test log with instructions for when that test fails.

[erights]

From #698 (comment)

The compartment-mapper package is not internal to SES

Doh! That's what I was missing. No change suggested. Resolving.

For some reason I cannot comment on this conversation in within that conversation.

[erights]

From #698 (comment)

I’ll adopt this and would like to know what case the previous code is not robust against.

Object.prototype.hasOwnProperty = myEvilFunction;

Obviously impossible in SES, but still a concern outside of SES. Our spec would use the internal hasOwnProperty, not whatever the current value of the property is.

[erights]

From #698 (comment)

I’ll adopt this and would like to know what case the previous code is not robust against.

Object.prototype.hasOwnProperty = myEvilFunction;

Obviously impossible in SES, but still a concern outside of SES. Our spec would use the internal hasOwnProperty, not whatever the current value of the property is.

Also

Function.prototype.call = myEvilFunction;

[erights]

LGTM!

[erights]

Would it make sense to rename the property names to match these much clearer variable names?

[kriskowal]

It might. They’re very verbose for a JSON file, but it might be a good idea.