Merge pull request #2720 from JuliaLang/backports-release-1.7

Backports for 1.7

Author: KristofferC

src/API.jl

@@ -1435,7 +1435,9 @@ function instantiate(ctx::Context; manifest::Union{Bool, Nothing}=nothing,
         return instantiate(Context(); manifest=manifest, update_registry=update_registry, verbose=verbose, kwargs...)
     end
     if (!isfile(ctx.env.manifest_file) && manifest === nothing) || manifest == false
-        up(ctx; update_registry=update_registry)
+        # given no manifest exists, only allow invoking a registry update if there are project deps
+        allow_registry_update = isfile(ctx.env.project_file) && !isempty(ctx.env.project.deps)
+        up(ctx; update_registry = update_registry && allow_registry_update)
         return
     end
     if !isfile(ctx.env.manifest_file) && manifest == true

src/REPLMode/REPLMode.jl

@@ -692,6 +692,7 @@ function try_prompt_pkg_add(pkgs::Vector{Symbol})
         println(ctx.io)
         return false
     end
+    resp = strip(resp)
     if lowercase(resp) in ["y", "yes"]
         API.add(string.(available_pkgs))
         if length(available_pkgs) < length(pkgs)

src/Resolve/Resolve.jl

@@ -341,57 +341,18 @@ function verify_solution(sol::Vector{Int}, graph::Graph)
     return true
 end
 
+
 """
-Push the given solution to a local optimium if needed: keeps increasing
-the states of the given solution as long as no constraints are violated.
-It also removes unnecessary parts of the solution which are unconnected
-to the required packages.
+Uninstall unreachable packages:
+start from the required ones and keep only the packages reachable from them along the graph.
 """
-function enforce_optimality!(sol::Vector{Int}, graph::Graph)
+function _uninstall_unreachable!(sol::Vector{Int}, why::Vector{Union{Symbol,Int}}, graph::Graph)
     np = graph.np
     spp = graph.spp
     gadj = graph.gadj
     gmsk = graph.gmsk
     gconstr = graph.gconstr
-    pkgs = graph.data.pkgs
 
-    # keep a track for the log
-    why = Union{Symbol,Int}[0 for p0 = 1:np]
-
-    restart = true
-    while restart
-        restart = false
-        for p0 = 1:np
-            s0 = sol[p0]
-            s0 == spp[p0] && (why[p0] = :uninst; continue) # the package is not installed
-
-            # check if bumping to the higher version would violate a constraint
-            gconstr[p0][s0+1] || (why[p0] = :constr; continue)
-
-            # check if bumping to the higher version would violate a constraint
-            viol = false
-            for (j1,p1) in enumerate(gadj[p0])
-                s1 = sol[p1]
-                msk = gmsk[p0][j1]
-                if !msk[s1, s0+1]
-                    viol = true
-                    why[p0] = p1
-                    break
-                end
-            end
-            viol && continue
-
-            # So the solution is non-optimal: we bump it manually
-            sol[p0] += 1
-            restart = true
-        end
-    end
-
-    # Finally uninstall unneeded packages:
-    # start from the required ones and keep only
-    # the packages reachable from them along the graph.
-    # (These should have been removed in the previous step, but in principle
-    # an unconnected yet self-sustaining cycle may have survived.)
     uninst = trues(np)
     staged = Set{Int}(p0 for p0 = 1:np if !gconstr[p0][end])
     seen = copy(staged)
@@ -400,7 +361,7 @@ function enforce_optimality!(sol::Vector{Int}, graph::Graph)
         staged_next = Set{Int}()
         for p0 in staged
             s0 = sol[p0]
-            @assert s0 < spp[p0]
+            s0 == spp[p0] && continue
             uninst[p0] = false
             for (j1,p1) in enumerate(gadj[p0])
                 gmsk[p0][j1][end,s0] && continue # the package is not required by p0 at version s0
@@ -415,6 +376,173 @@ function enforce_optimality!(sol::Vector{Int}, graph::Graph)
         sol[p0] = spp[p0]
         why[p0] = :uninst
     end
+end
+
+"""
+Push the given solution to a local optimum if needed: keeps increasing
+the states of the given solution as long as no constraints are violated.
+It might also install additional packages, if needed to bump the ones already
+installed.
+It also removes unnecessary parts of the solution which are unconnected
+to the required packages.
+"""
+function enforce_optimality!(sol::Vector{Int}, graph::Graph)
+    np = graph.np
+    spp = graph.spp
+    gadj = graph.gadj
+    gmsk = graph.gmsk
+    gconstr = graph.gconstr
+    pkgs = graph.data.pkgs
+
+    # keep a track for the log
+    why = Union{Symbol,Int}[0 for p0 = 1:np]
+
+    # Strategy:
+    # There's a cycle in which first the unnecessary (unconnected) packages are removed,
+    # then we make a pass over the whole packages trying to bump each of them.
+    # We repeat the above two steps until no further action is allowed.
+    # When attempting to bump a package, we may attempt to bump or install other packages
+    # if needed. Except if the bump would uninstall a package, in which cases we don't
+    # touch anything else: we do it only if it has no consequence at all. This strategy
+    # favors installing packages as needed.
+    # During the bumping pass, we keep an upper and lower bound for each package, which
+    # progressively shrink. These are used when adjusting for the effect of an attempted bump.
+    # The way it's written should ensure that no package is ever downgraded (unless it was
+    # originally unneeded, and then got removed, and later reinstalled to a lower version as
+    # a consequence of a bump of some other package).
+
+    # move_up is used to keep track of which packages can move up
+    # (they start installed and can be bumped) and which down (they start uninstalled and
+    # can be installed)
+    move_up = BitVector(undef, length(sol))
+    # lower and upper bounds on the valid range of each package
+    upperbound = similar(spp)
+    lowerbound = similar(spp)
+    # backup space for restoring the state if an attempted bump fails
+    bk_sol = similar(sol)
+    bk_lowerbound = similar(lowerbound)
+    bk_upperbound = similar(upperbound)
+
+    # auxiliary sets to perform breadth-first search on the graph
+    seen = Set{Int}()
+    staged = Set{Int}()
+    staged_next = Set{Int}()
+
+    old_sol = similar(sol)       # to detect if we made any changes
+    allsols = Set{Vector{Int}}() # used to make 100% sure we avoid infinite loops
+
+    while true
+        copy!(old_sol, sol)
+        push!(allsols, copy(sol))
+
+        # step 1: uninstall unneded packages
+        _uninstall_unreachable!(sol, why, graph)
+
+        # setp 2: try to bump each installed package in turn
+
+        move_up .= sol .≠ spp
+        copy!(upperbound, spp)
+        let move_up = move_up
+            lowerbound .= [move_up[p0] ? sol[p0] : 1 for p0 = 1:np]
+        end
+
+        for p0 = 1:np
+            s0 = sol[p0]
+            s0 == spp[p0] && (why[p0] = :uninst; continue) # the package is not installed
+            move_up[p0] || continue # the package is only installed as a result of a previous bump, skip it
+
+            @assert upperbound[p0] == spp[p0]
+
+            # pick the next version that doesn't violate a constraint (if any)
+            bump_range = collect(s0+1:spp[p0])
+            bump = let gconstr = gconstr
+                findfirst(v0->gconstr[p0][v0], bump_range)
+            end
+
+            # no such version was found, skip this package
+            bump ≡ nothing && (why[p0] = :constr; continue)
+
+            # assume that we will succeed in bumping the version (otherwise we
+            # roll-back at the end)
+
+            new_s0 = bump_range[bump]
+            try_uninstall = new_s0 == spp[p0] # are we trying to uninstall a package?
+
+            copy!(bk_sol, sol)
+            copy!(bk_lowerbound, lowerbound)
+            copy!(bk_upperbound, upperbound)
+            sol[p0] = new_s0
+
+            # if we're trying to uninstall, the bump is "soft": we don't update the
+            # lower bound so that the package can be reinstalled later in the pass
+            # if needed by another package
+            try_uninstall || (lowerbound[p0] = new_s0) # note that we're in the move_up case
+
+            empty!(seen)
+            empty!(staged)
+            empty!(staged_next)
+            push!(staged, p0)
+
+            while !isempty(staged)
+                for f0 in staged
+                    for (j1,f1) in enumerate(gadj[f0])
+                        f1 == p0 && continue
+                        f1 ∈ seen && continue
+                        s1 = sol[f1]
+                        msk = gmsk[f0][j1]
+                        if try_uninstall
+                            bump_range = [s1] # when uninstalling, no further changes are allowed
+                        else
+                            lb1 = lowerbound[f1]
+                            ub1 = upperbound[f1]
+                            @assert lb1 ≤ s1 ≤ ub1
+                            if move_up[f1]
+                                s1 > lb1 && @assert s1 == spp[f1]
+                                    # the arrangement of the range gives precedence to improving the
+                                    # current situation, but allows reinstalling a package if needed
+                                bump_range = vcat(s1:ub1, s1-1:-1:lb1)
+                            else
+                                bump_range = collect(ub1:-1:lb1)
+                            end
+                        end
+                        bump = let gconstr = gconstr
+                            findfirst(v1->(gconstr[f1][v1] && msk[v1, sol[f0]]), bump_range)
+                        end
+                        if bump ≡ nothing
+                            why[p0] = f1 # TODO: improve this? (ideally we might want the path from p0 to f1)
+                            @goto abort
+                        end
+                        new_s1 = bump_range[bump]
+                        sol[f1] = new_s1
+                        new_s1 == s1 && continue
+                        push!(staged_next, f1)
+                        if move_up[f1]
+                            lowerbound[f1] = new_s1
+                        else
+                            upperbound[f1] = new_s1
+                        end
+                    end
+                end
+                union!(seen, staged)
+                staged, staged_next = staged_next, staged
+                empty!(staged_next)
+            end
+
+            # if we're here the bump was successful, there's nothing more to do
+            continue
+
+            ## abort the bumping: restore the solution
+            @label abort
+
+            copy!(sol, bk_sol)
+            copy!(lowerbound, bk_lowerbound)
+            copy!(upperbound, bk_upperbound)
+        end
+        sol ≠ old_sol || break
+        # It might be possible in principle to contrive a situation in which
+        # the solutions oscillate
+        sol ∈ allsols && break
+    end
 
     @assert verify_solution(sol, graph)
 

src/Resolve/graphtype.jl

@@ -44,7 +44,7 @@ mutable struct ResolveLog
     pool::Dict{UUID,ResolveLogEntry}
 
     # journal: record all messages in order (shared between all entries)
-    journal::Vector{Tuple{UUID,String}}
+    journal::ResolveJournal
 
     # exact: keeps track of whether the resolve process is still exact, or
     #        heuristics have been employed
@@ -1019,6 +1019,8 @@ function propagate_constraints!(graph::Graph, sources::Set{Int} = Set{Int}(); lo
         Set{Int}(p0 for p0 = 1:np if !gconstr[p0][end]) :
         sources
 
+    seen = copy(staged)
+
     while !isempty(staged)
         staged_next = Set{Int}()
         for p0 in staged
@@ -1048,6 +1050,8 @@ function propagate_constraints!(graph::Graph, sources::Set{Int} = Set{Int}(); lo
                 if gconstr1 ≠ old_gconstr1
                     push!(staged_next, p1)
                     log_events && log_event_implicit_req!(graph, p1, added_constr1, p0)
+                elseif p1 ∉ seen
+                    push!(staged_next, p1)
                 end
                 if !any(gconstr1)
                     if exact
@@ -1060,6 +1064,7 @@ function propagate_constraints!(graph::Graph, sources::Set{Int} = Set{Int}(); lo
                 end
             end
         end
+        union!(seen, staged_next)
         staged = staged_next
     end
     return graph

src/Types.jl

@@ -464,15 +464,50 @@ function write_env_usage(source_file::AbstractString, usage_filepath::AbstractSt
     # Ensure that log dir exists
     !ispath(logdir()) && mkpath(logdir())
 
-    # Generate entire entry as a string first
-    entry = sprint() do io
-        TOML.print(io, Dict(source_file => [Dict("time" => now())]))
-    end
-
-    # Append entry to log file in one chunk
     usage_file = joinpath(logdir(), usage_filepath)
-    open(usage_file, append=true) do io
-        write(io, entry)
+    timestamp = now()
+
+    ## Atomically write usage file
+    while true
+        # read existing usage file
+        usage = if isfile(usage_file)
+            TOML.parsefile(usage_file)
+        else
+            Dict{String, Any}()
+        end
+
+        # record new usage
+        usage[source_file] = [Dict("time" => timestamp)]
+
+        # keep only latest usage info
+        for k in keys(usage)
+            times = map(d -> Dates.DateTime(d["time"]), usage[k])
+            usage[k] = [Dict("time" => maximum(times))]
+        end
+
+        # Write to a temp file in the same directory as the destination
+        temp_usage_file = tempname(logdir())
+        open(temp_usage_file, "w") do io
+            TOML.print(io, usage, sorted=true)
+        end
+
+        # Move the temp file into place, replacing the original
+        mv(temp_usage_file, usage_file, force = true)
+
+        # Check that the new file has what we want in it
+        new_usage = if isfile(usage_file)
+            TOML.parsefile(usage_file)
+        else
+            Dict{String, Any}()
+        end
+        if haskey(new_usage, source_file)
+            for e in new_usage[source_file]
+                if Dates.DateTime(e["time"]) >= timestamp
+                    return
+                end
+            end
+        end
+        # If not, try again
     end
 end
 

test/resolve.jl

@@ -407,11 +407,19 @@ end
 
     @test sanity_tst(ResolveData.deps_data, ResolveData.problematic_data)
     @test resolve_tst(ResolveData.deps_data, ResolveData.reqs_data, ResolveData.want_data)
+
+    ## DEPENDENCY SCHEME 12: A LARGER, MORE DIFFICULT REALISTIC EXAMPLE
+    ## ref Pkg.jl issue #1949
+
+    include("resolvedata2.jl")
+
+    @test sanity_tst(ResolveData2.deps_data, ResolveData2.problematic_data)
+    @test resolve_tst(ResolveData2.deps_data, ResolveData2.reqs_data, ResolveData2.want_data)
 end
 
 @testset "nasty" begin
     VERBOSE && @info("SCHEME NASTY")
-    ## DEPENDENCY SCHEME 12: A NASTY CASE
+    ## DEPENDENCY SCHEME 13: A NASTY CASE
 
     include("NastyGenerator.jl")
     deps_data, reqs_data, want_data, problematic_data = NastyGenerator.generate_nasty(5, 20, q=20, d=4, sat = true)