diff --git a/base/loading.jl b/base/loading.jl index 5bc3c2ef77098..6c4541296f269 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -3160,7 +3160,7 @@ function parse_cache_header(f::IO, cachefile::AbstractString) includes_srcfiles = CacheHeaderIncludes[] includes_depfiles = CacheHeaderIncludes[] - for (i, inc) in enumerate(includes) + for inc in includes if inc.filename ∈ srcfiles push!(includes_srcfiles, inc) else @@ -3168,23 +3168,63 @@ function parse_cache_header(f::IO, cachefile::AbstractString) end end - # determine depot for @depot replacement for include() files and include_dependency() files separately - srcfiles_depot = resolve_depot(first(srcfiles)) - if srcfiles_depot === :no_depot_found - @debug("Unable to resolve @depot tag include() files from cache file $cachefile", srcfiles) - elseif srcfiles_depot === :not_relocatable - @debug("include() files from $cachefile are not relocatable", srcfiles) - else + + # The @depot resolution logic for include() files: + # 1. If the cache is not relocatable because of an absolute path, + # we ignore that path for the depot search. + # Recompilation will be triggered by stale_cachefile() if that absolute path does not exist. + # 2. If we can't find a depot for a relocatable path, + # we still replace it with the depot we found from other files. + # Recompilation will be triggered by stale_cachefile() because the resolved path does not exist. + # 3. We require that relocatable paths all resolve to the same depot. + # 4. We explicitly check that all relocatable paths resolve to the same depot. This has two reasons: + # - We want to scan all source files in order to provide logs for 1. and 2. above. + # - It is possible that a depot might be missing source files. + # Assume that we have two depots on DEPOT_PATH, depot_complete and depot_incomplete. + # If DEPOT_PATH=["depot_complete","depot_incomplete"] then no recompilation shall happen, + # because depot_complete will be picked. + # If DEPOT_PATH=["depot_incomplete","depot_complete"] we trigger recompilation and + # hopefully a meaningful error about missing files is thrown. + # If we were to just select the first depot we find, then whether recompilation happens would + # depend on whether the first relocatable file resolves to depot_complete or depot_incomplete. + srcdepot = nothing + any_not_relocatable = false + any_no_depot_found = false + multiple_depots_found = false + for src in srcfiles + depot = resolve_depot(src) + if depot === :not_relocatable + any_not_relocatable = true + elseif depot === :no_depot_found + any_no_depot_found = true + elseif isnothing(srcdepot) + srcdepot = depot + elseif depot != srcdepot + multiple_depots_found = true + end + end + if any_no_depot_found + @debug("Unable to resolve @depot tag for at least one include() file from cache file $cachefile", srcfiles, _group=:relocatable) + end + if any_not_relocatable + @debug("At least one include() file from $cachefile is not relocatable", srcfiles, _group=:relocatable) + end + if multiple_depots_found + @debug("Some include() files from $cachefile are distributed over multiple depots", srcfiles, _group=:relocatable) + elseif !isnothing(srcdepot) for inc in includes_srcfiles - inc.filename = restore_depot_path(inc.filename, srcfiles_depot) + inc.filename = restore_depot_path(inc.filename, srcdepot) end end + + # unlike include() files, we allow each relocatable include_dependency() file to resolve + # to a separate depot, #52161 for inc in includes_depfiles depot = resolve_depot(inc.filename) if depot === :no_depot_found - @debug("Unable to resolve @depot tag for include_dependency() file $(inc.filename) from cache file $cachefile", srcfiles) + @debug("Unable to resolve @depot tag for include_dependency() file $(inc.filename) from cache file $cachefile", _group=:relocatable) elseif depot === :not_relocatable - @debug("include_dependency() file $(inc.filename) from $cachefile is not relocatable", srcfiles) + @debug("include_dependency() file $(inc.filename) from $cachefile is not relocatable", _group=:relocatable) else inc.filename = restore_depot_path(inc.filename, depot) end diff --git a/test/.gitignore b/test/.gitignore index fc55a0df3a173..20bf199b87c74 100644 --- a/test/.gitignore +++ b/test/.gitignore @@ -4,3 +4,4 @@ /libccalltest.* /relocatedepot /RelocationTestPkg2/src/foo.txt +/RelocationTestPkg*/Manifest.toml diff --git a/test/Makefile b/test/Makefile index 31f8f20615d7a..1b9cb377c943d 100644 --- a/test/Makefile +++ b/test/Makefile @@ -43,6 +43,7 @@ relocatedepot: @cp -R $(SRCDIR)/RelocationTestPkg1 $(SRCDIR)/relocatedepot @cp -R $(SRCDIR)/RelocationTestPkg2 $(SRCDIR)/relocatedepot @cp -R $(SRCDIR)/RelocationTestPkg3 $(SRCDIR)/relocatedepot + @cp -R $(SRCDIR)/RelocationTestPkg4 $(SRCDIR)/relocatedepot @cd $(SRCDIR) && \ $(call PRINT_JULIA, $(call spawn,RELOCATEDEPOT="" $(JULIA_EXECUTABLE)) --check-bounds=yes --startup-file=no --depwarn=error ./runtests.jl $@) @@ -55,6 +56,7 @@ revise-relocatedepot: revise-% : @cp -R $(SRCDIR)/RelocationTestPkg1 $(SRCDIR)/relocatedepot @cp -R $(SRCDIR)/RelocationTestPkg2 $(SRCDIR)/relocatedepot @cp -R $(SRCDIR)/RelocationTestPkg3 $(SRCDIR)/relocatedepot + @cp -R $(SRCDIR)/RelocationTestPkg4 $(SRCDIR)/relocatedepot @cd $(SRCDIR) && \ $(call PRINT_JULIA, $(call spawn,RELOCATEDEPOT="" $(JULIA_EXECUTABLE)) --check-bounds=yes --startup-file=no --depwarn=error ./runtests.jl --revise $*) diff --git a/test/RelocationTestPkg4/Project.toml b/test/RelocationTestPkg4/Project.toml new file mode 100644 index 0000000000000..8334a684f064e --- /dev/null +++ b/test/RelocationTestPkg4/Project.toml @@ -0,0 +1,6 @@ +name = "RelocationTestPkg4" +uuid = "d423d817-d7e9-49ac-b245-9d9d6db0b429" +version = "0.1.0" + +[deps] +RelocationTestPkg1 = "854e1adb-5a97-46bf-a391-1cfe05ac726d" diff --git a/test/RelocationTestPkg4/src/RelocationTestPkg4.jl b/test/RelocationTestPkg4/src/RelocationTestPkg4.jl new file mode 100644 index 0000000000000..d24a51d19a918 --- /dev/null +++ b/test/RelocationTestPkg4/src/RelocationTestPkg4.jl @@ -0,0 +1,5 @@ +module RelocationTestPkg4 + +greet() = print("Hello World!") + +end # module RelocationTestPkg4 diff --git a/test/relocatedepot.jl b/test/relocatedepot.jl index c9c8bd38a245f..039d422c35e25 100644 --- a/test/relocatedepot.jl +++ b/test/relocatedepot.jl @@ -17,10 +17,12 @@ function test_harness(@nospecialize(fn); empty_load_path=true, empty_depot_path= end end -# We test relocation with three dummy pkgs: -# - RelocationTestPkg1 - no include_dependency -# - RelocationTestPkg2 - with include_dependency tracked by `mtime` -# - RelocationTestPkg3 - with include_dependency tracked by content +# We test relocation with these dummy pkgs: +# - RelocationTestPkg1 - pkg with no include_dependency +# - RelocationTestPkg2 - pkg with include_dependency tracked by `mtime` +# - RelocationTestPkg3 - pkg with include_dependency tracked by content +# - RelocationTestPkg4 - pkg with no dependencies; will be compiled such that the pkgimage is +# not relocatable, but no repeated recompilation happens upon loading if !test_relocated_depot @@ -123,6 +125,27 @@ if !test_relocated_depot end end + @testset "precompile RelocationTestPkg4" begin + # test for #52346 and https://github.com/JuliaLang/julia/issues/53859#issuecomment-2027352004 + # If a pkgimage is not relocatable, no repeated precompilation should occur. + pkgname = "RelocationTestPkg4" + test_harness(empty_depot_path=false) do + push!(LOAD_PATH, @__DIR__) + # skip this dir to make the pkgimage not relocatable + filter!(DEPOT_PATH) do depot + !startswith(@__DIR__, depot) + end + pkg = Base.identify_package(pkgname) + cachefiles = Base.find_all_in_cache_path(pkg) + rm.(cachefiles, force=true) + @test Base.isprecompiled(pkg) == false + @test Base.isrelocatable(pkg) == false # because not precompiled + Base.require(pkg) + @test Base.isprecompiled(pkg, ignore_loaded=true) == true + @test Base.isrelocatable(pkg) == false + end + end + @testset "#52161" begin # Take the src files from two pkgs Example1 and Example2, # which are each located in depot1 and depot2, respectively, and @@ -246,7 +269,7 @@ else pkgname = "RelocationTestPkg3" test_harness() do push!(LOAD_PATH, joinpath(@__DIR__, "relocatedepot")) - push!(DEPOT_PATH, joinpath(@__DIR__, "relocatedepot")) + push!(DEPOT_PATH, joinpath(@__DIR__, "relocatedepot")) # required to find src files push!(DEPOT_PATH, joinpath(@__DIR__, "relocatedepot", "julia")) # contains cache file pkg = Base.identify_package(pkgname) @test Base.isprecompiled(pkg) == true @@ -257,4 +280,17 @@ else end end + @testset "load RelocationTestPkg4 from test/relocatedepot" begin + pkgname = "RelocationTestPkg4" + test_harness() do + push!(LOAD_PATH, @__DIR__, "relocatedepot") + push!(DEPOT_PATH, joinpath(@__DIR__, "relocatedepot")) # required to find src files + push!(DEPOT_PATH, joinpath(@__DIR__, "relocatedepot", "julia")) # contains cache file + pkg = Base.identify_package(pkgname) + # precompiled but not relocatable + @test Base.isprecompiled(pkg) == true + @test Base.isrelocatable(pkg) == false + end + end + end