Skip to content

Commit

Permalink
Use shader's Cargo.toml for rust-gpu version/toolchain
Browse files Browse the repository at this point in the history
We can still override the version and toolchain with CLI args. This
commit just changes the defaults.

I feel like there could be a lot of edge cases for this implementation.
So I consider it more of a first draft.

Notable changes:
* `Spirv` renamed to `SpirvCLI`.
* New `impl SpirvSource` that does all the shader crate querying.
* CLI args change: `--spirv-builder` is now split into
  `--spirv-builder-source` and `--spirv-builder-version`.
* `--shader-crate` now lives on the `install` subcommand, which
  because it is inherited by `build` (where it was originally), means it
  doesn't make much of a difference.
* The full build test now has a teardown function making it idempotent.
* Cache folder structure now has sub folders for `spirv-builder-cli`'s
  and `rust-gpu` repos.

Things to consider:
* Can we remove `TARGET_SPECS` now, because we have a copy of the
  `rust-gpu` repo, we can get them from there instead?
* We have to copy the `shader-crate-template` outside of the workspace
  to get the tests to run. This is because recent Cargo now uses version
  4 in `Cargo.lock` and the shader crate toolchain uses an older version
  of Cargo that doesn't recognise that version. But that doesn't seem
  right. Why is Cargo looking outside of the `shader-crate-template`
  anyway??
* What's the UX expectations for changing `spirv-std` versions or
  overriding `rust-gpu` versions and toolchain? I think in most cases
  `spirv-builder-cli` rebuilds should occur because the changes will
  cause cache directory changes. But I think it'd be good to at least
  manually test this.
* Should the main build test be moved out into an integrations tests
  folder? Also it'd be good to have each test run in its own cache
  folder so that we can run more than one full build test at once. I'd
  still like to test each `spirv-builder-cli` feature, ie:
  `spirv-builder-pre-cli` and `spirv-builder-0_10`. We're currently only
  testing `spirv-builder-pre-cli`.
  • Loading branch information
tombh committed Dec 17, 2024
1 parent 87de5da commit 2f19f07
Show file tree
Hide file tree
Showing 11 changed files with 796 additions and 196 deletions.
209 changes: 203 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ relative-path = "1.9.3"
serde = { version = "1.0.214", features = ["derive"] }
serde_json = "1.0.132"
toml = "0.8.19"
test-log = "0.2.16"

[workspace.lints.rust]
missing_docs = "warn"
Expand All @@ -46,7 +47,9 @@ pattern_type_mismatch = { level = "allow", priority = 1 }
print_stdout = { level = "allow", priority = 1 }
std_instead_of_alloc = { level = "allow", priority = 1 }

# TODO: Try to not depend on these lints
# TODO: Try to not depend on allowing these lints
unwrap_used = { level = "allow", priority = 1 }
get_unwrap = { level = "allow", priority = 1 }
expect_used = { level = "allow", priority = 1 }
panic = { level = "allow", priority = 1 }

Loading

0 comments on commit 2f19f07

Please sign in to comment.