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

Add prefix argument to mktempdir() #23634

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 45 additions & 32 deletions base/file.jl
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,8 @@ function touch(path::AbstractString)
end
end

const temp_prefix = "jl_"

if Sys.iswindows()

function tempdir()
Expand All @@ -265,13 +267,17 @@ function tempdir()
resize!(temppath,lentemppath)
return transcode(String, temppath)
end

tempname(uunique::UInt32=UInt32(0)) = tempname(tempdir(), uunique)
const temp_prefix = cwstring("jl_")

function tempname(temppath::AbstractString,uunique::UInt32)
tempp = cwstring(temppath)
temppfx = cwstring(temp_prefix)
tname = Vector{UInt16}(32767)
uunique = ccall(:GetTempFileNameW,stdcall,UInt32,(Ptr{UInt16},Ptr{UInt16},UInt32,Ptr{UInt16}), tempp,temp_prefix,uunique,tname)
lentname = findfirst(iszero,tname)-1
uunique = ccall(:GetTempFileNameW,stdcall,UInt32,
(Ptr{UInt16}, Ptr{UInt16}, UInt32, Ptr{UInt16}),
tempp, temppfx, uunique, tname)
lentname = findfirst(iszero, tname)-1
if uunique == 0 || lentname <= 0
error("GetTempFileName failed: $(Libc.FormatMessage())")
end
Expand All @@ -284,22 +290,6 @@ function mktemp(parent=tempdir())
return (filename, Base.open(filename, "r+"))
end

function mktempdir(parent=tempdir())
seed::UInt32 = rand(UInt32)
while true
if (seed & typemax(UInt16)) == 0
seed += 1
end
filename = tempname(parent, seed)
ret = ccall(:_wmkdir, Int32, (Ptr{UInt16},), cwstring(filename))
if ret == 0
return filename
end
systemerror(:mktempdir, Libc.errno()!=Libc.EEXIST)
seed += 1
end
end

else # !windows
# Obtain a temporary filename.
function tempname()
Expand All @@ -322,13 +312,6 @@ function mktemp(parent=tempdir())
return (b, fdio(p, true))
end

# Create and return the name of a temporary directory
function mktempdir(parent=tempdir())
b = joinpath(parent, "tmpXXXXXX")
p = ccall(:mkdtemp, Cstring, (Cstring,), b)
systemerror(:mktempdir, p == C_NULL)
return unsafe_string(p)
end

end # os-test

Expand Down Expand Up @@ -356,13 +339,42 @@ is an open file object for this path.
mktemp(parent)

"""
mktempdir(parent=tempdir())
mktempdir(parent=tempdir(); prefix="$(repr(temp_prefix))")

Create a temporary directory in the `parent` directory and return its path.
If `parent` does not exist, throw an error.
An optional `prefix` to the directory name can be provided.
"""
mktempdir(parent)
function mktempdir(parent=tempdir(); prefix=temp_prefix)
i = endof(parent)
while i >= 1 && parent[i:i] == path_separator
i -= 1
end
parent = parent[1:i]
i = 1
while i <= endof(prefix) && prefix[i:i] == path_separator
i += 1
end
prefix = prefix[i:end]

tpath = "$(parent)$(path_separator)$(prefix)XXXXXX"
Copy link
Member

Choose a reason for hiding this comment

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

If prefix contains the letter X this is going to be wrong; it's hard to say if that's avoidable or not, given the way the underlying uv_fs_mkdtemp API works.

Copy link
Member

Choose a reason for hiding this comment

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

My understanding is that the last 6 characters need to be X, but it doesn't matter what the other characters are.

Copy link
Member

Choose a reason for hiding this comment

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

Yes. It's not even clear why these chars need to be X rather than any other ASCII char.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Links to documentation for uv_fs_mkdtemp and mkdtemp given below:

http://docs.libuv.org/en/v1.x/fs.html
https://linux.die.net/man/3/mkdtemp


req = Libc.malloc(_sizeof_uv_fs)
try
ret = ccall(:uv_fs_mkdtemp, Int32,
(Ptr{Void}, Ptr{Void}, Cstring, Ptr{Void}),
eventloop(), req, tpath, C_NULL)
if ret < 0
ccall(:uv_fs_req_cleanup, Void, (Ptr{Void},), req)
uv_error("mktempdir", ret)
end
path = unsafe_string(ccall(:jl_uv_fs_t_path, Ptr{Cchar}, (Ptr{Void},), req))
ccall(:uv_fs_req_cleanup, Void, (Ptr{Void},), req)
return path
finally
Libc.free(req)
end
end

