Skip to content

Commit

Permalink
Allow include() and include_dependency() files to resolve to diff…
Browse files Browse the repository at this point in the history 
…erent depots (JuliaLang#52750)

Fixes JuliaLang#52161

JuliaLang#52161 is an awkward use of the relocation feature in the sense that it
attempts to load the `include()` and `include_dependency` files of a pkg
from two separate depots.
The problem there is that the value with which we replace the `@depot`
tag for both `include()` and `include_dependency()` files is determined
by trying to relocate only the `include()` files. We then end up not
finding the `include_dependency()` files.

Solution:
@staticfloat noted that the pkg slugs in depot paths like
`@depot/packages/Foo/1a2b3c/src/Foo.jl` are already enough to (weakly)
content-address a depot.
This means that we should be able to load any `include()` file of a pkg
from any `depot` that contains a precompile cache, provided the hashes
match.
The same logic can be extended to `include_dependency()` files, which
this PR does.

Note that we continue to use only one file from the `include()` files to
determine the depot which we use to relocate all `include()` files.
[this behavior is kept from master]
But for `include_dependency()` files we allow each file to resolve to a
different depot. This way the MWE given in JuliaLang#52161 should be extendable
to two README files being located in two different pkgs that lie in two
different depots.

---

Side note: JuliaLang#49866 started with explicitly verifying that all `include()`
files come from the same depot. In JuliaLang#52346 this was already relaxed to
pick the first depot for which any `include()` file can be resolved to.
This works, because if any other file might be missing from that depot
then this is caught in `stalecache()`.
  • Loading branch information
@fatteneder
fatteneder committed Jan 8, 2024
1 parent 8dc2c30 commit 55113b5
Show file tree
Hide file tree
Showing 4 changed files with 128 additions and 52 deletions.
60 changes: 32 additions & 28 deletions base/loading.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2734,22 +2734,10 @@ function restore_depot_path(path::AbstractString, depot::AbstractString)
replace(path, r"^@depot" => depot; count=1)
end

# Find depot in DEPOT_PATH for which all @depot tags from the `includes`
# can be replaced so that they point to a file on disk each.
function resolve_depot(includes::Union{AbstractVector,AbstractSet})
# `all` because it's possible to have a mixture of includes inside and outside of the depot
if all(includes) do inc
!startswith(inc, "@depot")
end
return :fully_outside_depot
end
function resolve_depot(inc::AbstractString)
startswith(inc, "@depot") || return :not_relocatable
for depot in DEPOT_PATH
# `any` because it's possible to have a mixture of includes inside and outside of the depot
if any(includes) do inc
isfile(restore_depot_path(inc, depot))
end
return depot
end
isfile(restore_depot_path(inc, depot)) && return depot
end
return :no_depot_found
end
Expand Down Expand Up @@ -2837,25 +2825,41 @@ function parse_cache_header(f::IO, cachefile::AbstractString)
l = read(f, Int32)
clone_targets = read(f, l)

# determine path for @depot replacement from srctext files only, e.g. ignore any include_dependency files
srcfiles = srctext_files(f, srctextpos, includes)
depot = resolve_depot(srcfiles)
keepidx = Int[]
for (i, chi) in enumerate(includes)
chi.filename ∈ srcfiles && push!(keepidx, i)
end
if depot === :no_depot_found
@debug("Unable to resolve @depot tag in cache file $cachefile", srcfiles)
elseif depot === :fully_outside_depot
@debug("All include dependencies in cache file $cachefile are outside of a depot.", srcfiles)

includes_srcfiles = CacheHeaderIncludes[]
includes_depfiles = CacheHeaderIncludes[]
for (i, inc) in enumerate(includes)
if inc.filename ∈ srcfiles
push!(includes_srcfiles, inc)
else
push!(includes_depfiles, inc)
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
for inc in includes
for inc in includes_srcfiles
inc.filename = restore_depot_path(inc.filename, srcfiles_depot)
end
end
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)
elseif depot === :not_relocatable
@debug("include_dependency() file $(inc.filename) from $cachefile is not relocatable", srcfiles)
else
inc.filename = restore_depot_path(inc.filename, depot)
end
end
includes_srcfiles_only = includes[keepidx]

return modules, (includes, includes_srcfiles_only, requires), required_modules, srctextpos, prefs, prefs_hash, clone_targets, flags
return modules, (includes, includes_srcfiles, requires), required_modules, srctextpos, prefs, prefs_hash, clone_targets, flags
end

