From a30c46c16f6ce3e69a2c78b3658392ed5536f62f Mon Sep 17 00:00:00 2001 From: Cody Tapscott Date: Fri, 27 Sep 2024 17:06:51 -0400 Subject: [PATCH] =?UTF-8?q?Allow=20ext=20=E2=86=92=20ext=20dependency=20if?= =?UTF-8?q?=20triggers=20are=20a=20strict=20superset?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- NEWS.md | 4 ++++ base/loading.jl | 20 ++++++++++---------- base/precompilation.jl | 35 ++++++++++++++++++++++++++--------- 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/NEWS.md b/NEWS.md index 20b17cf6f68cde..7a4af7d7769c3a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -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 ----------------------------- diff --git a/base/loading.jl b/base/loading.jl index d36059fd36cdc8..7d2c1649197b6e 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -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 @@ -1528,6 +1526,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) @@ -1558,7 +1557,7 @@ function run_extension_callbacks(pkgid::PkgId) for extid in extids @assert extid.ntriggers > 0 extid.ntriggers -= 1 - if extid.ntriggers == 0 + if extid.ntriggers == 0 && (loadable_extensions === nothing || extid.id in loadable_extensions) push!(extids_to_load, extid) end end @@ -2868,7 +2867,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) @@ -2942,7 +2941,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)))) """) @@ -2999,18 +2999,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 @@ -3050,7 +3050,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 diff --git a/base/precompilation.jl b/base/precompilation.jl index 0cd0f6346c3083..6bea0db369abc7 100644 --- a/base/precompilation.jl +++ b/base/precompilation.jl @@ -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 @@ -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)) @@ -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), @@ -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