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

Refactor Cargo.toml Significantly #6980

Merged
merged 3 commits into from
Jan 23, 2025

Conversation

cwfitzgerald
Copy link
Member

@cwfitzgerald cwfitzgerald commented Jan 23, 2025

Sorry this bad boy got a bit out of control, but I think puts us in a much better place than we were before.

Step 1 towards #6975

  • Replaces Cargo.toml crate renames with extern crate renames at the top of the relevant libraries.
  • Vastly cleans up the target-soup in wgpu/Cargo.toml and wgpu-hal/Cargo.toml and adds a variety of comments.
  • Stop using separate tables for each dependency.
  • Stop using wgt in examples and tests.
  • Always put local dependencies on top.

@cwfitzgerald cwfitzgerald requested review from a team and crowlKats as code owners January 23, 2025 19:18
@cwfitzgerald cwfitzgerald force-pushed the cw/cargo-toml-refactor branch 6 times, most recently from 6dca5bb to 3240917 Compare January 23, 2025 20:21
@cwfitzgerald cwfitzgerald force-pushed the cw/cargo-toml-refactor branch from 3240917 to bc98515 Compare January 23, 2025 20:28
@@ -55,7 +55,7 @@ env:
RUSTFLAGS: -D warnings
RUSTDOCFLAGS: -D warnings
WASM_BINDGEN_TEST_TIMEOUT: 300 # 5 minutes
CACHE_SUFFIX: c # cache busting
CACHE_SUFFIX: d # cache busting
Copy link
Member Author

Choose a reason for hiding this comment

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

Not strictly necessary, but I thought I had a cache bug, and it doesn't hurt to make sure the caches are clear every once and again

Comment on lines +13 to +14
extern crate wgpu_core as wgc;
extern crate wgpu_types as wgt;
Copy link
Member Author

Choose a reason for hiding this comment

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

Lookie!

@@ -35,7 +35,7 @@ use core::fmt;
///
/// [skip]: super::TestParameters::skip
/// [expect_fail]: super::TestParameters::expect_fail
/// [`AdapterInfo`]: wgt::AdapterInfo
/// [`AdapterInfo`]: wgpu::AdapterInfo
Copy link
Member Author

Choose a reason for hiding this comment

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

We constantly used wgt in tests when we shouldn't have.

Comment on lines +37 to +39
[lints.rust]
unexpected_cfgs = { level = "warn", check-cfg = ['cfg(web_sys_unstable_apis)'] }

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved all the lints to be in the same order

@@ -68,7 +68,7 @@ struct Example<A: hal::Api> {
instance: A::Instance,
adapter: A::Adapter,
surface: A::Surface,
surface_format: wgt::TextureFormat,
surface_format: wgpu_types::TextureFormat,
Copy link
Member Author

Choose a reason for hiding this comment

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

Changed these because these are supposed to be examples.

Comment on lines +67 to +70
spirv = ["naga/spv-in", "wgpu-core?/spirv"]

## Enable accepting GLSL shaders as input.
glsl = ["naga/glsl-in", "wgc/glsl"]
glsl = ["naga/glsl-in", "wgpu-core?/glsl"]
Copy link
Member Author

Choose a reason for hiding this comment

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

subtle: we dropped a ? here, and previously would always get wgpu-core even if webgl wasn't enabled.

Comment on lines +154 to +156
########################################
# Target Specific Feature Dependencies #
########################################
Copy link
Member Author

Choose a reason for hiding this comment

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

This section was recast such that instead of trying to get exact right combination of targets for each feature, we just list out the target categories and list out each feature. This is soooo much more clear.

Comment on lines -222 to -223
#[cfg_attr(docsrs, doc(cfg(any(wgpu_core, naga))))]
#[cfg(any(wgpu_core, naga))]
Copy link
Member Author

Choose a reason for hiding this comment

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

This was wrong, but wasn't caught until some of the cfgs were fixed up.

@cwfitzgerald cwfitzgerald force-pushed the cw/cargo-toml-refactor branch from bc98515 to 184c51d Compare January 23, 2025 21:16
@ErichDonGubler ErichDonGubler changed the title Refactor Cargo.toml Signifigantly Refactor Cargo.toml Significantly Jan 23, 2025
@jimblandy
Copy link
Member

jimblandy commented Jan 23, 2025

@cwfitzgerald Does this change the names of the wgpu features people need to select to get, say wgpu_core?
It seems like the way it was written before, it'd need to be "wgc", but now it's "wgpu_core", no?

If so, then we need a CHANGELOG.md note about this.

Copy link
Member

@jimblandy jimblandy left a comment

Choose a reason for hiding this comment

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

Everything outside of deno_webgpu looks good to me, except as noted. The wgpu_hal/Cargo.toml changes are especially nice.

deno_webgpu/Cargo.toml Outdated Show resolved Hide resolved
wgpu/Cargo.toml Show resolved Hide resolved
wgpu/Cargo.toml Outdated Show resolved Hide resolved
@cwfitzgerald cwfitzgerald enabled auto-merge (squash) January 23, 2025 22:40
@cwfitzgerald cwfitzgerald merged commit d8e7ab1 into gfx-rs:trunk Jan 23, 2025
31 checks passed
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.

2 participants