Skip to content

Commit

Permalink
[release-1.11] Prevent pre-compilation package from triggering its ow…
Browse files Browse the repository at this point in the history
…n extensions (#56666) (#56676)

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 not supposed
to happen during pre-compilation of `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 attempts both) and leads to confusing errors about
missing cache files.

We used to cover up this bad behavior w/ an erroneous cycle warning
(fixed by #55910), but now we need to be sure this works.
  • Loading branch information
topolarity authored Nov 24, 2024
1 parent 5ef1884 commit 4a7b1d1
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 1 deletion.
5 changes: 4 additions & 1 deletion base/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1392,6 +1392,7 @@ function run_module_init(mod::Module, i::Int=1)
end

function run_package_callbacks(modkey::PkgId)
(modkey == precompilation_target) && return nothing
run_extension_callbacks(modkey)
assert_havelock(require_lock)
unlock(require_lock)
Expand Down Expand Up @@ -1521,7 +1522,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
Expand All @@ -1534,6 +1535,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
Expand Down Expand Up @@ -2967,6 +2969,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))))
""")
Expand Down
34 changes: 34 additions & 0 deletions test/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1209,6 +1209,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)
Expand Down
9 changes: 9 additions & 0 deletions test/project/Extensions/DepWithParentExt.jl/Project.toml
Original file line number Diff line number Diff line change
@@ -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"
6 changes: 6 additions & 0 deletions test/project/Extensions/DepWithParentExt.jl/ext/ParentExt.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module ParentExt

using Parent
using DepWithParentExt

end
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module DepWithParentExt

greet() = print("Hello dep w/ ext for parent dep!")

end # module DepWithParentExt
16 changes: 16 additions & 0 deletions test/project/Extensions/Parent.jl/Manifest.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# This file is machine-generated - editing it directly is not advised

julia_version = "1.11.1"
manifest_format = "2.0"
project_hash = "5d72c155f50f076d28b74de819d417878ffb0965"

[[deps.DepWithParentExt]]
path = "../DepWithParentExt.jl"
uuid = "8a35c396-5ffc-40d2-b7ec-e8ed2248da32"
version = "0.1.0"

[deps.DepWithParentExt.extensions]
ParentExt = "Parent"

[deps.DepWithParentExt.weakdeps]
Parent = "58cecb9c-f68a-426e-b92a-89d456ae7acc"
6 changes: 6 additions & 0 deletions test/project/Extensions/Parent.jl/Project.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
name = "Parent"
uuid = "58cecb9c-f68a-426e-b92a-89d456ae7acc"
version = "0.1.0"

[deps]
DepWithParentExt = "8a35c396-5ffc-40d2-b7ec-e8ed2248da32"
7 changes: 7 additions & 0 deletions test/project/Extensions/Parent.jl/src/Parent.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module Parent

using DepWithParentExt

greet() = print("Hello parent!")

end # module Parent

0 comments on commit 4a7b1d1

Please sign in to comment.