Skip to content

Conversation

timon-schelling
Copy link
Collaborator

@timon-schelling timon-schelling commented Aug 7, 2025

Requires #3096 and #3103

And we would follow a non-main rust-gpu rev currently!

@Firestar99
Copy link
Collaborator

I'd merge this with the shaders togehter, since we'll be jumping rev quite a few more times

@timon-schelling
Copy link
Collaborator Author

timon-schelling commented Aug 11, 2025

I suggest merging this separately and updating the rev in your pr, this should not have a influence on other devs.

@timon-schelling
Copy link
Collaborator Author

@Firestar99 I will merge this right before #2985 when we actually need it.

@Firestar99 Firestar99 force-pushed the provide-cargo-rust-gpu-to-nix-devs branch from b9dde9a to 75bbe25 Compare August 14, 2025 12:44
@Keavon Keavon changed the title Provide cargo rust-gpu env to nix devs Add a Nix configuration for the cargo rust-gpu environment Aug 18, 2025
@Firestar99 Firestar99 marked this pull request as draft August 18, 2025 17:20
@Firestar99
Copy link
Collaborator

Converting to draft to prevent merging, for now, as I'm may have to update the rust-gpu rev quite a bit.

.nix/flake.nix Outdated
Comment on lines 66 to 75
execWithRustGPUEnvironment = pkgs.writeShellScriptBin "rust-gpu" ''
#!${pkgs.lib.getExe pkgs.bash}

filtered_args=()
for arg in "$@"; do
case "$arg" in
+nightly|+nightly-*) ;;
*) filtered_args+=("$arg") ;;
esac
done

export PATH="${pkgs.lib.makeBinPath [ rustGPUToolchainPkg pkgs.spirv-tools ]}:$PATH"
export RUSTC_CODEGEN_SPIRV_PATH="${rustc_codegen_spirv}/lib/librustc_codegen_spirv.so"

exec ${"\${filtered_args[@]}"}
'';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's so much easier to use RUSTUP_TOOLCHAIN for this, I was just asking @Firestar99 why the + syntax when the env var also works.

All you need in the script, if you want to have dynamic dispatch, is:

''
  #!${pkgs.lib.getExe pkgs.bash}
  exec "${toolchains}/${"\$"}{RUSTUP_TOOLCHAIN:-stable}/bin/$(basename "$0")" "$@"
''

(I don't like that ${"\$"} but am not sure I can make it much nicer)

I suppose something different than what you are doing today is you would want to define toolchains as one of those sym trees with one directory per supported toolchain - maybe the oxalica overlay rust-bin has something along those lines? (it's been a while since I looked at it anyway)


Although, if cargo-gpu/spirv-builder switched to setting RUSTUP_TOOLCHAIN, the fact that this script only has to filter out the + syntax, makes me think the script can just go away? (since I guess there is no dynamic dispatch right now, just a pesky first argument)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want devs mistaking the pinned rust-gpu rust nightly toolchain for a normal nightly toolchain. We want a way to ensure that it is only ever usable to compile rust-gpu code. A wrapper script is the best way to ensure that in my opinion.

But getting rid of the filter logic would be great. Thanks for the comment.

Copy link
Collaborator

@Firestar99 Firestar99 Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just want to note that on master, we kill the RUSTUP_TOOLCHAIN env var. I'm unsure if my branch has these changes already or not, so we could see behaviour differences between now and me rebasing the branch.

https://github.com/Rust-GPU/rust-gpu/blob/e0dfdd1112d9b2cf27d10e25104dc997db9acdf0/crates/spirv-builder/src/cargo_cmd.rs#L52

Yes we do need a better solution for this, but since this has to work within the next week, it'll have to wait.

@Firestar99 Firestar99 force-pushed the provide-cargo-rust-gpu-to-nix-devs branch from 75bbe25 to 17e8fd1 Compare August 26, 2025 18:00
@Firestar99 Firestar99 changed the base branch from master to shaders_table_color_to_color August 26, 2025 18:00
@0HyperCube 0HyperCube force-pushed the shaders_table_color_to_color branch from ace6707 to 9735bfe Compare August 26, 2025 18:29
@Firestar99 Firestar99 force-pushed the shaders_table_color_to_color branch from 9735bfe to ad83e7f Compare August 26, 2025 19:26
@0HyperCube 0HyperCube force-pushed the shaders_table_color_to_color branch from ad83e7f to 9735bfe Compare August 26, 2025 20:52
@Firestar99 Firestar99 force-pushed the shaders_table_color_to_color branch from 66f9e4c to 6c9ee70 Compare August 27, 2025 09:43
@Firestar99 Firestar99 force-pushed the provide-cargo-rust-gpu-to-nix-devs branch from 17e8fd1 to c2f4f47 Compare August 27, 2025 12:08
@Firestar99 Firestar99 changed the title Add a Nix configuration for the cargo rust-gpu environment Shaders: Nix configuration for the rust-gpu toolchain Aug 27, 2025
@Firestar99 Firestar99 force-pushed the shaders_table_color_to_color branch from 6c9ee70 to b68c3dc Compare August 28, 2025 07:32
Base automatically changed from shaders_table_color_to_color to master August 28, 2025 13:16
@Firestar99 Firestar99 force-pushed the provide-cargo-rust-gpu-to-nix-devs branch from c2f4f47 to 0fd1e87 Compare August 28, 2025 13:43
@Firestar99 Firestar99 force-pushed the provide-cargo-rust-gpu-to-nix-devs branch from 0fd1e87 to 63d832e Compare August 29, 2025 11:07
@Firestar99 Firestar99 force-pushed the provide-cargo-rust-gpu-to-nix-devs branch from 63d832e to 0bdc23d Compare August 30, 2025 12:34
@Firestar99 Firestar99 marked this pull request as ready for review August 30, 2025 12:56
@Firestar99 Firestar99 requested a review from Keavon August 30, 2025 12:56
@timon-schelling timon-schelling removed the request for review from Keavon August 30, 2025 16:35
@timon-schelling timon-schelling changed the title Shaders: Nix configuration for the rust-gpu toolchain Add nix configuration for the rust-gpu toolchain Aug 30, 2025
@timon-schelling timon-schelling merged commit 101ae6d into master Aug 30, 2025
4 checks passed
@timon-schelling timon-schelling deleted the provide-cargo-rust-gpu-to-nix-devs branch August 30, 2025 16:42
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.

4 participants