Stream: git-wasmtime

Topic: wasmtime / issue #3609 Cranelift: sha2 dependency introdu...

view this post on Zulip Wasmtime GitHub notifications bot (Dec 16 2021 at 17:26):

bjorn3 opened issue #3609:

   Compiling version_check v0.9.3
   Compiling typenum v1.14.0
   Compiling opaque-debug v0.3.0
   Compiling cpufeatures v0.2.1
   Compiling generic-array v0.14.4
   Compiling block-buffer v0.9.0
   Compiling digest v0.9.0
   Compiling sha2 v0.9.8

This regresses compilation time by a couple of seconds and requires me to add them to the list of permitted dependencies in rustc at https://github.com/rust-lang/rust/blob/f8402169aaa12e7bbb9630796a8caec90a3055ca/src/tools/tidy/src/deps.rs#L236

view this post on Zulip Wasmtime GitHub notifications bot (Dec 17 2021 at 00:04):

sunfishcode commented on issue #3609:

sha2 and its dependencies already appear in the PERMITTED_DEPENDENCIES list. Is it valuable for cranelift to avoid dependencies that rustc itself already depends on?

view this post on Zulip Wasmtime GitHub notifications bot (Dec 17 2021 at 06:32):

bjorn3 commented on issue #3609:

I'm trying to keep the dependency list as small as reasonably possible.

view this post on Zulip Wasmtime GitHub notifications bot (Dec 17 2021 at 06:50):

cfallin commented on issue #3609:

That's definitely a good first-order principle and I share that instinct too -- but in this case I think it's worthwhile to see the context: computing a hash in the build script and checking against the manifest lets us avoid building the whole ISLE compiler, so it's replacing an even larger dependency tree.

The best option w.r.t. dependency-set size is to build without any up-to-date checks at all, but that's a huge developer-friction issue if it is the default when hacking away in a local checkout. (Forget rebuild-isle, spend time debugging mysterious issue, discover source was never updated, etc.)

One option that may be reasonable is to put the manifest check under an on-by-default feature (say check-isle-up-to-date) so that we have safety / least-surprising behavior by default, but dependents that care strongly about minimal dependencies can specify default-features = false and then turn back on the features they need. The crate as uploaded to crates.io or on main in the git repo should always have up-to-date generated source, so skipping the checks (and thus sha512-hashing) is safe. Thoughts?

view this post on Zulip Wasmtime GitHub notifications bot (Dec 17 2021 at 09:24):

bjorn3 commented on issue #3609:

One option that may be reasonable is to put the manifest check under an on-by-default feature (say check-isle-up-to-date)

That sounds great!

view this post on Zulip Wasmtime GitHub notifications bot (Dec 17 2021 at 21:15):

cfallin closed issue #3609:

   Compiling version_check v0.9.3
   Compiling typenum v1.14.0
   Compiling opaque-debug v0.3.0
   Compiling cpufeatures v0.2.1
   Compiling generic-array v0.14.4
   Compiling block-buffer v0.9.0
   Compiling digest v0.9.0
   Compiling sha2 v0.9.8

This regresses compilation time by a couple of seconds and requires me to add them to the list of permitted dependencies in rustc at https://github.com/rust-lang/rust/blob/f8402169aaa12e7bbb9630796a8caec90a3055ca/src/tools/tidy/src/deps.rs#L236

Last updated: Nov 22 2024 at 16:03 UTC