Skip to content

Commit

Permalink
Cranelift ISLE build script: make manifest check optional, to avoid d…
Browse files Browse the repository at this point in the history
…ependencies where needed.

Some use-cases are very sensitive to the number of crates pulled in,
both for audit-related reasons and for build-time reasons.

Our manifest-based ISLE-up-to-date approach in #3534 was meant to help
with this while still avoiding the footgun of a completely opt-in source
rebuild: by including generated source in the checked-in tree, and just
checking that source is up to date, we allow for fast builds without the
whole ISLE compiler meta-step, but still catch attempted use of stale
generated source (and allow the developer to opt-in to regenerating the
in-tree source).

Unfortunately this still requires the hash algorithm itself (`sha2`)
which turns out to pull in a number of other small crates. In cases
where we know the source won't be stale -- for example, depending on the
`main` branch in git, or a published crate version of
`cranelift-codegen` -- the checks are actually not needed at all.

This PR thus introduces a feature `check-isle` that controls whether
`build.rs` does the checks. It is on by default, so developer safety
remains: if someone checks out the source, modifies some ISLE, and then
does a `cargo build`, they will get an error that helps them with the
proper steps to regenerate the source. But, dependencies that know what
they are doing can turn it off with `default-features = false`.

I've verified that we have the expected small dependency tree now:

```
$ cargo tree --depth 1 -p cranelift-codegen --no-default-features --features "std unwind all-arch"
cranelift-codegen v0.79.0 (/home/cfallin/work/wasmtime/cranelift/codegen)
├── cranelift-bforest v0.79.0 (/home/cfallin/work/wasmtime/cranelift/bforest)
├── cranelift-codegen-shared v0.79.0 (/home/cfallin/work/wasmtime/cranelift/codegen/shared)
├── cranelift-entity v0.79.0 (/home/cfallin/work/wasmtime/cranelift/entity)
├── gimli v0.26.1
├── log v0.4.14
├── regalloc v0.0.33
├── smallvec v1.6.1
└── target-lexicon v0.12.0
[build-dependencies]
├── cranelift-codegen-meta v0.79.0 (/home/cfallin/work/wasmtime/cranelift/codegen/meta)
└── sha2 v0.9.8
[dev-dependencies]
└── criterion v0.3.5
```

Fixes #3609.
  • Loading branch information
cfallin committed Dec 17, 2021
1 parent e94ebc2 commit 19203e5
Show file tree
Hide file tree
Showing 3 changed files with 342 additions and 308 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,8 @@ jobs:
submodules: true
- name: Install Rust
run: rustup update stable && rustup default stable
- run: cd cranelift/codegen && cargo build --features "all-arch completely-skip-isle-for-ci-deterministic-check"
# Run without `check-isle` feature (see note in `cranelift/codegen/build.rs` for why).
- run: cd cranelift/codegen && cargo build --no-default-features --features "std unwind all-arch"
- run: ci/ensure_deterministic_build.sh

# Perform release builds of `wasmtime` and `libwasmtime.so`. Builds on
Expand Down
16 changes: 14 additions & 2 deletions cranelift/codegen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ miette = { version = "3", features = ["fancy"], optional = true }
sha2 = "0.9.8"

[features]
default = ["std", "unwind"]
default = ["std", "unwind", "check-isle"]

# The "std" feature enables use of libstd. The "core" feature enables use
# of some minimal std-like replacement libraries. At least one of these two
Expand All @@ -58,6 +58,13 @@ testing_hooks = []
# This enables unwind info generation functionality.
unwind = ["gimli"]

# The "check-isle" feature checks that ISLE-generated source is
# up-to-date during build, according to a manifest of file hashes
# created the last time the source was regenerated. Use the
# "rebuild-isle" feature (which depends on this feature) to
# additionally regenerate the source.
check-isle = []

# ISA targets for which we should build.
# If no ISA targets are explicitly enabled, the ISA target for the host machine is enabled.
x86 = []
Expand Down Expand Up @@ -91,7 +98,12 @@ regalloc-snapshot = ["bincode", "regalloc/enable-serde"]
souper-harvest = ["souper-ir", "souper-ir/stringify"]

# Recompile ISLE DSL source files into their generated Rust code.
rebuild-isle = ["cranelift-isle", "miette", "cranelift-codegen-meta/rebuild-isle"]
rebuild-isle = [
"check-isle",
"cranelift-isle",
"miette",
"cranelift-codegen-meta/rebuild-isle",
]

# A hack to skip the ISLE-rebuild logic when testing for determinism
# with the "Meta deterministic check" CI job.
Expand Down
Loading

0 comments on commit 19203e5

Please sign in to comment.