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

make tempname on windows match unix behavior #25530

Merged
merged 1 commit into from
Jan 15, 2018
Merged

Conversation

JeffBezanson
Copy link
Member

Here is a possible fix to #9053. From re-reading the thread I decided the following:

  • While tempname is not recommended, it has uses and it would be annoying for us to remove it.
  • We could potentially change (and/or rename) the function to make it create (but not open) the file, which would be safer, but it's not clear that all use cases want the file to exist.

So this PR does the following:

  • Change tempname on windows not to create the file, like unix. According to the docs it is possible to call GetTempFileNameW such that it doesn't create the file.
  • Add a warning to the docs.
  • Deprecate (with an error) tempname(::UInt32) because it's just weird.
  • Change a use of tempname in Pkg to mktemp

@JeffBezanson JeffBezanson added the system:windows Affects only Windows label Jan 12, 2018
base/file.jl Outdated
@@ -344,6 +358,9 @@ tempdir()
tempname()

Generate a unique temporary file path.
Warning: this can lead to race conditions if another process obtains the same
Copy link
Member

Choose a reason for hiding this comment

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

You could make this a Documenter-style warning:

!!! warning

    This can lead to race conditions if another process obtains the same
    file name and creates the file before you are able to.
    Using [`mktemp()`](@ref) is recommended instead.

@ararslan ararslan added deprecation This change introduces or involves a deprecation filesystem Underlying file system and functions that use it labels Jan 12, 2018
@ararslan
Copy link
Member

+1, I like this approach. I was looking into this the other day and I found that in a lot of cases tempname is actually what you want, so this makes sense.

base/file.jl Outdated
@@ -344,6 +358,12 @@ tempdir()
tempname()

Generate a unique temporary file path.
Copy link
Member

Choose a reason for hiding this comment

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

It would probably be good to specify here that the file is not created.

@JeffBezanson JeffBezanson force-pushed the jb/tempname branch 3 times, most recently from 0209dc7 to 50d8320 Compare January 15, 2018 01:20
also deprecate `tempname(::UInt32)` and add a warning to the docs.
@JeffBezanson JeffBezanson changed the title RFC: make tempname on windows match unix behavior make tempname on windows match unix behavior Jan 15, 2018
@JeffBezanson JeffBezanson merged commit 5e2ff12 into master Jan 15, 2018
@JeffBezanson JeffBezanson deleted the jb/tempname branch January 15, 2018 17:38
@ararslan
Copy link
Member

Nice to see another 4-digit issue resolved. 🙂

@bjarthur
Copy link
Contributor

great to see more similarity in cross-platform behavior! in this regard, don't forget about #24626 and #24675

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation This change introduces or involves a deprecation filesystem Underlying file system and functions that use it system:windows Affects only Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants