Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Naga tests don't build without features #7038

Open
jimblandy opened this issue Jan 30, 2025 · 1 comment · May be fixed by #7039
Open

Naga tests don't build without features #7038

jimblandy opened this issue Jan 30, 2025 · 1 comment · May be fixed by #7039
Labels
area: infrastructure Testing, building, coordinating issues type: enhancement New feature or request

Comments

@jimblandy
Copy link
Member

#6938 (use hashbrown in more crates) causes cargo nextest run in the naga subdirectory to fail:

cargo nextest run
   Compiling naga v24.0.0 (/home/jimb/wgpu/naga)
error[E0277]: the trait bound `hashbrown::set::HashSet<spirv::Capability, BuildHasherDefault<rustc_hash::FxHasher>>: Deserialize<'_>` is not satisfied
    --> naga/tests/snapshots.rs:45:19
     |
45   |     capabilities: naga::FastHashSet<spirv::Capability>,
     |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Deserialize<'_>` is not implemented for `hashbrown::set::HashSet<spirv::Capability, BuildHasherDefault<rustc_hash::FxHasher>>`
     |

I think the issue here is that serde itself implements Deserialize for std::hash::HashSet, but not hashbrown::HashSet.

Apparently CI doesn't run Naga tests without features.
@jimblandy jimblandy linked a pull request Jan 30, 2025 that will close this issue
Open
jimblandy added a commit to jimblandy/wgpu that referenced this issue Jan 30, 2025
@jimblandy
[naga] Require hashbrown's serde feature as a dev-dependency.
d04df38 
Add `hashbrown` with the `"serde"` feature as a development dependency
for Naga. Regardless of whether Naga's `deserialize` feature is
enabled, the snapshot tests need to deserialize parameters saved in
files as RON text.

It would also suffice to use `std::collections::HashSet` in the
snapshot tests, although we would need to build a `naga::FashHashSet`
from the std `HashSet` at one point. Adding the dev-dependency seems
slightly simpler.

Fixes gfx-rs#7038.
@jimblandy jimblandy moved this from Todo to In Progress in WebGPU for Firefox Jan 30, 2025
@brody4hire
Copy link
Contributor

My one suggestion is that you can do nextest on a single crate from the project root directory with a command like this:

cargo nextest run -p naga

and yes, this IS also broken by #6938. I would personally favor both updating CI to test these & update README to clarify this. I would be happy to contribute these updates, if needed.

@cwfitzgerald cwfitzgerald added type: enhancement New feature or request area: infrastructure Testing, building, coordinating issues labels Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area: infrastructure Testing, building, coordinating issues type: enhancement New feature or request
Projects
WebGPU for Firefox
Status: In Progress
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[naga] Require hashbrown's serde feature as a dev-dependency. jimblandy/wgpu
3 participants
@jimblandy @brody4hire @cwfitzgerald