Skip to content

Commit

Permalink
Merge pull request #24904 from JuliaLang/cb/resolvefix
Browse files Browse the repository at this point in the history 
Fixes and improvements to Pkg resolver
  • Loading branch information
@carlobaldassi
carlobaldassi authored Dec 5, 2017
2 parents 9b52fc0 + 208f633 commit 8fb8db1
Show file tree
Hide file tree
Showing 7 changed files with 204 additions and 84 deletions.
6 changes: 4 additions & 2 deletions base/pkg/entry.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,8 +474,10 @@ function resolve(
have :: Dict = Read.free(instd),
upkgs :: Set{String} = Set{String}()
)
orig_reqs = reqs
reqs, bktrc = Query.requirements(reqs, fixed, avail)
bktrc = Query.init_resolve_backtrace(reqs, fixed)
orig_reqs = deepcopy(reqs)
Query.check_fixed(reqs, fixed, avail)
Query.propagate_fixed!(reqs, bktrc, fixed)
deps, conflicts = Query.dependencies(avail, fixed)

for pkg in keys(reqs)
Expand Down
127 changes: 82 additions & 45 deletions base/pkg/query.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module Query
import ...Pkg.PkgError
using ..Types

function requirements(reqs::Dict, fix::Dict, avail::Dict)
function init_resolve_backtrace(reqs::Requires, fix::Dict{String,Fixed} = Dict{String,Fixed}())
bktrc = ResolveBacktrace()
for (p,f) in fix
bktrc[p] = ResolveBacktraceItem(:fixed, f.version)
Expand All @@ -14,7 +14,10 @@ function requirements(reqs::Dict, fix::Dict, avail::Dict)
bktrcp = get!(bktrc, p) do; ResolveBacktraceItem() end
push!(bktrcp, :required, vs)
end
return bktrc
end

function check_fixed(reqs::Requires, fix::Dict{String,Fixed}, avail::Dict)
for (p1,f1) in fix
for p2 in keys(f1.requires)
haskey(avail, p2) || haskey(fix, p2) || throw(PkgError("unknown package $p2 required by $p1"))
Expand All @@ -26,7 +29,9 @@ function requirements(reqs::Dict, fix::Dict, avail::Dict)
warn("$p1 is fixed at $(f1.version) conflicting with requirement for $p2: $(f2.requires[p1])")
end
end
reqs = deepcopy(reqs)
end

function propagate_fixed!(reqs::Requires, bktrc::ResolveBacktrace, fix::Dict{String,Fixed})
for (p,f) in fix
merge_requires!(reqs, f.requires)
for (rp,rvs) in f.requires
Expand All @@ -37,7 +42,7 @@ function requirements(reqs::Dict, fix::Dict, avail::Dict)
for (p,f) in fix
delete!(reqs, p)
end
reqs, bktrc
reqs
end

# Specialized copy for the avail argument below because the deepcopy is slow
Expand All @@ -53,39 +58,71 @@ function availcopy(avail)
return new_avail
end

# Generate a reverse dependency graph (package names only)
function gen_backdeps(avail::Dict)
backdeps = Dict{String,Set{String}}()
for (ap,av) in avail, (v,a) in av, rp in keys(a.requires)
s = get!(backdeps, rp) do; Set{String}() end
push!(s, ap)
end
return backdeps
end

function dependencies(avail::Dict, fix::Dict = Dict{String,Fixed}("julia"=>Fixed(VERSION)))
avail = availcopy(avail)
conflicts = Dict{String,Set{String}}()
to_expunge = VersionNumber[]
emptied = String[]
backdeps = gen_backdeps(avail)

