Skip to content

Commit

Permalink
mkpath always returns the original path (#54857)
Browse files Browse the repository at this point in the history
fix #54826

related performance test code:
```julia
using BenchmarkTools
using Base: IOError

function checkmode(mode::Integer)
    if !(0 <= mode <= 511)
        throw(ArgumentError("Mode must be between 0 and 511 = 0o777"))
    end
    mode
end

function mkpath(path::AbstractString; mode::Integer = 0o777)
    dir = dirname(path)
    # stop recursion for `""`, `"/"`, or existed dir
    (path == dir || isdir(path)) && return path
    mkpath(dir, mode = checkmode(mode))
    try
        # cases like `mkpath("x/")` will cause an error if `isdir(path)` is skipped
        # the error will not be rethrowed, but it may be slower, and thus we avoid it in advance
        isdir(path) || mkdir(path, mode = mode)
    catch err
        # If there is a problem with making the directory, but the directory
        # does in fact exist, then ignore the error. Else re-throw it.
        if !isa(err, IOError) || !isdir(path)
            rethrow()
        end
    end
    return path
end

function mkpath_2(path::AbstractString; mode::Integer = 0o777)
    dir = dirname(path)
    # stop recursion for `""` and `"/"` or existed dir
    (path == dir || isdir(path)) && return path
    mkpath_2(dir, mode = checkmode(mode))
    try
        mkdir(path, mode = mode)
    catch err
        # If there is a problem with making the directory, but the directory
        # does in fact exist, then ignore the error. Else re-throw it.
        if !isa(err, IOError) || !isdir(path)
            rethrow()
        end
    end
    return path
end

versioninfo()
display(@benchmark begin rm("A", recursive=true, force=true); mkpath("A/B/C/D/") end)
display(@benchmark begin rm("A", recursive=true, force=true); mkpath_2("A/B/C/D/") end)
```
output:
```
Julia Version 1.10.4
Commit 48d4fd4 (2024-06-04 10:41 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: macOS (x86_64-apple-darwin22.4.0)
  CPU: 16 × Intel(R) Core(TM) i9-9900K CPU @ 3.60GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, skylake)
Threads: 1 default, 0 interactive, 1 GC (on 16 virtual cores)
Environment:
  JULIA_EDITOR = code
  JULIA_NUM_THREADS = 
BenchmarkTools.Trial: 8683 samples with 1 evaluation.
 Range (min … max):  473.972 μs …  18.867 ms  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     519.704 μs               ┊ GC (median):    0.00%
 Time  (mean ± σ):   571.261 μs ± 378.851 μs  ┊ GC (mean ± σ):  0.00% ± 0.00%

   ▂█▇▄▁                                                         
  ▃███████▇▅▄▄▃▃▃▂▂▂▂▂▂▂▂▂▂▂▂▂▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ ▂
  474 μs           Histogram: frequency by time          961 μs <

 Memory estimate: 5.98 KiB, allocs estimate: 65.
BenchmarkTools.Trial: 6531 samples with 1 evaluation.
 Range (min … max):  588.122 μs …  17.449 ms  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     660.071 μs               ┊ GC (median):    0.00%
 Time  (mean ± σ):   760.333 μs ± 615.759 μs  ┊ GC (mean ± σ):  0.00% ± 0.00%

  ██▆▄▃▁▁                                                       ▁
  ██████████▆▇█▇▆▆▅▅▅▄▁▄▁▄▄▃▄▃▁▃▃▁▃▁▁▁▄▁▁▁▁▁▁▁▃▃▁▁▁▃▁▁▃▁▁▁▁▅▅▄▇ █
  588 μs        Histogram: log(frequency) by time        4.2 ms <

 Memory estimate: 5.63 KiB, allocs estimate: 72.
```
  • Loading branch information
ctarn authored Jul 15, 2024
1 parent b88f64f commit 7676196
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 7 deletions.
16 changes: 10 additions & 6 deletions base/file.jl
Original file line number Diff line number Diff line change
Expand Up @@ -230,23 +230,27 @@ julia> mkpath("intermediate_dir/actually_a_directory.txt") # creates two directo
julia> isdir("intermediate_dir/actually_a_directory.txt")
true
julia> mkpath("my/test/dir/") # returns the original `path`
"my/test/dir/"
```
"""
function mkpath(path::AbstractString; mode::Integer = 0o777)
isdirpath(path) && (path = dirname(path))
dir = dirname(path)
(path == dir || isdir(path)) && return path
mkpath(dir, mode = checkmode(mode))
parent = dirname(path)
# stop recursion for `""`, `"/"`, or existing dir
(path == parent || isdir(path)) && return path
mkpath(parent, mode = checkmode(mode))
try
mkdir(path, mode = mode)
# The `isdir` check could be omitted, then `mkdir` will throw an error in cases like `x/`.
# Although the error will not be rethrown, we avoid it in advance for performance reasons.
isdir(path) || mkdir(path, mode = mode)
catch err
# If there is a problem with making the directory, but the directory
# does in fact exist, then ignore the error. Else re-throw it.
if !isa(err, IOError) || !isdir(path)
rethrow()
end
end
path
return path
end

# Files that were requested to be deleted but can't be by the current process
Expand Down
7 changes: 6 additions & 1 deletion test/file.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1491,7 +1491,7 @@ mktempdir() do dir
@test isfile(name1)
@test isfile(name2)
namedir = joinpath(dir, "chalk")
namepath = joinpath(dir, "chalk","cheese","fresh")
namepath = joinpath(dir, "chalk", "cheese", "fresh")
@test !ispath(namedir)
@test mkdir(namedir) == namedir
@test isdir(namedir)
Expand All @@ -1500,7 +1500,12 @@ mktempdir() do dir
@test isdir(namepath)
@test mkpath(namepath) == namepath
@test isdir(namepath)
# issue 54826
namepath_dirpath = joinpath(dir, "x", "y", "z", "")
@test mkpath(namepath_dirpath) == namepath_dirpath
end
@test mkpath("") == ""
@test mkpath("/") == "/"

# issue #30588
@test realpath(".") == realpath(pwd())
Expand Down

0 comments on commit 7676196

Please sign in to comment.