Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable multivalue in tests #4216

Closed
wants to merge 5 commits into from

Conversation

RunDevelopment
Copy link
Contributor

@RunDevelopment RunDevelopment commented Oct 20, 2024

I couldn't really find a lot of docs on how to disable this, so I spend like an hour figuring this out.

I also slightly changed how reference.rs determines the repo root, test file dir, and target dir. I needed to do this, because debugging in VSCode sets the cwd to the workspace root by default, which messed up all paths. Now debugging this just works.

I also changed the target dir for the references. This stops the cache of all crates from constantly being invalidated, which massively speeds up repeated runs of tests and rust analyzer for me.

@RunDevelopment
Copy link
Contributor Author

Okay, after some more local testing, I could not find a way to remove the multivalue and reference-types features. Even setting RUSTFLAGS to -C target-feature=-multivalue does nothing. And I know that I'm using RUSTFLAGS correctly, because changing to add a target feature (e.g. simd128) does work as expected.

@RunDevelopment
Copy link
Contributor Author

Okay, rust-lang/rust#109807 and rust-lang/rust#128511 have a similar issue, and the solution is that std needs to re-built to disable default features. However, this is (AFAIK) only possible with a nightly flag: -Zbuild-std.

So we are pretty much stuck with multivalue and reference-types, if understand this correctly. This poses a significant problem though, because it means that Ci won't test code with those features missing anymore, so we might break older compiler version unintentionally.

Not sure what the best path forward here is. If it is decided that CI tests default target features, then I'll change the PR to add those target features in reference.rs so the output of different compiler versions is consistent (since contributor may not always run the latest compiler).


To prevent such issues in the future, it might also be a good idea to run some tests on the beta channel of the compiler to give us 6 weeks time to detect and fix similar problems. E.g. this would have prevented #4207.

@daxpedda
Copy link
Collaborator

Yes, expanding our testing to older compiler versions (maybe just MSRV and beta) would be great, I'm happy to look at any improvements!

We should also look into testing with wasm32v1-none ASAP.

I'm going to close this in favor of #4235, but let me know if you want me to re-open if you want to get the other adjustments in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants