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?
* 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 39932ec
Show file tree
Hide file tree
Showing 12 changed files with 813 additions and 203 deletions.
18 changes: 11 additions & 7 deletions .github/workflows/push.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@ env:
jobs:
install-and-build-shaders:
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]
os:
- ubuntu-latest
- macos-latest
- windows-latest
runs-on: ${{ matrix.os }}
defaults:
run:
Expand All @@ -27,15 +31,15 @@ jobs:
- run: rustup default stable
- run: rustup update
- run: cargo test
- run: cargo install --path crates/cargo-gpu
- run: cargo gpu install
- run: cargo gpu build --shader-crate crates/shader-crate-template --output-dir test-shaders
- run: ls -lah test-shaders
- run: cat test-shaders/manifest.json
# - run: cargo install --path crates/cargo-gpu
# - run: cargo gpu install
# - run: cargo gpu build --shader-crate crates/shader-crate-template --output-dir test-shaders
# - run: ls -lah test-shaders
# - run: cat test-shaders/manifest.json

clippy:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: moonrepo/setup-rust@v1
- run: cargo clippy
- run: cargo clippy -- --deny warnings
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 39932ec

Please sign in to comment.