From 22ace53f2bad8d1355fd71a237ef7f1c5735aa90 Mon Sep 17 00:00:00 2001 From: KristofferC Date: Mon, 10 Jun 2024 11:09:22 +0200 Subject: [PATCH 1/3] add a failing test due of extension loading when trigger is in the sysimage --- test/loading.jl | 22 +++++++++++++++++-- .../Extensions/GotLibdlExt/Project.toml | 9 ++++++++ .../Extensions/GotLibdlExt/ext/LibdlExt.jl | 5 +++++ .../Extensions/GotLibdlExt/src/GotLibdlExt.jl | 5 +++++ 4 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 test/project/Extensions/GotLibdlExt/Project.toml create mode 100644 test/project/Extensions/GotLibdlExt/ext/LibdlExt.jl create mode 100644 test/project/Extensions/GotLibdlExt/src/GotLibdlExt.jl diff --git a/test/loading.jl b/test/loading.jl index 9230bc75599e6..6647bdc4f4fed 100644 --- a/test/loading.jl +++ b/test/loading.jl @@ -1115,7 +1115,7 @@ end cmd_proj_ext = `$(Base.julia_cmd()) $compile --startup-file=no -e $test_ext_proj` proj = joinpath(@__DIR__, "project", "Extensions") cmd_proj_ext = addenv(cmd_proj_ext, "JULIA_LOAD_PATH" => join([joinpath(proj, "HasExtensions.jl"), joinpath(proj, "EnvWithDeps")], sep)) - run(cmd_proj_ext) + @test success(cmd_proj_ext) end # Sysimage extensions @@ -1134,8 +1134,26 @@ end """ cmd = `$(Base.julia_cmd()) --startup-file=no -e $sysimg_ext_test_code` cmd = addenv(cmd, "JULIA_LOAD_PATH" => join([proj, "@stdlib"], sep)) - run(cmd) + @test success(cmd) + # Failure of loading some extensions when trigger is in the sysimage + # The test below requires that LinearAlgebra is in the sysimage and that it has not been loaded yet + # and that it depends on Libdl. + # 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 + proj_libdlext = joinpath(@__DIR__, "project", "Extensions", "GotLibdlExt") + sysimg_ext_test_code = """ + #uuid_key_la = Base.PkgId(Base.UUID("37e2e46d-f89d-539d-b4ee-838fcccc9c8e"), "LinearAlgebra") + # Base.in_sysimage(uuid_key_la) || error("LinearAlgebra not in sysimage") + #uuid_key_libdl = Base.PkgId(Base.UUID("8f399da3-3557-5675-b5ff-fb832c97cbdb"), "Libdl") + # Base.in_sysimage(uuid_key_libdl) || error("Libdl not in sysimage") + using GotLibdlExt + using LinearAlgebra + Base.get_extension(GotLibdlExt, :LibdlExt) 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_libdlext, "@stdlib"], sep)) + @test_broken success(cmd) # Extensions in implicit environments old_load_path = copy(LOAD_PATH) diff --git a/test/project/Extensions/GotLibdlExt/Project.toml b/test/project/Extensions/GotLibdlExt/Project.toml new file mode 100644 index 0000000000000..1702df034fd74 --- /dev/null +++ b/test/project/Extensions/GotLibdlExt/Project.toml @@ -0,0 +1,9 @@ +name = "GotLibdlExt" +uuid = "a549ab1b-9b57-4c18-8a82-af63d7789335" +version = "0.1.0" + +[weakdeps] +Libdl = "8f399da3-3557-5675-b5ff-fb832c97cbdb" + +[extensions] +LibdlExt = "Libdl" diff --git a/test/project/Extensions/GotLibdlExt/ext/LibdlExt.jl b/test/project/Extensions/GotLibdlExt/ext/LibdlExt.jl new file mode 100644 index 0000000000000..7f0c7ecf60844 --- /dev/null +++ b/test/project/Extensions/GotLibdlExt/ext/LibdlExt.jl @@ -0,0 +1,5 @@ +module LibdlExt + +using Libdl, GotLibdlExt + +end diff --git a/test/project/Extensions/GotLibdlExt/src/GotLibdlExt.jl b/test/project/Extensions/GotLibdlExt/src/GotLibdlExt.jl new file mode 100644 index 0000000000000..89207fc8dfd5e --- /dev/null +++ b/test/project/Extensions/GotLibdlExt/src/GotLibdlExt.jl @@ -0,0 +1,5 @@ +module GotLibdlExt + +greet() = print("Hello World!") + +end # module GotLibdlExt From 73ed021ab1bbda56576dd31f95f2488dab819584 Mon Sep 17 00:00:00 2001 From: Kristoffer Carlsson Date: Mon, 10 Jun 2024 12:52:59 +0200 Subject: [PATCH 2/3] Update loading.jl --- test/loading.jl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/loading.jl b/test/loading.jl index 6647bdc4f4fed..65a6b6e796b4f 100644 --- a/test/loading.jl +++ b/test/loading.jl @@ -1143,10 +1143,10 @@ end # We run this test in a new process so we are not vulnerable to a previous test having loaded LinearAlgebra proj_libdlext = joinpath(@__DIR__, "project", "Extensions", "GotLibdlExt") sysimg_ext_test_code = """ - #uuid_key_la = Base.PkgId(Base.UUID("37e2e46d-f89d-539d-b4ee-838fcccc9c8e"), "LinearAlgebra") - # Base.in_sysimage(uuid_key_la) || error("LinearAlgebra not in sysimage") - #uuid_key_libdl = Base.PkgId(Base.UUID("8f399da3-3557-5675-b5ff-fb832c97cbdb"), "Libdl") - # Base.in_sysimage(uuid_key_libdl) || error("Libdl not in sysimage") + uuid_key_la = Base.PkgId(Base.UUID("37e2e46d-f89d-539d-b4ee-838fcccc9c8e"), "LinearAlgebra") + Base.in_sysimage(uuid_key_la) || error("LinearAlgebra not in sysimage") + uuid_key_libdl = Base.PkgId(Base.UUID("8f399da3-3557-5675-b5ff-fb832c97cbdb"), "Libdl") + Base.in_sysimage(uuid_key_libdl) || error("Libdl not in sysimage") using GotLibdlExt using LinearAlgebra Base.get_extension(GotLibdlExt, :LibdlExt) isa Module || error("expected extension to load") From 4760a51a5d020271a2829742f70be82c4812fc81 Mon Sep 17 00:00:00 2001 From: KristofferC Date: Mon, 10 Jun 2024 14:52:02 +0200 Subject: [PATCH 3/3] record dependencies when `requiring` a package and also mark those deps as explicitly loaded this is important for packages in the sysimage whose dependencies should also be marked as loaded when they themselves are loaded --- base/loading.jl | 37 +++++++++++++++++++++++++++++-------- test/loading.jl | 17 ++++++++++++++--- 2 files changed, 43 insertions(+), 11 deletions(-) diff --git a/base/loading.jl b/base/loading.jl index c6dfd15933692..a0d04e4467d12 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -1458,7 +1458,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 !in(trigger_id, explicit_loaded_modules) || haskey(package_locks, trigger_id) trigger1 = get!(Vector{ExtensionId}, EXT_DORMITORY, trigger_id) push!(trigger1, gid) else @@ -2148,6 +2148,8 @@ function require(into::Module, mod::Symbol) end end +const require_map = Dict{PkgId, Set{PkgId}}() + function __require(into::Module, mod::Symbol) @lock require_lock begin LOADING_CACHE[] = LoadingCache() @@ -2192,7 +2194,13 @@ function __require(into::Module, mod::Symbol) path = binpack(uuidkey) push!(_require_dependencies, (into, path, UInt64(0), UInt32(0), 0.0)) end - return _require_prelocked(uuidkey, env) + push!(get!(Set{PkgId}, require_map, PkgId(into)), uuidkey) + try + return _require_prelocked(uuidkey, env) + catch + delete!(require_map, PkgId(into)) + rethrow() + end finally LOADING_CACHE[] = nothing end @@ -2268,12 +2276,11 @@ function __require_prelocked(uuidkey::PkgId, env=nothing) end insert_extension_triggers(uuidkey) # After successfully loading, notify downstream consumers - run_package_callbacks(uuidkey) + update_explicit_loaded_modules(uuidkey) else m = get(loaded_modules, uuidkey, nothing) - if m !== nothing && !haskey(explicit_loaded_modules, uuidkey) - explicit_loaded_modules[uuidkey] = m - run_package_callbacks(uuidkey) + if m !== nothing && !in(uuidkey, explicit_loaded_modules) + update_explicit_loaded_modules(uuidkey) end newm = root_module(uuidkey) end @@ -2290,7 +2297,7 @@ const pkgorigins = Dict{PkgId,PkgOrigin}() const loaded_modules = Dict{PkgId,Module}() # Emptied on Julia start -const explicit_loaded_modules = Dict{PkgId,Module}() +const explicit_loaded_modules = Set{PkgId}() const loaded_modules_order = Vector{Module}() const module_keys = IdDict{Module,PkgId}() # the reverse @@ -2313,12 +2320,26 @@ root_module_key(m::Module) = @lock require_lock module_keys[m] end push!(loaded_modules_order, m) loaded_modules[key] = m - explicit_loaded_modules[key] = m + update_explicit_loaded_modules(key) module_keys[m] = key end nothing end + +function update_explicit_loaded_modules(key::PkgId) + if !in(key, explicit_loaded_modules) + push!(explicit_loaded_modules, key) + run_package_callbacks(key) + deps = get(require_map, key, nothing) + if deps !== nothing + for dep in deps + update_explicit_loaded_modules(dep) + end + end + end +end + register_root_module(Core) register_root_module(Base) register_root_module(Main) diff --git a/test/loading.jl b/test/loading.jl index 65a6b6e796b4f..7ae05e5c22858 100644 --- a/test/loading.jl +++ b/test/loading.jl @@ -1125,18 +1125,17 @@ end 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") + in(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") + in(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)) @test success(cmd) - # Failure of loading some extensions when trigger is in the sysimage # The test below requires that LinearAlgebra is in the sysimage and that it has not been loaded yet # and that it depends on Libdl. # If it gets moved out, this test will need to be updated. @@ -1153,6 +1152,18 @@ end """ cmd = `$(Base.julia_cmd()) --startup-file=no -e $sysimg_ext_test_code` cmd = addenv(cmd, "JULIA_LOAD_PATH" => join([proj_libdlext, "@stdlib"], sep)) + @test success(cmd) + + # Failure of loading some extensions when loading a precompiled package with a trigger in the sysimage + sysimg_ext_test_code = """ + uuid_key_la = Base.PkgId(Base.UUID("37e2e46d-f89d-539d-b4ee-838fcccc9c8e"), "LinearAlgebra") + using GotLibdlExt + using SparseArrays # loads LinearAlgebra + haskey(Base.loaded_modules, uuid_key_la) || error("LinearAlgebra not loaded") + Base.get_extension(GotLibdlExt, :LibdlExt) 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_libdlext, "@stdlib"], sep)) @test_broken success(cmd) # Extensions in implicit environments