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

Having spirv-builder / num-traits as a build-dependency in a shader crate fails the shader build #986

Closed
Firestar99 opened this issue Jan 10, 2023 · 3 comments
Labels
t: bug Something isn't working

Comments

@Firestar99
Copy link

Firestar99 commented Jan 10, 2023

Expected Behaviour

Being able to use [build-dependencies] with crates depending on num-traits in shader crates.

Example & Steps To Reproduce

Example Repo: https://github.com/Firestar99/rust-gpu-build-dependecies-bug
(Note that I forced use-installed-tools feature in this repo)

On the master branch you'll have a working rust-gpu setup with /engine being an std rust crate for cpu code and /shader being the no-std shader crate with it's lib.rs being copied from your simplest-shader example. When you cargo run the main function in the engine crate will just build the shaders from the shader crate using spirv_builder.

Now switch to branch broken, where in /shader/Cargo.toml the [build-dependencies] the crate spirv-builder will be added. From what I've understood build-dependencies only affect the dependencies for the build script and are completely independent from the normal dependencies. As the shader crate does NOT even have a build script, this should literally not affect the build in any way. But still building the shader crate (by running the engine crate) fails the build for the crate num-traits:

error[E0463]: can't find crate for `std`
  --> /home/<user>/.cargo/registry/src/github.com-1ecc6299db9ec823/num-traits-0.2.15/src/lib.rs:21:1
   |
21 | extern crate std;
   | ^^^^^^^^^^^^^^^^^ can't find crate
   |
   = note: the `spirv-unknown-vulkan1.1` target may not support the standard library
   = help: consider building the standard library from source with `cargo build -Zbuild-std`
[...]

I would have expected that num-traits would be build twice: first for target "x64" with default features "std" for the build script execution and then again for target "spirv" with features "libm" for the normal cross-compile build. But for some reason that second compile does not seem to happen, and instead cargo/rust decides to reuse the compile / features from the first compile.

Rust-gpu from master branch does not change the result (Branch broken-git-master).

BUT NOTE that copying this example into a rust-gpu checkout and adding the engine and shader crates to the workspace makes it build correctly for whatever reason. Similarly adding the build dependency to any of your shader crates in /examples works just fine. So if you are testing this issue you need to add rust-gpu via a standard dependency and may not have it in your workspace.

Thanks and Have fun cracking this hard nut 😃

System Info

  • Rust: nightly-2022-10-29
  • OS: Ubuntu 22.10 x64
  • GPU: irrelevant
  • SPIR-V: irrelevant
@Firestar99 Firestar99 added the t: bug Something isn't working label Jan 10, 2023
@Firestar99
Copy link
Author

Firestar99 commented Jan 10, 2023

Example use-case

I would like to have my shaders next to my cpu-code and not in two different directories, even if I then have to spam a lot of #[cfg(not(target_arch = "spirv"))] for cpu-only code. Thus I need to put them in the same shader crate. To make this work I added a build script to that crate which builds itself, which would look something like this:

compile "engine" target x64
   compile build script "engine" target native (x64)
      compile "spirv-builder"
         compile "num-traits" default features "std"
   run build script "engine"
       spirv-builder on "engine"
           compile "engine" target spirv
              compile build script "engine" target native (x64)
                 compile "spirv-builder"          // including this causes the build to fail later
                     compile "num-traits" default features "std"
              run build script "engine"
                  detects `env::var("CARGO_CFG_TARGET_ARCH") == "spirv"` and exits to prevent recursion       
                  // the earliest place to exit recursion afaik
              compile "spirv-std"
                  compile "num-traits" features "libm"
                     reuses "num-traits" features "std" // build fail!

To do this I'd need spirv-builder in my [build-dependencies], which would of course be picked up by both compile targets "x64" and "spirv" to get compiled for the native arch. And as build dependencies always compiled for the native arch I cannot easily exclude the dependency in the toml with [target.'cfg(not(target_arch = "spirv"))'.build-dependencies].

One potential workaround idea I have is to add a shader crate, but make it have an empty src and only a build script which uses spirv-builder to build shaders from the engine crate. The engine crate then depends on the shader crate to import any generated symbols / binaries, but only if not building shaders to avoid recursion.

Doesn't work, it fails exactly the same way, see the new branch broken-circular-workaround I made. Any suggestions for any other workarounds are welcome 😄

@eddyb
Copy link
Contributor

eddyb commented Apr 3, 2023

I would like to have my shaders next to my cpu-code and not in two different directories, even if I then have to spam a lot of #[cfg(not(target_arch = "spirv"))] for cpu-only code

This is unsupported and incredibly dangerous/misleading (by accidentally implying things about e.g. data layouts, that are not actually true), as Rust has nothing like CUDA/SYCL's hybrid compilations (and even those are either very hacky or have to force the host to behave more like the device, to ensure compatibility).

IMO Rust-GPU will only be able to officially support this when same-crate proc macros are supported by Rust (which itself would be a huge endeavor), and maybe even that might not be enough.

Far more likely that you'll end up using something like this instead:


BUT NOTE that copying this example into a rust-gpu checkout and adding the engine and shader crates to the workspace makes it build correctly for whatever reason.

That said, this makes me not want to close this issue as wontfix just yet, because this sounds like your Cargo workspace is stuck on Rust 2015 bug-compat mode (and you need to add resolver = "2" to your [workspace] to opt into the Rust 2021 fixes - sadly this is needed with virtual manifests because there is no [package] edition = ...).

@Firestar99
Copy link
Author

Firestar99 commented Apr 3, 2023

Adding this to the root cargo.toml fixes the issue:

[workspace.package]
edition = "2021"

or

[workspace]
resolver = "2"

Some official documentation regarding that would be quite helpful: :D
#1022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants