feat(agoric-cli): allow NPM-based agoric install to work with dapps

[michaelfig]

closes: #3857 closes: #4169 closes: #4170

Description

With these changes, the agoric NPM package can function without the rest of the SDK installed.

Many thanks to @mhofman for enumerating several of the problems that the prior version had, and suggesting Verdaccio for integration-testing the results.

• repurpose agoric start --docker-tag=XXX to turn on Docker mode instead of --no-sdk, which is different.
• build @agoric/xsnap binaries before testing (with agoric start --rebuild), as they are not distributed on NPM
• several fixes to package.json files to distribute necessary things
• integration test uses Verdaccio to test installing from a local NPM registry
• when upgrading a Dapp, ensure that the required agoric start packages (@agoric/cosmic-swingset, @agoric/solo) are listed in ./package.json

Security Considerations

Documentation Considerations

After we gain some confidence in this setup, we could recommend using npx agoric instead of having to install the full SDK just to test a dapp.

Testing Considerations

Integration test added using Verdaccio.

[mhofman]

This looks great. I need to try this locally! Also wondering if I'd be able to run the loadgen with these changes without having the SDK installed locally, which would be neat.

I just have some small nits about child process exitCodes and a duplication of yarn workspace logic in the cli.

I think we'll need a follow-up to make the xsnap package more node-like (either building from embedded source, or pulling a binary from GitHub). Or maybe we can punt on that until xsnap moves under endo.

[mhofman]

Also, is the goal to still require agoric install for dapps, even when using a non local sdk?

If we removed the AGORIC_INSTALL preinstall check, I think it should even work from my reading of the code with a dapp which has agoric installed only as a dependency (no global install of the cli).

[michaelfig]

Also wondering if I'd be able to run the loadgen with these changes without having the SDK installed locally, which would be neat.

That would be possible as one particular mode (like "run npx agoric install beta, then loadgen against it"), but as you know, only certain SDK revisions will actually support that. Hopefully, all of them going forward.

Is the goal to still require agoric install for dapps, even when using a non local SDK?

The goal is to allow dapps to get by just with yarn (for now, maybe eventually other package managers). agoric install is for when you want to test against different SDKs (possibly unpublished) or upgrade your dapp's SDK dependencies, and that should be all.

If we removed the AGORIC_INSTALL preinstall check, I think it should even work from my reading of the code with a dapp which has agoric installed only as a dependency (no global install of the cli).

Yes, the preinstall check was just temporary until we could sort this process out.

[michaelfig]

I think we'll need a follow-up to make the xsnap package more node-like (either building from embedded source, or pulling a binary from GitHub). Or maybe we can punt on that until xsnap moves under endo.

Yeah, that's a bit of a can of worms, especially now that some package managers are moving away from implicit preinstall scripts. I agree that we can wait to improve this later.

[mhofman]

Feedback on process: it was hard for me to figure out what had changed since my first review because the updates were squashed into the initial commits. Making it harder, the PR was rebased on the latest master.

While I agree it's cleaner to fixup commits addressing feedback before merging, it'd be easier for reviewing if that was done after final approval so that reviews can be incremental. In that case rebasing wouldn't matter as much (aka I'd trust the rebase didn't introduce changes to the already reviewed commits).

[michaelfig]

Feedback on process: it was hard for me to figure out what had changed since my first review because the updates were squashed into the initial commits. Making it harder, the PR was rebased on the latest master.

I think we need to have an engg process discussion to decide how to navigate through the different requirements for PR reviewers. Ideally, that would result in a flowchart for how to publish changes to PRs, so I don't have to think as much. :)

[mhofman]

As soon as this is merged, I'll give it a try straight from NPM :)