Running ./x test compiler only runs unit tests for rustc_codegen_cranelift

[Zalathar]

In theory, this command should run cargo test for all of the compiler crates in compiler/:

# (stage 0 isn't essential, but it's faster than using a later stage)
./x test compiler --stage=0

Instead, it just prints this message and exits successfully:

cranelift not in rust.codegen-backends. skipping

---

This seems to be because test::CodegenCranelift::should_run registers compiler/rustc_codegen_cranelift as its path, which has the side-effect of hijacking compiler in bootstrap's wacky path-resolution logic. That prevents test::CrateLibrustc from running, which is what would run the unit tests for the vast majority of compiler crates.

The same problem is also theoretically present for test::CodegenGCC, though currently the cranelift step takes priority for whatever reason.

[lqd]

Is this only locally or are tests now also ignored on CI? This must be a somewhat recent regression, I remember this working before; some bisection would be good I guess.

[Zalathar]

Is this only locally or are tests now also ignored on CI? This must be a somewhat recent regression, I remember this working before; some bisection would be good I guess.

I think this only affects manually running ./x test compiler; jobs that just do ./x test and rely on the defaults should be unaffected.

[Zalathar]

I haven't done a bisection, but based on my fix, it seems likely that this has been broken ever since cg_clif tests were added to bootstrap.

[jyn514]

This seems to be because test::CodegenCranelift::should_run registers compiler/rustc_codegen_cranelift as its path, which has the side-effect of hijacking compiler in bootstrap's wacky path-resolution logic

i suspect this is a new regression since bootstrap started supporting prefixes as well as suffixes. cc #133492, @MarcoIeni

[MarcoIeni]

Is this only locally or are tests now also ignored on CI?

In CI we run tests with the --skip mechanism. So maybe this bug doesn't affect CI.

i suspect this is a new regression since bootstrap started supporting prefixes as well as suffixes. cc #133492, @MarcoIeni

It might be! (I haven't understood the bug completely, I just checked this issue quickly).
Hopefully the bisection process will verify this 👍

A quick remediation might be only applying the login introduced in that PR only to --skip maybe.

[jieyouxu]

It might be! (I haven't understood the bug completely, I just checked this issue quickly).

Having PathSet::check also match on prefixes in addition to suffixes might be somewhat problematic, see #134919 (comment) for more detailed explanation.

[Zalathar]

I can confirm that reverting #133492 seems to fix this, without having to reorder the test steps as in #134919 (though there's no harm in reordering them either, so it's fine for that PR to land).

So this suggests that a revert would probably be a good idea in the short term, though we would want @MarcoIeni 's input on how to avoid breaking #134427.