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.

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

Fix wasm examples failing to compile #2524

Merged
merged 2 commits into from
Mar 6, 2022

Conversation

Liamolucko
Copy link
Contributor

Description
The web examples were broken by 476b6a1, because apparently enabling a feature in any package enables it for the whole workspace, and the wasm examples fail to compile with Vulkan enabled.

It wasn't failing in CI because CI specifies -p wgpu, which seems to stop wgpu-info's enabling of features having an effect.

Testing
If you try to run a wasm example on master, it'll fail to compile due to libloading being included as a part of vulkan support, which doesn't support wasm. With this change, it works again.

They were broken by 476b6a1, because apparently enabling a feature in any package enables it for the whole workspace, and the wasm examples fail to compile with vulkan enabled.

It wasn't failing in CI because CI specifies `-p wgpu`, which seems to stop `wgpu-info`'s enabling of features having an effect.
@cwfitzgerald
Copy link
Member

That's some wild stuff, my worry though now is that cargo build --target wasm32-unknown-unknown will now fail to compile as wgpu-info won't compile. Maybe the solution is to have the features enabled on non-wasm only (so have two wgpu deps, one unconditionally with no features, and one with these two features)

@kvark
Copy link
Member

kvark commented Mar 6, 2022

@cwfitzgerald do we need wgpu-info on the Web at all? If not, we could add a top-level guard on wgpu-info's code to only work on native, and then it will happily compile (as an empty crate) for the web.

@cwfitzgerald
Copy link
Member

No we don't, so that would be a perfectly viable solution too.

@kvark
Copy link
Member

kvark commented Mar 6, 2022

@Liamolucko would you be open to making this change to wgpu-info, making it an empty crate for wasm targets?

@Liamolucko
Copy link
Contributor Author

@Liamolucko would you be open to making this change to wgpu-info, making it an empty crate for wasm targets?

Okay, done.

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cwfitzgerald cwfitzgerald enabled auto-merge (squash) March 6, 2022 04:36
@cwfitzgerald cwfitzgerald merged commit 0ac9ce0 into gfx-rs:master Mar 6, 2022
@Liamolucko Liamolucko deleted the wasm-no-vulkan branch March 6, 2022 05:32
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.

3 participants