function parse_cache_header(cachefile::String)
Expand Down
4 changes: 2 additions & 2 deletions test/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ relocatedepot:
@cp -R $(SRCDIR)/RelocationTestPkg1 $(SRCDIR)/relocatedepot
@cp -R $(SRCDIR)/RelocationTestPkg2 $(SRCDIR)/relocatedepot
@cd $(SRCDIR) && \
$(call PRINT_JULIA, $(call spawn,JULIA_DEBUG=loading RELOCATEDEPOT="" JULIA_DEPOT_PATH=$(SRCDIR)/relocatedepot/julia $(JULIA_EXECUTABLE)) --check-bounds=yes --startup-file=no --depwarn=error ./runtests.jl $@)
$(call PRINT_JULIA, $(call spawn,JULIA_DEBUG=loading RELOCATEDEPOT="" $(JULIA_EXECUTABLE)) --check-bounds=yes --startup-file=no --depwarn=error ./runtests.jl $@)

revise-relocatedepot: revise-% :
@rm -rf $(SRCDIR)/relocatedepot
Expand All @@ -54,7 +54,7 @@ revise-relocatedepot: revise-% :
@cp -R $(SRCDIR)/RelocationTestPkg1 $(SRCDIR)/relocatedepot
@cp -R $(SRCDIR)/RelocationTestPkg2 $(SRCDIR)/relocatedepot
@cd $(SRCDIR) && \
$(call PRINT_JULIA, $(call spawn,JULIA_DEBUG=loading RELOCATEDEPOT="" JULIA_DEPOT_PATH=$(SRCDIR)/relocatedepot/julia $(JULIA_EXECUTABLE)) --check-bounds=yes --startup-file=no --depwarn=error ./runtests.jl --revise $*)
$(call PRINT_JULIA, $(call spawn,JULIA_DEBUG=loading RELOCATEDEPOT="" $(JULIA_EXECUTABLE)) --check-bounds=yes --startup-file=no --depwarn=error ./runtests.jl --revise $*)

