Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
ISLE: guard against stale generated source in default build config.
Currently, the `build.rs` script that generates Rust source from the ISLE DSL will only do this generation if the `rebuild-isle` Cargo feature is specified. By default, it is not. This is based on the principle that we (the build script) do not modify the source tree as managed by git; git-managed files are strictly a human-managed and human-edited resource. By adding the opt-in Cargo feature, a developer is requesting the build script to perform an explicit action. (In my understanding at least, this principle comes from the general philosophy of hermetic builds: the output should be a pure function of the input, and part of this is that the input is read-only. If we modify the source tree, then all bets are off.) Unfortunately, requiring the opt-in feature also creates a footgun that is easy to hit: if a developer modifies the ISLE DSL source, but forgets to specify the Cargo feature, then the compiler will silently be built successfully with stale source, and will silently exclude any changes that were made. The generated source is checked into git for a good reason: we want DSL compiler to not affect build times for the overwhelmingly common case that Cranelift is used as a dependency but the backends are not being actively developed. (This overhead comes mainly from building `islec` itself.) So, what to do? This PR implements a middle ground first described in [this conversation](bytecodealliance#3506 (comment)), in which we: - Generate a hash (SHA-512) of the ISLE DSL source and produce a "manifest" of ISLE inputs alongside the generated source; and - Always read the ISLE DSL source, and see if the manifest is still valid, on builds that do not have the opt-in "rebuild" feature. This allows us to know whether the ISLE compiler output would have been the same (modulo changes to the DSL compiler itself, which are out-of-scope here), without actually building the ISLE compiler and running it. If the compiler-backend developer modifies an ISLE source file and then tries to build `cranelift-codegen` without adding the `rebuild-isle` Cargo feature, they get the following output: ```text Error: the ISLE source files that resulted in the generated Rust source * src/isa/x64/lower/isle/generated_code.rs have changed but the generated source was not rebuilt! These ISLE source files are: * src/clif.isle * src/prelude.isle * src/isa/x64/inst.isle * src/isa/x64/lower.isle Please add `--features rebuild-isle` to your `cargo build` command if you wish to rebuild the generated source, then include these changes in any git commits you make that include the changes to the ISLE. For example: $ cargo build -p cranelift-codegen --features rebuild-isle (This build script cannot do this for you by default because we cannot modify checked-into-git source without your explicit opt-in.) ``` which will tell them exactly what they need to do to fix the problem! Note that I am leaving the "Rebuild ISLE" CI job alone for now, because otherwise, we are trusting whomever submits a PR to generate the correct generated source. In other words, the manifest is a communication from the checked-in tree to the developer, but we still need to verify that the checked-in generated Rust source and the manifest are correct with respect to the checked-in ISLE source.
- Loading branch information