for (fp,fx) in fix
delete!(avail, fp)
for (ap,av) in avail, (v,a) in copy(av)
if satisfies(fp, fx.version, a.requires)
delete!(a.requires, fp)
else
haskey(conflicts, ap) || (conflicts[ap] = Set{String}())
push!(conflicts[ap], fp)
haskey(backdeps, fp) || continue
# for (ap,av) in avail
for ap in backdeps[fp]
haskey(avail, ap) || continue
av = avail[ap]
empty!(to_expunge)
for (v,a) in av
if satisfies(fp, fx.version, a.requires)
delete!(a.requires, fp)
else
conflicts_ap = get!(conflicts, ap) do; Set{String}() end
push!(conflicts_ap, fp)
# don't delete v from av right away so as not to screw up iteration
push!(to_expunge, v)
end
end
for v in to_expunge
delete!(av, v)
end
isempty(av) && push!(emptied, ap)
end
end
again = true
while again
again = false
while !isempty(emptied)
deleted_pkgs = String[]
for (ap,av) in avail
if isempty(av)
delete!(avail, ap)
push!(deleted_pkgs, ap)
again = true
end
for ap in emptied
delete!(avail, ap)
push!(deleted_pkgs, ap)
end
empty!(emptied)

for dp in deleted_pkgs
for (ap,av) in avail, (v,a) in copy(av)
if haskey(a.requires, dp)
haskey(conflicts, ap) || (conflicts[ap] = Set{String}())
union!(conflicts[ap], conflicts[dp])
haskey(backdeps, dp) || continue
for ap in backdeps[dp]
haskey(avail, ap) || continue
av = avail[ap]
empty!(to_expunge)
for (v,a) in av
haskey(a.requires, dp) || continue
conflicts_ap = get!(conflicts, ap) do; Set{String}() end
union!(conflicts_ap, conflicts[dp])
push!(to_expunge, v)
end
for v in to_expunge
delete!(av, v)
end
isempty(av) && push!(emptied, ap)
end
end
end
Expand Down Expand Up @@ -273,23 +310,28 @@ function filter_versions(reqs::Requires, deps::Dict{String,Dict{VersionNumber,Av
# Collect all required packages
isreq = Dict{String,Bool}(rp=>true for a in values(fdepsp) for rp in keys(a.requires))
# Compute whether a required package appears in all requirements
for a in values(fdepsp), rp in keys(isreq)
haskey(a.requires, rp) || (isreq[rp] = false)
for rp in keys(isreq)
isreq[rp] = all(haskey(a.requires, rp) for a in values(fdepsp))
end

# Create a list of candidates for new implicit requirements
staged_new = Set{String}()
for a in values(fdepsp)
for (rp,rvs) in a.requires
# Skip packages that may not be required
isreq[rp] || continue
# Compute the union of the version sets
snvs = get!(staged_next, rp, copy(rvs))
for a in values(fdepsp), (rp,rvs) in a.requires
# Skip packages that may not be required
isreq[rp] || continue
# Compute the union of the version sets
if haskey(staged_next, rp)
snvs = staged_next[rp]
union!(snvs, rvs)
push!(staged_new, rp)
else
snvs = copy(rvs)
staged_next[rp] = snvs
end
push!(staged_new, rp)
end
for rp in staged_new
@assert isreq[rp]
srvs = staged_next[rp]
isreq[rp] || continue
bktrcp = get!(bktrc, rp) do; ResolveBacktraceItem(); end
push!(bktrcp, p=>bktrc[p], srvs)
if isa(bktrcp.versionreq, VersionSet) && isempty(bktrcp.versionreq)
Expand All @@ -306,7 +348,7 @@ function filter_versions(reqs::Requires, deps::Dict{String,Dict{VersionNumber,Av
filtered_deps = Dict{String,Dict{VersionNumber,Available}}()
for (p,depsp) in deps
filtered_deps[p] = Dict{VersionNumber,Available}()
allowedp = get(allowed, p, Dict{VersionNumber,Bool}())
allowedp = get(allowed, p) do; Dict{VersionNumber,Bool}() end
fdepsp = filtered_deps[p]
for (vn,a) in depsp
get(allowedp, vn, true) || continue
Expand All @@ -326,6 +368,9 @@ end
# Preliminarily calls filter_versions.
function prune_versions(reqs::Requires, deps::Dict{String,Dict{VersionNumber,Available}}, bktrc::ResolveBacktrace)
filtered_deps, allowed = filter_versions(reqs, deps, bktrc)
if !isempty(reqs)
filtered_deps = dependencies_subset(filtered_deps, Set{String}(keys(reqs)))
end

# To each version in each package, we associate a BitVector.
# It is going to hold a pattern such that all versions with
Expand Down Expand Up @@ -432,7 +477,7 @@ function prune_versions(reqs::Requires, deps::Dict{String,Dict{VersionNumber,Ava
# Recompute deps. We could simplify them, but it's not worth it
new_deps = Dict{String,Dict{VersionNumber,Available}}()

for (p,depsp) in deps
for (p,depsp) in filtered_deps
@assert !haskey(new_deps, p)
if !haskey(pruned_vers, p)
new_deps[p] = depsp
Expand Down Expand Up @@ -533,18 +578,10 @@ function undirected_dependencies_subset(deps::Dict{String,Dict{VersionNumber,Ava
return subdeps(deps, allpkgs)
end

function prune_dependencies(reqs::Requires, deps::Dict{String,Dict{VersionNumber,Available}})
bktrc = ResolveBacktrace()
for (p,vs) in reqs
bktrc[p] = ResolveBacktraceItem(:required, vs)
end
return prune_dependencies(reqs, deps, bktrc)
end

function prune_dependencies(reqs::Requires, deps::Dict{String,Dict{VersionNumber,Available}}, bktrc::ResolveBacktrace)
deps = dependencies_subset(deps, Set{String}(keys(reqs)))
function prune_dependencies(reqs::Requires,
deps::Dict{String,Dict{VersionNumber,Available}},
bktrc::ResolveBacktrace = init_resolve_backtrace(reqs))
deps, _ = prune_versions(reqs, deps, bktrc)

return deps
end

Expand Down
40 changes: 24 additions & 16 deletions base/pkg/resolve.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,7 @@ function sanity_check(deps::Dict{String,Dict{VersionNumber,Available}},
end
end

vers = Vector{Tuple{String,VersionNumber,VersionNumber}}()
for (p,d) in deps, vn in keys(d)
lvns = VersionNumber[Iterators.filter(vn2->(vn2>vn), keys(d))...]
nvn = isempty(lvns) ? typemax(VersionNumber) : minimum(lvns)
push!(vers, (p,vn,nvn))
end
vers = [(p,vn) for (p,d) in deps for vn in keys(d)]
sort!(vers, by=pvn->(-ndeps[pvn[1]][pvn[2]]))

nv = length(vers)
Expand All @@ -89,16 +84,34 @@ function sanity_check(deps::Dict{String,Dict{VersionNumber,Available}},
checked = falses(nv)

problematic = Vector{Tuple{String,VersionNumber,String}}()

i = 1
psl = 0
for (p,vn,nvn) in vers
for (p,vn) in vers
ndeps[p][vn] == 0 && break
checked[i] && (i += 1; continue)

sub_reqs = Dict{String,VersionSet}(p=>VersionSet([vn, nvn]))
local sub_deps::Dict{String,Dict{VersionNumber,Available}}
fixed = Dict{String,Fixed}(p=>Fixed(vn, deps[p][vn].requires), "julia"=>Fixed(VERSION))
sub_reqs = Dict{String,VersionSet}()
bktrc = Query.init_resolve_backtrace(sub_reqs, fixed)
Query.propagate_fixed!(sub_reqs, bktrc, fixed)
sub_deps = Query.dependencies_subset(deps, Set{String}([p]))
sub_deps, conflicts = Query.dependencies(sub_deps, fixed)

try
sub_deps = Query.prune_dependencies(sub_reqs, deps)
for pkg in keys(sub_reqs)
if !haskey(sub_deps, pkg)
if "julia" in conflicts[pkg]
throw(PkgError("$pkg can't be installed because it has no versions that support $VERSION " *
"of julia. You may need to update METADATA by running `Pkg.update()`"))
else
sconflicts = join(conflicts[pkg], ", ", " and ")
throw(PkgError("$pkg's requirements can't be satisfied because " *
"of the following fixed packages: $sconflicts"))
end
end
end
Query.check_requirements(sub_reqs, sub_deps, fixed)
sub_deps = Query.prune_dependencies(sub_reqs, sub_deps, bktrc)
catch err
isa(err, PkgError) || rethrow(err)
## info("ERROR MESSAGE:\n" * err.msg)
Expand Down Expand Up @@ -133,11 +146,6 @@ function sanity_check(deps::Dict{String,Dict{VersionNumber,Available}},
end
end
if ok
let p0 = interface.pdict[p]
svn = red_pvers[p0][sol[p0]]
@assert svn == vn
end

for p0 = 1:red_np
s0 = sol[p0]
if s0 != red_spp[p0]
Expand Down
16 changes: 16 additions & 0 deletions base/pkg/resolve/fieldvalue.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,22 @@ FieldValue(l0::Integer, l1::VersionWeight) = FieldValue(l0, l1, zero(VersionWeig
FieldValue(l0::Integer) = FieldValue(l0, zero(VersionWeight))
FieldValue() = FieldValue(0)

# This isn't nice, but it's for debugging only anyway
function Base.show(io::IO, a::FieldValue)
print(io, a.l0)
a == FieldValue(a.l0) && return
print(io, ".", a.l1)
a == FieldValue(a.l0, a.l1) && return
print(io, ".", a.l2)
a == FieldValue(a.l0, a.l1, a.l2) && return
print(io, ".", a.l3)
a == FieldValue(a.l0, a.l1, a.l2, a.l3) && return
print(io, ".", a.l4)
a == FieldValue(a.l0, a.l1, a.l2, a.l3, a.l4) && return
print(io, ".", a.l5)
return
end

const Field = Vector{FieldValue}

Base.zero(::Type{FieldValue}) = FieldValue()
Expand Down
22 changes: 18 additions & 4 deletions base/pkg/resolve/interface.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ mutable struct Interface

function Interface(reqs::Requires, deps::Dict{String,Dict{VersionNumber,Available}})
# generate pkgs
pkgs = sort!(String[Set{String}(keys(deps))...])
pkgs = sort!(String[keys(deps)...])

np = length(pkgs)

Expand Down Expand Up @@ -216,9 +216,23 @@ function verify_solution(sol::Vector{Int}, interface::Interface)
if sol[p0] == v0
for (rp, rvs) in a.requires
p1 = pdict[rp]
sol[p1] != spp[p1] || return false
vn = pvers[p1][sol[p1]]
vn ∈ rvs || return false
if sol[p1] == spp[p1]
println("""
VERIFICATION ERROR: REQUIRED DEPENDENCY NOT INSTALLED
package p=$p (p0=$p0) version=$vn (v0=$v0) requires package rp=$rp in version set rvs=$rvs
but package $rp is not being installed (p1=$p1 sol[p1]=$(sol[p1]) == spp[p1]=$(spp[p1]))
""")
return false
end
vn1 = pvers[p1][sol[p1]]
if vn1 ∉ rvs
println("""
VERIFICATION ERROR: INVALID VERSION
package p=$p (p0=$p0) version=$vn (v0=$v0) requires package rp=$rp in version set rvs=$rvs
but package $rp version is being set to $vn1 (p1=$p1 sol[p1]=$(sol[p1]) spp[p1]=$(spp[p1]))
""")
return false
end
end
end
end
Expand Down
Loading

0 comments on commit 8fb8db1

Please sign in to comment.