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.

Sign up for GitHub

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

Force xsnap rebuild #9618

Merged
merged 3 commits into from
Jul 1, 2024
Merged

Force xsnap rebuild #9618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
467435a
fix(xsnap): force rebuild if build config changes
mhofman Jun 30, 2024
a22772e
fix(agd): check xsnap was rebuilt
mhofman Jun 30, 2024
2b53896
feat(xsnap): force rebuild if binary version mismatch
mhofman Jun 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .dockerignore
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/usr/bin/env bash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good enough, it assumes that bash is available, but I'll admit that's likely, and easier than figuring out how to write this in the purely sh subset

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well arguably this is already posix 2022 compliant, it's just that most shells still don't support -o pipefail in their posix mode.

I suppose I could check for the existence of the binary first before trying to invoke it


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
View file
Open in desktop
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, so we update build.config.env if it differs from the const configEnvs here in build.js, and build.config.env is added as a *.o dependency via EXTRA_DEPS, so modifying most of src/build.js won't cause a rebuild, but modifying the -D__has_builtin or the XSNAP_VERSION value will.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or if forceBuild is passed, which is if we detect a version mismatch, in case someone mucked around with the binary directly

}

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
2 changes: 1 addition & 1 deletion packages/xsnap/xsnap-native
Open in desktop
Submodule xsnap-native updated 5 files
+3 −0 xsnap/makefiles/lin/xsnap-worker.mk
+3 −0 xsnap/makefiles/mac/xsnap-worker.mk
+16 −4 xsnap/sources/xsnap-worker.c
+3 −2 xsnap/sources/xsnap.c
+6 −1 xsnap/sources/xsnapPlatform.c
3 changes: 1 addition & 2 deletions repoconfig.sh
View file
Open in desktop
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
View file
Open in desktop
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}"

Comment on lines -225 to -236
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad to see this go! 💯

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well arguably it moved

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