xsnap doesn't rebuild when make env changes #9614
Comments
mhofman
added
bug
This was referenced Jun 29, 2024
Merged
mhofman
added a commit
to agoric-labs/xsnap-pub
that referenced
this issue
Jul 1, 2024
refs: Agoric/agoric-sdk#9614 Add an `EXTRA_DEPS` environment to the makefile to enable agoric-sdk to trigger a rebuild if some env options change like `XSNAP_VERSION` or `CC`. Drive-by deprecation of the `-n` command since the agoric-sdk PR will get rid of it in favor of checking the `XSNAP_VERSION` already embedded in `-v` Tested by building both manually invoking the makefile with and without the `EXTRA_DEPS` set, and through the build script changed in agoric-sdk.
mhofman
pushed a commit
that referenced
this issue
Jul 1, 2024
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
This was referenced Jul 1, 2024
mergify bot
added a commit
that referenced
this issue
Sep 12, 2024
…it clean` (#9634) closes: #9633 refs: #9614 ## Description Adds `git submodule foreach --recursive git clean -xdf` to build instructions in the release template. ### Security Considerations None. ### Scaling Considerations n/a ### Documentation Considerations This PR updates documentation intended for node operators. ### Testing Considerations n/a ### Upgrade Considerations n/a
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
Changing env passed to xsnap doesn't cause xsnap to rebuild, even though the env would affect the build output.
This recently caused app hashes for a validator on devnet (#9602), and also results in the updated package version to not be included in the output.
To Reproduce
xsnap-worker -vreturns the wrong package version (note that u15 does not actually change the xsnap package version)Expected behavior
Rebuild of xsnap on upgrades which change anything about xsnap
Additional context
This is tangentially related to #7012 since we'd need to force a rebuild if the version is plumbed through as env.
The text was updated successfully, but these errors were encountered: