search

google / renameio

Package renameio provides a way to atomically create or replace a file or symbolic link.
Apache License 2.0
609 stars 27 forks source link

WriteFile is broken on Windows #17

Closed dhaavi closed 4 years ago

dhaavi commented 5 years ago

I've read a couple issues on Windows support, and I'm not sure what the current support state is.

Anyway, the WriteFile function is broken on Windows, as *os.File.Chmod() used in the function is not supported on Windows: https://github.com/google/renameio/blob/a368f9987532a68a3d676566141654a81aa8100b/writefile.go#L29

twpayne commented 5 years ago

It does not seem possible to do atomic file replacement on Windows, see the discussion in #1.

If you want a chmod function that works on Windows, consider https://github.com/hectane/go-acl/blob/master/chmod.go#L14 (more recent version at https://github.com/twpayne/go-acl/blob/chezmoi/chmod.go#L14).

dhaavi commented 5 years ago

I also just noticed that these other referenced issues belong to another project: twpayne/chezmoi I guess that caused some confusion here.

While not being atomic, it would still be better than just writing files directly, wouldn't it?

I don't care about chmod other than it breaking Windows compatibility.

twpayne commented 5 years ago

The first link is to hectane/go-acl. This library has a pending pull request (https://github.com/hectane/go-acl/pull/14) that improves Windows support. The second link is to a friendly fork that includes that PR.

dhaavi commented 5 years ago

I think I did a poor job communicating what I am trying to achieve here.

In my case, I don't care about the chmod call, but want the library working.

Do you want to "fix" WriteFile by just leaving out the chmod call on Windows? As far as I understand, this is also how many Go std utils work: You give them permissions via os.FileMode, but they are actually only used on POSIX systems.

Or, do you want to let WriteFile broken as is on Windows until you find a real fix? Then I'd just remove the chmod call myself for the few times I need it.

stapelberg commented 5 years ago

I don’t do Windows development, so I’ll rely on a pull request containing a fix. Thanks in advance :)

andhe commented 4 years ago

Ok, so apparently f.Chmod(perm) errors out on windows as the syscall is not available but os.Chmod(filename, perm) does not error! Maybe we can just change the code to use the latter (with t.Name() as filename) ...

twpayne commented 4 years ago

This issue is (arguably) fixed by #20. It may be worth re-opening #1 for when Windows supports atomic file operations.