Skip to content

Merge cargo-gpu into rust-gpu#553

Open
Firestar99 wants to merge 181 commits intomainfrom
cargo-gpu
Open

Merge cargo-gpu into rust-gpu#553
Firestar99 wants to merge 181 commits intomainfrom
cargo-gpu

Conversation

@Firestar99
Copy link
Copy Markdown
Member

@Firestar99 Firestar99 commented Apr 8, 2026

Merges the cargo-gpu repository into the rust-gpu repository, to make it easier to edit spirv-builder and cargo-gpu at the same time. It's pretty annoying to juggle two PRs in two different projects, both of which need to pass, and when rust-gpu merges you still have to update the cargo-gpu rev before merging that one. Way easier if they're in the same repo!

This PR conserves the entire history of cargo-gpu by adding it as a git subtree and then moving files into the proper places, which is why this PR is so big. I recommend just reviewing the last commits, prefixed with cargo-gpu:.

I feel like I should ping @schell @tombh so that they're both aware of this.

CI

I've merged the CI actions from cargo-gpu into the rust-gpu ones. The cargo-gpu CI checks backwards compatibility with various old rust-gpu versions, which leads to a significant increase in the amount of jobs. This goes beyond the 20 or so parallel jobs github gives for free, meaning that some jobs must wait on others to complete, increase total CI time by 1-2min at the minimum, maybe more if the windows jobs have to wait. We may want to consider:

  • delaying the fast jobs by a few seconds, similarly to what abundance does, to give the long-running jobs a higher priority
    • Implemented by waiting on the 1min cargo-deny job.
  • only run the backwards compat tests when needed via a file filter on the cargo-gpu, cargo-gpu-install, spirv-builder crates and Cargo.lock
  • or just buying more CI minutes from github

shader-crate-template

I'm not sure if we want to keep the shader-crate-template crate as it is right now:

  • definitely move it out of /crates
  • maybe replace it with our examples
  • It's referencing the rust-gpu repo by git rev instead of local. Changing this requires adjusting the xtask for testing cargo-gpu as well.

I feel like a followup PR would be better suited for this.

schell and others added 30 commits October 19, 2024 08:25
feature: specify spirv-builder source by Cargo.toml dep verbatim
…olchain

feat: use rustup to install toolchain and components
fix: provide spirv-builder with a path to a specific target-spec file
@tombh
Copy link
Copy Markdown

tombh commented Apr 8, 2026

Sounds good!

@Firestar99 Firestar99 force-pushed the cargo-gpu branch 3 times, most recently from 0cc7fc8 to 3fafa5b Compare April 8, 2026 13:29
@Firestar99 Firestar99 marked this pull request as ready for review April 8, 2026 16:34
@LegNeato
Copy link
Copy Markdown
Collaborator

LegNeato commented Apr 9, 2026

Thanks for doing this!

@schell
Copy link
Copy Markdown
Contributor

schell commented Apr 9, 2026

🎉 🙇

@Firestar99
Copy link
Copy Markdown
Member Author

image

There are no conflicts with main, this branch is based on current main. This is github's conflict analysis bugging out, likely due to the subtree merge. Can I get admin perms or someone with admin perms just push this branch to main? @LegNeato @eddyb

@nazar-pc
Copy link
Copy Markdown
Contributor

nazar-pc commented Apr 9, 2026

IMHO rebase is generally a bad approach to merging PRs. With the primary reason being that you can no longer do git bisect after merge on commits from main because despite rebasing cleanly they are no longer guaranteed to work correctly or compile at all. Retaining original commits ensures they all still compile and run as developer intended and tested originally.

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.

10 participants