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

Fix and enforce MSRV by building all features during CI #6755

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

BenWiederhake
Copy link
Collaborator

This PR:

  • downgrades fts-sys and selinux-sys (erroneously upgraded in Bump fts-sys and selinux-sys #6339) to achieve MSRV 1.70.0 again
  • enforces MSRV by actually compiling all code during CI.

Note that the latter requires that we split the text step into two steps, because cargo nextest --all-features would also include features such as test_unimplemented and expensive_tests, which would always break.

Fixes #6728.

Specifically:

$ cargo update -p selinux-sys --precise 0.6.10
    Updating crates.io index
      Adding bindgen v0.69.4
      Adding home v0.5.9
      Adding itertools v0.12.1
      Adding lazycell v1.3.0
 Downgrading selinux-sys v0.6.12 -> v0.6.10
      Adding which v4.4.2
$ cargo update -p fts-sys --precise 0.2.9
    Updating crates.io index
    Removing bindgen v0.70.1
 Downgrading fts-sys v0.2.11 -> v0.2.9

It also required itertools, once_cell, and zip.
@BenWiederhake BenWiederhake force-pushed the dev-enforce-msrv-all-features branch from 558f74b to 162055d Compare September 30, 2024 22:24
@BenWiederhake
Copy link
Collaborator Author

  • Some of our code uses itertools' chunk_by feature, which is only introduced in 0.13.0
  • Any fts-sys version that is old enough to work with rust 1.70.0 depends on itertools < 0.13.0, e.g. 0.12.1
  • If we use both itertools 0.12.1 and 0.13.0 at the same time, then a lint complains about that.

I see only these options:

  • Raise MSRV to >= 1.77 (ugh!)
  • Permit using two different versions of itertools at the same time (ugh!)
  • Claim that we still have an MSRV of 1.70 for "most" of the code, but it breaks with --feature chcon (ugh!)

All of these are terrible. @sylvestre How would you like to proceed?

@samueltardieu
Copy link
Contributor

samueltardieu commented Oct 6, 2024

* Raise MSRV to >= 1.77 (ugh!)

What is really the problem with raising the MSRV to 1.77.0? Debian stable uses rust 1.65.0 by default, which is older than 1.70 anyway so people stuck with Debian stable have to use rustup and can probably use 1.77.0 (which is more than 6 months old). Debian testing uses 1.80.1, so this is ok.

Who are the "customers" of coreutils who can use 1.70.0 and cannot use 1.77.0? Is it worth holding back using newer versions of commodity packages such as itertools for them? Can't they work with an earlier version of coreutils? Will they want to use the current daily version anyway if they are not allowed to upgrade their compilers, why could they upgrade their coreutils?

@BenWiederhake
Copy link
Collaborator Author

Feel free to open a PR to do so, but I don't want to make a decision on this.

@samueltardieu
Copy link
Contributor

@sylvestre?

@cakebaker cakebaker mentioned this pull request Nov 18, 2024
@sylvestre
Copy link
Contributor

@samueltardieu @BenWiederhake
we aren't at a stage of the project that we have to worry at the LTS support.
So, IMHO, we can be relaxed about upgrading the rust minimal version for now

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

Successfully merging this pull request may close these issues.

MSRV >= 1.77 when building with --all-features
3 participants