search

Rust-GPU / rust-gpu

🐉 Making Rust a first-class language and ecosystem for GPU shaders 🚧
https://rust-gpu.github.io
Apache License 2.0
956 stars 27 forks source link

$CARGO_MANIFEST_DIR "leakage" (used at runtime) due to target-specs vendoring. #22

Open abel465 opened 1 month ago

abel465 commented 1 month ago

I cant keep rust-gpu updated for my projects since this commit

Example & Steps To Reproduce

I have made a minimal example to demonstrate

nix run github:abel465/rust-gpu-nixos-example/b117267d254310df77e69debde9e510ed193ff56

The shader builds just fine for the above command. it uses this version of rust-gpu

> nix run github:abel465/rust-gpu-nixos-example/6c9e2ca63da09ab1d119a06a596cd41f84255876
cargo:rerun-if-env-changed=RUSTGPU_CODEGEN_ARGS
cargo:rerun-if-env-changed=RUSTGPU_RUSTFLAGS
error: target path "/build/cargo-vendor-dir/spirv-builder-0.9.0/target-specs/spirv-unknown-vulkan1.1.json" is not a valid file

Caused by:
  No such file or directory (os error 2)
thread 'main' panicked at runner/src/main.rs:17:42:
called `Result::unwrap()` on an `Err` value: BuildFailed
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

The above command is the same except it uses the next commit of rust-gpu which seems to hard code CARGO_MANIFEST_DIR into the binary. This is problematic because the build environment is no longer available during runtime with nix

System Info

schell commented 1 month ago

From what I can tell, this is hooking into the nightly-only feature of target specification explained in this RFC. Here is the relevant part:

Rather than hardcoding a specific set of behaviors per-target, with no recourse for escaping them, the compiler would also use this mechanism when deciding how to build for a given target. The process would look like:

Look up the target triple in an internal map, and load that configuration if it exists. If that fails, check if the target name exists as a file, and try loading that. If the file does not exist, look up .json in the RUST_TARGET_PATH, which is a colon-separated list of directories.

My guess is that CARGO_MANIFEST_DIR is one of the directories in RUST_TARGET_PATH.

I think a possible fix here would be to include_str these json files and then write them into another place on RUST_TARGET_PATH if they don't already exist.

LegNeato commented 1 month ago

Thanks for the report! This does indeed look like a regression.

eddyb commented 1 month ago

My guess is that CARGO_MANIFEST_DIR is one of the directories in RUST_TARGET_PATH.

No, we just pass the full path. The assumption is that spirv-builder's source is available at runtime.

I think a possible fix here would be to include_str these json files and then write them into another place on RUST_TARGET_PATH if they don't already exist.

