Skip to content

Conversation

tuguzT
Copy link
Contributor

@tuguzT tuguzT commented Jul 16, 2025

Requires Rust-GPU/rust-gpu#335

See that PR for details as well

Resolves #93
Resolves #77

@tuguzT tuguzT changed the title Make toolchain & codegen installation succeed when called by Miri Make toolchain & backend installation succeed when called by Miri Jul 16, 2025
@tuguzT tuguzT changed the title Make toolchain & backend installation succeed when called by Miri Make backend installation succeed when called by Miri Jul 16, 2025
@tuguzT tuguzT marked this pull request as ready for review July 16, 2025 14:54
@Firestar99 Firestar99 self-requested a review July 16, 2025 21:52
@Firestar99
Copy link
Member

Firestar99 commented Jul 17, 2025

Miri works, but ran into some trouble with clippy: The install action when using a local path to rust-gpu seems to use the wrong toolchain for some reason. Will have a go at it myself.

[2025-07-17T08:52:22Z DEBUG cargo_gpu::install] building artifacts with `cd "/home/firestar99/workspace/frameworks/rust-gpu" && env -u CARGO_CFG_FEATURE [...] "cargo" "+nightly-2025-06-23" "build" "--release" "-p" "rustc_codegen_spirv" "--lib"`
[...]
error[E0514]: found crate `strum` compiled by an incompatible version of rustc
 --> crates/rustc_codegen_spirv-target-specs/src/lib.rs:5:5
  |
5 | use strum::{Display, EnumIter, EnumString, IntoStaticStr};
  |     ^^^^^
  |
  = note: the following crate versions were found:
          crate `strum` compiled by rustc 1.89.0-nightly (be19eda0d 2025-06-22): /home/firestar99/workspace/frameworks/rust-gpu/target/release/deps/libstrum-23be4bc9b25a9a01.rmeta
  = help: please recompile that crate using this compiler (rustc 1.88.0 (6b00bc388 2025-06-23)) (consider running `cargo clean` first)

@Firestar99 Firestar99 changed the title Make backend installation succeed when called by Miri Make backend installation succeed when called by Miri or Clippy Jul 17, 2025
@tuguzT
Copy link
Contributor Author

tuguzT commented Jul 17, 2025

The install action when using a local path to rust-gpu seems to use the wrong toolchain for some reason.

Having the same problem with Clippy.

