Skip to content

RFC: for the release, change tempfile cleanup default back to false #33741

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

RFC: for the release, change tempfile cleanup default back to false #33741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ New library functions

* The `splitpath` function now accepts any `AbstractString` whereas previously it only accepted paths of type `String` ([#33012]).
* The `tempname` function now takes an optional `parent::AbstractString` argument to give it a directory in which to attempt to produce a temporary path name ([#33090]).
* The `tempname` function now takes a `cleanup::Bool` keyword argument defaulting to `true`, which causes the process to try to ensure that any file or directory at the path returned by `tempname` is deleted upon process exit ([#33090]).
* The `tempname` function now takes a `cleanup::Bool` keyword argument defaulting to `false` (may change in a future release), which causes the process to try to ensure that any file or directory at the path returned by `tempname` is deleted upon process exit ([#33090]).
* The `readdir` function now takes a `join::Bool` keyword argument defaulting to `false`, which when set causes `readdir` to join its directory argument with each listed name ([#33113]).
* `readdir` output is now guaranteed to be sorted. The `sort` keyword allows opting out of sorting to get names in OS-native order ([#33542]).
* The new `only(x)` function returns the one-and-only element of a collection `x`, and throws an `ArgumentError` if `x` contains zero or multiple elements. ([#33129])
Expand Down
18 changes: 9 additions & 9 deletions base/file.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,13 +504,13 @@ function _win_tempname(temppath::AbstractString, uunique::UInt32)
return transcode(String, tname)
end

function mktemp(parent::AbstractString=tempdir(); cleanup::Bool=true)
function mktemp(parent::AbstractString=tempdir(); cleanup::Bool=false)
filename = _win_tempname(parent, UInt32(0))
cleanup && temp_cleanup_later(filename)
return (filename, Base.open(filename, "r+"))
end

function tempname(parent::AbstractString=tempdir(); cleanup::Bool=true)
function tempname(parent::AbstractString=tempdir(); cleanup::Bool=false)
isdir(parent) || throw(ArgumentError("$(repr(parent)) is not a directory"))
seed::UInt32 = rand(UInt32)
while true
Expand All @@ -529,7 +529,7 @@ end
else # !windows

# Obtain a temporary filename.
function tempname(parent::AbstractString=tempdir(); cleanup::Bool=true)
function tempname(parent::AbstractString=tempdir(); cleanup::Bool=false)
isdir(parent) || throw(ArgumentError("$(repr(parent)) is not a directory"))
p = ccall(:tempnam, Cstring, (Cstring, Cstring), parent, temp_prefix)
systemerror(:tempnam, p == C_NULL)
Expand All @@ -540,7 +540,7 @@ function tempname(parent::AbstractString=tempdir(); cleanup::Bool=true)
end

# Create and return the name of a temporary file along with an IOStream
function mktemp(parent::AbstractString=tempdir(); cleanup::Bool=true)
function mktemp(parent::AbstractString=tempdir(); cleanup::Bool=false)
b = joinpath(parent, temp_prefix * "XXXXXX")
p = ccall(:mkstemp, Int32, (Cstring,), b) # modifies b
systemerror(:mktemp, p == -1)
Expand All @@ -553,7 +553,7 @@ end # os-test


"""
tempname(parent=tempdir(); cleanup=true) -> String
tempname(parent=tempdir(); cleanup=false) -> String

Generate a temporary file path. This function only returns a path; no file is
created. The path is likely to be unique, but this cannot be guaranteed due to
Expand All @@ -573,8 +573,8 @@ there is nothing to cleanup unless you create a file or directory there. If
you do and `clean` is `true` it will be deleted upon process termination.

!!! compat "Julia 1.4"
The `parent` and `cleanup` arguments were added in 1.4. Prior to Julia 1.4
the path `tempname` would never be cleaned up at process termination.
The `parent` and `cleanup` arguments were added in 1.4.
In a future version, the `cleanup` parameter may start to default to true.

!!! warning

Expand All @@ -595,7 +595,7 @@ the temporary file is automatically deleted when the process exits.
mktemp(parent)

"""
mktempdir(parent=tempdir(); prefix=$(repr(temp_prefix)), cleanup=true) -> path
mktempdir(parent=tempdir(); prefix=$(repr(temp_prefix)), cleanup=false) -> path

Create a temporary directory in the `parent` directory with a name
constructed from the given prefix and a random suffix, and return its path.
Expand All @@ -604,7 +604,7 @@ If `parent` does not exist, throw an error. The `cleanup` option controls whethe
the temporary directory is automatically deleted when the process exits.
"""
function mktempdir(parent::AbstractString=tempdir();
prefix::AbstractString=temp_prefix, cleanup::Bool=true)
prefix::AbstractString=temp_prefix, cleanup::Bool=false)
if isempty(parent) || occursin(path_separator_re, parent[end:end])
# append a path_separator only if parent didn't already have one
tpath = "$(parent)$(prefix)XXXXXX"
Expand Down
17 changes: 10 additions & 7 deletions test/file.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,35 +60,37 @@ using Random
@test_throws ArgumentError tempname(randstring())
end

child_eval(code::String) = eval(Meta.parse(readchomp(`$(Base.julia_cmd()) -E $code`)))
child_parsestr(code::String) = Meta.parse(readchomp(`$(Base.julia_cmd()) -E $code`))::String

@testset "mktemp/dir basic cleanup" begin
# mktemp without cleanup
t = child_eval("t = mktemp(cleanup=false)[1]; @assert isfile(t); t")
t = child_parsestr("t = mktemp(cleanup=false)[1]; @assert isfile(t); t")
@test isfile(t)
rm(t, force=true)
@test !ispath(t)
# mktemp with cleanup
t = child_eval("t = mktemp()[1]; @assert isfile(t); t")
t = child_parsestr("t = mktemp(cleanup=true)[1]; @assert isfile(t); t")
@test !ispath(t)
# mktempdir without cleanup
t = child_eval("t = mktempdir(cleanup=false); touch(joinpath(t, \"file.txt\")); t")
t = child_parsestr("t = mktempdir(cleanup=false); touch(joinpath(t, \"file.txt\")); t")
@test isfile("$t/file.txt")
rm(t, recursive=true, force=true)
@test !ispath(t)
# mktempdir with cleanup
t = child_eval("t = mktempdir(); touch(joinpath(t, \"file.txt\")); t")
t = child_parsestr("t = mktempdir(cleanup=true); touch(joinpath(t, \"file.txt\")); t")
@test !ispath(t)
# tempname without cleanup
t = child_eval("t = tempname(cleanup=false); touch(t); t")
t = child_parsestr("t = tempname(cleanup=false); touch(t); t")
@test isfile(t)
rm(t, force=true)
@test !ispath(t)
# tempname with cleanup
t = child_eval("t = tempname(); touch(t); t")
t = child_parsestr("t = tempname(cleanup=true); touch(t); t")
@test !ispath(t)
end

@test_broken "mktemp with cleanup=true failing on Windows CI"
#=
import Base.Filesystem: TEMP_CLEANUP_MIN, TEMP_CLEANUP_MAX, TEMP_CLEANUP

function with_temp_cleanup(f::Function, n::Int)
Expand Down Expand Up @@ -255,6 +257,7 @@ no_error_logging(f::Function) =
end
end
end
=#

#######################################################################
# This section tests some of the features of the stat-based file info #
Expand Down