[backports-release-1.11] Allow ext → ext dependency if triggers are a strict superset (#56368)

This is an effort at a proper workaround (feature?) to resolve
#56204.

For example:
```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.

We'll also need to land #49891, so that we guarantee these load in the
right order.

Author: topolarity

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 triggers of any
+  extension they wish to depend upon (+ at least one other trigger). Ext-to-ext dependencies
+  that don't meet this requirement are now blocked from using `Base.get_extension` during pre-
+  compilation, to prevent extension cycles [#55557].
 
 Compiler/Runtime improvements
 -----------------------------

base/loading.jl

@@ -733,8 +733,9 @@ function manifest_uuid_path(env::String, pkg::PkgId)::Union{Nothing,String,Missi
         proj = implicit_manifest_uuid_path(env, pkg)
         proj === nothing || return proj
         # if not found
-        parentid = get(EXT_PRIMED, pkg, nothing)
-        if parentid !== nothing
+        triggers = get(EXT_PRIMED, pkg, nothing)
+        if triggers !== nothing
+            parentid = triggers[1]
             _, parent_project_file = entry_point_and_project_file(env, parentid.name)
             if parent_project_file !== nothing
                 parentproj = project_file_name_uuid(parent_project_file, parentid.name)
@@ -1387,9 +1388,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
@@ -1418,7 +1417,7 @@ mutable struct ExtensionId
     ntriggers::Int # how many more packages must be defined until this is loaded
 end
 
-const EXT_PRIMED = Dict{PkgId, PkgId}() # Extension -> Parent
+const EXT_PRIMED = Dict{PkgId,Vector{PkgId}}() # Extension -> Parent + Triggers (parent is always first)
 const EXT_DORMITORY = Dict{PkgId,Vector{ExtensionId}}() # Trigger -> Extensions that can be triggered by it
 const EXT_DORMITORY_FAILED = ExtensionId[]
 
@@ -1509,14 +1508,15 @@ function _insert_extension_triggers(parent::PkgId, extensions::Dict{String, Any}
         if haskey(EXT_PRIMED, id) || haskey(Base.loaded_modules, id)
             continue  # extension is already primed or loaded, don't add it again
         end
-        EXT_PRIMED[id] = parent
+        EXT_PRIMED[id] = trigger_ids = PkgId[parent]
         gid = ExtensionId(id, parent, 1 + length(triggers), 1 + length(triggers))
         trigger1 = get!(Vector{ExtensionId}, EXT_DORMITORY, parent)
         push!(trigger1, gid)
         for trigger in triggers
             # TODO: Better error message if this lookup fails?
             uuid_trigger = UUID(totaldeps[trigger]::String)
             trigger_id = PkgId(uuid_trigger, trigger)
+            push!(trigger_ids, trigger_id)
             if !haskey(explicit_loaded_modules, trigger_id) || haskey(package_locks, trigger_id)
                 trigger1 = get!(Vector{ExtensionId}, EXT_DORMITORY, trigger_id)
                 push!(trigger1, gid)
@@ -1528,6 +1528,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 +1559,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
@@ -2542,7 +2543,17 @@ function _require(pkg::PkgId, env=nothing)
                 # double-check the search now that we have lock
                 m = _require_search_from_serialized(pkg, path, UInt128(0), true)
                 m isa Module && return m
-                return compilecache(pkg, path; reasons)
+                triggers = get(EXT_PRIMED, pkg, nothing)
+                loadable_exts = nothing
+                if triggers !== nothing # extension
+                    loadable_exts = PkgId[]
+                    for (ext′, triggers′) in EXT_PRIMED
+                        if triggers′ ⊊ triggers
+                            push!(loadable_exts, ext′)
+                        end
+                    end
+                end
+                return compilecache(pkg, path; reasons, loadable_exts)
             end
             loaded isa Module && return loaded
             if isnothing(loaded) # maybe_cachefile_lock returns nothing if it had to wait for another process
@@ -2865,19 +2876,26 @@ function check_package_module_loaded(pkg::PkgId)
     return nothing
 end
 
+# protects against PkgId and UUID being imported and losing Base prefix
+_pkg_str(_pkg::PkgId) = (_pkg.uuid === nothing) ? "Base.PkgId($(repr(_pkg.name)))" : "Base.PkgId(Base.UUID(\"$(_pkg.uuid)\"), $(repr(_pkg.name)))"
+_pkg_str(_pkg::Vector) = sprint(show, eltype(_pkg); context = :module=>nothing) * "[" * join(map(_pkg_str, _pkg), ",") * "]"
+_pkg_str(_pkg::Pair{PkgId}) = _pkg_str(_pkg.first) * " => " * repr(_pkg.second)
+_pkg_str(_pkg::Nothing) = "nothing"
+
 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)
     depot_path = map(abspath, DEPOT_PATH)
     dl_load_path = map(abspath, DL_LOAD_PATH)
     load_path = map(abspath, Base.load_path())
     # if pkg is a stdlib, append its parent Project.toml to the load path
-    parentid = get(EXT_PRIMED, pkg, nothing)
-    if parentid !== nothing
+    triggers = get(EXT_PRIMED, pkg, nothing)
+    if triggers !== nothing
+        parentid = triggers[1]
         for env in load_path
             project_file = env_project_file(env)
             if project_file === true
@@ -2895,22 +2913,6 @@ function create_expr_cache(pkg::PkgId, input::String, output::String, output_o::
     any(path -> path_sep in path, load_path) &&
         error("LOAD_PATH entries cannot contain $(repr(path_sep))")
 
-    deps_strs = String[]
-    # protects against PkgId and UUID being imported and losing Base prefix
-    function pkg_str(_pkg::PkgId)
-        if _pkg.uuid === nothing
-            "Base.PkgId($(repr(_pkg.name)))"
-        else
-            "Base.PkgId(Base.UUID(\"$(_pkg.uuid)\"), $(repr(_pkg.name)))"
-        end
-    end
-    for (pkg, build_id) in concrete_deps
-        push!(deps_strs, "$(pkg_str(pkg)) => $(repr(build_id))")
-    end
-    deps_eltype = sprint(show, eltype(concrete_deps); context = :module=>nothing)
-    deps = deps_eltype * "[" * join(deps_strs, ",") * "]"
-    precomp_stack = "Base.PkgId[$(join(map(pkg_str, vcat(Base.precompilation_stack, pkg)), ", "))]"
-
     if output_o === nothing
         # remove options that make no difference given the other cache options
         cacheflags = CacheFlags(cacheflags, opt_level=0)
@@ -2941,10 +2943,11 @@ function create_expr_cache(pkg::PkgId, input::String, output::String, output_o::
     # write data over stdin to avoid the (unlikely) case of exceeding max command line size
     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.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))))
+        Base.track_nested_precomp($(_pkg_str(vcat(Base.precompilation_stack, pkg))))
+        Base.loadable_extensions = $(_pkg_str(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)), $(_pkg_str(concrete_deps)), $(repr(source_path(nothing))))
         """)
     close(io.in)
     return io
@@ -2999,18 +3002,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 +3053,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

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,12 @@ 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
+                                    # for extensions, any extension in our direct dependencies is one we have a right to load
+                                    # for packages, we may load any extension (all possible triggers are accounted for above)
+                                    loadable_exts = haskey(exts, pkg) ? filter((dep)->haskey(exts, dep), depsmap[pkg]) : nothing
+                                    Base.compilecache(pkg, sourcepath, std_pipe, std_pipe, keep_loaded_modules;
+                                                      flags, cacheflags, loadable_exts)
                                 end
                             end
                             if ret isa Base.PrecompilableError

test/loading.jl

@@ -1158,6 +1158,74 @@ end
         cmd = addenv(cmd, "JULIA_LOAD_PATH" => proj)
         @test occursin("Hello Cycles!", String(read(cmd)))
 
+        # Extension-to-extension dependencies
+
+        mktempdir() do depot # Parallel pre-compilation
+            code = """
+            Base.disable_parallel_precompile = false
+            using ExtToExtDependency
+            Base.get_extension(ExtToExtDependency, :ExtA) isa Module || error("expected extension to load")
+            Base.get_extension(ExtToExtDependency, :ExtAB) isa Module || error("expected extension to load")
+            ExtToExtDependency.greet()
+            """
+            proj = joinpath(@__DIR__, "project", "Extensions", "ExtToExtDependency")
+            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 ext-to-ext!", String(read(cmd)))
+        end
+        mktempdir() do depot # Serial pre-compilation
+            code = """
+            Base.disable_parallel_precompile = true
+            using ExtToExtDependency
+            Base.get_extension(ExtToExtDependency, :ExtA) isa Module || error("expected extension to load")
+            Base.get_extension(ExtToExtDependency, :ExtAB) isa Module || error("expected extension to load")
+            ExtToExtDependency.greet()
+            """
+            proj = joinpath(@__DIR__, "project", "Extensions", "ExtToExtDependency")
+            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 ext-to-ext!", String(read(cmd)))
+        end
+
+        mktempdir() do depot # Parallel pre-compilation
+            code = """
+            Base.disable_parallel_precompile = false
+            using CrossPackageExtToExtDependency
+            Base.get_extension(CrossPackageExtToExtDependency.CyclicExtensions, :ExtA) isa Module || error("expected extension to load")
+            Base.get_extension(CrossPackageExtToExtDependency, :ExtAB) isa Module || error("expected extension to load")
+            CrossPackageExtToExtDependency.greet()
+            """
+            proj = joinpath(@__DIR__, "project", "Extensions", "CrossPackageExtToExtDependency")
+            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 x-package ext-to-ext!", String(read(cmd)))
+        end
+        mktempdir() do depot # Serial pre-compilation
+            code = """
+            Base.disable_parallel_precompile = true
+            using CrossPackageExtToExtDependency
+            Base.get_extension(CrossPackageExtToExtDependency.CyclicExtensions, :ExtA) isa Module || error("expected extension to load")
+            Base.get_extension(CrossPackageExtToExtDependency, :ExtAB) isa Module || error("expected extension to load")
+            CrossPackageExtToExtDependency.greet()
+            """
+            proj = joinpath(@__DIR__, "project", "Extensions", "CrossPackageExtToExtDependency")
+            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 x-package ext-to-ext!", String(read(cmd)))
+        end
+
     finally
         try
             rm(depot_path, force=true, recursive=true)

test/project/Extensions/CrossPackageExtToExtDependency/Manifest.toml

@@ -0,0 +1,32 @@
+# This file is machine-generated - editing it directly is not advised
+
+julia_version = "1.11.1"
+manifest_format = "2.0"
+project_hash = "dc35c2cf8c6b82fb5b9624c9713c2df34ca30499"
+
+[[deps.CyclicExtensions]]
+deps = ["ExtDep"]
+path = "../CyclicExtensions"
+uuid = "17d4f0df-b55c-4714-ac4b-55fa23f7355c"
+version = "0.1.0"
+weakdeps = ["SomePackage"]
+
+    [deps.CyclicExtensions.extensions]
+    ExtA = ["SomePackage"]
+    ExtB = ["SomePackage"]
+
+[[deps.ExtDep]]
+deps = ["SomeOtherPackage", "SomePackage"]
+path = "../ExtDep.jl"
+uuid = "fa069be4-f60b-4d4c-8b95-f8008775090c"
+version = "0.1.0"
+
+[[deps.SomeOtherPackage]]
+path = "../SomeOtherPackage"
+uuid = "178f68a2-4498-45ee-a775-452b36359b63"
+version = "0.1.0"
+
+[[deps.SomePackage]]
+path = "../SomePackage"
+uuid = "678608ae-7bb3-42c7-98b1-82102067a3d8"
+version = "0.1.0"

test/project/Extensions/CrossPackageExtToExtDependency/Project.toml

@@ -0,0 +1,12 @@
+name = "CrossPackageExtToExtDependency"
+uuid = "30f07f2e-c47e-40db-93a2-cbc4d1b301cc"
+version = "0.1.0"
+
+[deps]
+CyclicExtensions = "17d4f0df-b55c-4714-ac4b-55fa23f7355c"
+
+[weakdeps]
+SomePackage = "678608ae-7bb3-42c7-98b1-82102067a3d8"
+
+[extensions]
+ExtAB = ["CyclicExtensions", "SomePackage"]

test/project/Extensions/CrossPackageExtToExtDependency/ext/ExtAB.jl

@@ -0,0 +1,12 @@
+module ExtAB
+
+using CrossPackageExtToExtDependency
+using SomePackage
+using CyclicExtensions
+
+const ExtA = Base.get_extension(CyclicExtensions, :ExtA)
+if !(ExtA isa Module)
+    error("expected extension to load")
+end
+
+end