Resolver feature request: compatibility on optional dependencies

[KristofferC]

@carlobaldassi, we are thinking about possible ways of adding conditional dependencies. In almost all cases it seems there is a need for a package to be able to add compatibility to a package that does not have to end up getting installed.

So as an example:

[deps]
A = "..."
B = "..."

[weakdeps]
C = "..."

[compat]
A = "0.1"
B = "0.2"
C = "0.3"

In this case, we want to require C to have the "0.3" compat if the package C ends up getting installed. But if it happens that C does not get installed, then that is also fine.

How hard do you think something like that would be to implement. And do you see any immediate problems conceptually by such a thing?

I guess one question is how much score you give C being installed vs the score of upgrading other versions (that might be on lower versions if C is installed). But the existing resolver already have to deal with questions like that I guess.

[carlobaldassi]

As far as I can tell, that should be pretty easy to do, almost straightforward. The "not installed" condition is treated by the resolver like one possible version (the highest-priority one, when available). At the moment, requiring a package just forbids this version, and the compat requirements possibly forbid other versions too. We could just allow the resolver to mask out the undesirable versions (say, from "0.0" to "0.2" in your example) but not the "uninstalled" version. All of this masking-out happens in a very preliminary phase, when building the graph, so there should really be no problem in just effectively removing the undesirable versions at that stage and let the rest stay as-is. There may be some small work to do on the logger, possibly, in order to provide the correct diagnostic error messages. In terms of the priority attributed to installing the package, what the resolver would do I guess is to try to avoid installing the package unless it helps with getting a better version of the other installed packages. Same as any other non-required package, essentially. To me, this seems ok.

[KristofferC]

That's nice to hear. Thinking about how this should be passed to the resolver; right now, we pass the following:

# Compat
# {package => {package_version => {dependency => compat}}}
all_compat::Dict{UUID,Dict{VersionNumber,Dict{UUID,VersionSpec}}}()

# Fixed
# {package => (version, {dependency => compat})}
fixed::Dict{UUID, Requires.Fixed}

So both of the {dependency => compat} in these data structures can be wither "weak" (which does not require them to be installed) or strong (like a normal dependency).

One way is to just have weak versions of these data structures:

weak_all_compat
weak_fixed

with the same type as those above. Alternative, we generalize the Dict{UUID,VersionSpec} to be something like

struct Dependency
    uuid::UUID
    vspec::VersionSpec
    isweak::Bool
end
all_compat::Dict{UUID,Dict{VersionNumber,Dependency}}()

Any opinion about that? And if I implemented the Pkg side of those things, do you think you have time to add the stuff for propagating that into the resolver?

[IanButterworth]

@carlobaldassi say VersionSpec now has a weak::Bool field, is there more than this needed?

diff --git a/src/Resolve/graphtype.jl b/src/Resolve/graphtype.jl
index c2f43558f0..d5c482915f 100644
--- a/src/Resolve/graphtype.jl
+++ b/src/Resolve/graphtype.jl
@@ -278,8 +278,10 @@ mutable struct Graph
             req_msk = Dict{Int,BitVector}()
             for (p1, vs) in req
                 pv = pvers[p1]
-                req_msk_p1 = BitVector(undef, spp[p1] - 1)
-                @inbounds for i in 1:spp[p1] - 1
+                # if it's a weak dep, allow it to be uninstalled
+                num_allowed = spp[p1] - (vs.weak ? 0 : 1)
+                req_msk_p1 = BitVector(undef, num_allowed)
+                @inbounds for i in 1:num_allowed
                     req_msk_p1[i] = pv[i] ∈ vs
                 end
                 req_msk[p1] = req_msk_p1

[carlobaldassi]

Sorry for the long hiatus. @IanButterworth that code would try to call pvers[p1][spp[p1]] which is out of bounds, see https://github.com/JuliaLang/Pkg.jl/blob/91b4e05b57b146c78fcd066c2e1942735b9a6ddc/src/Resolve/graphtype.jl#L128-L129 Re-reading the thread, I think had likely misunderstood the original idea. Still, this should not affect the simplicity of implementation hopefully, it's just that instead of acting on req_msk the change should go into gmsk. As soon as I can, I'll try to produce an implementation based on the assumption that VersionSpec has a weak field, as in Ian's proposal, and try to evaluate all the implications in the code and whether it breaks any assumption in any of the stages (I don't think so).

[IanButterworth]

Great. Note that VersionSpec having a weak field is just one idea of how to communicate it to the resolver, but I guess it's a reasonable starting point

[IanButterworth]

@carlobaldassi are you on the julia slack? I couldn't find you

[IanButterworth]

friendly bump @carlobaldassi

[IanButterworth]

I've tried a number of times to figure out what to do based on your suggestions @carlobaldassi but I've not been able to figure it out. I find the variable names and code in the resolver a little tricky to follow. I've not worked on graph code before, so it may be a familiarity thing.

If you are able to give any more pointers, or an estimate of when you might be able to work on it, it would be great to know, as having a working conditional deps demo working soon would be helpful for a number of reasons.

[carlobaldassi]

Sorry Ian, I've been really strapped for time lately (family reasons, mostly). I am hoping to work on this during this weekend.