time-rs / time

The most used Rust library for date and time handling.
https://time-rs.github.io
Apache License 2.0
1.1k stars 277 forks source link

error[E0557]: feature has been removed #618

Closed da2ce7 closed 1 year ago

da2ce7 commented 1 year ago

Unable to build in latest nightly:

error[E0557]: feature has been removed
  --> /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/time-0.3.28/src/lib.rs:79:39
   |
79 | #![cfg_attr(coverage_nightly, feature(no_coverage))]
   |                                       ^^^^^^^^^^^ feature has been removed
   |
   = note: renamed to `coverage_attribute`

https://github.com/time-rs/time/blob/6aa8db51b9949c46b352579c9e951056b8ce333f/time/src/lib.rs#L79C1-L79C53

QuantumDancer commented 1 year ago

Caused by https://github.com/rust-lang/rust/pull/114656

jhpratt commented 1 year ago

Thanks for the info; I wasn't aware it was changing syntax. Ultimately it's only a CI thing, as it is opt-in. Given that, I'll get around to updating it in the coming days as it's not urgent.

DDtKey commented 1 year ago

Hi @jhpratt

It's unfortunately not only CI thing for time crate. It affects dependent code in case of running coverage checks by llvm-cov, so it would be great to have a new version with fixed behavior.

jhpratt commented 1 year ago

I will point out that coverage_nightly is only passed on nightly as the name implies. There are no stability guarantees when using nightly — including from time. There is currently not a release planned, as I still have a few things I want to wrap up before that will occur.

DDtKey commented 1 year ago

Yes, for sure. I don't expect any "guarantees" related to this feature. It's just pretty common way to use llvm-cov under nightly compiler, because no_coverage/coverage(off) is still nightly feature.

I'm wondering why it's not possible to make minor (patch) release to solve the issue for nightly builds? It would be helpful. You can see that there are several GH issues(and these are only public ones) already referred to this one, and most of them had to disable coverage step for a while.

Regardless of the decision, I do appreciate all your efforts to maintain the time crate. Thank you!

jhpratt commented 1 year ago

I'm wondering why it's not possible to make minor (patch) release to solve the issue for nightly builds?

Pretty much as I said — there's a couple things that aren't complete that I want to include. They're pretty minor and as such shouldn't take too long, but that's the reason.

djc commented 1 year ago

That doesn't feel like a satisfying answer to me. The time crate has been breaking a bunch of downstream CI setups, a PR was submitted with a fix 5 days ago, and semver-compatible releases ought to be cheap. You could presumably have published this fix and held back your "couple things" for another semver-compatible release?

jhpratt commented 1 year ago

As I said, if you're using nightly, breakage is expected. I do not create releases solely to fix something that happens only on nightly. I am working on other things at the moment and will create a release when I feel it is appropriate. I'm not going to be pressured into anything. If it is affecting companies' CI, they are free to sponsor me to ensure that I have more time to work on things.

DDtKey commented 1 year ago

TBH, it's hard to understand. There is a simple win-win strategy here, make a semver compatible release and solve problems for many users. In turn, this will save you from the need to answer the same questions or argue.

Especially considering that the fix was already done by an external contributor, but it turns out that people must either turn off their CI or patch the version themselves. In addition, it's not only about companies' CI, it affects other open source projects as well where coverage is kinda important

jhpratt commented 1 year ago

There's no point in me reiterating the same thing over and over. Locking this issue.