Cyclic package dependency with certain package features set

[ldm0]

Create a simple cargo project with these dependencies in Cargo.toml

parking_lot = { version = "0.12.3", features = ["deadlock_detection"] }
once_cell = { version = "1.19.0", features = ["parking_lot"] }
cc = { version = "1.1.0", features = ["parallel"] }

Run cargo check, then cargo will emits error:

error: cyclic package dependency: package `backtrace v0.3.73` depends on itself. Cycle:
package `backtrace v0.3.73`
    ... which satisfies dependency `backtrace = "^0.3.60"` of package `parking_lot_core v0.9.10`
    ... which satisfies dependency `parking_lot_core = "^0.9.3"` of package `once_cell v1.19.0`
    ... which satisfies dependency `once_cell = "^1.19"` of package `cc v1.1.0`
    ... which satisfies dependency `cc = "^1.0.97"` of package `backtrace v0.3.73`

The dependencies formed a cycle after #1037 (which introduces the once_cell dependency)

[NobodyXu]

Thanks for reporting, I will have a look to see if we could remove that dependency...

[NobodyXu]

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:

cc-rs/src/parallel/job_token.rs

Line 189 in bd5a4ec

.get_or_try_init(|| HelperThread::new(self.jobserver))?;

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()))
    }
}

[ldm0]

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.

[NobodyXu]

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

[ldm0]

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. :-/

[NobodyXu]

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.

[NobodyXu]

Alternative solution if we don't want to bump MSRV, is to vendor the once_cell::sync