nalgebra icon indicating copy to clipboard operation
nalgebra copied to clipboard

Update syn to `2.0`

Open yotamofek opened this issue 2 years ago • 2 comments

Requires no adaptations.

yotamofek avatar Dec 31 '23 09:12 yotamofek

Cargo didn't find [email protected] for some reason. Seems spurious and unrelated to the syn update.

CAD97 avatar Jan 13 '24 04:01 CAD97

Ah great, now it failed due to a different spurious and unrelated problem 😅 Let's run CI again..

Edit: seems the previous failure was not spurious and was fixed by 7866bcee5c4f3c1f9d9d86116fd96d000e7453b9?

yotamofek avatar Jan 14 '24 09:01 yotamofek

Friendly ping? Would be nice to get this merged (and ideally, released) to reduce compile-times for downstream projects.

djc avatar Mar 18 '24 15:03 djc

Friendly ping? Would be nice to get this merged (and ideally, released) to reduce compile-times for downstream projects.

I agree. However, if I run cargo tree -p nalgebra --all-features, there are a few (opt-in) dependencies that still seem to rely on syn 1.0. Having both syn 1.0 and syn 2.0 in the dependency tree seems strictly worse. Perhaps we can upgrade these too though. It's not clear to me how much effort is required, as I suspect some packages might simply not have been updated to support syn 2.0.

Andlon avatar Mar 18 '24 16:03 Andlon

Actually, I realized that I hadn't updated dependencies in my dev environment for a while. After running cargo update, I see that the only other dependency that has not been updated to use syn 2.0 is rkyv. Since we anyway might have both syn 1.0 and syn 2.0 in the tree at this point due to serde using 2.0, I think we should go ahead and merge this as-is, and then log an issue for rkyv to also upgrade its syn version to 2.0.

Andlon avatar Mar 18 '24 16:03 Andlon

@sebcrozet: it's unclear to me how to handle this correctly with respect to release. I think we need to:

  • bump nalgebra-macros to v0.2.2 (I believe patch version should be sufficient?)
  • change nalgebra to depend on nalgebra-macros v0.2.2.

However, as far as I can tell, we don't actually need a new nalgebra release for this. If we release nalgebra-macros with a new patch version, then existing nalgebra users should get the change through cargo update.

Andlon avatar Mar 18 '24 16:03 Andlon

However, as far as I can tell, we don't actually need a new nalgebra release for this. If we release nalgebra-macros with a new patch version, then existing nalgebra users should get the change through cargo update.

Yes, that sounds correct. I think that means you don't even need to change nalgebra to bump the nalgebra-macros dependency.

djc avatar Mar 18 '24 16:03 djc

Resolves #1403 (I forgot to check PRs before opening an issue ^^)

tautropfli avatar Jun 11 '24 21:06 tautropfli

Thank you all for making/reviewing this PR!

sebcrozet avatar Jun 22 '24 16:06 sebcrozet