-
Notifications
You must be signed in to change notification settings - Fork 33
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
update to syn 2.0 #175
update to syn 2.0 #175
Conversation
# the start of `[dependencies]` (avoiding unnecessary rebuilds). | ||
[build-dependencies] | ||
syn = { version = "1", features = ["extra-traits", "full"] } | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these only exist to unify features across dependency trees
This made absolutely no difference in my testing with syn 2, A being with and B without this dependency. Tests were run by changing the line, cargo clean
, cargo update
and time cargo compiletest --target-env vulkan1.2
on a 6900HS (8/16 cores) with installed tools:
native spirv dep real user
A1 43.43s 9.35s 1m7 9m19
A2 41.16s 8.79s 1m5 8m59
B1 42.48s 8.84s 1m6 9m1
B2 42.37s 9.00s 1m6 9m0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are other similar dependencies below I have not touched.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will discuss this more in-depth off this PR, but for historical context, cargo clean
with a single Cargo build after it is probably completely unimpacted by what this was fixing.
The issue was switching between things like the wgpu
example and compiletest
etc. - and the debugging process to track down the rebuild causes was honestly a bit absurd (there might be better tools now, or Cargo functionality, but I don't know off the top of my head).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Example of the issue at play (after running each of these commands once):
$ cargo build -p compiletests --release --no-default-features --features use-installed-tools
Compiling rustc_codegen_spirv v0.9.0 (/home/eddy/Projects/rust-gpu/crates/rustc_codegen_spirv)
Compiling compiletests v0.0.0 (/home/eddy/Projects/rust-gpu/tests)
Finished `release` profile [optimized] target(s) in 2.92s
$ cargo build -p example-runner-wgpu --release --no-default-features --features use-installed-tools
Compiling rustc_codegen_spirv v0.9.0 (/home/eddy/Projects/rust-gpu/crates/rustc_codegen_spirv)
Compiling spirv-builder v0.9.0 (/home/eddy/Projects/rust-gpu/crates/spirv-builder)
Compiling example-runner-wgpu v0.0.0 (/home/eddy/Projects/rust-gpu/examples/runners/wgpu)
Finished `release` profile [optimized] target(s) in 3.58s
$ cargo build -p compiletests --release --no-default-features --features use-installed-tools
Compiling rustc_codegen_spirv v0.9.0 (/home/eddy/Projects/rust-gpu/crates/rustc_codegen_spirv)
Compiling compiletests v0.0.0 (/home/eddy/Projects/rust-gpu/tests)
Finished `release` profile [optimized] target(s) in 2.64s
$ cargo build -p example-runner-wgpu --release --no-default-features --features use-installed-tools
Compiling rustc_codegen_spirv v0.9.0 (/home/eddy/Projects/rust-gpu/crates/rustc_codegen_spirv)
Compiling spirv-builder v0.9.0 (/home/eddy/Projects/rust-gpu/crates/spirv-builder)
Compiling example-runner-wgpu v0.0.0 (/home/eddy/Projects/rust-gpu/examples/runners/wgpu)
Finished `release` profile [optimized] target(s) in 3.44s
I did try re-adding the build-dep you removed (with version 2
instead of 1
), and nothing changed, so you didn't make anything worse (I was expecting this to have bitrotten anyway).
0859f97
to
ba0f934
Compare
ba0f934
to
d8c3116
Compare
This was the last dependency in my project that still uses syn 1.0 :D
Note that within the repo there remains one usage of syn 1.0 on some targets (I assume windows only):