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

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

Open
abel465 opened this issue Oct 1, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@abel465
Copy link

abel465 commented Oct 1, 2024

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

  • Rust: 1.78.0-nightly (3b1717c05 2024-03-10)
@schell
Copy link
Contributor

schell commented Oct 1, 2024

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 LegNeato added the bug Something isn't working label Oct 1, 2024
@LegNeato
Copy link
Collaborator

LegNeato commented Oct 1, 2024

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

@eddyb
Copy link
Collaborator

eddyb commented Oct 7, 2024

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: eddyb/abel465-rust-gpu-nixos-example@184b4d7

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 eddyb changed the title Can no longer package with nix $CARGO_MANIFEST_DIR "leakage" (used at runtime) due to target-specs vendoring. Oct 7, 2024
@eddyb
Copy link
Collaborator

eddyb commented Oct 7, 2024

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
Copy link
Author

abel465 commented Oct 8, 2024

eddyb thanks for the solution!

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

No branches or pull requests

4 participants