-
Notifications
You must be signed in to change notification settings - Fork 82
spirv_builder feature compile_codegen to disable building rustc_codegen_spirv
#245
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
Conversation
0f65bdd to
1c0baee
Compare
db5a368 to
5235161
Compare
436846c to
b619665
Compare
8790849 to
b8f659b
Compare
ignore that, my IDE was just derpy I guess |
dd6961d to
eca83df
Compare
|
This is looking great @Firestar99 , what's left? |
|
I'd wait and merge this PR together with Rust-GPU/cargo-gpu#69, in case I find something that still needs adjustment. |
…ven on first completion, code cleanup
f35e492 to
74d1c66
Compare
schell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
|
Something I forgot to mention here: The default |
| # Pinning clap, as newer versions have raised min rustc version without being marked a breaking change | ||
| clap = { version = "=4.5.37", optional = true, features = ["derive"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really annoying, can this constraint be lifted now that the toolchain was upgraded? This prevents me from using newer clap version in the workspace downstream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I seem to remember that I pinned that. Running cargo-gpu against newer rust-gpu versions was failing. cargo-gpu has tests for the last ~2 years of rust-gpu versions. So if you want to bump clap, and cargo-gpu tests pass then I think it's fine. And if they fail, then what @schell and I discussed is that cargo-gpu should aim to support the last 2 versions of rust-gpu. But I'm sure that could be relaxed if there's good reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that you no longer directly depend on old spirv-builder versions since I refactored cargo-gpu, we always use the newest one. Only the codegen backend is now compiled in a potentially quite old toolchain, which doesn't need clap.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already updated in #349, everything looks good so far
See writeup at Rust-GPU/cargo-gpu#69
breaking change:
MetadataPrintoutchanged fromFulltoNone(controls printing ofcargo::rerun-if-changed="path/to/file"for build script)