Skip to content

Conversation

@Firestar99
Copy link
Member

@Firestar99 Firestar99 commented Dec 15, 2025

  • replace enum MetadataPrintout with struct BuildScriptConfig for more flexibility
    • Primarily: spirv_builder.build_script.defaults allows us to add new options like below in the future
    • MetadataPrintout::DependencyOnly is now spirv_builder.build_script.dependency_info = Some(true) and defaults to spirv_builder.build_script.defaults (property above)
    • MetadataPrintout::Full is now spirv_builder.build_script.env_shader_spv_path = Some(true) and defaults to false (preventing conflicts with multimodule)
  • forward rustc warnings and errors as build script warnings (via cargo::warning={})
    • includes support for ansi colors!

Example error

Image cause github doesn't support ansi in code blocks. Also note how in the shader warning, the final "space-engine-shader (lib) generated 1 warning" is missing. This is due to cargo --quiet also suppressing those, next to the other status messages.
image

close #346
close Rust-GPU/cargo-gpu#103

Comment on lines 743 to 783
if self.print_metadata == MetadataPrintout::Full {
println!("cargo:rustc-env={}={}", env_var, spirv_module.display());
if self.build_script.env_shader_spv_path() {
println!(
"cargo::rustc-env=SHADER_SPV_PATH={}",
spirv_module.display()
);
Copy link
Member Author

@Firestar99 Firestar99 Dec 15, 2025

Choose a reason for hiding this comment

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

Since I'm already changing this part, I've considered adding some more ways to "send" the shader binaries from the build script into the source code, like multimodule support for env vars or better source code gen. But I feel like exploring that would be better in a followup PR.

Copy link
Member Author

@Firestar99 Firestar99 Dec 16, 2025

Choose a reason for hiding this comment

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

Note: the code is outdated, I decided to go back on renaming the env var to SHADER_SPV_PATH since it breaks the android build

@Firestar99 Firestar99 marked this pull request as ready for review December 16, 2025 11:40
@LegNeato
Copy link
Collaborator

Would it make sense to split builder into two public crates, one for using in build.rs and one for using from the CLI / other tools?

@Firestar99
Copy link
Member Author

Firestar99 commented Dec 23, 2025

I don't think we should split spirv-builder like that. cargo-gpu being built on top may also be used from different use-cases, and would suddenly need to use two different libraries.

But what does make sense is splitting cargo-gpu into two crates, but differently: One cargo-gpu with all the cli goodness with all it's dependencies, specifically clap. The other cargo-gpu-install that just has the install action, since that's the only thing really needed for build scripts and watch()-ing binaries, and no cli deps. See this issue and the rejected PR1 and PR2. EDIT: here's my attempt at it: Rust-GPU/cargo-gpu#132

Setting a single bool when used from a build script is far simpler, just use whatever method you found in another project, and make sure you adjust this one value to switch between build script and watch in your binary mode. Though I do agree slightly error-prone since it's easy to miss.

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.

Reporting warnings when building in build script Support propagating warnings from build.rs

3 participants