"""
mktemp(f::Function, parent=tempdir())
Expand All @@ -381,13 +393,14 @@ function mktemp(fn::Function, parent=tempdir())
end

"""
mktempdir(f::Function, parent=tempdir())
mktempdir(f::Function, parent=tempdir(); prefix="$(repr(temp_prefix))")
Copy link
Member

Choose a reason for hiding this comment

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

I think this will include two sets of quotes (quotes are included in the output of repr of a string).

Copy link
Member

Choose a reason for hiding this comment

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

See previous comment: #23634 (comment)


Apply the function `f` to the result of [`mktempdir(parent)`](@ref) and remove the
Apply the function `f` to the result of [`mktempdir(parent; prefix)`](@ref) and remove the
temporary directory upon completion.
An optional `prefix` to the directory name can be provided.
"""
function mktempdir(fn::Function, parent=tempdir())
tmpdir = mktempdir(parent)
function mktempdir(fn::Function, parent=tempdir(); prefix=temp_prefix)
tmpdir = mktempdir(parent; prefix=prefix)
try
fn(tmpdir)
finally
Expand Down
1 change: 1 addition & 0 deletions src/sys.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ JL_DLLEXPORT int32_t jl_nb_available(ios_t *s)
JL_DLLEXPORT int jl_sizeof_uv_fs_t(void) { return sizeof(uv_fs_t); }
JL_DLLEXPORT void jl_uv_fs_req_cleanup(uv_fs_t *req) { uv_fs_req_cleanup(req); }
JL_DLLEXPORT char *jl_uv_fs_t_ptr(uv_fs_t *req) { return (char*)req->ptr; }
JL_DLLEXPORT char *jl_uv_fs_t_path(uv_fs_t *req) { return (char*)req->path; }
JL_DLLEXPORT int jl_uv_fs_result(uv_fs_t *f) { return f->result; }

// --- stat ---
Expand Down
77 changes: 77 additions & 0 deletions test/file.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1183,3 +1183,80 @@ if !Sys.iswindows()
test_22566()
end
end # !Sys.iswindows

function test_22922()
def_prefix = "jl_"
tst_prefix = "ABCDEF"
mktempdir() do tmpdir
filename = basename(tmpdir)
@test startswith(filename, def_prefix)
end
mktempdir(; prefix=tst_prefix) do tmpdir
filename = basename(tmpdir)
@test startswith(filename, tst_prefix)
end
# Special character prefix tests
tst_prefix="#!@%^&()"
mktempdir(; prefix=tst_prefix) do tmpdir
filename = basename(tmpdir)
@test startswith(filename, tst_prefix)
end

# Behavioral differences across OS types
if Sys.iswindows()
@test_throws Base.UVError mktempdir(; prefix="*")
@test_throws Base.UVError mktempdir(; prefix="cdcdccd/")

# The API accepts "c:/ and c:\\" as valid prefixes.
# The parent directory is ignored in that case.
# The temp directory created is of form "C:\\XXXXXX"
mktempdir("C:\\dir_notexisting"; prefix=tempdir()) do tmpdir
@test startswith(tmpdir, tempdir())
@test isdir(tmpdir)
filename = basename(tmpdir)
@test length(filename) == 6
end

# Although, underlying directory created is of type c:\\xxx\\yyy\\..., the returned
# directory name by the API is in the format as specified in the prefix
# parameter.
mktempdir("C:/dir_notexisting"; prefix=replace(tempdir(), '\\', '/')) do tmpdir
@test startswith(tmpdir, replace(tempdir(), '\\', '/'))
@test !startswith(tmpdir, tempdir())
@test isdir(tmpdir)
filename = basename(tmpdir)
@test length(filename) == 6
end
else
# '/' is accepted in a prefix but depends on the overall path and permissions.
# A carefully crafted parent directory and prefix combination can actually
# create a directory as the example below.
# The file created will be of format "/tmp/XXXXXX"
mktempdir(; prefix="/") do tmpdir
@test startswith(tmpdir, "/tmp/")
filename = basename(tmpdir)
@test length(filename) == 6
end

mktempdir(; prefix="////abc") do tmpdir
@test startswith(tmpdir, "/tmp/")
filename = basename(tmpdir)
@test length(filename) == 9
end

mktempdir("/"; prefix="tmp/") do tmpdir
@test startswith(tmpdir, "/tmp/")
filename = basename(tmpdir)
@test length(filename) == 6
end
end

# Unicode test
tst_prefix="∃x∀y"
mktempdir(; prefix=tst_prefix) do tmpdir
filename = basename(tmpdir)
@test startswith(filename, tst_prefix)
end
end

test_22922()