feat: allow depending on self

[Eh2406]

In general PubGrub allows cyclical imports. It is totally okay for A to depend on B which depends on A. We have a special exception for A depending on A. This is extra code and kind of inconsistent.

The argument for the error message was to force the user to choose one of the two reasonable semantics:

• Self dependencies are okay, but meaningless. In which case the dependency provider should filter them out.
• Self dependencies are not okay. In which case the dependency provider should return Unavailable with a reason.

I believe that the UV fork of this project removed this code to implement the "self dependencies are okay" semantics.

In Cargo we (currently) take a somewhat inconsistent position. We treat the self dependency as okay for dependency resolution, but then error later due to the cyclical import. I would like to maintain bug for bug compatibility as long as possible to avoid tying upgrading to PubGrub to other breaking changes. The easiest way to implement this "it's not a problem for resolution but the edge does exist for cycle checking" is for PubGrub to simply not error. (Although if we decide not to merge this I have other options.)

[mpizenberg]

It seems to have been added for version 0.2, back in 2020 (

pubgrub/CHANGELOG.md

Line 54 in ff09b46

`SelfDependency`, `ErrorChoosingPackageVersion` and `ErrorInShouldCancel`.

).

I think I added it because cyclical dependencies are forbidden in Elm, and I don’t think its trivial to figure out without the solver signaling them. But your comment about cargo behavior seems to imply it is not too complex

[konstin]

I believe cyclical dependencies are something that the caller should decide on when processing the metadata (so they can allow it in python and ban it in elm), so not something that pubgrub should check.

[mpizenberg]

The thing is non-trivial cyclical dependencies are not easy to spot without the dependency solver. And I can see 3 different situations:

1. All cyclical dependencies are allowed
2. Cyclical dependencies are tolerated while solving but just discarded as invalid solutions, so the solver continues
3. Cyclical are strictly forbidden and any discovered cyclic dependency that may occur while building your app and not having published yet your new version but using a dependency that depends on your very package at an earlier (compatible range) version should raise an error to prevent you from going through making a release.

How do you differentiate these 3 behaviors without the help of the solver and only metadata?

[mpizenberg]

My bad, the current solver doesn’t check the above 3 anyway. It’s just checked in direct dependencies. Well I guess then it’s an easy check on the provider part to decide.

[Eh2406]

In general PubGrub allows cyclical imports. It is totally okay for A to depend on B which depends on A. We have a special exception for A depending on A. This is extra code and kind of inconsistent.

The only check that PubGrub currently supports if the trivial one.

If you have a structure like

    fn get_dependencies(
        &self,
        package: &P,
        version: &VS::V,
    ) -> Result<Dependencies<P, VS, Self::M>, NewErr> {
        let mut out = Dependencies::default();
        ...
        Ok(Dependencies::Available(out))
    }

then to maintain the current behavior you just have to add

        if out.contains_key(package) {
           return Err(NewErr::New());
        }

or to backtrack on self dependencies you add

        if out.contains_key(package) {
           return Ok(Dependencies::Unavailable("self dep"));
        }