Skip to content

Commit d6d6f2d

Browse files
committed
loading: Delete normpath where possible
Unix path semantics have a design flaw, where if `a` is a symbolic directory link, `x/a/..` refer's to the link target's parent directory rather than `x/`. As a result, `normpath` is not in general safe to run. We were doing this all over the place (either explicitly, or implicitly in `abspath`) causing us to fail to load files if their paths contained this pattern. Fix this by rm'ing normpath in most places in loading and adding an `abspath` kwarg that skips normalization. Where paths show up in printing, we can possibly put some back after verifying that the path resolution result does not change using `samefile`, but let's do that on a case-by-case basis.
1 parent a63cca8 commit d6d6f2d

File tree

5 files changed

+56
-45
lines changed

5 files changed

+56
-45
lines changed

base/initdefs.jl

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -302,15 +302,15 @@ function load_path_expand(env::AbstractString)::Union{String, Nothing}
302302
path = joinpath(depot, "environments", name)
303303
isdir(path) || continue
304304
for proj in project_names
305-
file = abspath(path, proj)
305+
file = joinpath(path, proj)
306306
isfile_casesensitive(file) && return file
307307
end
308308
end
309309
isempty(DEPOT_PATH) && return nothing
310-
return abspath(DEPOT_PATH[1], "environments", name, project_names[end])
310+
return abspath(DEPOT_PATH[1], "environments", name, project_names[end]; normalize=false)
311311
end
312312
# otherwise, it's a path
313-
path = abspath(env)
313+
path = abspath(env; normalize=false)
314314
if isdir(path)
315315
# directory with a project file?
316316
for proj in project_names
@@ -337,7 +337,7 @@ function active_project(search_load_path::Bool=true)
337337
# there are backedges here from abspath(::AbstractString, ::String)
338338
project = project::String
339339
if !isfile_casesensitive(project) && basename(project) project_names
340-
project = abspath(project, "Project.toml")
340+
project = abspath(project, "Project.toml"; normalize=false)
341341
end
342342
return project
343343
end

