Skip to content

Commit 5eaad53

Browse files
authored
Remove ArgumentError() in parse_cache_header() when @depot cannot be resolved (#51989)
In the original implementation of relocatable package cache files, `parse_cache_header()` was made responsible for resolving `@depot/`-relative paths, and it threw an `ArgumentError()` if it could not find one. However, this could happen for a few reasons: 1. The `JULIA_DEPOT_PATH` could be set incorrectly, so the appropriate source text files could not be found in an appropriate `packages/` directory. 2. The package could have been upgraded, leaving a cache file for an older version on-disk, but no matching source files. For a concrete example of (2), see the following `bash` script: #!/bin/bash set -euo pipefail TEMP_PATH=$(mktemp -d) JULIA=${JULIA:-julia} echo JULIA: ${JULIA} export JULIA_DEPOT_PATH="${TEMP_PATH}" trap 'rm -rf ${TEMP_PATH}' EXIT # Create a `Foo.jl` that has an `internal.jl` within it FOO_DIR=${JULIA_DEPOT_PATH}/dev/Foo mkdir -p ${FOO_DIR}/src cat >${FOO_DIR}/Project.toml <<EOF name = "Foo" uuid = "00000000-0000-0000-0000-000000000001" version = "1.0.0" EOF cat >${FOO_DIR}/src/Foo.jl <<EOF module Foo include("internal.jl") end EOF cat >${FOO_DIR}/src/internal.jl <<EOF # Nothing to see here, folks EOF ${JULIA} -e "import Pkg; Pkg.develop(\"Foo\"); Pkg.precompile()" # Change `Foo` to no longer depend on `internal.jl`; this should invalidate its cache files cat >${FOO_DIR}/src/Foo.jl <<EOF module Foo end EOF rm -f ${FOO_DIR}/src/internal.jl # This should print `SUCCESS!`, not `FAILURE!` ${JULIA} -e 'using Foo; println("SUCCESS!")' || echo "FAILURE!" This pull request removes the `ArgumentError()`, as it seems reasonable to require `parse_cache_header()` to not throw errors in cases like these. We instead rely upon a higher level validation to reject a cachefile whose depot cannot be found, which should happen when `stale_cachefile()` later checks to ensure that all includes are found on-disk, (which will be false, as these paths start with `@depot/`, an unlikely path prefix to exist).
1 parent 2a84214 commit 5eaad53

File tree

2 files changed

+63
-4
lines changed

2 files changed

+63
-4
lines changed

base/loading.jl

+6-4
Original file line numberDiff line numberDiff line change
@@ -2722,11 +2722,9 @@ function parse_cache_header(f::IO, cachefile::AbstractString)
27222722
chi.filename srcfiles && push!(keepidx, i)
27232723
end
27242724
if depot === :no_depot_found
2725-
throw(ArgumentError("""
2726-
Failed to determine depot from srctext files in cache file $cachefile.
2727-
- Make sure you have adjusted DEPOT_PATH in case you relocated depots."""))
2725+
@debug("Unable to resolve @depot tag in cache file $cachefile", srcfiles)
27282726
elseif depot === :missing_depot_tag
2729-
@debug "Missing @depot tag for include dependencies in cache file $cachefile."
2727+
@debug("Missing @depot tag for include dependencies in cache file $cachefile.", srcfiles)
27302728
else
27312729
for inc in includes
27322730
inc.filename = restore_depot_path(inc.filename, depot)
@@ -3279,6 +3277,10 @@ end
32793277
end
32803278
for chi in includes
32813279
f, fsize_req, hash_req, ftime_req = chi.filename, chi.fsize, chi.hash, chi.mtime
3280+
if startswith(f, "@depot/")
3281+
@debug("Rejecting stale cache file $cachefile because its depot could not be resolved")
3282+
return true
3283+
end
32823284
if !ispath(f)
32833285
_f = fixup_stdlib_path(f)
32843286
if isfile(_f) && startswith(_f, Sys.STDLIB)

test/loading.jl

+57
Original file line numberDiff line numberDiff line change
@@ -1274,3 +1274,60 @@ end
12741274
@test_throws ErrorException Base.check_src_module_wrap(p, fpath)
12751275
end
12761276
end
1277+
1278+
@testset "relocatable upgrades #51989" begin
1279+
mktempdir() do depot
1280+
# Create fake `Foo.jl` package with two files:
1281+
foo_path = joinpath(depot, "dev", "Foo")
1282+
mkpath(joinpath(foo_path, "src"))
1283+
open(joinpath(foo_path, "src", "Foo.jl"); write=true) do io
1284+
println(io, """
1285+
module Foo
1286+
include("internal.jl")
1287+
end
1288+
""")
1289+
end
1290+
open(joinpath(foo_path, "src", "internal.jl"); write=true) do io
1291+
println(io, "const a = \"asd\"")
1292+
end
1293+
open(joinpath(foo_path, "Project.toml"); write=true) do io
1294+
println(io, """
1295+
name = "Foo"
1296+
uuid = "00000000-0000-0000-0000-000000000001"
1297+
version = "1.0.0"
1298+
""")
1299+
end
1300+
1301+
# In our depot, `dev` and then `precompile` this `Foo` package.
1302+
@test success(addenv(
1303+
`$(Base.julia_cmd()) --startup-file=no -e 'import Pkg; Pkg.develop("Foo"); Pkg.precompile(); exit(0)'`,
1304+
"JULIA_DEPOT_PATH" => depot,
1305+
))
1306+
1307+
# Get the size of the generated `.ji` file so that we can ensure that it gets altered
1308+
foo_compiled_path = joinpath(depot, "compiled", "v$(VERSION.major).$(VERSION.minor)", "Foo")
1309+
cache_path = joinpath(foo_compiled_path, only(filter(endswith(".ji"), readdir(foo_compiled_path))))
1310+
cache_size = filesize(cache_path)
1311+
1312+
# Next, remove the dependence on `internal.jl` and delete it:
1313+
rm(joinpath(foo_path, "src", "internal.jl"))
1314+
open(joinpath(foo_path, "src", "Foo.jl"); write=true) do io
1315+
truncate(io, 0)
1316+
println(io, """
1317+
module Foo
1318+
end
1319+
""")
1320+
end
1321+
1322+
# Try to load `Foo`; this should trigger recompilation, not an error!
1323+
@test success(addenv(
1324+
`$(Base.julia_cmd()) --startup-file=no -e 'using Foo; exit(0)'`,
1325+
"JULIA_DEPOT_PATH" => depot,
1326+
))
1327+
1328+
# Ensure that there is still only one `.ji` file (it got replaced
1329+
# and the file size changed).
1330+
@test length(filter(endswith(".ji"), readdir(foo_compiled_path))) == 1
1331+
@test filesize(cache_path) != cache_size
1332+
end
1333+
end

0 commit comments

Comments
 (0)