From bf379e2a2fefa029defb5468765d6761ca18196b Mon Sep 17 00:00:00 2001 From: KristofferC Date: Wed, 5 Apr 2023 11:19:09 +0200 Subject: [PATCH 1/2] Revert "delay loading of extensions as much as possible (#48674)" This reverts commit e3043a875d432ec4bd7c073874a654b55421438f. --- base/loading.jl | 87 +++++++++++++++++++++++-------------------------- 1 file changed, 40 insertions(+), 47 deletions(-) diff --git a/base/loading.jl b/base/loading.jl index b82028216663b..c36990fd3a07e 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -1080,6 +1080,7 @@ function register_restored_modules(sv::SimpleVector, pkg::PkgId, path::String) end function run_package_callbacks(modkey::PkgId) + run_extension_callbacks(modkey) assert_havelock(require_lock) unlock(require_lock) try @@ -1204,55 +1205,51 @@ function run_extension_callbacks(extid::ExtensionId) return succeeded end -function run_extension_callbacks() +function run_extension_callbacks(pkgid::PkgId) assert_havelock(require_lock) - loaded_triggers = collect(intersect(keys(Base.loaded_modules), keys(Base.EXT_DORMITORY))) - sort!(loaded_triggers; by=x->x.uuid) - for pkgid in loaded_triggers - # take ownership of extids that depend on this pkgid - extids = pop!(EXT_DORMITORY, pkgid, nothing) - extids === nothing && continue - for extid in extids - if extid.ntriggers > 0 - # It is possible that pkgid was loaded in an environment - # below the one of the parent. This will cause a load failure when the - # pkg ext tries to load the triggers. Therefore, check this first - # before loading the pkg ext. - pkgenv = identify_package_env(extid.id, pkgid.name) - ext_not_allowed_load = false - if pkgenv === nothing + # take ownership of extids that depend on this pkgid + extids = pop!(EXT_DORMITORY, pkgid, nothing) + extids === nothing && return + for extid in extids + if extid.ntriggers > 0 + # It is possible that pkgid was loaded in an environment + # below the one of the parent. This will cause a load failure when the + # pkg ext tries to load the triggers. Therefore, check this first + # before loading the pkg ext. + pkgenv = identify_package_env(extid.id, pkgid.name) + ext_not_allowed_load = false + if pkgenv === nothing + ext_not_allowed_load = true + else + pkg, env = pkgenv + path = locate_package(pkg, env) + if path === nothing ext_not_allowed_load = true - else - pkg, env = pkgenv - path = Base.locate_package(pkg, env) - if path === nothing - ext_not_allowed_load = true - end - end - if ext_not_allowed_load - @debug "Extension $(extid.id.name) of $(extid.parentid.name) will not be loaded \ - since $(pkgid.name) loaded in environment lower in load path" - # indicate extid is expected to fail - extid.ntriggers *= -1 - else - # indicate pkgid is loaded - extid.ntriggers -= 1 end end - if extid.ntriggers < 0 - # indicate pkgid is loaded - extid.ntriggers += 1 - succeeded = false + if ext_not_allowed_load + @debug "Extension $(extid.id.name) of $(extid.parentid.name) will not be loaded \ + since $(pkgid.name) loaded in environment lower in load path" + # indicate extid is expected to fail + extid.ntriggers *= -1 else - succeeded = true - end - if extid.ntriggers == 0 - # actually load extid, now that all dependencies are met, - # and record the result - succeeded = succeeded && run_extension_callbacks(extid) - succeeded || push!(EXT_DORMITORY_FAILED, extid) + # indicate pkgid is loaded + extid.ntriggers -= 1 end end + if extid.ntriggers < 0 + # indicate pkgid is loaded + extid.ntriggers += 1 + succeeded = false + else + succeeded = true + end + if extid.ntriggers == 0 + # actually load extid, now that all dependencies are met, + # and record the result + succeeded = succeeded && run_extension_callbacks(extid) + succeeded || push!(EXT_DORMITORY_FAILED, extid) + end end return end @@ -1276,7 +1273,7 @@ function retry_load_extensions() end prepend!(EXT_DORMITORY_FAILED, failed) end - nothing + return end """ @@ -1669,10 +1666,6 @@ function _require_prelocked(uuidkey::PkgId, env=nothing) else newm = root_module(uuidkey) end - # Load extensions when not precompiling and not in a nested package load - if JLOptions().incremental == 0 && isempty(package_locks) - run_extension_callbacks() - end return newm end From aa16ceac581d5a94e82496cc66c2600f7c31dd80 Mon Sep 17 00:00:00 2001 From: Kristoffer Date: Sun, 9 Apr 2023 17:07:54 +0200 Subject: [PATCH 2/2] add a test that extension is available during precompilation --- .../HasDepWithExtensions.jl/src/HasDepWithExtensions.jl | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/project/Extensions/HasDepWithExtensions.jl/src/HasDepWithExtensions.jl b/test/project/Extensions/HasDepWithExtensions.jl/src/HasDepWithExtensions.jl index d64cbc680e3a5..5c1f2d1f301aa 100644 --- a/test/project/Extensions/HasDepWithExtensions.jl/src/HasDepWithExtensions.jl +++ b/test/project/Extensions/HasDepWithExtensions.jl/src/HasDepWithExtensions.jl @@ -4,10 +4,18 @@ using HasExtensions: HasExtensions, HasExtensionsStruct using ExtDep: ExtDepStruct # Loading ExtDep makes the extension "Extension" load +const m = Base.get_extension(HasExtensions, :Extension) +m isa Module || error("extension not loaded during precompilation") + function do_something() HasExtensions.foo(HasExtensionsStruct()) == 1 || error() HasExtensions.foo(ExtDepStruct()) == 2 || error() return true end +function __init__() + m = Base.get_extension(HasExtensions, :Extension) + m isa Module || error("extension not loaded during __init__") +end + end # module