Extensions: make loading of extensions independent of what packages are in the sysimage (JuliaLang#52841)

When triggers of extension are in the sysimage it is easy to end up with
cycles in package loading. Say we have a package A with exts BExt and
CExt and say that B and C is in the sysimage.

- Upon loading A, we will immidiately start to precompile BExt (because
the trigger B is "loaded" by virtue of being in the sysimage).
- BExt will load A which will cause CExt to start precompiling (again
because C is in the sysimage).
- CExt will load A which will now cause BExt to start loading and we get
a cycle.

This is fixed in this PR by instead of looking at what modules are
loaded, we look at what modules are actually `require`d and only use
that to drive the loading of extensions.

Fixes JuliaLang#52132.

(cherry picked from commit 08d229f)

Author: KristofferC

base/Base.jl

@@ -595,6 +595,7 @@ function __init__()
     init_load_path()
     init_active_project()
     append!(empty!(_sysimage_modules), keys(loaded_modules))
+    empty!(explicit_loaded_modules)
     if haskey(ENV, "JULIA_MAX_NUM_PRECOMPILE_FILES")
         MAX_NUM_PRECOMPILE_FILES[] = parse(Int, ENV["JULIA_MAX_NUM_PRECOMPILE_FILES"])
     end

base/loading.jl

@@ -1344,7 +1344,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) || (trigger_id == precompilation_target)
+            if !haskey(explicit_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
@@ -1891,6 +1891,11 @@ function __require_prelocked(uuidkey::PkgId, env=nothing)
             REPL_MODULE_REF[] = newm
         end
     else
+        m = get(loaded_modules, uuidkey, nothing)
+        if m !== nothing
+            explicit_loaded_modules[uuidkey] = m
+            run_package_callbacks(uuidkey)
+        end
         newm = root_module(uuidkey)
     end
     return newm
@@ -1905,6 +1910,8 @@ PkgOrigin() = PkgOrigin(nothing, nothing, nothing)
 const pkgorigins = Dict{PkgId,PkgOrigin}()
 
 const loaded_modules = Dict{PkgId,Module}()
+# Emptied on Julia start
+const explicit_loaded_modules = Dict{PkgId,Module}()
 const loaded_modules_order = Vector{Module}()
 const module_keys = IdDict{Module,PkgId}() # the reverse
 
@@ -1928,6 +1935,7 @@ root_module_key(m::Module) = @lock require_lock module_keys[m]
     end
     push!(loaded_modules_order, m)
     loaded_modules[key] = m
+    explicit_loaded_modules[key] = m
     module_keys[m] = key
     end
     nothing
@@ -2110,7 +2118,6 @@ function _require_from_serialized(uuidkey::PkgId, path::String, ocachepath::Unio
 end
 
 
-
 # relative-path load
 
 """

test/loading.jl

@@ -795,10 +795,8 @@ end
 @testset "`Base.project_names` and friends" begin
     # Some functions in Pkg assumes that these tuples have the same length
     n = length(Base.project_names)
+    @test length(Base.manifest_names) == n
     @test length(Base.preferences_names) == n
-
-    # there are two manifest names per project name
-    @test length(Base.manifest_names) == 2n
 end
 
 @testset "Manifest formats" begin
@@ -827,33 +825,6 @@ end
     end
 end
 
-@testset "Manifest name preferential loading" begin
-    mktempdir() do tmp
-        proj = joinpath(tmp, "Project.toml")
-        touch(proj)
-        for man_name in (
-            "Manifest.toml",
-            "JuliaManifest.toml",
-            "Manifest-v$(VERSION.major).$(VERSION.minor).toml",
-            "JuliaManifest-v$(VERSION.major).$(VERSION.minor).toml"
-            )
-            touch(joinpath(tmp, man_name))
-            man = basename(Base.project_file_manifest_path(proj))
-            @test man == man_name
-        end
-    end
-    mktempdir() do tmp
-        # check that another version isn't preferred
-        proj = joinpath(tmp, "Project.toml")
-        touch(proj)
-        touch(joinpath(tmp, "Manifest-v1.5.toml"))
-        @test Base.project_file_manifest_path(proj) == nothing
-        touch(joinpath(tmp, "Manifest.toml"))
-        man = basename(Base.project_file_manifest_path(proj))
-        @test man == "Manifest.toml"
-    end
-end
-
 @testset "error message loading pkg bad module name" begin
     mktempdir() do tmp
         old_loadpath = copy(LOAD_PATH)
@@ -1033,16 +1004,6 @@ end
 end
 
 @testset "Extensions" begin
-    test_ext = """
-    function test_ext(parent::Module, ext::Symbol)
-        _ext = Base.get_extension(parent, ext)
-        _ext isa Module || error("expected extension \$ext to be loaded")
-        _pkgdir = pkgdir(_ext)
-        _pkgdir == pkgdir(parent) != nothing || error("unexpected extension \$ext pkgdir path: \$_pkgdir")
-        _pkgversion = pkgversion(_ext)
-        _pkgversion == pkgversion(parent) || error("unexpected extension \$ext version: \$_pkgversion")
-    end
-    """
     depot_path = mktempdir()
     try
         proj = joinpath(@__DIR__, "project", "Extensions", "HasDepWithExtensions.jl")
@@ -1053,25 +1014,20 @@ end
             cmd = """
             $load_distr
             begin
-                $ew $test_ext
                 $ew push!(empty!(DEPOT_PATH), $(repr(depot_path)))
                 using HasExtensions
                 $ew using HasExtensions
                 $ew Base.get_extension(HasExtensions, :Extension) === nothing || error("unexpectedly got an extension")
                 $ew HasExtensions.ext_loaded && error("ext_loaded set")
                 using HasDepWithExtensions
                 $ew using HasDepWithExtensions
-                $ew test_ext(HasExtensions, :Extension)
                 $ew Base.get_extension(HasExtensions, :Extension).extvar == 1 || error("extvar in Extension not set")
                 $ew HasExtensions.ext_loaded || error("ext_loaded not set")
                 $ew HasExtensions.ext_folder_loaded && error("ext_folder_loaded set")
                 $ew HasDepWithExtensions.do_something() || error("do_something errored")
                 using ExtDep2
                 $ew using ExtDep2
                 $ew HasExtensions.ext_folder_loaded || error("ext_folder_loaded not set")
-                using ExtDep3
-                $ew using ExtDep3
-                $ew HasExtensions.ext_dep_loaded || error("ext_dep_loaded not set")
             end
             """
             return `$(Base.julia_cmd()) $compile --startup-file=no -e $cmd`
@@ -1114,14 +1070,11 @@ end
 
         test_ext_proj = """
         begin
-            $test_ext
             using HasExtensions
             using ExtDep
-            test_ext(HasExtensions, :Extension)
+            Base.get_extension(HasExtensions, :Extension) isa Module || error("expected extension to load")
             using ExtDep2
-            test_ext(HasExtensions, :ExtensionFolder)
-            using ExtDep3
-            test_ext(HasExtensions, :ExtensionDep)
+            Base.get_extension(HasExtensions, :ExtensionFolder) isa Module || error("expected extension to load")
         end
         """
         for compile in (`--compiled-modules=no`, ``)
@@ -1131,40 +1084,23 @@ end
             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
-
+        # Sysimage extensions
+        # The test below requires that LinearAlgebra is in the sysimage and that it has not been loaded yet.
+        # if it gets moved out, this test will need to be updated.
+        # We run this test in a new process so we are not vulnerable to a previous test having loaded LinearAlgebra
+        sysimg_ext_test_code = """
+            uuid_key = Base.PkgId(Base.UUID("37e2e46d-f89d-539d-b4ee-838fcccc9c8e"), "LinearAlgebra")
+            Base.in_sysimage(uuid_key) || error("LinearAlgebra not in sysimage")
+            haskey(Base.explicit_loaded_modules, uuid_key) && error("LinearAlgebra already loaded")
+            using HasExtensions
+            Base.get_extension(HasExtensions, :LinearAlgebraExt) === nothing || error("unexpectedly got an extension")
+            using LinearAlgebra
+            haskey(Base.explicit_loaded_modules, uuid_key) || error("LinearAlgebra not loaded")
+            Base.get_extension(HasExtensions, :LinearAlgebraExt) isa Module || error("expected extension to load")
+        """
+        cmd =  `$(Base.julia_cmd()) --startup-file=no -e $sysimg_ext_test_code`
+        cmd = addenv(cmd, "JULIA_LOAD_PATH" => join([proj, "@stdlib"], sep))
+        run(cmd)
     finally
         try
             rm(depot_path, force=true, recursive=true)
@@ -1265,28 +1201,3 @@ end
 @testset "Upgradable stdlibs" begin
     @test success(`$(Base.julia_cmd()) --startup-file=no -e 'using DelimitedFiles'`)
 end
-
-@testset "Fallback for stdlib deps if manifest deps aren't found" begin
-    mktempdir() do depot
-        # This manifest has a LibGit2 entry that is missing LibGit2_jll, which should be
-        # handled by falling back to the stdlib Project.toml for dependency truth.
-        badmanifest_test_dir = joinpath(@__DIR__, "project", "deps", "BadStdlibDeps.jl")
-        @test success(addenv(
-            `$(Base.julia_cmd()) --project=$badmanifest_test_dir --startup-file=no -e 'using LibGit2'`,
-            "JULIA_DEPOT_PATH" => depot * Base.Filesystem.pathsep(),
-        ))
-    end
-end
-
-@testset "extension path computation name collision" begin
-    old_load_path = copy(LOAD_PATH)
-    try
-        empty!(LOAD_PATH)
-        push!(LOAD_PATH, joinpath(@__DIR__, "project", "Extensions", "ExtNameCollision_A"))
-        push!(LOAD_PATH, joinpath(@__DIR__, "project", "Extensions", "ExtNameCollision_B"))
-        ext_B = Base.PkgId(Base.uuid5(Base.identify_package("ExtNameCollision_B").uuid, "REPLExt"), "REPLExt")
-        @test Base.locate_package(ext_B) == joinpath(@__DIR__, "project",  "Extensions", "ExtNameCollision_B", "ext", "REPLExt.jl")
-    finally
-        copy!(LOAD_PATH, old_load_path)
-    end
-end

test/project/Extensions/HasDepWithExtensions.jl/Manifest.toml

@@ -1,8 +1,8 @@
 # This file is machine-generated - editing it directly is not advised
 
-julia_version = "1.10.6"
+julia_version = "1.10.0"
 manifest_format = "2.0"
-project_hash = "eed4f16fdd2e22799229e480394c255a569eb19c"
+project_hash = "d523b3401f72a1ed34b7b43749fd2655c6b78542"
 
 [[deps.ExtDep]]
 deps = ["SomePackage"]
@@ -15,22 +15,20 @@ path = "../ExtDep2"
 uuid = "55982ee5-2ad5-4c40-8cfe-5e9e1b01500d"
 version = "0.1.0"
 
-[[deps.ExtDep3]]
-path = "../ExtDep3.jl"
-uuid = "a5541f1e-a556-4fdc-af15-097880d743a1"
-version = "0.1.0"
-
 [[deps.HasExtensions]]
-deps = ["ExtDep3"]
 path = "../HasExtensions.jl"
 uuid = "4d3288b3-3afc-4bb6-85f3-489fffe514c8"
 version = "0.1.0"
-weakdeps = ["ExtDep", "ExtDep2"]
 
     [deps.HasExtensions.extensions]
     Extension = "ExtDep"
-    ExtensionDep = "ExtDep3"
     ExtensionFolder = ["ExtDep", "ExtDep2"]
+    LinearAlgebraExt = "LinearAlgebra"
+
+    [deps.HasExtensions.weakdeps]
+    ExtDep = "fa069be4-f60b-4d4c-8b95-f8008775090c"
+    ExtDep2 = "55982ee5-2ad5-4c40-8cfe-5e9e1b01500d"
+    LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
 
 [[deps.SomePackage]]
 path = "../SomePackage"

test/project/Extensions/HasExtensions.jl/Project.toml

@@ -8,8 +8,10 @@ ExtDep3 = "a5541f1e-a556-4fdc-af15-097880d743a1"
 [weakdeps]
 ExtDep = "fa069be4-f60b-4d4c-8b95-f8008775090c"
 ExtDep2 = "55982ee5-2ad5-4c40-8cfe-5e9e1b01500d"
+LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
 
 [extensions]
 Extension = "ExtDep"
 ExtensionDep = "ExtDep3"
 ExtensionFolder = ["ExtDep", "ExtDep2"]
+LinearAlgebraExt = "LinearAlgebra"

test/project/Extensions/HasExtensions.jl/ext/LinearAlgebraExt.jl

@@ -0,0 +1,3 @@
+module LinearAlgebraExt
+
+end