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

Enable MOVEFILE_COPY_ALLOWED by default for renameFileInternal for Windows #189

Closed
jasagredo opened this issue Sep 18, 2024 · 2 comments
Closed

Comments

@jasagredo
Copy link

jasagredo commented Sep 18, 2024

Currently only MOVEFILE_REPLACE_EXISTING is enabled but I don't think there would be a downside to enable the mentioned flag. The documentation for that one says:

If the file is to be moved to a different volume, the function simulates the move by using the CopyFile and DeleteFile functions.

My motivation here is: I'm trying to make Cabal use the global temp directory for creating temp files instead of the final destination directory (as GetTempFileName doesn't support long paths, which is one of the few remaining uses of functions that can't use long paths in cabal) but the temp dir in GHA (and potentially in the user's system could be) is in C: and the working dir is in D:, so renaming paths fails because the mentioned flag is not set.

Similarly, if I override the temp dir to something in D: (like ${{ runner.temp }}) cabal can't copy files to the store which is then in C:.

@Rufflewind
Copy link
Member

Rufflewind commented Sep 19, 2024

renameFile is atomic on Unix-like systems and by design it will never work across file systems.

On Windows the situation is less clear (#109) but that doesn't mean we should intentionally weaken the whatever guarantees that might currently hold.

Enabling MOVEFILE_COPY_ALLOWED would cause the behavior to diverge further between Windows and Unix-like systems. This can lead to surprises for users when porting code from Windows to Linux or OS X where renameFile would fail when copying data across file systems.

The recommended solution is to copy the file to the destination and then delete the original files. This is effective for all platforms and does not rely on platform-specific features like MOVEFILE_COPY_ALLOWED. It also makes it self-evident that the operation is not atomic.

@jasagredo
Copy link
Author

I didn't know that renameFile could fail on Linux/OSX when copying across file systems. I think your argument for not enabling the flag is convincing. Thanks for explaining it.

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 a pull request may close this issue.

2 participants