Skip to content

Commit

Permalink
Prevent pre-compilation package from triggering its own extensions (#…
Browse files Browse the repository at this point in the history
…56666)

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 committed Nov 24, 2024
1 parent c0478d8 commit e6c7c92
Show file tree
Hide file tree
Showing 8 changed files with 88 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 @@ -1215,6 +1215,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 @@ -1338,7 +1339,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(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 @@ -1350,6 +1351,7 @@ end

loading_extension::Bool = false
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 @@ -2342,6 +2344,7 @@ function create_expr_cache(pkg::PkgId, input::String, output::String, output_o::
write(io.in, """
empty!(Base.EXT_DORMITORY) # If we have a custom sysimage with `EXT_DORMITORY` prepopulated
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)), $deps, $(repr(source_path(nothing))))
""")
Expand Down
35 changes: 35 additions & 0 deletions test/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1101,6 +1101,41 @@ end
cmd_proj_ext = addenv(cmd_proj_ext, "JULIA_LOAD_PATH" => join([joinpath(proj, "HasExtensions.jl"), joinpath(proj, "EnvWithDeps")], sep))
run(cmd_proj_ext)
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.10.6"
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 e6c7c92

Please sign in to comment.