Skip to content

Conversation

@Keno
Copy link
Member

@Keno Keno commented Nov 26, 2025

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.

@Keno Keno force-pushed the kf/loadingnonormpath branch from 5d65187 to 9e4c09a Compare November 26, 2025 02:31
Keno added a commit to JuliaLang/Pkg.jl that referenced this pull request Nov 26, 2025
7z appears to normalize internally causing it to fail to extract archives that have this pattern. One way to reproduce is to set DEPOT_PATH to a path that has a symlink/.. in the path. We get this wrong in Julia too, c.f. JuliaLang/julia#60251
@Keno Keno force-pushed the kf/loadingnonormpath branch from 9e4c09a to d6d6f2d Compare November 26, 2025 05:06
Keno added a commit to JuliaLang/Pkg.jl that referenced this pull request Nov 26, 2025
7z appears to normalize internally causing it to fail to extract archives that have this pattern. One way to reproduce is to set DEPOT_PATH to a path that has a symlink/.. in the path. We get this wrong in Julia too, c.f. JuliaLang/julia#60251
Keno added a commit to timholy/Revise.jl that referenced this pull request Nov 26, 2025
Fixes test failures with JuliaLang/julia#60251.
Also fixes an unrelated test where Revise was using the wrong PkgId
for Core.Compiler (failure not seen on CI, because it's explicitly
disabled there).
Keno added a commit to timholy/Revise.jl that referenced this pull request Nov 27, 2025
Fixes test failures with JuliaLang/julia#60251.
Also fixes an unrelated test where Revise was using the wrong PkgId
for Core.Compiler (failure not seen on CI, because it's explicitly
disabled there).
Keno added a commit to timholy/Revise.jl that referenced this pull request Nov 27, 2025
Fixes test failures with JuliaLang/julia#60251.
Also fixes an unrelated test where Revise was using the wrong PkgId
for Core.Compiler (failure not seen on CI, because it's explicitly
disabled there).
Keno added a commit to timholy/Revise.jl that referenced this pull request Nov 27, 2025
* Adjust for base paths possibly being non-normalized

Fixes test failures with JuliaLang/julia#60251.
Also fixes an unrelated test where Revise was using the wrong PkgId
for Core.Compiler (failure not seen on CI, because it's explicitly
disabled there).

* Try more realpaths

* relpath?

* WIP

* Try to avoid calling realpath internally
@Keno
Copy link
Member Author

Keno commented Nov 28, 2025

Alright, we have some tests that specifically test that we strip . and convert / to \ on Windows. That's safe to do AFAIK, so I will introduce a kwarg to normpath that only allows safe transformations and use that version of normpath in the appropriate places. Arguably that should have been the semantics in the first place, but our current normpath function follows python semantics. Put it on the list of API evolution I guess.

@Keno Keno force-pushed the kf/loadingnonormpath branch from 770f1e5 to ff57fe1 Compare November 28, 2025 01:47
KristofferC pushed a commit to JuliaLang/Pkg.jl that referenced this pull request Nov 28, 2025
7z appears to normalize internally causing it to fail to extract archives that have this pattern. One way to reproduce is to set DEPOT_PATH to a path that has a symlink/.. in the path. We get this wrong in Julia too, c.f. JuliaLang/julia#60251

(cherry picked from commit 52d7fa3)
@Keno Keno force-pushed the kf/loadingnonormpath branch from ff57fe1 to af8a57d Compare November 28, 2025 19:16
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 `normpath`/`abspath` kwarg
that skips unsafe 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.
@Keno Keno force-pushed the kf/loadingnonormpath branch from af8a57d to f16cf92 Compare November 28, 2025 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants