Skip to content

Commit

Permalink
Allow ext → ext dependency if triggers are a strict superset
Browse files Browse the repository at this point in the history 
This allows for one extension to depend on another if its triggers are
a strict superset of the other's. For example, in a Project.toml:

```toml
[extensions]
PlottingExt = "Plots"
StatisticsPlottingExt = ["Plots", "Statistics"]
```

Here `StatisticsPlottingExt` is allowed to depend on `PlottingExt`
This provides a way to declare `ext → ext` dependencies while still
avoiding any extension cycles. The same trick can also be used to make
an extension in one package depend on an extension provided in another.

Also requires something like #49891, so that we guarantee these load in
the right order.
  • Loading branch information
@topolarity
topolarity committed Oct 29, 2024
1 parent 05182dc commit f01c6e0
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 19 deletions.
4 changes: 4 additions & 0 deletions NEWS.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ Language changes
omit the default user depot ([#51448]).
* Precompilation cache files are now relocatable and their validity is now verified through
a content hash of their source files instead of their `mtime` ([#49866]).
* Extensions may now depend on other extensions, if their triggers include all of the triggers
of any extension they wish to depend upon (+ at least one other trigger). In contrast to prior
versions, ext-to-ext dependencies that don't meet this requirement are now blocked during pre-
compilation to prevent extension cycles [#55557].

Compiler/Runtime improvements
-----------------------------
Expand Down
20 changes: 10 additions & 10 deletions base/loading.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1387,9 +1387,7 @@ function run_module_init(mod::Module, i::Int=1)
end

function run_package_callbacks(modkey::PkgId)
if !precompiling_extension
run_extension_callbacks(modkey)
end
run_extension_callbacks(modkey)
assert_havelock(require_lock)
unlock(require_lock)
try
Expand Down Expand Up @@ -1527,6 +1525,7 @@ function _insert_extension_triggers(parent::PkgId, extensions::Dict{String, Any}
end

loading_extension::Bool = false
loadable_extensions::Union{Nothing,Vector{PkgId}} = nothing
precompiling_extension::Bool = false
function run_extension_callbacks(extid::ExtensionId)
assert_havelock(require_lock)
Expand Down Expand Up @@ -1565,7 +1564,7 @@ function run_extension_callbacks(pkgid::PkgId)
else
succeeded = true
end
if extid.ntriggers == 0
if extid.ntriggers == 0 && (loadable_extensions === nothing || extid.id in loadable_extensions)
# actually load extid, now that all dependencies are met,
# and record the result
succeeded = succeeded && run_extension_callbacks(extid)
Expand Down Expand Up @@ -2870,7 +2869,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=``, cacheflags::CacheFlags=CacheFlags(),
internal_stderr::IO = stderr, internal_stdout::IO = stdout, isext::Bool=false)
internal_stderr::IO = stderr, internal_stdout::IO = stdout, loadable_exts::Union{Vector{PkgId},Nothing}=nothing)
@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 @@ -2944,7 +2943,8 @@ 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 | isext)
Base.loadable_extensions = $(loadable_exts)
Base.precompiling_extension = $(loading_extension)
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 @@ -3001,18 +3001,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}(), isext::Bool=false)
function compilecache(pkg::PkgId, internal_stderr::IO = stderr, internal_stdout::IO = stdout; flags::Cmd=``, reasons::Union{Dict{String,Int},Nothing}=Dict{String,Int}(), loadable_exts::Union{Vector{PkgId},Nothing}=nothing)
@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, isext)
return compilecache(pkg, path, internal_stderr, internal_stdout; flags, reasons, loadable_exts)
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}(), isext::Bool=false)
reasons::Union{Dict{String,Int},Nothing}=Dict{String,Int}(), loadable_exts::Union{Vector{PkgId},Nothing}=nothing)

@nospecialize internal_stderr internal_stdout
# decide where to put the resulting cache file
Expand Down Expand Up @@ -3052,7 +3052,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, cacheflags, internal_stderr, internal_stdout, isext)
p = create_expr_cache(pkg, path, tmppath, tmppath_o, concrete_deps, flags, cacheflags, internal_stderr, internal_stdout, loadable_exts)

if success(p)
if cache_objects
Expand Down
35 changes: 26 additions & 9 deletions base/precompilation.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,7 @@ function _precompilepkgs(pkgs::Vector{String},
return name
end

triggers = Dict{Base.PkgId,Vector{Base.PkgId}}()
for (dep, deps) in env.deps
pkg = Base.PkgId(dep, env.names[dep])
Base.in_sysimage(pkg) && continue
Expand All @@ -427,25 +428,22 @@ function _precompilepkgs(pkgs::Vector{String},
# add any extensions
pkg_exts = Dict{Base.PkgId, Vector{Base.PkgId}}()
for (ext_name, extdep_uuids) in env.extensions[dep]
ext_deps = Base.PkgId[]
push!(ext_deps, pkg) # depends on parent package
ext_uuid = Base.uuid5(pkg.uuid, ext_name)
ext = Base.PkgId(ext_uuid, ext_name)
triggers[ext] = Base.PkgId[pkg] # depends on parent package
all_extdeps_available = true
for extdep_uuid in extdep_uuids
extdep_name = env.names[extdep_uuid]
if extdep_uuid in keys(env.deps)
push!(ext_deps, Base.PkgId(extdep_uuid, extdep_name))
push!(triggers[ext], Base.PkgId(extdep_uuid, extdep_name))
else
all_extdeps_available = false
break
end
end
all_extdeps_available || continue
ext_uuid = Base.uuid5(pkg.uuid, ext_name)
ext = Base.PkgId(ext_uuid, ext_name)
filter!(!Base.in_sysimage, ext_deps)
depsmap[ext] = ext_deps
exts[ext] = pkg.name
pkg_exts[ext] = ext_deps
pkg_exts[ext] = depsmap[ext] = filter(!Base.in_sysimage, triggers[ext])
end
if !isempty(pkg_exts)
pkg_exts_map[pkg] = collect(keys(pkg_exts))
Expand All @@ -461,6 +459,16 @@ function _precompilepkgs(pkgs::Vector{String},
append!(direct_deps, keys(filter(d->last(d) in keys(env.project_deps), exts)))

@debug "precompile: deps collected"

# An extension effectively depends on another extension if it has a strict superset of its triggers
for ext_a in keys(exts)
for ext_b in keys(exts)
if triggers[ext_a] ⊋ triggers[ext_b]
push!(depsmap[ext_a], ext_b)
end
end
end

# this loop must be run after the full depsmap has been populated
for (pkg, pkg_exts) in pkg_exts_map
# find any packages that depend on the extension(s)'s deps and replace those deps in their deps list with the extension(s),
Expand Down Expand Up @@ -817,7 +825,16 @@ function _precompilepkgs(pkgs::Vector{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, isext = haskey(exts, pkg))
keep_loaded_modules = false
if haskey(exts, pkg)
# any extension in our direct dependencies is one we have a right to load
loadable_exts = filter((dep)->haskey(exts, dep), depsmap[pkg])
Base.compilecache(pkg, sourcepath, std_pipe, std_pipe, keep_loaded_modules;
flags, cacheflags, loadable_exts)
else
Base.compilecache(pkg, sourcepath, std_pipe, std_pipe, keep_loaded_modules;
flags, cacheflags)
end
end
end
if ret isa Base.PrecompilableError
Expand Down

0 comments on commit f01c6e0

Please sign in to comment.