search

rust-lang / log

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

Remove build.rs file #543

Closed GuillaumeGomez closed 1 year ago

GuillaumeGomez commented 1 year ago

Fixes #489.

This is another take on #490. Since Rust 1.60, cfg(target_has_atomic = "...") is stable so we can now use it. However, it'll change the MSRV to 1.60 as well.

I used target_has_atomic = "ptr" (and not target_has_atomic = "8" for example) because that's what is used in the std for AtomicUsize. You can see it in the macro declaring the AtomicUsize here.

cc @nnethercote (since you're the one who suggested this change)

EDIT: Just saw that this solution was mentioned by @KodrAus in the linked issue.

GuillaumeGomez commented 1 year ago

Once it's figured out, we can then decide what to do with this PR. No hurry. ;)

KodrAus commented 1 year ago

1.60.0 is quite a jump, but is still just over 12 months old. I personally think the removal of a build.rs makes it worth it. I don't think it's worth doing a minor bump for this purely because we couldn't shim an 0.5.x build in 0.4.x without also bumping the MSRV of 0.4.x anyways, and wouldn't want to issue a breaking release for log that didn't automatically forward 0.4.x through 0.5.x.

Thomasdezeeuw commented 1 year ago

We could do a release with a bump to 1.60 and be prepared to yank it if people complain... Not an ideal solution, but maybe we're making a bigger deal of the MSRV bump than needed.

KodrAus commented 1 year ago

In this case, I think we've got something really tangible to offer people for the bump too; one less dependency running arbitrary code at build-time.

In our release notes we can mention why we couldn't do this in a minor bump, because the log macros for multiple versions need to forward through the same global logger. We can't even pull just this mechanism out into its own crate because that's the bit that needs the MSRV bump.

GuillaumeGomez commented 1 year ago

Thanks everyone!