From a1dbfd0aaed0977ffd97674d460ebde56ca78223 Mon Sep 17 00:00:00 2001 From: Cody Tapscott Date: Sun, 24 Nov 2024 12:09:33 +0000 Subject: [PATCH 1/2] Test extensions of "parent" dependencies These are the main correctness fix from #55910, so it's important that we have test coverage for it. --- test/loading.jl | 34 +++++++++++++++++++ .../DepWithParentExt.jl/Project.toml | 9 +++++ .../DepWithParentExt.jl/ext/ParentExt.jl | 6 ++++ .../src/DepWithParentExt.jl | 5 +++ .../Extensions/Parent.jl/Manifest.toml | 20 +++++++++++ .../project/Extensions/Parent.jl/Project.toml | 7 ++++ .../Extensions/Parent.jl/src/Parent.jl | 7 ++++ 7 files changed, 88 insertions(+) create mode 100644 test/project/Extensions/DepWithParentExt.jl/Project.toml create mode 100644 test/project/Extensions/DepWithParentExt.jl/ext/ParentExt.jl create mode 100644 test/project/Extensions/DepWithParentExt.jl/src/DepWithParentExt.jl create mode 100644 test/project/Extensions/Parent.jl/Manifest.toml create mode 100644 test/project/Extensions/Parent.jl/Project.toml create mode 100644 test/project/Extensions/Parent.jl/src/Parent.jl diff --git a/test/loading.jl b/test/loading.jl index 1cc20548d9bc8..09f96e1f43578 100644 --- a/test/loading.jl +++ b/test/loading.jl @@ -1220,6 +1220,40 @@ end @test occursin("Hello x-package ext-to-ext!", String(read(cmd))) end + # Extensions for "parent" dependencies + # (i.e. an `ExtAB` where A depends on / loads B, but B provides the extension) + + mktempdir() do depot # Parallel pre-compilation + code = """ + Base.disable_parallel_precompile = false + using Parent + Base.get_extension(getfield(Parent, :DepWithParentExt), :ParentExt) isa Module || error("expected extension to load") + Parent.greet() + """ + proj = joinpath(@__DIR__, "project", "Extensions", "Parent.jl") + cmd = `$(Base.julia_cmd()) --startup-file=no -e $code` + cmd = addenv(cmd, + "JULIA_LOAD_PATH" => proj, + "JULIA_DEPOT_PATH" => depot * Base.Filesystem.pathsep(), + ) + @test occursin("Hello parent!", String(read(cmd))) + end + mktempdir() do depot # Serial pre-compilation + code = """ + Base.disable_parallel_precompile = true + using Parent + Base.get_extension(getfield(Parent, :DepWithParentExt), :ParentExt) isa Module || error("expected extension to load") + Parent.greet() + """ + proj = joinpath(@__DIR__, "project", "Extensions", "Parent.jl") + cmd = `$(Base.julia_cmd()) --startup-file=no -e $code` + cmd = addenv(cmd, + "JULIA_LOAD_PATH" => proj, + "JULIA_DEPOT_PATH" => depot * Base.Filesystem.pathsep(), + ) + @test occursin("Hello parent!", String(read(cmd))) + end + finally try rm(depot_path, force=true, recursive=true) diff --git a/test/project/Extensions/DepWithParentExt.jl/Project.toml b/test/project/Extensions/DepWithParentExt.jl/Project.toml new file mode 100644 index 0000000000000..bc487252ced4e --- /dev/null +++ b/test/project/Extensions/DepWithParentExt.jl/Project.toml @@ -0,0 +1,9 @@ +name = "DepWithParentExt" +uuid = "8a35c396-5ffc-40d2-b7ec-e8ed2248da32" +version = "0.1.0" + +[weakdeps] +Parent = "58cecb9c-f68a-426e-b92a-89d456ae7acc" + +[extensions] +ParentExt = "Parent" diff --git a/test/project/Extensions/DepWithParentExt.jl/ext/ParentExt.jl b/test/project/Extensions/DepWithParentExt.jl/ext/ParentExt.jl new file mode 100644 index 0000000000000..56176d2f5921d --- /dev/null +++ b/test/project/Extensions/DepWithParentExt.jl/ext/ParentExt.jl @@ -0,0 +1,6 @@ +module ParentExt + +using Parent +using DepWithParentExt + +end diff --git a/test/project/Extensions/DepWithParentExt.jl/src/DepWithParentExt.jl b/test/project/Extensions/DepWithParentExt.jl/src/DepWithParentExt.jl new file mode 100644 index 0000000000000..3d4ebc4ebf8a0 --- /dev/null +++ b/test/project/Extensions/DepWithParentExt.jl/src/DepWithParentExt.jl @@ -0,0 +1,5 @@ +module DepWithParentExt + +greet() = print("Hello dep w/ ext for parent dep!") + +end # module DepWithParentExt diff --git a/test/project/Extensions/Parent.jl/Manifest.toml b/test/project/Extensions/Parent.jl/Manifest.toml new file mode 100644 index 0000000000000..eb0c323ac36f5 --- /dev/null +++ b/test/project/Extensions/Parent.jl/Manifest.toml @@ -0,0 +1,20 @@ +# This file is machine-generated - editing it directly is not advised + +julia_version = "1.12.0-DEV" +manifest_format = "2.0" +project_hash = "b6ac643184d62cc94427c9aa665ff1fb63d66038" + +[[deps.DepWithParentExt]] +path = "../DepWithParentExt.jl" +uuid = "8a35c396-5ffc-40d2-b7ec-e8ed2248da32" +version = "0.1.0" +weakdeps = ["Parent"] + + [deps.DepWithParentExt.extensions] + ParentExt = "Parent" + +[[deps.Parent]] +deps = ["DepWithParentExt"] +path = "." +uuid = "58cecb9c-f68a-426e-b92a-89d456ae7acc" +version = "0.1.0" diff --git a/test/project/Extensions/Parent.jl/Project.toml b/test/project/Extensions/Parent.jl/Project.toml new file mode 100644 index 0000000000000..d62594cf15d3f --- /dev/null +++ b/test/project/Extensions/Parent.jl/Project.toml @@ -0,0 +1,7 @@ +name = "Parent" +uuid = "58cecb9c-f68a-426e-b92a-89d456ae7acc" +version = "0.1.0" +authors = ["Cody Tapscott "] + +[deps] +DepWithParentExt = "8a35c396-5ffc-40d2-b7ec-e8ed2248da32" diff --git a/test/project/Extensions/Parent.jl/src/Parent.jl b/test/project/Extensions/Parent.jl/src/Parent.jl new file mode 100644 index 0000000000000..471f4b13ecca3 --- /dev/null +++ b/test/project/Extensions/Parent.jl/src/Parent.jl @@ -0,0 +1,7 @@ +module Parent + +using DepWithParentExt + +greet() = print("Hello parent!") + +end # module Parent From 06f851903a85eecaa06ad1caf59f4a07af1c9d54 Mon Sep 17 00:00:00 2001 From: Cody Tapscott Date: Sun, 24 Nov 2024 12:11:06 +0000 Subject: [PATCH 2/2] Prevent pre-compilation target package from triggering extensions It is possible for an extension `ExtAB` to be loadable by one of its triggers, e.g. if A loads B. However this loading is only supposed to happen after loading for A is finished, so it shouldn't be included as part of pre-compiling A. Getting this wrong means disagreeing with the scheduled pre-compile jobs (A is not scheduled to depend on or generate a cache file for ExtAB but accidentally does both) and leads to confusing errors about missing cache files. To avoid trying to use / generate a cache file for ExtAB while still pre-compiling A, this change tracks the package being currently pre- compiled so that its extension triggers can be ignored. --- base/loading.jl | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/base/loading.jl b/base/loading.jl index ae54ba19038e9..0a70564077692 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -1433,6 +1433,7 @@ function run_module_init(mod::Module, i::Int=1) end function run_package_callbacks(modkey::PkgId) + @assert modkey != precompilation_target run_extension_callbacks(modkey) assert_havelock(require_lock) unlock(require_lock) @@ -1562,7 +1563,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(Base.loaded_modules, trigger_id) || haskey(package_locks, trigger_id) + if !haskey(Base.loaded_modules, trigger_id) || haskey(package_locks, trigger_id) || (trigger_id == precompilation_target) trigger1 = get!(Vector{ExtensionId}, EXT_DORMITORY, trigger_id) push!(trigger1, gid) else @@ -1575,6 +1576,7 @@ end loading_extension::Bool = false loadable_extensions::Union{Nothing,Vector{PkgId}} = nothing precompiling_extension::Bool = false +precompilation_target::Union{Nothing,PkgId} = nothing function run_extension_callbacks(extid::ExtensionId) assert_havelock(require_lock) succeeded = try @@ -3081,6 +3083,7 @@ function create_expr_cache(pkg::PkgId, input::String, output::String, output_o:: Base.track_nested_precomp($(_pkg_str(vcat(Base.precompilation_stack, pkg)))) Base.loadable_extensions = $(_pkg_str(loadable_exts)) Base.precompiling_extension = $(loading_extension) + Base.precompilation_target = $(_pkg_str(pkg)) Base.include_package_for_output($(_pkg_str(pkg)), $(repr(abspath(input))), $(repr(depot_path)), $(repr(dl_load_path)), $(repr(load_path)), $(_pkg_str(concrete_deps)), $(repr(source_path(nothing)))) """)