Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prevent loading other extensions when precompiling an extension #55589

Merged
merged 4 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions base/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1387,7 +1387,9 @@ function run_module_init(mod::Module, i::Int=1)
end

function run_package_callbacks(modkey::PkgId)
run_extension_callbacks(modkey)
if !precompiling_extension
run_extension_callbacks(modkey)
end
assert_havelock(require_lock)
unlock(require_lock)
try
Expand Down Expand Up @@ -2843,7 +2845,7 @@ end

const PRECOMPILE_TRACE_COMPILE = Ref{String}()
function create_expr_cache(pkg::PkgId, input::String, output::String, output_o::Union{Nothing, String},
concrete_deps::typeof(_concrete_dependencies), flags::Cmd=``, internal_stderr::IO = stderr, internal_stdout::IO = stdout)
concrete_deps::typeof(_concrete_dependencies), flags::Cmd=``, internal_stderr::IO = stderr, internal_stdout::IO = stdout, isext::Bool=false)
@nospecialize internal_stderr internal_stdout
rm(output, force=true) # Remove file if it exists
output_o === nothing || rm(output_o, force=true)
Expand Down Expand Up @@ -2912,7 +2914,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.track_nested_precomp($precomp_stack)
Base.precompiling_extension = $(loading_extension)
Base.precompiling_extension = $(loading_extension | isext)
Base.precompiling_package = true
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 Expand Up @@ -2970,18 +2972,18 @@ This can be used to reduce package load times. Cache files are stored in
`DEPOT_PATH[1]/compiled`. See [Module initialization and precompilation](@ref)
for important notes.
"""
function compilecache(pkg::PkgId, internal_stderr::IO = stderr, internal_stdout::IO = stdout; flags::Cmd=``, reasons::Union{Dict{String,Int},Nothing}=Dict{String,Int}())
function compilecache(pkg::PkgId, internal_stderr::IO = stderr, internal_stdout::IO = stdout; flags::Cmd=``, reasons::Union{Dict{String,Int},Nothing}=Dict{String,Int}(), isext::Bool=false)
@nospecialize internal_stderr internal_stdout
path = locate_package(pkg)
path === nothing && throw(ArgumentError("$(repr("text/plain", pkg)) not found during precompilation"))
return compilecache(pkg, path, internal_stderr, internal_stdout; flags, reasons)
return compilecache(pkg, path, internal_stderr, internal_stdout; flags, reasons, isext)
end

const MAX_NUM_PRECOMPILE_FILES = Ref(10)

function compilecache(pkg::PkgId, path::String, internal_stderr::IO = stderr, internal_stdout::IO = stdout,
keep_loaded_modules::Bool = true; flags::Cmd=``, cacheflags::CacheFlags=CacheFlags(),
reasons::Union{Dict{String,Int},Nothing}=Dict{String,Int}())
reasons::Union{Dict{String,Int},Nothing}=Dict{String,Int}(), isext::Bool=false)

@nospecialize internal_stderr internal_stdout
# decide where to put the resulting cache file
Expand Down Expand Up @@ -3021,7 +3023,7 @@ function compilecache(pkg::PkgId, path::String, internal_stderr::IO = stderr, in
close(tmpio_o)
close(tmpio_so)
end
p = create_expr_cache(pkg, path, tmppath, tmppath_o, concrete_deps, flags, internal_stderr, internal_stdout)
p = create_expr_cache(pkg, path, tmppath, tmppath_o, concrete_deps, flags, internal_stderr, internal_stdout, isext)

if success(p)
if cache_objects
Expand Down
47 changes: 1 addition & 46 deletions base/precompilation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -435,51 +435,6 @@ function precompilepkgs(pkgs::Vector{String}=String[];
# consider exts of direct deps to be direct deps so that errors are reported
append!(direct_deps, keys(filter(d->last(d) in keys(env.project_deps), exts)))

# An extension effectively depends on another extension if it has all the the
# dependencies of that other extension
function expand_dependencies(depsmap)
function visit!(visited, node, all_deps)
if node in visited
return
end
push!(visited, node)
for dep in get(Set{Base.PkgId}, depsmap, node)
if !(dep in all_deps)
push!(all_deps, dep)
visit!(visited, dep, all_deps)
end
end
end

depsmap_transitive = Dict{Base.PkgId, Set{Base.PkgId}}()
for package in keys(depsmap)
# Initialize a set to keep track of all dependencies for 'package'
all_deps = Set{Base.PkgId}()
visited = Set{Base.PkgId}()
visit!(visited, package, all_deps)
# Update depsmap with the complete set of dependencies for 'package'
depsmap_transitive[package] = all_deps
end
return depsmap_transitive
end

depsmap_transitive = expand_dependencies(depsmap)

for (_, extensions_1) in pkg_exts_map
for extension_1 in extensions_1
deps_ext_1 = depsmap_transitive[extension_1]
for (_, extensions_2) in pkg_exts_map
for extension_2 in extensions_2
extension_1 == extension_2 && continue
deps_ext_2 = depsmap_transitive[extension_2]
if issubset(deps_ext_2, deps_ext_1)
push!(depsmap[extension_1], extension_2)
end
end
end
end
end

@debug "precompile: deps collected"
# this loop must be run after the full depsmap has been populated
for (pkg, pkg_exts) in pkg_exts_map
Expand Down Expand Up @@ -852,7 +807,7 @@ function precompilepkgs(pkgs::Vector{String}=String[];
t = @elapsed ret = precompile_pkgs_maybe_cachefile_lock(io, print_lock, fancyprint, pkg_config, pkgspidlocked, hascolor) do
Base.with_logger(Base.NullLogger()) do
# The false here means we ignore loaded modules, so precompile for a fresh session
Base.compilecache(pkg, sourcepath, std_pipe, std_pipe, false; flags, cacheflags)
Base.compilecache(pkg, sourcepath, std_pipe, std_pipe, false; flags, cacheflags, isext = haskey(exts, pkg))
end
end
if ret isa Base.PrecompilableError
Expand Down
13 changes: 13 additions & 0 deletions test/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1155,6 +1155,19 @@ end
finally
copy!(LOAD_PATH, old_load_path)
end

# Extension with cycles in dependencies
code = """
using CyclicExtensions
Base.get_extension(CyclicExtensions, :ExtA) isa Module || error("expected extension to load")
Base.get_extension(CyclicExtensions, :ExtB) isa Module || error("expected extension to load")
CyclicExtensions.greet()
"""
proj = joinpath(@__DIR__, "project", "Extensions", "CyclicExtensions")
cmd = `$(Base.julia_cmd()) --startup-file=no -e $code`
cmd = addenv(cmd, "JULIA_LOAD_PATH" => proj)
@test occursin("Hello Cycles!", String(read(cmd)))

finally
try
rm(depot_path, force=true, recursive=true)
Expand Down
21 changes: 21 additions & 0 deletions test/project/Extensions/CyclicExtensions/Manifest.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# This file is machine-generated - editing it directly is not advised

julia_version = "1.10.4"
manifest_format = "2.0"
project_hash = "ec25ff8df3a5e2212a173c3de2c7d716cc47cd36"

[[deps.ExtDep]]
deps = ["SomePackage"]
path = "../ExtDep.jl"
uuid = "fa069be4-f60b-4d4c-8b95-f8008775090c"
version = "0.1.0"

[[deps.ExtDep2]]
path = "../ExtDep2"
uuid = "55982ee5-2ad5-4c40-8cfe-5e9e1b01500d"
version = "0.1.0"

[[deps.SomePackage]]
path = "../SomePackage"
uuid = "678608ae-7bb3-42c7-98b1-82102067a3d8"
version = "0.1.0"
13 changes: 13 additions & 0 deletions test/project/Extensions/CyclicExtensions/Project.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
name = "CyclicExtensions"
uuid = "17d4f0df-b55c-4714-ac4b-55fa23f7355c"
version = "0.1.0"

[deps]
ExtDep = "fa069be4-f60b-4d4c-8b95-f8008775090c"

[weakdeps]
SomePackage = "678608ae-7bb3-42c7-98b1-82102067a3d8"

[extensions]
ExtA = ["SomePackage"]
ExtB = ["SomePackage"]
6 changes: 6 additions & 0 deletions test/project/Extensions/CyclicExtensions/ext/ExtA.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module ExtA

using CyclicExtensions
using SomePackage

end
6 changes: 6 additions & 0 deletions test/project/Extensions/CyclicExtensions/ext/ExtB.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module ExtB

using CyclicExtensions
using SomePackage

end
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module CyclicExtensions

using ExtDep

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

end # module CyclicExtensions