From c588de6bfd15c4959f27e4026cacbc3950bacad4 Mon Sep 17 00:00:00 2001 From: Kristoffer Carlsson Date: Fri, 18 Oct 2024 14:42:23 +0200 Subject: [PATCH 1/2] Revert "Extensions: make loading of extensions independent of what packages are in the sysimage (#52841)" This reverts commit 08d229f4a7cb0c3ef5becddd1b3bc4f8f178b8e4. --- base/Base.jl | 1 - base/loading.jl | 14 ++++---------- test/loading.jl | 19 ------------------- .../HasDepWithExtensions.jl/Manifest.toml | 7 +------ .../Extensions/HasExtensions.jl/Project.toml | 2 -- .../HasExtensions.jl/ext/LinearAlgebraExt.jl | 3 --- 6 files changed, 5 insertions(+), 41 deletions(-) delete mode 100644 test/project/Extensions/HasExtensions.jl/ext/LinearAlgebraExt.jl diff --git a/base/Base.jl b/base/Base.jl index 8e6a3ec17bdaf..ab8493cd6fd32 100644 --- a/base/Base.jl +++ b/base/Base.jl @@ -602,7 +602,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) diff --git a/base/loading.jl b/base/loading.jl index 9dd5f9661da46..1395bf45cb3b6 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -1518,7 +1518,7 @@ function _insert_extension_triggers(parent::PkgId, extensions::Dict{String, Any} uuid_trigger = UUID(totaldeps[trigger]::String) trigger_id = PkgId(uuid_trigger, trigger) push!(trigger_ids, trigger_id) - 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 @@ -2392,9 +2392,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 @@ -2407,7 +2406,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}() @@ -2448,7 +2446,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 @@ -2480,9 +2477,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 @@ -3046,7 +3040,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 diff --git a/test/loading.jl b/test/loading.jl index c77c5e1e19fdd..ff24d972fddf3 100644 --- a/test/loading.jl +++ b/test/loading.jl @@ -1118,25 +1118,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 diff --git a/test/project/Extensions/HasDepWithExtensions.jl/Manifest.toml b/test/project/Extensions/HasDepWithExtensions.jl/Manifest.toml index 59bd802c9710a..98510dcb27733 100644 --- a/test/project/Extensions/HasDepWithExtensions.jl/Manifest.toml +++ b/test/project/Extensions/HasDepWithExtensions.jl/Manifest.toml @@ -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.SomeOtherPackage]] path = "../SomeOtherPackage" diff --git a/test/project/Extensions/HasExtensions.jl/Project.toml b/test/project/Extensions/HasExtensions.jl/Project.toml index fe21a1423f543..a02f5662d602d 100644 --- a/test/project/Extensions/HasExtensions.jl/Project.toml +++ b/test/project/Extensions/HasExtensions.jl/Project.toml @@ -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" diff --git a/test/project/Extensions/HasExtensions.jl/ext/LinearAlgebraExt.jl b/test/project/Extensions/HasExtensions.jl/ext/LinearAlgebraExt.jl deleted file mode 100644 index 19f87cb849417..0000000000000 --- a/test/project/Extensions/HasExtensions.jl/ext/LinearAlgebraExt.jl +++ /dev/null @@ -1,3 +0,0 @@ -module LinearAlgebraExt - -end From 79b1263b430769b42d2ae3b30c3999ff79eeab64 Mon Sep 17 00:00:00 2001 From: KristofferC Date: Mon, 21 Oct 2024 14:18:48 +0200 Subject: [PATCH 2/2] fix lookup when extension is in `[deps]` --- base/loading.jl | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/base/loading.jl b/base/loading.jl index 1395bf45cb3b6..8728c179765cc 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -934,14 +934,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 @@ -960,23 +960,25 @@ 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 @@ -984,6 +986,7 @@ function explicit_manifest_deps_get(project_file::String, where::PkgId, name::St 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