Sort of. The fundamental issue here is that spirv-builder would need to create either a temporary directory (that can get reused between builds!), or use the target dir (which we don't always control, and changing that would require potentially invoking cargo just to get that one piece of information, before invoking it for the actual build).

One compromise could be a rust-gpu dir in $XDG_CACHE_HOME (defaulting to e.g. ~/.cache), using one directory per target spec file, with the hash of the contents in the dir name (not unlike how /nix/store works).

I'm not 100% convinced we need to support this workflow, and I'm mildly suprised it ever worked in the first place.

I did look into whether it would be possible to ensure the source code doesn't get copied (but instead remains in /nix/store), and, well, the deep copy of $cargoDeps into the build dir is intentional.

Poking around locally (using nix derivation show ...), I found this path:

/nix/store/8li6vf8xg3l8s8dp1fv82fyf0grih4xf-spirv-builder-0.9.0/target-specs/spirv-unknown-vulkan1.1.json

Sadly, the use of a build-local copy cargo-vendor-dir directory as the replacement is hardcoded, even if in theory it could be pointing to.

You should still be able to use rustup (and not build your app w/ Nix) or compile all the shaders in the same Nix derivation (i.e. the intended usecase of using spirv-builder from a build script).

Turns out the workaround you'd need is really simple: https://github.com/eddyb/abel465-rust-gpu-nixos-example/commit/184b4d793431801ad5c73ed5edc3fe11976384bd

dontCargoSetupPostUnpack = true;
postUnpack = ''
  mkdir -p .cargo
  cat "$cargoDeps"/.cargo/config | sed "s|cargo-vendor-dir|$cargoDeps|" >> .cargo/config
  # HACK(eddyb) bypass cargoSetupPostPatchHook.
  export cargoDepsCopy="$cargoDeps"
'';

(thankfully the hooks rely on $cargoDeps i.e. the "template" for cargo-vendor-dir, to already have everything necessary, including .cargo/config, and substituting in the absolute path is enough)

$ nix run github:eddyb/abel465-rust-gpu-nixos-example/184b4d793431801ad5c73ed5edc3fe11976384bd
cargo:rerun-if-env-changed=RUSTGPU_CODEGEN_ARGS
cargo:rerun-if-env-changed=RUSTGPU_RUSTFLAGS
    Finished `release` profile [optimized] target(s) in 0.13s
cargo:rerun-if-changed=/home/eddy/.cache/rust-gpu-shaders/release/deps/libspirv_std_macros-ce236cb9475b6c8d.so
cargo:rerun-if-changed=/home/eddy/.cache/rust-gpu-shaders/spirv-unknown-vulkan1.1/release/deps/libbitflags-07d9e88686e85d9d.rlib
cargo:rerun-if-changed=/home/eddy/.cache/rust-gpu-shaders/spirv-unknown-vulkan1.1/release/deps/libbitflags-07d9e88686e85d9d.rmeta
cargo:rerun-if-changed=/home/eddy/.cache/rust-gpu-shaders/spirv-unknown-vulkan1.1/release/deps/libcompiler_builtins-804091cc4f3bf482.rlib
cargo:rerun-if-changed=/home/eddy/.cache/rust-gpu-shaders/spirv-unknown-vulkan1.1/release/deps/libcore-5f5d87e3b5f8ccd2.rlib
cargo:rerun-if-changed=/home/eddy/.cache/rust-gpu-shaders/spirv-unknown-vulkan1.1/release/deps/libglam-c250e8d47b2492d3.rlib
cargo:rerun-if-changed=/home/eddy/.cache/rust-gpu-shaders/spirv-unknown-vulkan1.1/release/deps/libglam-c250e8d47b2492d3.rmeta
cargo:rerun-if-changed=/home/eddy/.cache/rust-gpu-shaders/spirv-unknown-vulkan1.1/release/deps/liblibm-671ea443fbdaf40b.rlib
cargo:rerun-if-changed=/home/eddy/.cache/rust-gpu-shaders/spirv-unknown-vulkan1.1/release/deps/liblibm-671ea443fbdaf40b.rmeta
cargo:rerun-if-changed=/home/eddy/.cache/rust-gpu-shaders/spirv-unknown-vulkan1.1/release/deps/libnum_traits-b27e0d8a0540a5ab.rlib
cargo:rerun-if-changed=/home/eddy/.cache/rust-gpu-shaders/spirv-unknown-vulkan1.1/release/deps/libnum_traits-b27e0d8a0540a5ab.rmeta
cargo:rerun-if-changed=/home/eddy/.cache/rust-gpu-shaders/spirv-unknown-vulkan1.1/release/deps/librustc_std_workspace_core-e58b1b854099b25e.rlib
cargo:rerun-if-changed=/home/eddy/.cache/rust-gpu-shaders/spirv-unknown-vulkan1.1/release/deps/librustc_std_workspace_core-e58b1b854099b25e.rmeta
cargo:rerun-if-changed=/home/eddy/.cache/rust-gpu-shaders/spirv-unknown-vulkan1.1/release/deps/libspirv_std-48e55c5a5f9f0421.rlib
cargo:rerun-if-changed=/home/eddy/.cache/rust-gpu-shaders/spirv-unknown-vulkan1.1/release/deps/libspirv_std_types-62cd0220e3ac1702.rlib
cargo:rerun-if-changed=/home/eddy/.cache/rust-gpu-shaders/spirv-unknown-vulkan1.1/release/deps/libspirv_std_types-62cd0220e3ac1702.rmeta
cargo:rerun-if-changed=/nix/store/0xjid4ksaqp5psp5rwxvv1wqvsdlyydd-rust-gpu-nixos-example-0.0.0/lib/librustc_codegen_spirv.so
cargo:rerun-if-changed=/nix/store/0xjid4ksaqp5psp5rwxvv1wqvsdlyydd-rust-gpu-nixos-example-0.0.0/repo/shaders/shader/src/lib.rs
cargo:rustc-env=shader.spv=/home/eddy/.cache/rust-gpu-shaders/spirv-unknown-vulkan1.1/release/deps/shader.spv
eddyb commented 1 month ago

I've updated the title to reflect the exact technical problem here. We can keep the issue open in the general sense of the build system kerfuffle (even if @abel465 is satisfied with the above workaround), though I should note that long-term this would be overshadowed by much more drastic changes, e.g.:

abel465 commented 1 month ago

eddyb thanks for the solution!