base/loading.jl

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -964,7 +964,7 @@ function project_file_manifest_path(project_file::String)::Union{Nothing,String}
964964
manifest_path = get(cache.project_file_manifest_path, project_file, missing)
965965
manifest_path === missing || return manifest_path
966966
end
967-
dir = abspath(dirname(project_file))
967+
dir = abspath(dirname(project_file); normalize=false)
968968
isfile_casesensitive(project_file) || return nothing
969969
d = parsed_toml(project_file)
970970
base_manifest = workspace_manifest(project_file)
@@ -974,7 +974,7 @@ function project_file_manifest_path(project_file::String)::Union{Nothing,String}
974974
explicit_manifest = get(d, "manifest", nothing)::Union{String, Nothing}
975975
manifest_path = nothing
976976
if explicit_manifest !== nothing
977-
manifest_file = normpath(joinpath(dir, explicit_manifest))
977+
manifest_file = joinpath(dir, explicit_manifest)
978978
if isfile_casesensitive(manifest_file)
979979
manifest_path = manifest_file
980980
end
@@ -998,10 +998,10 @@ end
998998
# given a directory (implicit env from LOAD_PATH) and a name,
999999
# check if it is an implicit package
10001000
function entry_point_and_project_file_inside(dir::String, name::String)::Union{Tuple{Nothing,Nothing},Tuple{String,Nothing},Tuple{String,String}}
1001-
path = normpath(joinpath(dir, "src", "$name.jl"))
1001+
path = joinpath(dir, "src", "$name.jl")
10021002
isfile_casesensitive(path) || return nothing, nothing
10031003
for proj in project_names
1004-
project_file = normpath(joinpath(dir, proj))
1004+
project_file = joinpath(dir, proj)
10051005
isfile_casesensitive(project_file) || continue
10061006
return path, project_file
10071007
end
@@ -1018,7 +1018,7 @@ function entry_point_and_project_file(dir::String, name::String)::Union{Tuple{No
10181018
path, project_file = entry_point_and_project_file_inside(dir_jl, name)
10191019
path === nothing || return path, project_file
10201020
# check for less likely case with a bare file and no src directory last to minimize stat calls
1021-
path = normpath(joinpath(dir, "$name.jl"))
1021+
path = joinpath(dir, "$name.jl")
10221022
isfile_casesensitive(path) && return path, nothing
10231023
return nothing, nothing
10241024
end
@@ -1038,9 +1038,9 @@ end
10381038

10391039
# given a path, name, and possibly an entryfile, return the entry point
10401040
function entry_path(path::String, name::String, entryfile::Union{Nothing,String})::String
1041-
isfile_casesensitive(path) && return normpath(path)
1041+
isfile_casesensitive(path) && return path
10421042
entrypoint = entryfile === nothing ? joinpath("src", "$name.jl") : entryfile
1043-
return normpath(joinpath(path, entrypoint))
1043+
return joinpath(path, entrypoint)
10441044
end
10451045

10461046
## explicit project & manifest API ##
@@ -1204,7 +1204,7 @@ function explicit_manifest_uuid_load_spec(project_file::String, pkg::PkgId)::Uni
12041204
error("failed to find source of parent package: \"$name\"")
12051205
end
12061206
parent_path = parent_load_spec.path
1207-
p = normpath(dirname(parent_path), "..")
1207+
p = joinpath(dirname(parent_path), "..")
12081208
return PkgLoadSpec(find_ext_path(p, pkg.name), parent_load_spec.julia_syntax_version)
12091209
end
12101210
end
@@ -1228,7 +1228,7 @@ function explicit_manifest_entry_load_spec(manifest_file::String, pkg::PkgId, en
12281228
path = get(entry, "path", nothing)::Union{Nothing, String}
12291229
entryfile = get(entry, "entryfile", nothing)::Union{Nothing, String}
12301230
if path !== nothing
1231-
path = entry_path(normpath(abspath(dirname(manifest_file), path)), pkg.name, entryfile)
1231+
path = entry_path(joinpath(dirname(manifest_file), path), pkg.name, entryfile)
12321232
return PkgLoadSpec(path, syntax_version)
12331233
end
12341234
hash = get(entry, "git-tree-sha1", nothing)::Union{Nothing, String}
@@ -1248,7 +1248,7 @@ function explicit_manifest_entry_load_spec(manifest_file::String, pkg::PkgId, en
12481248
for slug in (version_slug(uuid, hash), version_slug(uuid, hash, 4))
12491249
for depot in DEPOT_PATH
12501250
path = joinpath(depot, "packages", pkg.name, slug)
1251-
ispath(path) && return PkgLoadSpec(entry_path(abspath(path), pkg.name, entryfile), syntax_version)
1251+
ispath(path) && return PkgLoadSpec(entry_path(abspath(path; normalize=false), pkg.name, entryfile), syntax_version)
12521252
end
12531253
end
12541254
# no depot contains the package, return missing to stop looking
@@ -3003,9 +3003,9 @@ function require_stdlib(package_uuidkey::PkgId, ext::Union{Nothing, String}, fro
30033003
if from_stdlib
30043004
# first since this is a stdlib, try to look there directly first
30053005
if ext === nothing
3006-
sourcepath = normpath(env, this_uuidkey.name, "src", this_uuidkey.name * ".jl")
3006+
sourcepath = joinpath(env, this_uuidkey.name, "src", this_uuidkey.name * ".jl")
30073007
else
3008-
sourcepath = find_ext_path(normpath(joinpath(env, package_uuidkey.name)), ext)
3008+
sourcepath = find_ext_path(joinpath(env, package_uuidkey.name), ext)
30093009
end
30103010
set_pkgorigin_version_path(this_uuidkey, sourcepath)
30113011
newm = _require_search_from_serialized(this_uuidkey, PkgLoadSpec(sourcepath, VERSION), UInt128(0), false; DEPOT_PATH=depot_path)
@@ -3191,13 +3191,14 @@ function evalfile(path::AbstractString, args::Vector{String}=String[])
31913191
end
31923192
evalfile(path::AbstractString, args::Vector) = evalfile(path, String[args...])
31933193

3194+
# Used in Pkg.jl
31943195
function load_path_setup_code(load_path::Bool=true)
31953196
code = """
3196-
append!(empty!(Base.DEPOT_PATH), $(repr(map(abspath, DEPOT_PATH))))
3197-
append!(empty!(Base.DL_LOAD_PATH), $(repr(map(abspath, DL_LOAD_PATH))))
3197+
append!(empty!(Base.DEPOT_PATH), $(repr(map(x -> abspath(x; normalize=false), DEPOT_PATH))))
3198+
append!(empty!(Base.DL_LOAD_PATH), $(repr(map(x -> abspath(x; normalize=false), DL_LOAD_PATH))))
31983199
"""
31993200
if load_path
3200-
load_path = map(abspath, Base.load_path())
3201+
load_path = map(x -> abspath(x; normalize=false), Base.load_path())
32013202
path_sep = Sys.iswindows() ? ';' : ':'
32023203
any(path -> path_sep in path, load_path) &&
32033204
error("LOAD_PATH entries cannot contain $(repr(path_sep))")
@@ -3283,9 +3284,9 @@ function create_expr_cache(pkg::PkgId, input::PkgLoadSpec, output::String, outpu
32833284
@nospecialize internal_stderr internal_stdout
32843285
rm(output, force=true) # Remove file if it exists
32853286
output_o === nothing || rm(output_o, force=true)
3286-
depot_path = String[abspath(x) for x in DEPOT_PATH]
3287-
dl_load_path = String[abspath(x) for x in DL_LOAD_PATH]
3288-
load_path = String[abspath(x) for x in Base.load_path()]
3287+
depot_path = String[abspath(x; normalize=false) for x in DEPOT_PATH]
3288+
dl_load_path = String[abspath(x; normalize=false) for x in DL_LOAD_PATH]
3289+
load_path = String[abspath(x; normalize=false) for x in Base.load_path()]
32893290
# if pkg is a stdlib, append its parent Project.toml to the load path
32903291
triggers = get(EXT_PRIMED, pkg, nothing)
32913292
if triggers !== nothing
@@ -3343,7 +3344,7 @@ function create_expr_cache(pkg::PkgId, input::PkgLoadSpec, output::String, outpu
33433344
Base.track_nested_precomp($(_pkg_str(vcat(Base.precompilation_stack, pkg))))
33443345
Base.loadable_extensions = $(_pkg_str(loadable_exts))
33453346
Base.precompiling_extension = $(loading_extension)
3346-
Base.include_package_for_output($(_pkg_str(pkg)), $(repr(abspath(input.path))), $(repr(input.julia_syntax_version)), $(repr(depot_path)), $(repr(dl_load_path)),
3347+
Base.include_package_for_output($(_pkg_str(pkg)), $(repr(abspath(input.path; normalize=false))), $(repr(input.julia_syntax_version)), $(repr(depot_path)), $(repr(dl_load_path)),
33473348
$(repr(load_path)), $(_pkg_str(concrete_deps)), $(repr(source_path(nothing))))
33483349
""")
33493350
close(io.in)
@@ -3372,7 +3373,7 @@ function compilecache_path(pkg::PkgId, prefs_hash::UInt64; flags::CacheFlags=Cac
33723373
cachepath = joinpath(DEPOT_PATH[1], entrypath)
33733374
isdir(cachepath) || mkpath(cachepath)
33743375
if pkg.uuid === nothing
3375-
abspath(cachepath, entryfile) * ".ji"
3376+
joinpath(cachepath, entryfile) * ".ji"
33763377
else
33773378
crc = _crc32c(project)
33783379
crc = _crc32c(unsafe_string(JLOptions().image_file), crc)
@@ -3387,7 +3388,7 @@ function compilecache_path(pkg::PkgId, prefs_hash::UInt64; flags::CacheFlags=Cac
33873388

33883389
crc = _crc32c(prefs_hash, crc)
33893390
project_precompile_slug = slug(crc, 5)
3390-
abspath(cachepath, string(entryfile, "_", project_precompile_slug, ".ji"))
3391+
joinpath(cachepath, string(entryfile, "_", project_precompile_slug, ".ji"))
33913392
end
33923393
end
33933394

@@ -3612,7 +3613,7 @@ function normalize_depots_for_relocation()
36123613
if isdirpath(d)
36133614
d = dirname(d)
36143615
end
3615-
push!(depots, abspath(d))
3616+
push!(depots, realpath(d))
36163617
end
36173618
return depots
36183619
end

base/path.jl

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,13 @@ julia> normpath("/home/myuser/../example.jl")
386386
julia> normpath("Documents/Julia") == joinpath("Documents", "Julia")
387387
true
388388
```
389+
390+
!!! warning
391+
Normalization may change the meaning of a path if the directory immediately
392+
preceding a ".." entry is a symbolic link. For example, if `a` is a symbolic
393+
link in `a/../b`, then this path will resolve to `b` in the link target's
394+
parent directory. Hoewver, `normpath("a/../b")` will return `b`, which will
395+
resolve `b` in the current working directory.
389396
"""
390397
function normpath(path::String)
391398
isabs = isabspath(path)
@@ -430,10 +437,10 @@ Convert a set of paths to a normalized path by joining them together and removin
430437
normpath(a::AbstractString, b::AbstractString...) = normpath(joinpath(a,b...))
431438

432439
"""
433-
abspath(path::AbstractString)::String
440+
abspath(path::AbstractString; normalize=true)::String
434441
435442
Convert a path to an absolute path by adding the current directory if necessary.
436-
Also normalizes the path as in [`normpath`](@ref).
443+
If `normalize` is true, also normalizes the path as in [`normpath`](@ref).
437444
438445
# Examples
439446
@@ -444,8 +451,11 @@ If you are in a directory called `JuliaExample` and the data you are using is tw
444451
Which gives a path like `"/home/JuliaUser/data/"`.
445452
446453
See also [`joinpath`](@ref), [`pwd`](@ref), [`expanduser`](@ref).
454+
455+
!!! compat "Julia 1.14"
456+
The `normalize` keyword argument was added in Julia 1.14.
447457
"""
448-
function abspath(a::String)::String
458+
function abspath(a::String; normalize=true)::String
449459
if !isabspath(a)
450460
cwd = pwd()
451461
a_drive, a_nodrive = splitdrive(a)
@@ -456,7 +466,7 @@ function abspath(a::String)::String
456466
a = joinpath(cwd, a)
457467
end
458468
end
459-
return normpath(a)
469+
return normalize ? normpath(a) : a
460470
end
461471

462472
"""
@@ -465,7 +475,7 @@ end
465475
Convert a set of paths to an absolute path by joining them together and adding the
466476
current directory if necessary. Equivalent to `abspath(joinpath(path, paths...))`.
467477
"""
468-
abspath(a::AbstractString, b::AbstractString...) = abspath(joinpath(a,b...))
478+
abspath(a::AbstractString, b::AbstractString...; kwargs...) = abspath(joinpath(a,b...); kwargs...)
469479

470480
if Sys.iswindows()
471481

test/cmdlineargs.jl

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -294,25 +294,25 @@ let exename = `$(Base.julia_cmd()) --startup-file=no --color=no`
294294

295295
# ~ expansion in --project and JULIA_PROJECT
296296
if !Sys.iswindows()
297-
let expanded = abspath(expanduser("~/foo/Project.toml"))
297+
let expanded = abspath(expanduser("~/foo/Project.toml"); normalize=false)
298298
@test expanded == readchomp(`$exename --project='~/foo' -e 'println(Base.active_project())'`)
299299
@test expanded == readchomp(setenv(`$exename -e 'println(Base.active_project())'`, "JULIA_PROJECT" => "~/foo", "HOME" => homedir()))
300300
end
301301
end
302302

303303
# handling of @projectname in --project and JULIA_PROJECT
304-
let expanded = abspath(Base.load_path_expand("@foo"))
304+
let expanded = abspath(Base.load_path_expand("@foo"); normalize=false)
305305
@test expanded == readchomp(`$exename --project='@foo' -e 'println(Base.active_project())'`)
306306
@test expanded == readchomp(addenv(`$exename -e 'println(Base.active_project())'`, "JULIA_PROJECT" => "@foo", "HOME" => homedir()))
307307
end
308308

309309
# --project=@script handling
310-
let expanded = abspath(joinpath(@__DIR__, "project", "ScriptProject"))
310+
let expanded = abspath(joinpath(@__DIR__, "project", "ScriptProject"); normalize=false)
311311
script = joinpath(expanded, "bin", "script.jl")
312312
# Check running julia with --project=@script both within and outside the script directory
313313
@testset "--@script from $name" for (name, dir) in [("project", expanded), ("outside", pwd())]
314-
@test joinpath(expanded, "Project.toml") == readchomp(Cmd(`$exename --project=@script $script`; dir))
315-
@test joinpath(expanded, "SubProject", "Project.toml") == readchomp(Cmd(`$exename --project=@script/../SubProject $script`; dir))
314+
@test samefile(joinpath(expanded, "Project.toml"), readchomp(Cmd(`$exename --project=@script $script`; dir)))
315+
@test samefile(joinpath(expanded, "SubProject", "Project.toml"), readchomp(Cmd(`$exename --project=@script/../SubProject $script`; dir)))
316316
end
317317
end
318318

test/loading.jl

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ end
274274
n = map(String, split(names, '.'))
275275
pkg = recurse_package(n...)
276276
@test pkg == PkgId(UUID(uuid), n[end])
277-
@test joinpath(@__DIR__, normpath(path)) == locate_package(pkg)
277+
@test samefile(joinpath(@__DIR__, path), locate_package(pkg))
278278
@test Base.compilecache_path(pkg, UInt64(0)) == Base.compilecache_path(pkg, UInt64(0))
279279
end
280280
@test identify_package("Baz") === nothing
@@ -350,19 +350,19 @@ module NotPkgModule; end
350350
@test Foo.which == "path"
351351

352352
@testset "pathof" begin
353-
@test pathof(Foo) == normpath(abspath(@__DIR__, "project/deps/Foo1/src/Foo.jl"))
353+
@test samefile(pathof(Foo), joinpath(@__DIR__, "project/deps/Foo1/src/Foo.jl"))
354354
@test pathof(NotPkgModule) === nothing
355355
end
356356

357357
@testset "pkgdir" begin
358-
@test pkgdir(Foo) == normpath(abspath(@__DIR__, "project/deps/Foo1"))
359-
@test pkgdir(Foo.SubFoo1) == normpath(abspath(@__DIR__, "project/deps/Foo1"))
360-
@test pkgdir(Foo.SubFoo2) == normpath(abspath(@__DIR__, "project/deps/Foo1"))
358+
@test samefile(pkgdir(Foo), joinpath(@__DIR__, "project/deps/Foo1"))
359+
@test samefile(pkgdir(Foo.SubFoo1), joinpath(@__DIR__, "project/deps/Foo1"))
360+
@test samefile(pkgdir(Foo.SubFoo2), joinpath(@__DIR__, "project/deps/Foo1"))
361361
@test pkgdir(NotPkgModule) === nothing
362362

363-
@test pkgdir(Foo, "src") == normpath(abspath(@__DIR__, "project/deps/Foo1/src"))
364-
@test pkgdir(Foo.SubFoo1, "src") == normpath(abspath(@__DIR__, "project/deps/Foo1/src"))
365-
@test pkgdir(Foo.SubFoo2, "src") == normpath(abspath(@__DIR__, "project/deps/Foo1/src"))
363+
@test samefile(pkgdir(Foo, "src"), joinpath(@__DIR__, "project/deps/Foo1/src"))
364+
@test samefile(pkgdir(Foo.SubFoo1, "src"), joinpath(@__DIR__, "project/deps/Foo1/src"))
365+
@test samefile(pkgdir(Foo.SubFoo2, "src"), joinpath(@__DIR__, "project/deps/Foo1/src"))
366366
@test pkgdir(NotPkgModule, "src") === nothing
367367
end
368368

@@ -1146,7 +1146,7 @@ end
11461146
_ext = Base.get_extension(parent, ext)
11471147
_ext isa Module || error("expected extension \$ext to be loaded")
11481148
_pkgdir = pkgdir(_ext)
1149-
_pkgdir == pkgdir(parent) != nothing || error("unexpected extension \$ext pkgdir path: \$_pkgdir")
1149+
samefile(_pkgdir, pkgdir(parent)) || error("unexpected extension \$ext pkgdir path: \$_pkgdir")
11501150
_pkgversion = pkgversion(_ext)
11511151
_pkgversion == pkgversion(parent) || error("unexpected extension \$ext version: \$_pkgversion")
11521152
end

0 commit comments

Comments
 (0)