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

Whats the MSRV for this crate? #85

Closed
stevenroose opened this issue Aug 19, 2020 · 22 comments · Fixed by #146
Closed

Whats the MSRV for this crate? #85

stevenroose opened this issue Aug 19, 2020 · 22 comments · Fixed by #146
Labels
feature-request Feature Request - Let's have some new things!

Comments

@stevenroose
Copy link

It would be helpful if you could mention the MSRV for this crate and also enforce that in Travis. Thanks :)

@Dygear
Copy link

Dygear commented Aug 29, 2020

Slightly off topic, but I have no idea what MSRV stands for. Not that it really matters as I'm an absolute nobody when it comes to this crate, but could you please define?

@PatatasDelPapa
Copy link

@Dygear I believe he's refering to Minimum Supported Rust Version. What is the absolute minimum version of the rust compiler that this crate support so that changing that requires a major version bump.

@CreepySkeleton
Copy link

changing that requires a major version bump.

This isn't true. Whether MSRV bump is a breaking change or not is a long-standing debate within the Rust community, and there's been different guidance on that. In my experience, these mainstream opinions prevail:

  • This crate will accompany each MSRV bump with a minor version bump, so our downstream users could use ~tilda dependency requirement and be sure it still builds with their compiler.

    I'd say this policy (or slight variations of it) is widespread among library crates. It isn't sunshine and bubbles though - there are pretty serious problems with tilda requirements in cargo.

  • This crate will be compatible with N rustc releases back (often 2 or 3). It may also work with earlier versions, but no guarantees.

    This policy is very widespread among libraries, too. Interestingly enough, it's often mixed with the first policy.

  • This crate will be guaranteed to compile with the latest stable rustc version at the time of release.

    This one is extremely common among binary crates.

  • This crate must be build with this particular version of nightly rustc, which is pinned in the rust-toolchain file and periodically updated.

    Well, there is a category of nightly-only crates. They have to pin the version because nightly compilers tend to be somewhat unstable at times 🤷.

