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

build xsnap from NPM tarball without git #8536

Closed
warner opened this issue Nov 16, 2023 · 6 comments · Fixed by #9113 or #9160
Closed

build xsnap from NPM tarball without git #8536

warner opened this issue Nov 16, 2023 · 6 comments · Fixed by #9113 or #9160
Assignees
Labels
devex developer experience enhancement New feature or request xsnap the XS execution tool

Comments

@warner
Copy link
Member

warner commented Nov 16, 2023

What is the Problem Being Solved?

Currently, @agoric/xsnap requires the git tool to be on $PATH before it can compile the C code that it needs. If the xsnap code is coming from an NPM tarball (i.e. any use outside of the Agoric-SDK), then the build scripts performs a git clone at install time, to fetch the code from github. This is a drag, and precludes some no-network/hermetic build environments.

The intended change is to build xsnap without using git at install time.

Description of the Design

package/xsnap depends upon Moddable's XS SDK (in a subdirectory named moddable/), as well as the "xsnap" application (in a subdirectory named xsnap-native/). This code lives upstream in a separate repository, specifically an agoric-labs fork of repos from Moddable's github organization.

In agoric-sdk, both subdirectories are registered as "git submodules". Currently, during build, our src/build.js performs a git submodule update to either populate these for the first time (i.e. in a brand new git clone https://github.com/Agoric/agoric-sdk worktree), or to update them (in case the developer switched branches, and the new branch calls for a different commit-id of the moddable/xsnap code). A file named build.env is updated to record the git repository URL and commit ID for each. This filename is listed in the package.json files: property, so it will be included in the generated tarball when publishing to the NPM registry. Then, if a build is performed from that NPM tarball, build.env is consulted to perform a git clone of the two dependency repos, dropping their code into the right subdirectories (as an independent git worktree, rather than a git submodule).

Instead, the new plan is:

  • use the presence of the moddable/ subdirectory, and the moddable/.git path (which might be a directory, or a redirection file), to determine which of the three cases we're in:
    • A: neither exists: we're in a new agoric-sdk checkout, and have not yet performed git submodule update --init
    • B: both exist: we're in an agoric-sdk checkout and have initialized the submodules
    • C: moddable/ exists but moddable/.git does not: we're in an unpacked NPM tarball
    • (perform the same checks for xsnap-native/)
  • in cases A and B, each yarn build should run git submodule update --init SUBDIR to either create or update the submodule
  • in case C, we do not use git, and leave the subdirectory source alone
  • in all cases, re-run the appropriate make command, to recompile any C code that might have changed

In addition, we'll change the files: property to include all of the necessary XS and xsnap source code in the npm pack tarball that is uploaded to the NPM registry. This allows case C to get the source it needs without git on the second machine.

We'll continue to create/update the build.env file with the URL and commit-ID of each submodule, and this file will continue to be included in the NPM tarball, however it will be for reference/auditing only (a hint to NPM users as to where the source files came from), and will not be read by the build process.

Security Considerations

Slightly improves security: the XS/xsnap source code is included in the uploaded tarball, rather than being downloaded at build time. This removes GitHub from the TCB of the second (downloading) machine. Auditors who inspect the source code of the NPM-hosted tarball (whose hash is generally included in yarn.lock -type files) can rely upon the code they see there, rather than hoping that GitHub will return the same code on each install.

Scaling Considerations

Somewhat worse: the generated NPM tarball is about 2.7MB in size, vs the current 35kB.

Test Plan

  • manual tests
    • yarn pack from an agoric-sdk checkout, then yarn install ../../agoric-xsnap-VER.tgz from a new environment (without /usr/bin/git) and a smoke test to make sure the resulting binary will work
  • automated tests
    • duplicate the above from an integration test, except it might be too hard to remove git from the environment
    • note that the package.json files: list needs to include things like modules/base64 and modules/textencoder, even though we omit all the XS device drivers (LCD panels, etc) and test262 subdirectories. So this automated test is very important, to make sure we don't accidentally start depending upon a file that we forgot to add to files:. If we made that mistake, agoric-sdk would keep working, but the tarballs we push to NPM would stop being usable

Upgrade Considerations

None: existing agoric-sdk worktrees will use the new logic, but will continue to keep their moddable/ and xsnap-native/ directories the way they are.

@warner warner added enhancement New feature or request xsnap the XS execution tool labels Nov 16, 2023
@warner warner self-assigned this Nov 16, 2023
warner added a commit that referenced this issue Nov 16, 2023
Use presence/absence of submodule directories and their `.git`
metadata to distinguish between an agoric-sdk clone and an unpacked
NPM tarball. Use `git submodule update --init` in the first case, and
leave the sources alone in the last case.

Still needs real tests, but manual testing passes.

refs #8536
@dckc dckc added the devex developer experience label Dec 6, 2023
@dckc
Copy link
Member

dckc commented Jan 10, 2024

This is a pain point for @kbennett2000 , whose internet bandwidth might be lower than usual.
He sometimes has to repeat this a few times to get it to work...

@kriskowal
Copy link
Member

Reverted #9122

@kriskowal kriskowal reopened this Mar 20, 2024
@kriskowal
Copy link
Member

When attacking this problem again, a litmus test is whether agoric/documentation can build against the branch, or a3p-integration works in agoric-sdk proper. Both of these exhibited the symptom of #9121.

@kriskowal
Copy link
Member

@warner’s branch is over here for the next attempt release-getting-started...warner/8536-xsnap-without-git

@kriskowal
Copy link
Member

Per conversation with @warner, a thing we haven’t tried yet is determining whether to shell out to make based on the presence of a source file:

  1. which should be present in an npm pack.
  2. which should be absent in a Docker image (and the bin must have been added by the host).
  3. which should be present in a checkout.

@kriskowal
Copy link
Member

Man, this problem is redonculous. yarn link from agoric-sdk produces a directory with empty moddable and xsnap-native directories.

https://github.com/Agoric/documentation/actions/runs/8460729116/job/23179429437?pr=1050

I’ll update the parameters and find a better test for whether to shell out to git submodule, probably a more specific path than moddable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devex developer experience enhancement New feature or request xsnap the XS execution tool
Projects
None yet
4 participants