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

Bump MSRV, update dependencies #184

Merged
merged 1 commit into from
Oct 17, 2023
Merged

Bump MSRV, update dependencies #184

merged 1 commit into from
Oct 17, 2023

Conversation

dralley
Copy link
Collaborator

@dralley dralley commented Oct 16, 2023

No description provided.

Copy link
Collaborator

@olivierlemasle olivierlemasle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dralley Why 1.71.0?

Even if times Minimum Rust version policy guarantees that at least the three latest Rust versions are supported (1.71, 1.72, 1.73), its latest version (0.3.30, released 3 days ago) is currently supporting rust >= 1.67.0:

See time's latest MSRV in Cargo.toml:

https://github.com/time-rs/time/blob/v0.3.30/time/Cargo.toml#L6

Also, see times MSRV CI tests:

https://github.com/time-rs/time/blob/v0.3.30/.github/workflows/build.yaml#L156

The other rpm's dependencies and rpm itself also work fine with rust 1.67.0, even with the dependency updates of this PR.

@dralley
Copy link
Collaborator Author

dralley commented Oct 16, 2023

We could use 1.67, but I would kinda rather do a bigger jump just to avoid needing to bump it again in another month, since this happens semi-frequently (unfortunately).

The main reason to keep an older MSRV is to make the package usable for building distro packages. 1.71 is currently available in CentOS 8 / 9 Stream, so I just went with that.

Of course, the situation is not exactly ideal, I wish either cargo was smart enough to pick a suitable version of time or our dependencies would avoid so many MSRV bumps.

@olivierlemasle
Copy link
Collaborator

olivierlemasle commented Oct 16, 2023

Ok, I understand.

The reason I hoped to keep the MSRV as low as possible is to avoid forcing the crates which depend on rpm to bump their own MSRV.

@dralley
Copy link
Collaborator Author

dralley commented Oct 16, 2023

You're not wrong, it flows endlessly downhill.

OK. I'll look at switching it to 1.67

@dralley
Copy link
Collaborator Author

dralley commented Oct 17, 2023

@olivierlemasle Is 1.67 OK?

Copy link
Collaborator

@olivierlemasle olivierlemasle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thanks @dralley!

Copy link
Collaborator

@olivierlemasle olivierlemasle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, I've just noticed that three of de dev-dependencies seem to be currently unused:

  • rsa
  • rsa-der (which depends on simple_asn1, bringing time in the dependency tree)
  • pretty_assertions

When removing these three dev-dependencies, cargo test --all-features still work (and the MSRV could then be 1.66 -- but I'm still ok with 1.67!).

@olivierlemasle olivierlemasle merged commit 93f29d9 into master Oct 17, 2023
11 checks passed
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.

2 participants