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

Should minrust be optional? #20

Closed
epage opened this issue Jul 13, 2019 · 11 comments · Fixed by #27
Closed

Should minrust be optional? #20

epage opened this issue Jul 13, 2019 · 11 comments · Fixed by #27
Labels
question Further information is requested

Comments

@epage
Copy link
Contributor

epage commented Jul 13, 2019

minrust is good for libraries where the dependent's rust version is less in your control. For binaries, the rust version is more in your control. Cutting minrust on these projects can save hassle (I've been finding its pretty brittle to verify it) and save time (both faster turnaround on PRs and keeping free CI usage to a minimum).

The main case I can think of for a minrust for binaries is if packagers need it but I thought I've seen comments that this isn't as big of a concern.

@epage epage added the question Further information is requested label Jul 13, 2019
@jonhoo
Copy link
Collaborator

jonhoo commented Jul 15, 2019

Yes, I think it's reasonable to make it possible to set minrust: false without requiring the user to set up all the stages themselves!

Alternatively we could go a step further and have azure/stages-lib.yml and azure/stages-bin.yml rather than all this branching?

@epage
Copy link
Contributor Author

epage commented Jul 15, 2019

Alternatively we could go a step further and have azure/stages-lib.yml and azure/stages-bin.yml rather than all this branching?

I'm unsure if we know enough to know how universal a specific lib/bin recommendation is

  • While my bin's don't need minrust, I don't know about others. burntsushi would probably be a good contact.
    • There are also bins that are also libs, confusing the matter further.
  • Someone iterating on a fresh crate might get frustrated the extra burden of minrust. I'm unsure if its best for us to encourage people for long term best practices or faster iteration in the prototyping stage.

@jonhoo
Copy link
Collaborator

jonhoo commented Jul 15, 2019

Yeah, those are both good points. Let's gently ping @BurntSushi and see if we can get some first-hand answers ^_^

For your last point, I'm inclined to say that we should aim for long-term best practices. If you're in the rapid prototyping stage, CI is probably a lower concern for you, and you'd be more okay with it breaking. minrust is then also less of a concern for you, and you'd probably set it explicitly to false.

@BurntSushi
Copy link

BurntSushi commented Jul 15, 2019

While it's on my nice-to-have list, I haven't actually moved anything to Azure yet, so I don't think I understand the operational implications of the minrust directive discussed here.

With that said, every Rust project I work on (whether libraries or binaries) has a pinned Rust version. Generally, my belief is that if one is going to increase the MSRV, then it should be done explicitly. A pinned Rust version in the CI helps with that.

For binaries, I usually keep the pinned version up to date with the latest stable release. In particular, I like to make sure that patch releases for a binary compile on the same version of the compiler as the previous minor version release. I wouldn't want someone's Rust compiler version preventing them from updating to a patch release of my software.

For libraries, I tend to be conservative in rough proportion with how widely used the library is. byteorder is one extreme end of this, where it still supports Rust 1.12. It's probably fine to bump this nowadays, but byteorder doesn't see a lot of active development so there's really not too much to be gained.

Personally, it would be great if most/all libraries documented a MSRV. A minimal way of doing that is to put a pinned Rust version in the CI configuration. If one doesn't do that and one cares about MSRV, then it becomes a research task to figure it out. This helps other library/binary maintainers that depend on your library. For binaries, I generally don't care what someone's MSRV policy is because I always keep my Rust installation updated.

@epage
Copy link
Contributor Author

epage commented Jul 15, 2019

While it's on my nice-to-have list, I haven't actually moved anything to Azure yet, so I don't think I understand the operational implications of the minrust directive discussed here.

Basically, we are offering a toolbox of templates that people can piece together as well as a one-stop template that provides a "good enough" pipeline. As part of this, we accept a MSRV parameter (minrust) so people can verify it.

I stopped using MSRV for my bin's because of (1) I didn't see enough value and wanted to reduce my CI time and (2) I've been running into a lot of issues verifying MSRV. Would love any input you have over on #22 for how we can make it more usable for people.

@jonhoo
Copy link
Collaborator

jonhoo commented Jul 15, 2019

@BurntSushi when you say that every project you work on has a pinned Rust version, do you mean a MSRV, or do you mean that your CI is tied to a particular Rust version? Does that mean that you do manual work on every new Rust stable release? Do you run CI against beta?

@BurntSushi
Copy link

when you say that every project you work on has a pinned Rust version, do you mean a MSRV, or do you mean that your CI is tied to a particular Rust version?

I mean MSRV.

Does that mean that you do manual work on every new Rust stable release?

No, definitely not. For libraries, the MSRV doesn't change frequently. For binaries, the MSRV only changes when I do a minor or major version release. Otherwise, the MSRV remains the same.

Do you run CI against beta?

Ah yeah, I run CI against a pinned version of Rust in addition to stable, beta and nightly.

I stopped using MSRV for my bin

Yeah I think that's reasonable/fine. It's libraries that I'm more concerned about.

@jonhoo
Copy link
Collaborator

jonhoo commented Jul 15, 2019

Thanks, that's helpful!

@epage I think what we'll want to do is encourage library authors to keep a MSRV (if we can push the ecosystem that way, that'd be awesome), but allow authors to opt out of it with something like minrust: false. Maybe what we should do is maintain two different example azure-pipelines.yml files, one for binaries (with a recent minrust) and one for libraries with a conservatively set minrust?

@BurntSushi
Copy link

BurntSushi commented Jul 15, 2019

I think what we'll want to do is encourage library authors to keep a MSRV (if we can push the ecosystem that way, that'd be awesome)

So just to be clear, one thing I say here is that even if a library doesn't want to maintain an official MSRV policy, it is still very useful to document a pinned version of Rust in the CI configuration, which is informative to others. encoding_rs does this for example, but will happily bump the MSRV in patch releases.

Also, it should go without saying that I am not the only perspective here. There are definitely some folks that think any kind of MSRV is harmful to the ecosystem since it holds it back. And there are others that just think it's too much maintenance burden to bother with. And there are still others that think bumping the MSRV requires a semver incompatible version bump. All of these perspectives are valid of course, but it's just good to be aware of them if you're consciously trying to push folks in a particular direction.

@epage
Copy link
Contributor Author

epage commented Jul 15, 2019

Also, it should go without saying that I am not the only perspective here.

This reminds me: I thought MSRV came up at some point in thinking Linux distributions would need it but I thought I saw a post from you saying something like it turned out they didn't need it after all. Is that memory accurate?

@BurntSushi
Copy link

Yes! I got lots of replies from distro packagers here: BurntSushi/ripgrep#1019

jonhoo added a commit that referenced this issue Jul 15, 2019
This also moves the MSRV check out into its own stage since it's not
_really_ part of the test stage.

Note that this change still leaves the default minimum Rust version as
1.32.0.

Fixes #20.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants