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 File.rename overwrite the destination file on Windows, like elsewhere #9038

Merged
merged 1 commit into from
Apr 10, 2020

Conversation

oprypin
Copy link
Member

@oprypin oprypin commented Apr 10, 2020

The compiler, for example, relies on it here:

File.rename(temporary_object_name, object_name)
But right now it throws an error that the file already exists.

You could argue whether it's desirable. For example, Python, to preserve backwards compatibility where the behavior differed like this for ages, decided to keep rename as replacing on Unix but erroring on Windows, but also added replace to replace on both. So on Unix they don't have any rename which would error out if the destination exists.

Another important caveat is that replacing a file like that is atomic on Unix, but on Windows that is impossible to achieve.

Let me know what you think, I could do the same as Python instead.

@RX14 RX14 added this to the 0.35.0 milestone Apr 10, 2020
@RX14 RX14 merged commit f3c8f31 into crystal-lang:master Apr 10, 2020
@RX14 RX14 added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:files labels Apr 10, 2020
Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

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

@RX14 That was a quick merge...
But I'll approve it after the fact 👍

@RX14
Copy link
Contributor

RX14 commented Apr 10, 2020

it's windows-only, you only need one approval

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. platform:windows topic:stdlib:files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants