Skip to content

Conversation

teoxoy
Copy link
Member

@teoxoy teoxoy commented Oct 6, 2025

Connections
Fixes Bug 1992500

Description
Fixes a regression introduced by #8265 where we were mistakenly writing a comma before the first buffer argument (if no other arguments were printed before it).

Testing
Enabled more CTS tests.

Squash or Rebase?
Rebase.

@ErichDonGubler ErichDonGubler self-assigned this Oct 6, 2025
@ErichDonGubler ErichDonGubler added type: bug Something isn't working backend: metal Issues with Metal area: naga back-end Outputs of naga shader conversion naga Shader Translator lang: Metal Metal Shading Language labels Oct 6, 2025
@ErichDonGubler ErichDonGubler changed the title Fix stray comma Fix stray comma in function signatures in Metal Oct 6, 2025
@ErichDonGubler ErichDonGubler enabled auto-merge (squash) October 6, 2025 18:09
@ErichDonGubler ErichDonGubler enabled auto-merge (squash) October 6, 2025 18:09
@cwfitzgerald
Copy link
Member

Does this need backporting?

@ErichDonGubler ErichDonGubler added the PR: needs back-porting PR with a fix that needs to land on crates label Oct 6, 2025
@ErichDonGubler
Copy link
Member

ErichDonGubler commented Oct 6, 2025

@cwfitzgerald: I'm not @teoxoy, but I'll say: yesplz

@andyleiserson
Copy link
Contributor

As far as we know it's only a CTS regression, right? I think backporting can at least wait for input from @teoxoy.

@teoxoy
Copy link
Member Author

teoxoy commented Oct 6, 2025

I think newer Metal versions are fine with the stray comma since I couldn't repro this issue on my M1 either.

@teoxoy
Copy link
Member Author

teoxoy commented Oct 6, 2025

I can put up another PR with the backport though.

@teoxoy
Copy link
Member Author

teoxoy commented Oct 6, 2025

@cwfitzgerald I pushed on the remote instead of the origin (my fork) so it ended up directly in the v27 branch... is that ok? Or should I reset, force push and still open a separate PR?

@ErichDonGubler ErichDonGubler merged commit 1072b87 into gfx-rs:trunk Oct 6, 2025
41 checks passed
@teoxoy teoxoy deleted the fix-stray-comma branch October 6, 2025 19:00
@cwfitzgerald
Copy link
Member

That's fine. I can bump version and release

@cwfitzgerald
Copy link
Member

@teoxoy seems ci failed on v27. Could you investigate?

@ErichDonGubler
Copy link
Member

@cwfitzgerald: I don't think this PR caused the CI failure; all failures report:

[INFO  wgpu_xtask::cts] Reading default test list from cts_runner/test.lst
[INFO  wgpu_xtask::cts] Fetching CTS
[INFO  wgpu_xtask::cts] Checking out CTS
fatal: unable to read tree (5e7bd6ed86201123ff6b0900974587550afb10e7)
Error: Failed to check out CTS

@andyleiserson
Copy link
Contributor

2ea52e0

(The problem is that I bumped the CTS version on trunk earlier today, and the CTS is cloned with --depth 1. So the cache from trunk will not work for the v27 branch. Certainly there's a better fix but changing the tag should suffice for now.)

@ErichDonGubler
Copy link
Member

@andyleiserson: Filed #8313.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: naga back-end Outputs of naga shader conversion backend: metal Issues with Metal lang: Metal Metal Shading Language naga Shader Translator PR: needs back-porting PR with a fix that needs to land on crates type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants