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

WriteFile is broken on Windows #17

Closed
dhaavi opened this issue Aug 21, 2019 · 7 comments
Closed

WriteFile is broken on Windows #17

dhaavi opened this issue Aug 21, 2019 · 7 comments

Comments

@dhaavi
Copy link

dhaavi commented Aug 21, 2019

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:

if err := t.Chmod(perm); err != nil {

@dhaavi dhaavi changed the title Writefile broken on Windows Writefile is broken on Windows Aug 21, 2019
@dhaavi dhaavi changed the title Writefile is broken on Windows WriteFile is broken on Windows Aug 21, 2019
@twpayne
Copy link
Contributor

twpayne commented Aug 21, 2019

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
Copy link
Author

dhaavi commented Aug 21, 2019

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
Copy link
Contributor

twpayne commented Aug 21, 2019

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

@dhaavi
Copy link
Author

dhaavi commented Aug 21, 2019

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
Copy link
Collaborator

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

@andhe
Copy link

andhe commented Sep 6, 2020

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
Copy link
Contributor

twpayne commented Sep 18, 2020

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

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.

4 participants