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

Upgrade colored and prepare for 0.7.0 release #128

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

Conversation

faern
Copy link

@faern faern commented Jul 12, 2023

Fixes #113

src/lib.rs Show resolved Hide resolved
CHANGELOG.md Outdated
@@ -1,6 +1,14 @@
Unreleased
==========


0.7.0 (2023-07-12)
Copy link
Author

Choose a reason for hiding this comment

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

The date might need to be adjusted unless this PR is accepted quickly. Or the package maintainers might want to handle this part of releasing themselves? Just tell me how you want it. I went with the path where this PR does everything needed before cargo publish for now, to make it easy :)

@faern
Copy link
Author

faern commented Jul 12, 2023

time bumped the MSRV to 1.65.0 in version 0.3.21. They (correctly) did not release this as a breaking change. But this also means that it's kind of pointless to track MSRV at all. Because we can't guarantee anything about our MSRVs when we can't do the same for our dependencies.

What we can do is to run the CI on cargo +nightly update -Z minimal-versions && cargo test to verify that our MSRV at least holds for the oldest compatible versions of all our dependencies.. Or skip the MSRV CIs completely. The maintainers need to step in here. But this issue has nothing with colored to do, it's just syslog.

We can also simply bump fern's MSRV to 1.65.0. But nothing stops our dependencies from bumping them even further with no notice.

@LingMan
Copy link

LingMan commented Jul 12, 2023

Other options would be to run the MSRV CI with cargo update -p time --precise 0.3.20, which would only downgrade that specific crate, or committing a locally crated Cargo.lock.msrv and then doing cp Cargo.lock.msrv Cargo.lock.
Both of these would avoid the need to install nightly rust on the MSRV CI job.

@faern
Copy link
Author

faern commented Jul 13, 2023

Other options would be to run the MSRV CI with cargo update -p time --precise 0.3.20 ...

I'll let the maintainers of fern step in here and express their wishes. But IMHO that's a very temporary and fragile solution. It's just a matter of time before some other dependency bumps their MSRV and this CI job fails for no apparent reason again. Following the same logic this crate will eventually have a large number of those workarounds to set specific versions of specific dependencies.

@polarathene
Copy link

polarathene commented Nov 26, 2023

This PR has been left open for a while, and no new commits to main since March.

Perhaps @daboross could add some additional maintainers?


this also means that it's kind of pointless to track MSRV at all. Because we can't guarantee anything about our MSRVs when we can't do the same for our dependencies.

IMHO that's a very temporary and fragile solution. It's just a matter of time before some other dependency bumps their MSRV and this CI job fails for no apparent reason again.

You can use -Z msrv-policy with nightly toolchain to resolve crates to an MSRV if they properly maintain their rust-version in Cargo.toml.

Useful for generating a Cargo.lock.msrv for CI usage, where you may have a few crates that need version pinning if they're not compatible yet with -Z msrv-policy.


Just to clarify, a project can set an MSRV and ensure that it satisfies that MSRV via CI, but this does not require the ability to have the MSRV supported without version pinning. It's the same as what the user would need to do to build with the MSRV, bumping it to avoid any pinning is just for convenience.

rust-version in Cargo.toml should convey what the actual MSRV is to build without failure (because the project itself uses an API or a dependency that actually requires an MSRV bump to support the functionality). If you're just interested in what the MSRV is without a Cargo.lock use something like cargo-msrv in CI to track that, it's a different expectation.

Upstream dependencies can MSRV bump as non-breaking semver change without issue, especially when they leverage rust-version. This explains it well.

config-rs 0.13.x still supports a MSRV of 1.56 by using -Z msrv-policy and only three pins.


TL;DR

rust-version is where the value is at for downstreams, provided it's maintained correctly (upstreams have no impact on MSRV then when they also maintain their rust-version).

@pronebird
Copy link

Bump

@faern
Copy link
Author

faern commented Jan 17, 2024

The original reason for this PR is gone (getting rid of atty as a transitive dependency). But I still think it makes sense to do this upgrade to colored 2. This gets rid of the also stale winapi crate, and in general follow the ecosystem forward. Sadly this PR also has to deal with MSRV stuff. Not really because this PR changes much around it, but because the CI for fern is currently broken if running straight off of main.

I went with the solution to check in a Cargo.lock.msrv in order to be able to maintain CI MSRV checks. However, I had to bump the lower bound on rust-version anyway, since the lockfile uses the new format introduced in Rust 1.38. Maybe it's possible to craft a lockfile of the older format 🤷 but to me that seems like madness for such an extreme backwards compatibility.

EDIT: I have not yet been able to craft a perectly working Cargo.lock.msrv. Neither of these produce a working version:

  • cargo update
  • cargo +nightly update -Z msrv-policy
  • cargo +nightly update -Z minimal-versions

But I'll continue investigating when I have time and try to handpick a combination of crates that do work.

The code did not change, but since the CI now uses a checked in
Cargo.lock.msrv of the new lockfile format introduced in Rust 1.38.
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.

Getting rid of atty as a transitive dependency (via colored)
4 participants