rust-lang / log

Logging implementation for Rust
https://docs.rs/log
Apache License 2.0
2.16k stars 250 forks source link

Prepare for 0.4.19 release #552

Closed KodrAus closed 1 year ago

KodrAus commented 1 year ago

cc @rust-lang/crate-maintainers @Thomasdezeeuw @JohnTitor

Besides the MSRV bump, this one should carry a low risk of breakage because we haven't touched the log! macros.

I would like to call special attention to the MSRV bump: 1.31.0 to 1.60.0. We haven't bumped it in a really long time, but I think the removal of our build.rs is a great win. If anyone has any strong objections though now is our last-responsible-moment to voice them!

What's Changed

KodrAus commented 1 year ago

Just leaving this open for a bit so there's time for people to see it 🙂

polazarus commented 1 year ago

As mentioned on r/rust, I think a point release is not enough for upping the MSRV. There are now some long running projects where updating packages or compilers are not easy. log being so popular makes it even more sensitive.

While some may think a major bump may not be advised, I think a good compromise is to bump the minor version. Making easy for those who require an old compiler to select a compatible log version: ~0.4 or 0.4.16 (which selects "0.4.17" if available).

And yes for most cargo.toml out there (in particular for cargo add users), there is no automatic "update" going on, but it's a feature not a bug.

BurntSushi commented 1 year ago

Bumping the MSRV on at least a minor version is perhaps appropriate for a crate whose leftmost non-zero version number is the major version, but that's not the case for log. For log, incrementing the minor version is equivalent to a putting out a semver incompatible release, and given that log is a public dependency, this would effectively break the ecosystem. The MSRV concerns are nowhere near the carnage that would ensue if log put out a semver incompatible release. (The "semver trick" could be employed to do it, but that works by updating the older version to depend on the newer version, which in turn means the older version gets an MSRV bump in a patch release anyway.)

I personally think an MSRV of Rust 1.60 (which was released about a year ago) is perfectly fine. Doing it in a minor version bump would be nice, but it doesn't make practical sense to do it here for log.

KodrAus commented 1 year ago

@polazarus That’s certainly not an unreasonable policy, unfortunately we can’t escape the need to bump the MSRV in the previous minor version for log in order to ensure logs produced from that version won’t be lost as the new version propagates. As you say, it’s a widely used and long-lived project, so we could expect multiple minor versions to be active for some time if we did that.

We also can’t create a stable core that includes just that bridging mechanism with a lower MSRV because it’s that mechanism that we’re bumping the MSRV for.

There’s a balance to be struck between maintaining toolchain compatibility, which is important to log, and keeping the project healthy by taking advantage of improvements in the language and tools that result in a better quality artefact. I think the balance struck here is on the edge of what we’d want to do, but is worth it.

demurgos commented 1 year ago

There was/is a long discussion around MSRV for libc:

This is relevant because one of the conclusion was that as long as you're not overly aggressive with bumps or using nightlies, bumping the MSRV should result in a minor or patch version change; not a major version change (ignoring leading zeros due to cargo's resolution rules). In this context, going from 0.4.17 to 0.4.18 is the exact right thing to do, log should not bump to 0.5.0.


Two relevant comments, but I recommend reading the issue as it raises many good points:

As long as libc doesn't bump major just to bump MSRV, downstream crates can maintain more conservative MSRV policy with non-trivial, but also not prohibitive, amount of work.

link

Rust is stable and nearly perfectly backwards compatible. Upgrading to a new version doesn't involve rewriting code (unlike bumping a dependency to a new major version). All Rust code you write today will compile fine for years even if you update your Rust compiler every six weeks.

link

NobodyXu commented 1 year ago

@KodrAus Would it be reasonable to create a log-core after this new release to avoid future hazards where major release is necessary (e.g. removal of API)?

Thomasdezeeuw commented 1 year ago

@KodrAus Would it be reasonable to create a log-core after this new release to avoid future hazards where major release is necessary (e.g. removal of API)?

No. Adding more crates into the mix only makes things harder, not easier.

FYI the reason for the bump is remove the build.rs script, see https://github.com/rust-lang/log/pull/543 and https://github.com/rust-lang/log/issues/489, no API is removed.

NobodyXu commented 1 year ago

No. Adding more crates into the mix only makes things harder, not easier.

Won't having a core makes it possible for two different major versions of log to still get the logging?

I.e. I imagine log-core could contain only the Log trait and setting/getting global logger.

Though then you would also have to add a trait for Metadata and Record, or just bring them into the log-core.

It sounds quite complicated and might be better to just keep them as one crate.

FYI the reason for the bump is remove the build.rs script, see https://github.com/rust-lang/log/pull/543 and https://github.com/rust-lang/log/issues/489, no API is removed.

Yeah I'm aware of that, in fact I did publish a reddit thread to bring attention to this for feedback.

Thomasdezeeuw commented 1 year ago

No. Adding more crates into the mix only makes things harder, not easier.

