Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix mkpath error handling #36126

Merged
merged 1 commit into from
Jun 3, 2020
Merged

Fix mkpath error handling #36126

merged 1 commit into from
Jun 3, 2020

Conversation

simonbyrne
Copy link
Contributor

The error thrown by mkdir when the directory already exists was changed in #33422.

Unfortunately, this is hard to test, though we came across it on a cluster with a shared file system.

cc: @jakebolewski @kpamnany

The error thrown by `mkdir` when the directory already exists was changed in #33422.
@@ -228,7 +228,7 @@ function mkpath(path::AbstractString; mode::Integer = 0o777)
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, SystemError) || !isdir(path)
if !isa(err, IOError) || !isdir(path)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to do?

Suggested change
if !isa(err, IOError) || !isdir(path)
if !(isa(err, IOError) && err.code == UV_EEXIST)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume it doesn't matter because of the isdir test?

julia> mkpath("/nonexistant")
ERROR: IOError: mkdir: permission denied (EACCES)
Stacktrace:
 [1] uv_error at ./libuv.jl:97 [inlined]
 [2] mkdir(::String; mode::UInt16) at ./file.jl:177
 [3] mkpath(::String; mode::UInt16) at ./file.jl:227
 [4] mkpath(::String) at ./file.jl:222
 [5] top-level scope at REPL[3]:1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguably in that case you would want the error to be propagated?

@simonbyrne simonbyrne requested a review from vtjnash June 3, 2020 03:30
@simonbyrne simonbyrne merged commit 912a8ed into master Jun 3, 2020
@simonbyrne simonbyrne deleted the sb/mkdir-ioerror branch June 3, 2020 22:53
KristofferC pushed a commit that referenced this pull request Jun 4, 2020
The error thrown by `mkdir` when the directory already exists was changed in #33422.

(cherry picked from commit 912a8ed)
simeonschaub pushed a commit to simeonschaub/julia that referenced this pull request Aug 11, 2020
The error thrown by `mkdir` when the directory already exists was changed in JuliaLang#33422.
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.

3 participants