Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

panic on cyclical dev-dependency with missing feature #12941

Closed
ehuss opened this issue Nov 9, 2023 · 2 comments
Closed

panic on cyclical dev-dependency with missing feature #12941

ehuss opened this issue Nov 9, 2023 · 2 comments
Labels
A-dependency-resolution Area: dependency resolution and the resolver C-bug Category: bug S-triage Status: This issue is waiting on initial triage.

Comments

@ehuss
Copy link
Contributor

ehuss commented Nov 9, 2023

Problem

A cyclical dev-dependency with a missing feature causes a panic:

thread 'main' panicked at src/cargo/util/graph.rs:149:20:
the only path was a cycle, no dependency graph has this shape
stack backtrace:
   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::option::expect_failed
   3: <cargo::util::graph::Graph<cargo::core::package_id::PackageId, im_rc::hash::set::HashSet<cargo::core::dependency::Dependency>>>::path_to::<<cargo::util::graph::Graph<cargo::core::package_id::PackageId, im_rc::hash::set::HashSet<cargo::core::dependency::Dependency>>>::path_to_bottom::{closure#0}, core::iter::adapters::flatten::FlatMap<core::option::IntoIter<&im_rc::ord::map::OrdMap<cargo::core::package_id::PackageId, im_rc::hash::set::HashSet<cargo::core::dependency::Dependency>>>, im_rc::ord::map::Iter<cargo::core::package_id::PackageId, im_rc::hash::set::HashSet<cargo::core::dependency::Dependency>>, <cargo::util::graph::Graph<cargo::core::package_id::PackageId, im_rc::hash::set::HashSet<cargo::core::dependency::Dependency>>>::edges::{closure#0}>>
   4: cargo::core::resolver::errors::describe_path_in_context
   5: cargo::core::resolver::errors::activation_error
   6: cargo::core::resolver::activate_deps_loop
   7: cargo::core::resolver::resolve
   8: cargo::ops::resolve::resolve_with_previous
   9: cargo::ops::cargo_generate_lockfile::generate_lockfile
  10: cargo::commands::generate_lockfile::exec
  11: cargo::cli::main
  12: cargo::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Steps

A test using cargo's test infrastructure:

#[cargo_test]
fn cyclical_dep_with_missing_feature() {
    // Checks for error handling when a cyclical dev-dependency specify a
    // feature that doesn't exist.
    let p = project()
        .file(
            "Cargo.toml",
            r#"
                [package]
                name = "foo"
                version = "0.1.0"

                [dev-dependencies]
                foo = { path = ".", features = ["missing"] }
            "#,
        )
        .file("src/lib.rs", "")
        .build();
    p.cargo("check")
        .with_status(101)
        .with_stderr("ENTER CORRECT ERROR HERE")
        .run();
}

Possible Solution(s)

I'm not sure.

Notes

No response

Version

cargo 1.75.0-nightly (b4d18d4bd 2023-10-31)
release: 1.75.0-nightly
commit-hash: b4d18d4bd3db6d872892f6c87c51a02999b80802
commit-date: 2023-10-31
host: aarch64-apple-darwin
libgit2: 1.7.1 (sys:0.18.1 vendored)
libcurl: 8.1.2 (sys:0.4.68+curl-8.4.0 system ssl:(SecureTransport) LibreSSL/3.3.6)
ssl: OpenSSL 1.1.1u  30 May 2023
os: Mac OS 14.0.0 [64-bit]
@ehuss ehuss added C-bug Category: bug A-dependency-resolution Area: dependency resolution and the resolver S-triage Status: This issue is waiting on initial triage. labels Nov 9, 2023
@Eh2406
Copy link
Contributor

Eh2406 commented Nov 13, 2023

Investigating now. By the way, how did you come across this?

bors added a commit that referenced this issue Nov 15, 2023
If the only path is a loop then counted as the shortest path.

This is a fix for #12941

This graph data structure is used to store dependency DAGs. Where each edge represents a dependency from a package to the package that fulfilled the dependency. Different parts of the resolver store this data in opposite directions, sometimes packages point at the things that depend on them other times packages point to the parents that required them. Error messages often need to report on why a package is in the graph, either by walking up toward parents or down toward children depending on how this graph is stored. #12678 unified the two different walking implementations, and replace them with a breadth first search so as to find the shortest path. This code ignored when edge pointed at a package that had already been reached, because that generally describes a longer path to an existing package.

Unfortunately, when I said this was a DAG that was a simplification. There can be cycles introduced as dev-dependencies. The existing code would reasonably ignore the cycles figuring that if we continue searching we would eventually find the root package (a package that nothing depended on). Missing the possibility that the root package created the cycle.

Now we search through the entire graph looking for a root package. If we do not find a root package we report the path to the last package we processed.
@Eh2406 Eh2406 closed this as completed Nov 15, 2023
@ehuss
Copy link
Contributor Author

ehuss commented Nov 16, 2023

By the way, how did you come across this?

I was just testing something for some other issue, and forgot to put in the [features] table, and hit the panic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependency-resolution Area: dependency resolution and the resolver C-bug Category: bug S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Development

No branches or pull requests

2 participants