Additionally, when testing locally just now (with rust-gpu pulled from Rust-GPU/rust-gpu#335) I've noticed that Clippy continues to fail for some reason with the same message on build phase (but Miri works!):

  error: failed to run `rustc` to learn about target-specific information

  Caused by:
    process didn't exit successfully: `C:\Users\tuguzT\.rustup\toolchains\stable-x86_64-pc-windows-msvc\bin\clippy-driver.exe C:\Users\tuguzT\.rustup\toolchains\nightly-2025-06-23-x86_64-pc-windows-msvc\bin\rustc.exe - --crate-name ___ --print=file-names -Zcodegen-backend=C:\Users\tuguzT\AppData\Local\rust-gpu\codegen\https___github_com_tuguzT_rust-gpu+b11f13bd\rustc_codegen_spirv.dll -Zbinary-dep-depinfo -Csymbol-mangling-version=v0 -Zcrate-attr=feature(register_tool) -Zcrate-attr=register_tool(rust_gpu) -Coverflow-checks=off -Cdebug-assertions=off -Zinline-mir=off -Zmir-enable-passes=-GVN -Zshare-generics=off -Cllvm-args=--spirv-metadata=full --target \\?\C:\Users\tuguzT\AppData\Local\rust-gpu\codegen\https___github_com_tuguzT_rust-gpu+b11f13bd\target-specs\spirv-unknown-vulkan1.2.json --crate-type bin --crate-type rlib --crate-type dylib --crate-type cdylib --crate-type staticlib --crate-type proc-macro --print=sysroot --print=split-debuginfo --print=crate-name --print=cfg -Wwarnings` (exit code: 1)
    --- stderr
    error: the option `Z` is only accepted on the nightly compiler

    help: consider switching to a nightly toolchain: `rustup default nightly`

    note: selecting a toolchain with `+toolchain` arguments require a rustup proxy; see <https://rust-lang.github.io/rustup/concepts/index.html>

    note: for more information about Rust's stability policy, see <https://doc.rust-lang.org/book/appendix-07-nightly-rust.html#unstable-features>

    error: 1 nightly option were parsed

  Error: BuildFailed

So, removing RUSTC_WRAPPER might be not enough for Clippy 🥲.

@Firestar99
Copy link
Member

Cargo clippy now works as well. I also unified env var cleaning in spirv-builder and added env var debug printing, so any issues like this should be easier to debug in the future.

@tuguzT feel free to give this branch another try :D

@tuguzT
Copy link
Contributor Author

tuguzT commented Jul 17, 2025

@Firestar99, thanks, I'll try this out until tomorrow

@Firestar99 Firestar99 changed the title Make backend installation succeed when called by Miri or Clippy Fix cargo-gpu in build script failing when called by Miri or Clippy Jul 17, 2025
@tuguzT
Copy link
Contributor Author

tuguzT commented Jul 17, 2025

I tested this PR (and rust-gpu one, too) on the sample project mentioned in the issue and on my own project in which I originally noticed this issue...
And it is working flawlessly!

Additionally, I haven't known it is possible to use PR's git commit hash when linking to the original repository, could have simplified testing for me if I had known it earlier.
Thanks for fixing these PRs!

@Firestar99
Copy link
Member

Firestar99 commented Jul 18, 2025

You can also use local paths like spirv-std = { path = "../rust-gpu/crates/spirv-std" } and it'll even rebuild the codegen backend within the same target dir to make it as easy as possible to dev and immediately test in another project :D

@Firestar99
Copy link
Member

FYI @tuguzT, we need to wait for rust-gpu's PR to merge anyway before we can update the spirv-builder rev to a commit on master (since the commits will be rebased)

@tuguzT
Copy link
Contributor Author

tuguzT commented Jul 21, 2025

Oh, I see...
I think force-push of this commit with another rust-gpu's git commit hash will do the trick (later)

@Firestar99
Copy link
Member

I was about to do that 🤣

@Firestar99 Firestar99 merged commit 98931f8 into Rust-GPU:main Jul 22, 2025
6 checks passed
@nazar-pc
Copy link
Contributor

This is great! I was also hoping this would also generate build script warnings when clippy is running for target_arch = "spirv", but currently it doesn't. Am I asking for too much?

@Firestar99
Copy link
Member

Firestar99 commented Jul 23, 2025

I don't think anyone has tried to run clippy with the spirv target. Technically, clippy doesn't need a codegen backend, it only needs rustc to understand that the spirv targets exist, that would be the only thing our codegen backend would be needed for.
For now, clippy was typically run on a CPU target, which would unfortunately ignore all #[gpu-only] functions and entry points.
Feel free to open an issue and post any potential research results there :D

@nazar-pc
Copy link
Contributor

Should I post it in this repo or rust-gpu?

Right now I only see warnings (like Rust-GPU/rust-gpu#324, which might have unexpected consequences) when I also get compilation errors, but I'd love to see them all the time. Both for cargo check and cargo clippy.

@Firestar99
Copy link
Member

I'd open it on rust-gpu, since it would be useful over there as well.

The problem with build scripts is that you only ever see their output if they fail. So many add #![deny(warnings)] to their lib.rs

@nazar-pc
Copy link
Contributor

Actually you can propagate warnings using cargo::warning=MESSAGE, which seems more relevant to cargo-gpu.

#![deny(warnings)] is a workaround, but often I do want for code to compile despite warnings, but I want to know that warnings are present too.

@Firestar99
Copy link
Member

To not spam this discussion, I created two issues:

@nazar-pc
Copy link
Contributor

I also opened #103 already

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.

cargo-gpu in build scripts breaks clippy and miri build script: cargo clippy fails
3 participants