Closed ldm0 closed 1 week ago
Thanks for reporting, I will have a look to see if we could remove that dependency...
once_cell
is used if feature cc/parallel
is enabled, so if that feature is disabled it should be ok.
We use once_cell
since parallel
feature needs get_or_try_init
:
The one in std OnceLock
is still unstable rust-lang/rust#109737 , and our MSRV is too low with objection to increase it #1144
And it looks like OnceLock::get_or_try_init
won't be stablized in the near future rust-lang/rust#107122 , so we need an alternative solution.
We can either roll our own get_or_try_init
, since we never calls get_or_try_init
again if it fails, the implementation can be relatively simple without unsafe
:
use std::sync::OnceLock;
pub struct OnceCell<T>(OnceLock<Option<T>>);
impl<T> OnceCell<T> {
pub fn get_or_try_init(&self, f: FnOnce() -> io::Result<T>) -> io::Result<&T> {
let err = None;
let ret = self.0.get_or_init(|| {
match f() {
Ok(ret) => Some(ret),
Err(error) => {
err = Some(error);
None
}
}
});
ret.as_ref().ok_or_else(|| err.unwrap_or_else(|| io::ErrorKind::Other.into()))
}
}
Yeah, I understand that OnceCell
is somehow necessary there, and MSRV bumping is not that easy :-(.
I haven't enabled cc/parallel
by myself, one node on the project dependency graph does: https://github.com/gyscos/zstd-rs/blob/main/zstd-safe/zstd-sys/Cargo.toml#L63. Therefore, I can't disable the feature flag without forking zstd-sys
, and I suspect there will be more dependencies enabling cc/parallel
in the future. As long as one crate on the dependency graph does this, the whole project will be affected.
I've already patched cc-rs
(by replacing OnceCell
with OnceLock
) to workaround this problem locally.
I've already patched cc-rs(by replacing OnceCell with OnceLock) to workaround this problem locally.
It would be awesome if you can open a PR with the patch!
I will do the code review
I've already patched cc-rs(by replacing OnceCell with OnceLock) to workaround this problem locally.
It would be awesome if you can open a PR with the patch!
I will do the code review
Yeah I am happy to open a PR.
However, the main problem is that OnceLock
was stablized in 1.70
, while MSRV of cc
is 1.67
.
Using OnceLock
is suitable for us but it breaks MSRV, so I suspect it's not for everyone. :-/
However, the main problem is that OnceLock was stablized in 1.70, while MSRV of cc is 1.67.
I would accept a PR, this msrv bump is necessary to fix the compilation error.
Alternative solution if we don't want to bump MSRV, is to vendor the once_cell::sync
Create a simple cargo project with these dependencies in Cargo.toml
Run
cargo check
, then cargo will emits error:The dependencies formed a cycle after https://github.com/rust-lang/cc-rs/pull/1037 (which introduces the
once_cell
dependency)