feat(xsnap): Build sensitivity to presence of build toolchain

[kriskowal]

closes: #8536

Description

For those just tuning in, @warner took a run at making yarn build xsnap just once when it’s initially installed. I then took a run at landing the described fix and discovered a problem with building in Docker and reverted.

The changes you see before you unrevert the revert and go on to address the issue with Docker. Except, we do not update .dockerignore. We do not do anything special for the Docker environment because the new Dockerfile.sdk doesn’t require accommodation. We might in some future change make the postinstall script gracefully handle a Docker environment that has a binary but no sources, and we’ve laid some groundwork, but that case is still not supported.

Elaborating on the original plan, it also turns out to be insufficient to assume the presence of a moddable directory implies the existence of checked-out files under some circumstances (Documentation CI). So, we must be more specific about a file that would have been checked out. I chose the xs.h header arbitrarily.

This attempt otherwise resembles #9113 and ensures the C sources get published to npm so the recipient doesn’t have to git update submodules and this presumably is why make remembers where it left off when you yarn again later.

Documentation integration PR (this regressed last time) Agoric/documentation#1050

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

[michaelfig]

This still needs some work. Coupling with the Docker host's platform is too strong, and produces broken binaries if your host platform's compiler is binary-incompatible with the platform used inside the containers.

[michaelfig]

I'm not convinced this is correct. The only way it would work is if the platform within the Docker container is (coincidentally) binary-compatible with the host platform's (outside of Docker) compiler toolchain, and the host built the binary before attempting to run the Docker build.

That bundling of a host-specific binary violates the idea of Docker builds being reproducible.

In practical terms, this means that I won't be able to run agoric-sdk/a3p-integration on my MacOS box, because the host platform's compiler toolchain is not binary-compatible with the Linux-based system inside the Docker containers.

[kriskowal]

I’ve removed this line from .dockerignore. It might be appropriate to remove the submodule exceptions as well, though they appear to be doing no harm.

[michaelfig]

This still needs some work.

Are you running into multiple builds because of bin/agd? That's a more encompassing issue that I'm looking to solve properly once I clear (or reprioritise) more of my queue.

[michaelfig]

If you still need these semantics, I’d suggest doing the xsnap build in a stage of Dockerfile.sdk which has the compiler toolchain (i.e. no host build necessary), and in a later stage:

RUN rm -rf packages/xsnap/moddable packages/xsnap/xsnap-native && mkdir -p packages/xsnap/xsnap-native/xsnap/build/bin
COPY --from=<earlier-stage> /usr/src/agoric-sdk/packages/xsnap/xsnap-native/xsnap/build/bin/* packages/xsnap/xsnap-native/xsnap/build/bin/

That doesn’t need a .dockerignore change.

[kriskowal]

I think this requires a base Docker image that includes the build toolchain, including make, git, and the CC, which are evidently absent.

Is it possible to contrive a mechanism that builds xsnap in one container and shares the build product into the next?

[kriskowal]

michaelfig answers out-of-band: Build stages. We can do this.

https://docs.docker.com/build/building/multi-stage/

[kriskowal]

Evidently, you already did this. I pulled out the .dockerignore change since it’s no longer helpful or necessary. I’ve also removed the ruminations on the fictional Docker environment where we want yarn build to work for xsnap when there’s a binary already present but no sources. I left the code mostly the same since there’s no downside.

[kriskowal]

@turadg, I got a tip from @michaelfig that you might have pending changes to Docker stuff that might overlap with this effort to speed up xsnap builds. Please take a look and let me know whether the wind is carrying us in the same direction.

[turadg]

My thing landed in a couple days ago: #8850 Sorry I didn't think to tag you. I had picked the reviewers before you took up xsnap.

Your branch is on top of that from master so no potential for conflicts.

That change was to have a Docker layer specifically for xsnap, so that the yarn install didn't try to build it again if it hadn't changed. I think that's good to keep and should reduce the number of cases you have to consider.

[kriskowal]

The Docker build of xsnap in a container with the build toolchain is great and I think does simplify the constraints on build.js. I’ve mostly adjusted the comments to reflect that simplification.

[kriskowal]

Are you running into multiple builds because of bin/agd? That's a more encompassing issue that I'm looking to solve properly once I clear (or reprioritise) more of my queue.

I have not seen this reported. I’ve only heard about lots of rebuilding of xsnap each time a dapp dev runs yarn.

[kriskowal]

In both the old and new build:env script, we fail if git is unavailable, since that is a requirement for --show-env, but now src/build.js says why. In the old code, we would short circuit if test didn’t exist, which is the case for an unpacked npm tarball, an environment where git might not exist and in which git should not be used. In the new code, we use the existence of a specific source file and the platform-specific binary for a more direct signal.

[kriskowal]

This is the bit that makes it unnecessary for the postinstall script to clone git submodules.

[kriskowal]

First one is slow, subsequent are fast.

[michaelfig]

LGTM!