Skip to content

Commit

Permalink
Force xsnap rebuild (#9618)
Browse files Browse the repository at this point in the history
closes: #9614
closes: #7012
refs: agoric-labs/xsnap-pub#49

## Description
Wire a mechanism to force rebuilds of xsnap when some build environments change, or if the xsnap binary is found to be out of date through the embedded xsnap package version.

Moves the `agd build` check to depend on a version check implemented by the xsnap package, which now only depends on the package version that is dynamically inserted instead of an explicit magic string that wasn't getting updated.

#7012 (comment) expressed concerns with relying on the xsnap package version, however:
- any changes to `xsnap-pub` or `moddable` submodules would result in a change to the checked in `build.env` file, which would be detected by `lerna` during the publish process. While a version bump would not apply for contributors of the sdk (aka not every xsnap change results in a version change), it will apply for anyone using published versions, even if the change is in submodules only.
- By having the version check look up the expected version from the `package.json` file directly, we avoid having to modify both  the `package.json` file and the check itself. Only the automatically generated single publish commit will trigger a forced rebuild, and satisfy the check on its own,
- At the same time we remove the dependency of `agd build` onto the the internal structure of xsnap.

### Security Considerations
Forces all validators to use the expected version of xsnap

### Scaling Considerations
Does a few more checks when building xsnap, and causes full rebuilds in some cases where they might not be strictly necessary.

### Documentation Considerations
Should all be transparent

### Testing Considerations
Manually tested by touching the xsnap package version, or reverting just the release binary to a previous copy (or deleting altogether). If the binary is meddled with like this, `agd build` checks will fail, but a manual `yarn build` will fix. I thought this was better than transparently forcing a rebuild in that abnormal situation.

### Upgrade Considerations
Smoother upgrades by validators
  • Loading branch information
mergify[bot] committed Jul 1, 2024
2 parents 54b2e2b + 2b53896 commit 78b6fec
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 20 deletions.
1 change: 1 addition & 0 deletions .dockerignore
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ packages/stat-logger
**/_agstate
.vagrant
endo-sha.txt
packages/xsnap/build.config.env
# When changing/adding entries here, make sure to search the whole project for
# `@@AGORIC_DOCKER_SUBMODULES@@`
packages/xsnap/moddable
Expand Down
1 change: 1 addition & 0 deletions packages/xsnap/.gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
build
build.config.env
dist
test/fixture-snap-pool/
test/fixture-snap-shot.xss
2 changes: 1 addition & 1 deletion packages/xsnap/build.env
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
MODDABLE_URL=https://github.com/agoric-labs/moddable.git
MODDABLE_COMMIT_HASH=f6c5951fc055e4ca592b9166b9ae3cbb9cca6bf0
XSNAP_NATIVE_URL=https://github.com/agoric-labs/xsnap-pub
XSNAP_NATIVE_COMMIT_HASH=2d8ccb76b8508e490d9e03972bb4c64f402d5135
XSNAP_NATIVE_COMMIT_HASH=eef9b67da5517ed18ff9e0073b842db20924eae3
2 changes: 2 additions & 0 deletions packages/xsnap/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"build:env": "node src/build.js --show-env > build.env",
"build:from-env": "{ cat build.env; echo node src/build.js; } | xargs env",
"build": "yarn build:bin && yarn build:env",
"check-version": "xsnap_version=$(./scripts/get_xsnap_version.sh); if test \"${npm_package_version}\" != \"${xsnap_version}\"; then echo \"xsnap version mismatch; expected '${npm_package_version}', got '${xsnap_version}'\"; exit 1; fi",
"postinstall": "npm run build:from-env",
"clean": "rm -rf xsnap-native/xsnap/build",
"lint": "run-s --continue-on-error lint:*",
Expand Down Expand Up @@ -56,6 +57,7 @@
"moddable/xs/makefiles",
"moddable/xs/platforms/*.h",
"moddable/xs/sources",
"scripts",
"src",
"xsnap-native/xsnap/makefiles",
"xsnap-native/xsnap/sources"
Expand Down
14 changes: 14 additions & 0 deletions packages/xsnap/scripts/get_xsnap_version.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/usr/bin/env bash

set -ueo pipefail

# the xsnap binary lives in a platform-specific directory
unameOut="$(uname -s)"
case "${unameOut}" in
Linux*) platform=lin ;;
Darwin*) platform=mac ;;
*) platform=win ;;
esac