embedding:
@$(MAKE) -C $(SRCDIR)/$@ check $(EMBEDDING_ARGS)
Expand Down
3 changes: 2 additions & 1 deletion test/precompile.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,8 @@ precompile_test_harness(false) do dir
modules, (deps, _, requires), required_modules, _... = Base.parse_cache_header(cachefile)
discard_module = mod_fl_mt -> mod_fl_mt.filename
@test modules == [ Base.PkgId(Foo) => Base.module_build_id(Foo) % UInt64 ]
@test map(x -> x.filename, deps) == [ Foo_file, joinpath(dir, "foo.jl"), joinpath(dir, "bar.jl") ]
# foo.jl and bar.jl are never written to disk, so they are not relocatable
@test map(x -> x.filename, deps) == [ Foo_file, joinpath("@depot", "foo.jl"), joinpath("@depot", "bar.jl") ]
@test requires == [ Base.PkgId(Foo) => Base.PkgId(string(FooBase_module)),
Base.PkgId(Foo) => Base.PkgId(Foo2),
Base.PkgId(Foo) => Base.PkgId(Test),
Expand Down
113 changes: 92 additions & 21 deletions test/relocatedepot.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ using Logging
include("testenv.jl")


function test_harness(@nospecialize(fn))
function test_harness(@nospecialize(fn); empty_load_path=true, empty_depot_path=true)
load_path = copy(LOAD_PATH)
depot_path = copy(DEPOT_PATH)
try
empty_load_path && empty!(LOAD_PATH)
empty_depot_path && empty!(DEPOT_PATH)
fn()
finally
copy!(LOAD_PATH, load_path)
Expand Down Expand Up @@ -66,9 +68,9 @@ if !test_relocated_depot

@testset "precompile RelocationTestPkg1" begin
pkgname = "RelocationTestPkg1"
test_harness() do
test_harness(empty_depot_path=false) do
push!(LOAD_PATH, @__DIR__)
push!(DEPOT_PATH, @__DIR__)
push!(DEPOT_PATH, @__DIR__) # required to make relocatable, but cache is written to DEPOT_PATH[1]
pkg = Base.identify_package(pkgname)
cachefiles = Base.find_all_in_cache_path(pkg)
rm.(cachefiles, force=true)
Expand All @@ -80,9 +82,9 @@ if !test_relocated_depot

@testset "precompile RelocationTestPkg2 (contains include_dependency)" begin
pkgname = "RelocationTestPkg2"
test_harness() do
test_harness(empty_depot_path=false) do
push!(LOAD_PATH, @__DIR__)
push!(DEPOT_PATH, string(@__DIR__, "/"))
push!(DEPOT_PATH, @__DIR__) # required to make relocatable, but cache is written to DEPOT_PATH[1]
pkg = Base.identify_package(pkgname)
cachefiles = Base.find_all_in_cache_path(pkg)
rm.(cachefiles, force=true)
Expand All @@ -93,27 +95,94 @@ if !test_relocated_depot
end
end

else

# must come before any of the load tests, because the will recompile and generate new cache files
@testset "attempt loading precompiled pkgs when depot is missing" begin
@testset "#52161" begin
# Take the src files from two pkgs Example1 and Example2,
# which are each located in depot1 and depot2, respectively, and
# add them as include_dependency()s to a new pkg Foo, which will be precompiled into depot3.
# After loading the include_dependency()s of Foo should refer to depot1 depot2 each.
test_harness() do
empty!(LOAD_PATH)
push!(LOAD_PATH, joinpath(@__DIR__, "relocatedepot"))
for pkgname in ("RelocationTestPkg1", "RelocationTestPkg2")
pkg = Base.identify_package(pkgname)
cachefile = only(Base.find_all_in_cache_path(pkg))
@test_throws ArgumentError("""
Failed to determine depot from srctext files in cache file $cachefile.
- Make sure you have adjusted DEPOT_PATH in case you relocated depots.""") Base.isprecompiled(pkg)
mktempdir() do depot1
# precompile Example in depot1
example1_root = joinpath(depot1, "Example1")
mkpath(joinpath(example1_root, "src"))
open(joinpath(example1_root, "src", "Example1.jl"); write=true) do io
println(io, """
module Example1
greet() = println("Hello from Example1!")
end
""")
end
open(joinpath(example1_root, "Project.toml"); write=true) do io
println(io, """
name = "Example1"
uuid = "00000000-0000-0000-0000-000000000001"
version = "1.0.0"
""")
end
pushfirst!(LOAD_PATH, depot1); pushfirst!(DEPOT_PATH, depot1)
pkg = Base.identify_package("Example1"); Base.require(pkg)
mktempdir() do depot2
# precompile Example in depot2
example2_root = joinpath(depot2, "Example2")
mkpath(joinpath(example2_root, "src"))
open(joinpath(example2_root, "src", "Example2.jl"); write=true) do io
println(io, """
module Example2
greet() = println("Hello from Example2!")
end
""")
end
open(joinpath(example2_root, "Project.toml"); write=true) do io
println(io, """
name = "Example2"
uuid = "00000000-0000-0000-0000-000000000002"
version = "1.0.0"
""")
end
pushfirst!(LOAD_PATH, depot2)
pushfirst!(DEPOT_PATH, depot2)
pkg = Base.identify_package("Example2")
Base.require(pkg)
mktempdir() do depot3
# precompile Foo in depot3
open(joinpath(depot3, "Foo.jl"), write=true) do io
println(io, """
module Foo
using Example1
using Example2
srcfile1 = joinpath(pkgdir(Example1), "src", "Example1.jl")
srcfile2 = joinpath(pkgdir(Example2), "src", "Example2.jl")
@show srcfile1
@show srcfile2
include_dependency(srcfile1)
include_dependency(srcfile2)
end
""")
end
pushfirst!(LOAD_PATH, depot3)
pushfirst!(DEPOT_PATH, depot3)
pkg = Base.identify_package("Foo")
Base.require(pkg)
cachefile = joinpath(depot3, "compiled",
"v$(VERSION.major).$(VERSION.minor)", "Foo.ji")
_, (deps, _, _), _... = Base.parse_cache_header(cachefile)
@test map(x -> x.filename, deps) ==
[ joinpath(depot3, "Foo.jl"),
joinpath(depot1, "Example1", "src", "Example1.jl"),
joinpath(depot2, "Example2", "src", "Example2.jl") ]
end
end
end
end
end


else

@testset "load stdlib from test/relocatedepot" begin
test_harness() do
push!(LOAD_PATH, joinpath(@__DIR__, "relocatedepot"))
push!(DEPOT_PATH, joinpath(@__DIR__, "relocatedepot"))
push!(LOAD_PATH, "@stdlib")
push!(DEPOT_PATH, joinpath(@__DIR__, "relocatedepot", "julia"))
# stdlib should be already precompiled
pkg = Base.identify_package("DelimitedFiles")
@test Base.isprecompiled(pkg) == true
Expand All @@ -124,7 +193,8 @@ else
pkgname = "RelocationTestPkg1"
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
Base.require(pkg) # re-precompile
Expand All @@ -136,7 +206,8 @@ else
pkgname = "RelocationTestPkg2"
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) == false # moving depot changes mtime of include_dependency
Base.require(pkg) # re-precompile
Expand Down

0 comments on commit 55113b5

Please sign in to comment.