From 01d11e980573f23afed71aa231589d725030f07f Mon Sep 17 00:00:00 2001 From: Florian Atteneder Date: Fri, 2 Aug 2024 23:35:44 +0200 Subject: [PATCH 1/7] account for path separator when inserting @depot tag --- base/loading.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/loading.jl b/base/loading.jl index 8d180845f942f..b7096a3fe06ec 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -3185,7 +3185,7 @@ function replace_depot_path(path::AbstractString) depot = dirname(depot) end - if startswith(path, depot) + if startswith(path, string(depot, Filesystem.pathsep())) || path == depot path = replace(path, depot => "@depot"; count=1) break end From cba4a7e6fb368bc4d5a0f916e59933f75484916f Mon Sep 17 00:00:00 2001 From: Florian Atteneder Date: Fri, 2 Aug 2024 23:36:32 +0200 Subject: [PATCH 2/7] unit test for issue 55340 --- base/loading.jl | 7 +++++++ test/relocatedepot.jl | 41 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/base/loading.jl b/base/loading.jl index b7096a3fe06ec..be0e751f07abb 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -3177,6 +3177,9 @@ mutable struct CacheHeaderIncludes end function replace_depot_path(path::AbstractString) + @static if Sys.iswindows() + path = replace(path, Filesystem.path_separator_re=>Filesystem.pathsep()) + end for depot in DEPOT_PATH !isdir(depot) && continue @@ -3185,6 +3188,10 @@ function replace_depot_path(path::AbstractString) depot = dirname(depot) end + @static if Sys.iswindows() + depot = replace(depot, Filesystem.path_separator_re=>Filesystem.pathsep()) + end + if startswith(path, string(depot, Filesystem.pathsep())) || path == depot path = replace(path, depot => "@depot"; count=1) break diff --git a/test/relocatedepot.jl b/test/relocatedepot.jl index 039d422c35e25..8fd3bbb3ce2c3 100644 --- a/test/relocatedepot.jl +++ b/test/relocatedepot.jl @@ -32,7 +32,12 @@ if !test_relocated_depot mktempdir() do dir pushfirst!(DEPOT_PATH, dir) path = dir*dir - @test Base.replace_depot_path(path) == "@depot"*dir + if Sys.iswindows() + # dirs start with a drive letter instead of a path separator + @test Base.replace_depot_path(path) == path + else + @test Base.replace_depot_path(path) == "@depot"*dir + end end end @@ -43,15 +48,45 @@ if !test_relocated_depot if isdirpath(DEPOT_PATH[1]) DEPOT_PATH[1] = dirname(DEPOT_PATH[1]) # strip trailing pathsep end - tag = joinpath("@depot", "") # append a pathsep + tag = string("@depot", Base.Filesystem.pathsep()) @test startswith(Base.replace_depot_path(path), tag) - DEPOT_PATH[1] = joinpath(DEPOT_PATH[1], "") # append a pathsep + DEPOT_PATH[1] = string(DEPOT_PATH[1], Base.Filesystem.pathsep()) @test startswith(Base.replace_depot_path(path), tag) popfirst!(DEPOT_PATH) @test !startswith(Base.replace_depot_path(path), tag) end end + # 55340 + test_harness(empty_depot_path=true) do + mktempdir() do dir + jlrc = joinpath(dir, "julia-rc2") + jl = joinpath(dir, "julia") + mkdir(jl) + push!(DEPOT_PATH, jl) + @test Base.replace_depot_path(jl) == "@depot" + @test Base.replace_depot_path(string(jl,Base.Filesystem.pathsep())) == + string("@depot",Base.Filesystem.pathsep()) + @test Base.replace_depot_path(jlrc) != "@depot-rc2" + @test Base.replace_depot_path(jlrc) == jlrc + end + if Sys.iswindows() + # windows accepts '\\' and '/' as path separators + mktempdir() do dir + jlrc = string(dir, "/", "julia-rc2") + jl = string(dir, "/", "julia") + mkdir(jl) + push!(DEPOT_PATH, jl) + @test Base.replace_depot_path(jl) == "@depot" + @test Base.replace_depot_path(string(jl,"/")) == string("@depot","\\") + @test Base.replace_depot_path(string(jl,"\\")) == string("@depot","\\") + @test Base.replace_depot_path(jlrc) != "@depot-rc2" + @test Base.replace_depot_path(jlrc) == + replace(jlrc, Base.Filesystem.path_separator_re=>"\\") + end + end + end + end @testset "restore path from @depot tag" begin From 8dcf3d6ba86c30ccf9f20d9d13169b7bb44d7b69 Mon Sep 17 00:00:00 2001 From: Florian Atteneder Date: Fri, 9 Aug 2024 22:35:41 +0200 Subject: [PATCH 3/7] document assumptions on replace_depot_path --- base/loading.jl | 9 ++------- test/relocatedepot.jl | 15 --------------- 2 files changed, 2 insertions(+), 22 deletions(-) diff --git a/base/loading.jl b/base/loading.jl index be0e751f07abb..7eea6db2cff3a 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -3176,10 +3176,9 @@ mutable struct CacheHeaderIncludes const modpath::Vector{String} # seemingly not needed in Base, but used by Revise end +# This method is only called from src/precompile.c and from within a precompilation process. +# Upon call it is safe to assume that `path` and elements of `DEPOT_PATH` are absolute and normalized. function replace_depot_path(path::AbstractString) - @static if Sys.iswindows() - path = replace(path, Filesystem.path_separator_re=>Filesystem.pathsep()) - end for depot in DEPOT_PATH !isdir(depot) && continue @@ -3188,10 +3187,6 @@ function replace_depot_path(path::AbstractString) depot = dirname(depot) end - @static if Sys.iswindows() - depot = replace(depot, Filesystem.path_separator_re=>Filesystem.pathsep()) - end - if startswith(path, string(depot, Filesystem.pathsep())) || path == depot path = replace(path, depot => "@depot"; count=1) break diff --git a/test/relocatedepot.jl b/test/relocatedepot.jl index 8fd3bbb3ce2c3..cb6468e7bd147 100644 --- a/test/relocatedepot.jl +++ b/test/relocatedepot.jl @@ -70,21 +70,6 @@ if !test_relocated_depot @test Base.replace_depot_path(jlrc) != "@depot-rc2" @test Base.replace_depot_path(jlrc) == jlrc end - if Sys.iswindows() - # windows accepts '\\' and '/' as path separators - mktempdir() do dir - jlrc = string(dir, "/", "julia-rc2") - jl = string(dir, "/", "julia") - mkdir(jl) - push!(DEPOT_PATH, jl) - @test Base.replace_depot_path(jl) == "@depot" - @test Base.replace_depot_path(string(jl,"/")) == string("@depot","\\") - @test Base.replace_depot_path(string(jl,"\\")) == string("@depot","\\") - @test Base.replace_depot_path(jlrc) != "@depot-rc2" - @test Base.replace_depot_path(jlrc) == - replace(jlrc, Base.Filesystem.path_separator_re=>"\\") - end - end end end From d03ff12ba56bef6c3f9dfa4d6fdfc21a1524095e Mon Sep 17 00:00:00 2001 From: Florian Atteneder Date: Sun, 11 Aug 2024 12:58:33 +0200 Subject: [PATCH 4/7] refactor some tests --- test/relocatedepot.jl | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/test/relocatedepot.jl b/test/relocatedepot.jl index cb6468e7bd147..653128e70e44f 100644 --- a/test/relocatedepot.jl +++ b/test/relocatedepot.jl @@ -26,21 +26,38 @@ end if !test_relocated_depot - @testset "insert @depot tag in path" begin + @testset "edge cases when inserting @depot tag in path" begin + # insert @depot only once for first match test_harness() do mktempdir() do dir pushfirst!(DEPOT_PATH, dir) - path = dir*dir if Sys.iswindows() # dirs start with a drive letter instead of a path separator - @test Base.replace_depot_path(path) == path + path = dir*Base.Filesystem.pathsep()*dir + @test Base.replace_depot_path(path) == "@depot"*Base.Filesystem.pathsep()*dir else + path = dir*dir @test Base.replace_depot_path(path) == "@depot"*dir end end + + # 55340 + empty!(DEPOT_PATH) + mktempdir() do dir + jlrc = joinpath(dir, "julia-rc2") + jl = joinpath(dir, "julia") + mkdir(jl) + push!(DEPOT_PATH, jl) + @test Base.replace_depot_path(jl) == "@depot" + @test Base.replace_depot_path(string(jl,Base.Filesystem.pathsep())) == + string("@depot",Base.Filesystem.pathsep()) + @test Base.replace_depot_path(jlrc) != "@depot-rc2" + @test Base.replace_depot_path(jlrc) == jlrc + end end + # deal with and without trailing path separators test_harness() do mktempdir() do dir pushfirst!(DEPOT_PATH, dir) @@ -57,21 +74,6 @@ if !test_relocated_depot end end - # 55340 - test_harness(empty_depot_path=true) do - mktempdir() do dir - jlrc = joinpath(dir, "julia-rc2") - jl = joinpath(dir, "julia") - mkdir(jl) - push!(DEPOT_PATH, jl) - @test Base.replace_depot_path(jl) == "@depot" - @test Base.replace_depot_path(string(jl,Base.Filesystem.pathsep())) == - string("@depot",Base.Filesystem.pathsep()) - @test Base.replace_depot_path(jlrc) != "@depot-rc2" - @test Base.replace_depot_path(jlrc) == jlrc - end - end - end @testset "restore path from @depot tag" begin From 8c6d7dbc9af67e647ba69ea7c379b5fd11a725ea Mon Sep 17 00:00:00 2001 From: Florian Atteneder Date: Wed, 18 Sep 2024 23:55:35 +0200 Subject: [PATCH 5/7] add license header --- test/relocatedepot.jl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/relocatedepot.jl b/test/relocatedepot.jl index 653128e70e44f..2ef6dec90dbc1 100644 --- a/test/relocatedepot.jl +++ b/test/relocatedepot.jl @@ -1,3 +1,5 @@ +# This file is a part of Julia. License is MIT: https://julialang.org/license + using Test From 0174e4397c3ad65bd346bdaee47e8ff37afd51e6 Mon Sep 17 00:00:00 2001 From: Florian Atteneder Date: Wed, 18 Sep 2024 23:57:20 +0200 Subject: [PATCH 6/7] normalize depot paths just before inserting @depot tag --- base/loading.jl | 26 +++++++++++++++----------- src/precompile.c | 17 +++++++++++++---- src/staticdata_utils.c | 20 ++++++++++++++------ 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/base/loading.jl b/base/loading.jl index 7eea6db2cff3a..1f9ffa7bc41ff 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -3176,17 +3176,8 @@ mutable struct CacheHeaderIncludes const modpath::Vector{String} # seemingly not needed in Base, but used by Revise end -# This method is only called from src/precompile.c and from within a precompilation process. -# Upon call it is safe to assume that `path` and elements of `DEPOT_PATH` are absolute and normalized. -function replace_depot_path(path::AbstractString) - for depot in DEPOT_PATH - !isdir(depot) && continue - - # Strip extraneous pathseps through normalization. - if isdirpath(depot) - depot = dirname(depot) - end - +function replace_depot_path(path::AbstractString, depots::Vector{String}=normalize_depots_for_relocation()) + for depot in depots if startswith(path, string(depot, Filesystem.pathsep())) || path == depot path = replace(path, depot => "@depot"; count=1) break @@ -3195,6 +3186,19 @@ function replace_depot_path(path::AbstractString) return path end +function normalize_depots_for_relocation() + depots = String[] + sizehint!(depots, length(DEPOT_PATH)) + for d in DEPOT_PATH + isdir(d) || continue + if isdirpath(d) + d = dirname(d) + end + push!(depots, d) + end + return depots +end + function restore_depot_path(path::AbstractString, depot::AbstractString) replace(path, r"^@depot" => depot; count=1) end diff --git a/src/precompile.c b/src/precompile.c index c40e867ea699e..ec4f7a69002c1 100644 --- a/src/precompile.c +++ b/src/precompile.c @@ -39,9 +39,17 @@ void write_srctext(ios_t *f, jl_array_t *udeps, int64_t srctextpos) { static jl_value_t *replace_depot_func = NULL; if (!replace_depot_func) replace_depot_func = jl_get_global(jl_base_module, jl_symbol("replace_depot_path")); + static jl_value_t *normalize_depots_func = NULL; + if (!normalize_depots_func) + normalize_depots_func = jl_get_global(jl_base_module, jl_symbol("normalize_depots_for_relocation")); ios_t srctext; - jl_value_t *deptuple = NULL; - JL_GC_PUSH2(&deptuple, &udeps); + jl_value_t *deptuple = NULL, *depots = NULL; + JL_GC_PUSH3(&deptuple, &udeps, &depots); + jl_task_t *ct = jl_current_task; + size_t last_age = ct->world_age; + ct->world_age = jl_atomic_load_acquire(&jl_world_counter); + depots = jl_apply(&normalize_depots_func, 1); + ct->world_age = last_age; for (size_t i = 0; i < len; i++) { deptuple = jl_array_ptr_ref(udeps, i); jl_value_t *depmod = jl_fieldref(deptuple, 0); // module @@ -60,13 +68,14 @@ void write_srctext(ios_t *f, jl_array_t *udeps, int64_t srctextpos) { } jl_value_t **replace_depot_args; - JL_GC_PUSHARGS(replace_depot_args, 2); + JL_GC_PUSHARGS(replace_depot_args, 3); replace_depot_args[0] = replace_depot_func; replace_depot_args[1] = abspath; + replace_depot_args[2] = depots; jl_task_t *ct = jl_current_task; size_t last_age = ct->world_age; ct->world_age = jl_atomic_load_acquire(&jl_world_counter); - jl_value_t *depalias = (jl_value_t*)jl_apply(replace_depot_args, 2); + jl_value_t *depalias = (jl_value_t*)jl_apply(replace_depot_args, 3); ct->world_age = last_age; JL_GC_POP(); diff --git a/src/staticdata_utils.c b/src/staticdata_utils.c index f39e5357c6782..c6bfcae1dca8d 100644 --- a/src/staticdata_utils.c +++ b/src/staticdata_utils.c @@ -752,6 +752,16 @@ static int64_t write_dependency_list(ios_t *s, jl_array_t* worklist, jl_array_t static jl_value_t *replace_depot_func = NULL; if (!replace_depot_func) replace_depot_func = jl_get_global(jl_base_module, jl_symbol("replace_depot_path")); + static jl_value_t *normalize_depots_func = NULL; + if (!normalize_depots_func) + normalize_depots_func = jl_get_global(jl_base_module, jl_symbol("normalize_depots_for_relocation")); + + jl_value_t *depots = NULL, *prefs_hash = NULL, *prefs_list = NULL; + JL_GC_PUSH2(&depots, &prefs_list); + last_age = ct->world_age; + ct->world_age = jl_atomic_load_acquire(&jl_world_counter); + depots = jl_apply(&normalize_depots_func, 1); + ct->world_age = last_age; // write a placeholder for total size so that we can quickly seek past all of the // dependencies if we don't need them @@ -764,13 +774,14 @@ static int64_t write_dependency_list(ios_t *s, jl_array_t* worklist, jl_array_t if (replace_depot_func) { jl_value_t **replace_depot_args; - JL_GC_PUSHARGS(replace_depot_args, 2); + JL_GC_PUSHARGS(replace_depot_args, 3); replace_depot_args[0] = replace_depot_func; replace_depot_args[1] = deppath; + replace_depot_args[2] = depots; ct = jl_current_task; size_t last_age = ct->world_age; ct->world_age = jl_atomic_load_acquire(&jl_world_counter); - deppath = (jl_value_t*)jl_apply(replace_depot_args, 2); + deppath = (jl_value_t*)jl_apply(replace_depot_args, 3); ct->world_age = last_age; JL_GC_POP(); } @@ -803,9 +814,6 @@ static int64_t write_dependency_list(ios_t *s, jl_array_t* worklist, jl_array_t write_int32(s, 0); // terminator, for ease of reading // Calculate Preferences hash for current package. - jl_value_t *prefs_hash = NULL; - jl_value_t *prefs_list = NULL; - JL_GC_PUSH1(&prefs_list); if (jl_base_module) { // Toplevel module is the module we're currently compiling, use it to get our preferences hash jl_value_t * toplevel = (jl_value_t*)jl_get_global(jl_base_module, jl_symbol("__toplevel__")); @@ -852,7 +860,7 @@ static int64_t write_dependency_list(ios_t *s, jl_array_t* worklist, jl_array_t write_int32(s, 0); write_uint64(s, 0); } - JL_GC_POP(); // for prefs_list + JL_GC_POP(); // for depots, prefs_list // write a dummy file position to indicate the beginning of the source-text pos = ios_pos(s); From b600a62a5e14ba640e8bb3271959d026fa20272f Mon Sep 17 00:00:00 2001 From: Florian Date: Thu, 19 Sep 2024 17:45:58 +0200 Subject: [PATCH 7/7] use abspath in depot normalization --- base/loading.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/loading.jl b/base/loading.jl index 1f9ffa7bc41ff..b5932c01ae732 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -3194,7 +3194,7 @@ function normalize_depots_for_relocation() if isdirpath(d) d = dirname(d) end - push!(depots, d) + push!(depots, abspath(d)) end return depots end