Of course there are some variations of those, and some less common policies (very less common), but I'd say that's it! The quoted phrase about "requires major bump" is inaccurate and misleading. Out of curiosity, can somebody give me an example of a reasonably popular (let's set the bar at 300 downloads per day on crates.io) crate that has adopted this policy and been enforcing it? I don't think I ever saw one.

@stevenroose
Copy link
Author

Hmm, I do agree with @PatatasDelPapa here that breaking compatibility with a certain compiler that was previously supported is definitely a breaking change to your library and thus requires a major version bump.

I think any crate that takes tracking their MSRV seriously (i.e. specifying it in the README and having a CI job enforcing that) has this approach. I've been using Rust professionally for more than 2 years now and we sometimes bump into crates violating this and we've banned them since.

I couldn't go over an exhaustive list, but some dependencies we use are

  • serde
  • rand
  • byteorder
  • jsonrpc
  • secp256k1

@CreepySkeleton
Copy link

CreepySkeleton commented Sep 25, 2020

And this is why I said there's no consensus 🤷 I think the trend among Rust maintainers is to be conservative and don't bump MSRV without a very good reason (bugfixes and very appealing features), so it happens quite seldom, especially in mature foundational crates such as serde and rand. But I don't think I ever saw a crate that actually (or even mostly) folloing your policy - they either "cement" the version and never bump it (most foundational crates) or use one of the policies I described above.

Out of morbid curiosity, I looked over the crates you listed and here's what I've found:

  • From rand readme

    Travis CI always has a build with a pinned version of Rustc matching the oldest supported Rust release. The current policy is that this can be updated in any Rand release if required, but the change must be noted in the changelog.

  • byteorder has recently had MSRV bump but they changed their mind shortly after.
  • secp256k1 - MSRV bump a month ago, minor version bump (this is pre 1.0 crate though).
  • jsonrpc seems to be indeed following your policy, but it has quite a few major releases.

None of the crates publicly define their MSRV policy, except rand.

So yeah, I'm convinced "major bumps" is not a widely adopted policy, in practice if not in theory. Probably because the majority of programmers strive to stay updated with the latest rustc, and updating the compiler is just a nuance for the most. Private companies can have different opinions about that, but then again, they tend to go with their own registries/forks.

@stevenroose
Copy link
Author

About secp256k1, the "minor" bumps pre-1.0.0 are considered "major" bumps and allow breaking changes. It's true that many crates are in the 0.x.y range, due to the zer0ver phenomenon. So from the ones you mention, all of them actually follow the rule in practice. Most just don't explicitly state so (perhaps because it can be considered implied by the semver definition of "breaking changes"?) while rand seems to state that they don't follow it, which might be an oversight because they bumped their MSRV several times in the past, every time doing a breaking semver bump along with it.

@stevenroose
Copy link
Author

Anyway, I was just trying to ask the maintainer if he'd be willing to commit to an MSRV, which seems not to be the case. That's fine.

@mackwic mackwic added the feature-request Feature Request - Let's have some new things! label Jun 30, 2022
@mackwic
Copy link
Collaborator

mackwic commented Jun 30, 2022

Hi I'm late for the party, sorry. I don't see an issue with an MSRV, that's OK.

@hwittenborn
Copy link
Member

See the linked issue (#131 (comment)), but I think the plan's probably going to be that the MSRV will be 1.70.0. Is that a problem for anyone here?

@mackwic
Copy link
Collaborator

mackwic commented Jul 2, 2023

Yes. I think 1.70.0 is far too recent to be MSRV, as colored is a "old" lib (first version was 2016) and we have many dependencies that may not upgrade so fast.

I think something in the range 1.15-1.45 would be more reasonable, with using probably this crate to provide sane defaults value for things like std::io::isTerminal()

What do you think ?

@hwittenborn
Copy link
Member

You're right, 1.70.0 is quite recent @mackwic. I was looking at other solutions for fixing #131 if we did that route, and the one crate I found is is-terminal. Their MSRV is 1.63 though, would that be an acceptable version for you?

@mackwic
Copy link
Collaborator

mackwic commented Jul 2, 2023 via email

@hwittenborn
Copy link
Member

@mackwic: I looked at the 5 most popular dependents of colored and the oldest MSRV I found was fern, which has theirs set to 1.31. I don't know how it's that old though because this crate uses Option::copied which only got added in 1.35.

If you're fine with the MSRV being 1.35 though, I've been able to get that building locally just fine. What do you think?

@mackwic
Copy link
Collaborator

mackwic commented Jul 2, 2023 via email

@mackwic
Copy link
Collaborator

mackwic commented Jul 2, 2023 via email

@hwittenborn
Copy link
Member

Sorry for the next comment, but I looked at is-terminal and it requires at least 1.56 since it uses the 2021 version of Rust (this crate does as well, but moving it back to 2018 worked fine).

I do think we should consider how to move away from atty before deciding anything because of that vulnerability it has, but that does increase the MSRV version quite a bit.

I don't know if you'd want to treat MSRVs as a major version bump or not though, do you have any thoughts? If not I think it could be fine having a higher MSRV, but if not I don't really know how to treat keeping atty in the package's dependency list.

@mackwic
Copy link
Collaborator

mackwic commented Jul 2, 2023

The fern issue (ref: daboross/fern#113) is discussing using this crate which has a MSRV or rust 1.49.

Let's ask them what they think of it there

@hwittenborn
Copy link
Member

hwittenborn commented Jul 2, 2023

Yeah that sounds good, I think using that crate might be what fixes #59 as well (I haven't looked enough into what this crate does to enable Windows support, if nothing currently I think that'd be the fix though).

@amitu
Copy link

amitu commented Jul 3, 2023

How about MSRV for colored 2.x to be something old, so current users do not have to worry, and also releasing colored 3, with a MSRV of 1.70.0. Since the two branches will differ only in a single dependency, maintaining 2.x for a year or so may be easy by making 2.x branch the "main branch" so all new features are add there, and 3.x branch can keep merging main.

More maintenance, but maybe manageable.

@hwittenborn
Copy link
Member

I like that logic @amitu, and I'd like to start working on a v3 of colored anyway (see #139). I'll have to figure out how old the MSRV can get, but then that could definitely get going.

@hwittenborn hwittenborn linked a pull request Jul 5, 2023 that will close this issue
@hwittenborn
Copy link
Member

See the linked PR for the planned MSRV approach. The MSRV is going to be 1.63, but whether or not MSRV bumps will be major/minor version bumps for the crate is being discussed there.

@stevenroose
Copy link
Author

I want to mention that I appreciate this thoughtful discussion taking MSRV seriously.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Feature Request - Let's have some new things!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants