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

rust-version and non-default features #671

Closed
tomtau opened this issue Jul 29, 2022 · 9 comments · Fixed by #683
Closed

rust-version and non-default features #671

tomtau opened this issue Jul 29, 2022 · 9 comments · Fixed by #683

Comments

@tomtau
Copy link
Contributor

tomtau commented Jul 29, 2022

Currently, rust-version is set to 1.56 and it builds fine with default features.
During the release process, we found that at least 1.61 is required for --all-features.
Shall we set rust-version to 1.61?

@tomtau
Copy link
Contributor Author

tomtau commented Jul 29, 2022

@CAD97 @NoahTheDuke

@tomtau
Copy link
Contributor Author

tomtau commented Jul 29, 2022

Right now, it seems it's not possible to set different MSRVs per feature flags: https://rust-lang.github.io/rfcs/2495-min-rust-version.html#cfg-based-msrvs in the manifest?

@NoahTheDuke
Copy link
Member

Related: #615 (comment)

@CAD97
Copy link
Contributor

CAD97 commented Jul 29, 2022

Also related: rust-lang/libs-team#72

Whilst it's not possible to key off of features, my opinion is that the rust-version should match the default feature set.

The ideal thing to do would be to downgrade the all-features MSRV, either by version sniffing or just changing what's adding the requirement.

Whatever we choose, we should test it in CI.

As far as if we want to bump MSRV again:

  • We should add a policy for upgrading MSRV
  • We should do a survey of rdeps's MSRV policies and avoid exceeding those
  • We should also strongly consider a "six month" minimum even if no rdeps currently rely on that
  • And track the T-libs discussion; it'll end up defacto setting the extended msrv policy for the ecosystem.

@CAD97
Copy link
Contributor

CAD97 commented Jul 29, 2022

direct rdeps

I checked rdeps with downloads >= 10k. Listed is just information from README/homepage/rustdoc; I did not check the rust-version key for any of these crates.

crate msrv policy notes
semver-parser - deprecated
handlebars stable patch-level change
nalgebra stable -
tera - no policy listed
json5 - no policy listed
insta - no policy listed
mmids - no policy listed
async-graphql-parser 1.57.0 no policy listed; regularly bumped
termwiz - no policy listed
css-modules - only exists on crates-io; docs-rs and repo are 404
jsonpath-rust - no policy listed
liquid - no policy listed
py_literal - no policy listed
comrak - no policy listed
msi - no policy listed

I know @jhpratt has considered potentially using pest in time; time has an MSRV policy of six months, with a explicit hole allowed for optional interoperability crate dependencies.

@NoahTheDuke
Copy link
Member

Thanks for the great research! 6 months is roughly stable-5, right? Seems like that's a reasonable bound given the discussion in the libs-team conversation.

@CAD97
Copy link
Contributor

CAD97 commented Jul 29, 2022

Yeah, 6 months is somewhere in stable-4/stable-5 (current stable is stable-0) since months aren't a clean number of weeks. stable-5 is cleanly always at least 6 months old.

@jhpratt
Copy link

jhpratt commented Jul 29, 2022

Six months is ~26 weeks. So stable - 4, plus about two weeks buffer. I don't count the weeks though, as it's easier to just add six to the month, saturating the day if necessary. So August 31 would turn into February 28.

The six months policy is in relatively high use among popular crates with a formal policy. For pest being potentially used in the time crate, while it would be an optional dependency, it would not be so directly (basically there's no "pest" feature, just "parsing" that would rely on pest). As such the caveat would not apply, and pest would need to fully comply with my MSRV policy to be used as a dependency. This is, in part, why the only dependencies enabled by default are ones I directly control.

@tomtau
Copy link
Contributor Author

tomtau commented Jul 30, 2022

FWIW:

Anyway, the culprit non-default feature in pest that needs 1.61 is const_prec_climber (it previously required nightly).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants