don't panic: on bad dependencies

[Eh2406]

Minimizing from the panic found in #34 (comment). I get the small test case of:

#[test]
fn should_always_find_a_satisfier() {
    let mut dependency_provider = OfflineDependencyProvider::<_, NumberVersion>::new();
    dependency_provider.add_dependencies("a", 0, vec![("b", Range::none())]);

    // Run the algorithm.
    let _ = resolve(&dependency_provider, "a", 0);
}

While this is a silly input, we should not panic on it.

[mpizenberg]

Nice find! Using Range::none() in dependencies invalidates the pattern for building incompatibilities reflecting dependencies in the form : { package: Positive(v), dependedUponPackage: Negative(range) }. Because if Range::none() is used, we get { package: Positive(v), dependedUponPackage: Negative(Range::none) } which is equivalent to { package: Positive(v) } (though the incompat is not reduced when building dependencies) which just says well we cannot pick that package version.

[mpizenberg]

There might be an incentive though to authorize people using Range::none() when they want to express directly that a package depend on another package not being in the dependency. In such case, we should probably identify that Range::none() and generate an incompatibility of the form { package: Positive(v), dependedUponPackage: Positive(any)} but that would be a half-backed solution for more advanced "anti-dependency" expressions and I propose to postpone that to the next version if we need it. For now, let's just try to fix the panic in meaningful way.

[mpizenberg]

This also makes me wonder how would the solver react to a dependency using the same package (I suppose packages in the hash map will be overwritten leading in pretty wrong conclusions). Something like add_dependency( "p", 0, vec![ ("p", whatever) ])

[Eh2406]

This is a hack but is the resolve when making dependencies I added:

            Some(x) => {
                if x.values().any(|r| r == &Range::none()) {
                    // todo: what is the correct Incompatibility?
                    state.add_incompatibility(|id| {
                        Incompatibility::unavailable_dependencies(id, p.clone(), v.clone())
                    });
                    continue;
                }
                x
            },

Witch gets this test to pass. What would be a more elegant solution?

[mpizenberg]

Witch gets this test to pass. What would be a more elegant solution?

Maybe we should fail, with a dedicated error type explaining why Range::none() should not be used for dependencies?