Won't having a core makes it possible for two different major versions of log to still get the logging?

I.e. I imagine log-core could contain only the Log trait and setting/getting global logger.

Though then you would also have to add a trait for Metadata and Record, or just bring them into the log-core.

It sounds quite complicated and might be better to just keep them as one crate.

I really don't see how a "log-core" crate would provide any benefit. The setting/getting of the global logger, which would be in the core according to your comment, would require the old build script (as it determined the atomic capabilities of the target). So that would mean that "log-core" would still required a MSRV bump when that build script would be removed.

But I think this is getting off-topic of this pr discussion.

notgull commented 1 year ago

Would this MSRV bump be able to wait until after the new Debian Stable releases? This bump would force a lot of crates that are currently MSRV-pinned to Debian Stable to either break their policy or forgo logging.

carllerche commented 1 year ago

It looks like Tokio depends on log via mio. This bump would break Tokio's stability targets. Our 1.18 LTS branch currently supports 1.49 and aims to be maintained until June 2023.

The log dependency in mio is more for debugging purposes, so I am investigating removing it.

KodrAus commented 1 year ago

Would this MSRV bump be able to wait until after the new Debian Stable releases?

Hmm, it looks like that doesn't have a fixed date yet but is likely sometime later this year. I'm not sure how long it then takes to meaningfully propagate to the point that most users on that platform who are also using the rustc that ships with it will have a version over 1.60.0, so don't think we're likely to to anchor to it.

This bump would break Tokio's stability targets.

We will at least wait until there's some resolution for mio before pushing ahead with this.

carllerche commented 1 year ago

We will at least wait until there's some resolution for mio before pushing ahead with this.

Thanks, I appreciate it :+1:

Kixunil commented 1 year ago

I also ask to wait until Debain stable release. It shouldn't be too long anyway.

KodrAus commented 1 year ago

I also ask to wait until Debain stable release. It shouldn't be too long anyway.

Hmm, I think that depends on what "too long anyway" means. If it's a few weeks, then maybe we can do that, but if it's a few months or longer I don't think that's reasonable. I also don't want to set the precedent of tying our MSRV to Debian since releases only seem to come every few years.

NobodyXu commented 1 year ago

@KodrAus Debian 12 is scheduled to release in mid 2023 and they are now under full hard freeze.

est31 commented 1 year ago

Note that Debian also has the rustc-mozilla package which is kept up date (at least for Debian standards): It is currently at 1.59 even on current stable as well as oldstable.

Regarding the MSRV choice, I think this update is fair given that there was a clear tangible benefit with build.rs being gone, and 1.60 being one year ago. It's always a tradeoff between the three parameters I think, how fundamental a crate is (log is very fundamental), how big the benefit is for users (and, less importantly, for developers say if some nice sugar can be used internally now), and lastly how long ago the release was.

Implementation wise, ideally the MSRV would be made explicit in Cargo.toml so that older compilers (those that support it) will error nicely instead of giving messages that are harder to comprehend. I've filed #557 for that, it will only error on a small range though as rust-version was stabilized not many versions before 1.60, but still I think it's good to have it.

Kixunil commented 1 year ago

@est31 how the hell I didn't know about that one?! This is revolutionary, thank you so much!!!

NobodyXu commented 1 year ago

Debian has scheduled the Full Freeze date to be Wednesday 2023-05-24 and release Debian 12 on 2023-06-10.

https://lists.debian.org/debian-devel-announce/2023/04/msg00007.html

KodrAus commented 1 year ago

Thanks for the update @NobodyXu!

Since that’s only a few weeks away I think it would be ok for us to wait for June 10th before releasing this. That should be enough time for anybody else needing to prepare to work around their own MSRV constraints to do so too.

KodrAus commented 1 year ago

🔔 Just a reminder to anyone subscribed that we’re about 2 weeks out from publishing this after June 10th.

KodrAus commented 1 year ago

For anybody following along the spaghetti commits going on right now; we're working on getting an unrelated patch out that doesn't bump the MSRV.

NobodyXu commented 1 year ago

@KodrAus You forgot to update description of the PR.

KodrAus commented 1 year ago

Thanks @NobodyXu! I’ve updated the changelog to reflect the actual changes between this release and the current.

NobodyXu commented 1 year ago

@KodrAus The release note for v0.4.18 is incorrect:

Remove build.rs file by @GuillaumeGomez in https://github.com/rust-lang/log/pull/543

This is the release log or v0.4.19 that you forgot to remove.

NobodyXu commented 1 year ago

@KodrAus The github release still contains the old release changelog https://github.com/rust-lang/log/releases/tag/0.4.18

KodrAus commented 1 year ago

Thanks @NobodyXu, I think that should be everywhere now. I was working in a bit of a hurry yesterday so I'm kind of glad the worst that happened was a few misleading changelog entries 🙂

notgull commented 1 year ago

Debian Bookworm has now been released. I'm perfectly fine with this version bump now.