Stream: git-wasmtime

Topic: wasmtime / PR #2957 Cranelift: restrict running tests dep...

view this post on Zulip Wasmtime GitHub notifications bot (Jun 01 2021 at 11:47):

olivierlemasle opened PR #2957 from tests-no-x86_64 to main:

These 5 tests fail with the error "This test requires x86_64 support." when
executed on another architecture.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 01 2021 at 11:48):

olivierlemasle edited PR #2957 from tests-no-x86_64 to main:

Restrict running tests dependant of x86_64

These 5 tests fail with the error "This test requires x86_64 support." when executed on another architecture.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 01 2021 at 13:18):

bnjbvr submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 01 2021 at 13:18):

bnjbvr submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 01 2021 at 13:18):

bnjbvr created PR review comment:

As bjorn3 pointed out, you might need something like #[cfg_attr(feature = "x86", ignore)] here and below instead.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 01 2021 at 13:22):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 01 2021 at 13:22):

bjorn3 created PR review comment:

I think you are missing a not().

view this post on Zulip Wasmtime GitHub notifications bot (Jun 01 2021 at 13:27):

olivierlemasle edited PR #2957 from tests-no-x86_64 to main.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 01 2021 at 13:30):

olivierlemasle submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 01 2021 at 13:30):

olivierlemasle created PR review comment:

Thank you! However, x86 is a feature of cranelift-codegen, not cranelift-frontend. Is there a way to check from the _frontend_ crate if the x86 backend is available?
(sorry if it's a newbie question!)

view this post on Zulip Wasmtime GitHub notifications bot (Jun 01 2021 at 13:39):

bnjbvr edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 01 2021 at 14:07):

bnjbvr created PR review comment:

Ah, it's a very good question. This particular crate shouldn't have tests taht depend on specific features of other crates, otherwise they wouldn't be run by our automation.

The behavior of cranelift-codegen, in the absence of specifying a target architecture for which one would like to compile, is to target the host architecture (this happens in cranelift-codegen's build.rs script). But that doesn't _enable_ the Cargo feature either, so we couldn't even guard against the cranelift-codegen's x86 feature here.

So I think that checking against the target_arch is actually the right way to go: on a x86_64 host, the x86_64 target will be enabled by default. Sorry for the back and forth here, you were probably right in the first place :)
Would still be nice to use the cfg_attr directive instead.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 01 2021 at 14:07):

bnjbvr submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 02 2021 at 17:02):

olivierlemasle updated PR #2957 from tests-no-x86_64 to main.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 02 2021 at 17:03):

olivierlemasle created PR review comment:

Thanks, I've updated the PR accordingly.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 02 2021 at 17:03):

olivierlemasle submitted PR review.

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

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 02 2021 at 17:35):

abrown submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 02 2021 at 17:35):

abrown merged PR #2957.

Last updated: Nov 22 2024 at 17:03 UTC