# extract the xsnap package version from the long version printed by xsnap-worker
"./xsnap-native/xsnap/build/bin/${platform}/release/xsnap-worker" -v | sed -e 's/^xsnap \([^ ]*\) (XS [^)]*)$/\1/g'
41 changes: 37 additions & 4 deletions packages/xsnap/src/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,16 +220,34 @@ const updateSubmodules = async (showEnv, { env, stdout, spawn, fs }) => {
* existsSync: typeof import('fs').existsSync,
* rmdirSync: typeof import('fs').rmdirSync,
* readFile: typeof import('fs').promises.readFile,
* writeFile: typeof import('fs').promises.writeFile,
* },
* os: {
* type: typeof import('os').type,
* }
* }} io
* @param {object} [options]
* @param {boolean} [options.forceBuild]
*/
const makeXsnap = async ({ spawn, fs, os }) => {
const makeXsnap = async ({ spawn, fs, os }, { forceBuild = false } = {}) => {
const pjson = await fs.readFile(asset('../package.json'), 'utf-8');
const pkg = JSON.parse(pjson);

const configEnvs = [
`XSNAP_VERSION=${pkg.version}`,
`CC=cc "-D__has_builtin(x)=1"`,
];

const configEnvFile = asset('../build.config.env');
const existingConfigEnvs = fs.existsSync(configEnvFile)
? await fs.readFile(configEnvFile, 'utf-8')
: '';

const expectedConfigEnvs = configEnvs.concat('').join('\n');
if (forceBuild || existingConfigEnvs.trim() !== expectedConfigEnvs.trim()) {
await fs.writeFile(configEnvFile, expectedConfigEnvs);
}

const platform = ModdableSDK.platforms[os.type()];
if (!platform) {
throw Error(`Unsupported OS found: ${os.type()}`);
Expand All @@ -241,8 +259,10 @@ const makeXsnap = async ({ spawn, fs, os }) => {
[
`MODDABLE=${ModdableSDK.MODDABLE}`,
`GOAL=${goal}`,
`XSNAP_VERSION=${pkg.version}`,
`CC=cc "-D__has_builtin(x)=1"`,
// Any other configuration variables that affect the build output
// should be placed in `configEnvs` to force a rebuild if they change
...configEnvs,
`EXTRA_DEPS=${configEnvFile}`,
'-f',
'xsnap-worker.mk',
],
Expand All @@ -263,6 +283,7 @@ const makeXsnap = async ({ spawn, fs, os }) => {
* existsSync: typeof import('fs').existsSync,
* rmdirSync: typeof import('fs').rmdirSync,
* readFile: typeof import('fs').promises.readFile,
* writeFile: typeof import('fs').promises.writeFile,
* },
* os: {
* type: typeof import('os').type,
Expand Down Expand Up @@ -328,7 +349,18 @@ async function main(args, { env, stdout, spawn, fs, os }) {

if (!showEnv) {
if (hasSource) {
await makeXsnap({ spawn, fs, os });
// Force a rebuild if for some reason the binary is out of date
// Since the make checks may not always detect that situation
let forceBuild = !hasBin;
if (hasBin) {
const npm = makeCLI('npm', { spawn });
await npm
.run(['run', '-s', 'check-version'], { cwd: asset('..') })
.catch(() => {
forceBuild = true;
});
}
await makeXsnap({ spawn, fs, os }, { forceBuild });
} else if (!hasBin) {
throw new Error(
'XSnap has neither sources nor a pre-built binary. Docker? .dockerignore? npm files?',
Expand All @@ -344,6 +376,7 @@ const run = () =>
spawn: childProcessTop.spawn,
fs: {
readFile: fsTop.promises.readFile,
writeFile: fsTop.promises.writeFile,
existsSync: fsTop.existsSync,
rmdirSync: fsTop.rmdirSync,
},
Expand Down
3 changes: 1 addition & 2 deletions repoconfig.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,14 @@ NODEJS_VERSION=v16
GOLANG_VERSION=1.20.3
GOLANG_DIR=golang/cosmos
GOLANG_DAEMON=$GOLANG_DIR/build/agd
XSNAP_VERSION=agoric-upgrade-10

# Args are major, minor and patch version numbers
golang_version_check() {
{
[ "$1" -eq 1 ] && [ "$2" -eq 20 ] && [ "$3" -ge 2 ] && return 0
[ "$1" -eq 1 ] && [ "$2" -ge 21 ] && return 0
[ "$1" -ge 2 ] && return 0
} 2>/dev/null
} 2> /dev/null
echo 1>&2 "need go version 1.20.2+, 1.21+, or 2+"
return 1
}
15 changes: 3 additions & 12 deletions scripts/agd-builder.sh
Original file line number Diff line number Diff line change
Expand Up @@ -219,21 +219,12 @@ $do_not_build || (
echo "At least $src is newer than gyp bindings"
(cd "$GOLANG_DIR" && lazy_yarn build:gyp)
}

# check the built xsnap version against the package version it should be using
(cd "${thisdir}/../packages/xsnap" && npm run -s check-version) || exit 1
fi
)

# the xsnap binary lives in a platform-specific directory
unameOut="$(uname -s)"
case "${unameOut}" in
Linux*) platform=lin ;;
Darwin*) platform=mac ;;
*) platform=win ;;
esac

# check the xsnap version against our baked-in notion of what version we should be using
xsnap_version=$("${thisdir}/../packages/xsnap/xsnap-native/xsnap/build/bin/${platform}/release/xsnap-worker" -n)
[[ "${xsnap_version}" == "${XSNAP_VERSION}" ]] || fatal "xsnap version mismatch; expected ${XSNAP_VERSION}, got ${xsnap_version}"

if $only_build; then
echo "Build complete." 1>&2
exit 0
Expand Down

0 comments on commit 78b6fec

Please sign in to comment.