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

snafu-derive syn is still in v1.0 #379

Closed
Enet4 opened this issue Jun 29, 2023 · 5 comments · Fixed by #382
Closed

snafu-derive syn is still in v1.0 #379

Enet4 opened this issue Jun 29, 2023 · 5 comments · Fixed by #382
Assignees
Labels
breaking change Likely requires a SemVer version bump found in the field A user of SNAFU found this when trying to use it maintenance Keeping the wheels turning upstream Related to a third-party component

Comments

@Enet4
Copy link
Collaborator

Enet4 commented Jun 29, 2023

A major version of syn (v2.0) was released a while ago. In my projects, all other syn dependents but snafu have done the jump. Would you be interested in a PR to migrate syn to version 2?

@shepmaster shepmaster added maintenance Keeping the wheels turning breaking change Likely requires a SemVer version bump found in the field A user of SNAFU found this when trying to use it upstream Related to a third-party component labels Jun 30, 2023
@shepmaster
Copy link
Owner

Yeah, I've been eyeing that. The "problem" is that it also comes with a MSRV bump, which means SNAFU will have to have a MSRV bump and thus bump the minor version. Bumping the MSRV should also involve checking and cleaning things like the rust_1_39 and rust_1_46 features. It would also allow us to finally move to edition 2021.

Rust 1.56 was 2021-10-21, so hopefully people will have been able to upgrade by now.

Have you tried bumping the syn version / any ideas what kind of changes are needed?

@8573
Copy link

8573 commented Jun 30, 2023

As a point of reference, Debian has Rust 1.63 in its stable release, 1.48 in "oldstable", and 1.41 in "oldoldstable" (long-term support out to 2024-06-30). Alpine Linux 3.15 and newer have Rust 1.56 and newer; Alpine 3.10, the oldest not-completely-unsupported version covered by the package catalog, has Rust 1.34. I'm not sure any other distro both packages Rust and is conservative in updating it similarly to Debian; Debian at least is the most famous. Fedora, Ubuntu, Arch, Gentoo, NixOS, and Guix seem to have Rust 1.56 or newer available for all supported releases of the distro (if it's not rolling-release), and I'm not sure OpenSUSE packages Rust at all except in a rolling-release version, in which Rust appears to be updated frequently.

@Enet4
Copy link
Collaborator Author

Enet4 commented Jul 2, 2023

Have you tried bumping the syn version / any ideas what kind of changes are needed?

Just gave it a try. The trickiest part was adjusting the DocComment definition to comply with the way that attributes are parsed in the new version (it's hard for me to explain why), the rest was renaming a type and changing a field access to a method call.

A good deal of the code changes would still be around requiring Rust 1.56, mostly to clean up conditional attributes and removing outdated content.
I pushed them all into this branch which I hope do both of these well. Let me know if you would be willing to consider a PR with this.

@shepmaster
Copy link
Owner

Sure, feel free to open that as a PR where we can iterate and comment.

Comments I would have left and we can discuss on the PR
  • It'll be fine to have this as separate PRs (e.g. upgrade syn, bump MSRV). The latter probably would need be committed first, but may be easier to be driven by the former. git rebase to the rescue!
  • We probably need to grep for (variations on) 1.34 / 1.39 / 1.46 to find places we talk about them in prose.
  • Why did snafu-derive get a default = [] feature?
  • The yes_async::async_body module can probably be inlined.
  • fn track_caller() can probably be inlined as well
  • Nice catch on the non_exhaustive note

This was referenced Jul 2, 2023
@8573
Copy link

8573 commented Jul 2, 2023

I'd like to say that I, supporting generous MSRVs, appreciate SNAFU's current generosity in its MSRV, but I, getting it for free, don't ask that that generosity be in any way permanent, particularly if it's incurring the cost in time-to-compile of an otherwise unnecessary and not small dependency.

@Enet4 Enet4 self-assigned this Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Likely requires a SemVer version bump found in the field A user of SNAFU found this when trying to use it maintenance Keeping the wheels turning upstream Related to a third-party component
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants