Skip to content

Commit

Permalink
Revert "Extensions: make loading of extensions independent of what pa…
Browse files Browse the repository at this point in the history
…ckages are in the sysimage (#52841) (#56234)

This reverts commit 08d229f.

There are some bugs now where extensions do not load when their package
has been put into the sysimage. #52841 was made because it was common to
get cycles otherwise but with
#55589 that should be much less
of a problem.

Subsumes #54750.
  • Loading branch information
KristofferC authored Oct 22, 2024
2 parents 31f7df6 + 319ee70 commit 5b677a1
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 57 deletions.
1 change: 0 additions & 1 deletion base/Base.jl
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,6 @@ function __init__()
init_load_path()
init_active_project()
append!(empty!(_sysimage_modules), keys(loaded_modules))
empty!(explicit_loaded_modules)
empty!(loaded_precompiles) # If we load a packageimage when building the image this might not be empty
for (mod, key) in module_keys
push!(get!(Vector{Module}, loaded_precompiles, key), mod)
Expand Down
49 changes: 23 additions & 26 deletions base/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -974,14 +974,14 @@ function explicit_manifest_deps_get(project_file::String, where::PkgId, name::St
entry = entry::Dict{String, Any}
uuid = get(entry, "uuid", nothing)::Union{String, Nothing}
uuid === nothing && continue
# deps is either a list of names (deps = ["DepA", "DepB"]) or
# a table of entries (deps = {"DepA" = "6ea...", "DepB" = "55d..."}
deps = get(entry, "deps", nothing)::Union{Vector{String}, Dict{String, Any}, Nothing}
if UUID(uuid) === where.uuid
found_where = true
# deps is either a list of names (deps = ["DepA", "DepB"]) or
# a table of entries (deps = {"DepA" = "6ea...", "DepB" = "55d..."}
deps = get(entry, "deps", nothing)::Union{Vector{String}, Dict{String, Any}, Nothing}
if deps isa Vector{String}
found_name = name in deps
break
found_name && @goto done
elseif deps isa Dict{String, Any}
deps = deps::Dict{String, Any}
for (dep, uuid) in deps
Expand All @@ -1000,30 +1000,33 @@ function explicit_manifest_deps_get(project_file::String, where::PkgId, name::St
return PkgId(UUID(uuid), name)
end
exts = extensions[where.name]::Union{String, Vector{String}}
weakdeps = get(entry, "weakdeps", nothing)::Union{Vector{String}, Dict{String, Any}, Nothing}
if (exts isa String && name == exts) || (exts isa Vector{String} && name in exts)
weakdeps = get(entry, "weakdeps", nothing)::Union{Vector{String}, Dict{String, Any}, Nothing}
if weakdeps !== nothing
if weakdeps isa Vector{String}
found_name = name in weakdeps
break
elseif weakdeps isa Dict{String, Any}
weakdeps = weakdeps::Dict{String, Any}
for (dep, uuid) in weakdeps
uuid::String
if dep === name
return PkgId(UUID(uuid), name)
for deps′ in [weakdeps, deps]
if deps′ !== nothing
if deps′ isa Vector{String}
found_name = name in deps′
found_name && @goto done
elseif deps′ isa Dict{String, Any}
deps′ = deps′::Dict{String, Any}
for (dep, uuid) in deps′
uuid::String
if dep === name
return PkgId(UUID(uuid), name)
end
end
end
end
end
end
end
# `name` is not an ext, do standard lookup as if this was the parent
return identify_package(PkgId(UUID(uuid), dep_name), name)
end
end
end
end
end
@label done
found_where || return nothing
found_name || return PkgId(name)
# Only reach here if deps was not a dict which mean we have a unique name for the dep
Expand Down Expand Up @@ -1554,7 +1557,7 @@ function _insert_extension_triggers(parent::PkgId, extensions::Dict{String, Any}
# TODO: Better error message if this lookup fails?
uuid_trigger = UUID(totaldeps[trigger]::String)
trigger_id = PkgId(uuid_trigger, trigger)
if !haskey(explicit_loaded_modules, trigger_id) || haskey(package_locks, trigger_id)
if !haskey(Base.loaded_modules, trigger_id) || haskey(package_locks, trigger_id)
trigger1 = get!(Vector{ExtensionId}, EXT_DORMITORY, trigger_id)
push!(trigger1, gid)
else
Expand Down Expand Up @@ -2428,9 +2431,8 @@ function __require_prelocked(uuidkey::PkgId, env=nothing)
insert_extension_triggers(uuidkey)
# After successfully loading, notify downstream consumers
run_package_callbacks(uuidkey)
elseif !haskey(explicit_loaded_modules, uuidkey)
explicit_loaded_modules[uuidkey] = m
run_package_callbacks(uuidkey)
else
newm = root_module(uuidkey)
end
return m
end
Expand All @@ -2443,7 +2445,6 @@ end
PkgOrigin() = PkgOrigin(nothing, nothing, nothing)
const pkgorigins = Dict{PkgId,PkgOrigin}()

const explicit_loaded_modules = Dict{PkgId,Module}() # Emptied on Julia start
const loaded_modules = Dict{PkgId,Module}() # available to be explicitly loaded
const loaded_precompiles = Dict{PkgId,Vector{Module}}() # extended (complete) list of modules, available to be loaded
const loaded_modules_order = Vector{Module}()
Expand Down Expand Up @@ -2483,7 +2484,6 @@ end
end
maybe_loaded_precompile(key, module_build_id(m)) === nothing && push!(loaded_modules_order, m)
loaded_modules[key] = m
explicit_loaded_modules[key] = m
module_keys[m] = key
end
nothing
Expand Down Expand Up @@ -2515,9 +2515,6 @@ loaded_modules_array() = @lock require_lock copy(loaded_modules_order)
# after unreference_module, a subsequent require call will try to load a new copy of it, if stale
# reload(m) = (unreference_module(m); require(m))
function unreference_module(key::PkgId)
if haskey(explicit_loaded_modules, key)
m = pop!(explicit_loaded_modules, key)
end
if haskey(loaded_modules, key)
m = pop!(loaded_modules, key)
# need to ensure all modules are GC rooted; will still be referenced
Expand Down Expand Up @@ -3126,7 +3123,7 @@ function compilecache(pkg::PkgId, path::String, internal_stderr::IO = stderr, in
# build up the list of modules that we want the precompile process to preserve
if keep_loaded_modules
concrete_deps = copy(_concrete_dependencies)
for (pkgreq, modreq) in loaded_modules # TODO: convert all relevant staleness heuristics to use explicit_loaded_modules instead
for (pkgreq, modreq) in loaded_modules
if !(pkgreq === Main || pkgreq === Core || pkgreq === Base)
push!(concrete_deps, pkgreq => module_build_id(modreq))
end
Expand Down
19 changes: 0 additions & 19 deletions test/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1129,25 +1129,6 @@ end
run(cmd_proj_ext)
end

# Sysimage extensions
# The test below requires that LinearAlgebra is in the sysimage and that it has not been loaded yet.
# if it gets moved out, this test will need to be updated.
# We run this test in a new process so we are not vulnerable to a previous test having loaded LinearAlgebra
sysimg_ext_test_code = """
uuid_key = Base.PkgId(Base.UUID("37e2e46d-f89d-539d-b4ee-838fcccc9c8e"), "LinearAlgebra")
Base.in_sysimage(uuid_key) || error("LinearAlgebra not in sysimage")
haskey(Base.explicit_loaded_modules, uuid_key) && error("LinearAlgebra already loaded")
using HasExtensions
Base.get_extension(HasExtensions, :LinearAlgebraExt) === nothing || error("unexpectedly got an extension")
using LinearAlgebra
haskey(Base.explicit_loaded_modules, uuid_key) || error("LinearAlgebra not loaded")
Base.get_extension(HasExtensions, :LinearAlgebraExt) isa Module || error("expected extension to load")
"""
cmd = `$(Base.julia_cmd()) --startup-file=no -e $sysimg_ext_test_code`
cmd = addenv(cmd, "JULIA_LOAD_PATH" => join([proj, "@stdlib"], sep))
run(cmd)


# Extensions in implicit environments
old_load_path = copy(LOAD_PATH)
try
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,12 @@ deps = ["ExtDep3"]
path = "../HasExtensions.jl"
uuid = "4d3288b3-3afc-4bb6-85f3-489fffe514c8"
version = "0.1.0"
weakdeps = ["ExtDep", "ExtDep2"]

[deps.HasExtensions.extensions]
Extension = "ExtDep"
ExtensionDep = "ExtDep3"
ExtensionFolder = ["ExtDep", "ExtDep2"]
LinearAlgebraExt = "LinearAlgebra"

[deps.HasExtensions.weakdeps]
ExtDep = "fa069be4-f60b-4d4c-8b95-f8008775090c"
ExtDep2 = "55982ee5-2ad5-4c40-8cfe-5e9e1b01500d"
LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"

[[deps.SomePackage]]
path = "../SomePackage"
Expand Down
2 changes: 0 additions & 2 deletions test/project/Extensions/HasExtensions.jl/Project.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,8 @@ ExtDep3 = "a5541f1e-a556-4fdc-af15-097880d743a1"
[weakdeps]
ExtDep = "fa069be4-f60b-4d4c-8b95-f8008775090c"
ExtDep2 = "55982ee5-2ad5-4c40-8cfe-5e9e1b01500d"
LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"

[extensions]
Extension = "ExtDep"
ExtensionDep = "ExtDep3"
ExtensionFolder = ["ExtDep", "ExtDep2"]
LinearAlgebraExt = "LinearAlgebra"

This file was deleted.

0 comments on commit 5b677a1

Please sign in to comment.