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

os: Rename("oldDir", "newExistingDir") doesn't work on Windows #14527

Closed
RezaSR opened this Issue Feb 26, 2016 · 7 comments

Comments

Projects
None yet
8 participants
@RezaSR

RezaSR commented Feb 26, 2016

  1. What version of Go are you using (go version)?
    1.5.3
  2. What operating system and processor architecture are you using (go env)?
    windows - amd64
  3. What did you do?
    Rename a folder to new name that is already a folder. It works fine on linux-amd64 but cause error on windows-amd64

http://play.golang.org/p/m__tRp4mZh

  1. What did you expect to see?
    Replacement of newDir on windows as it does on linux.
  2. What did you see instead?
    Error. Access denied. Even running as administrator!
@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz
Member

bradfitz commented Feb 26, 2016

@bradfitz bradfitz added the OS-Windows label Feb 26, 2016

@bradfitz bradfitz added this to the Go1.7 milestone Feb 26, 2016

@bradfitz bradfitz changed the title from os.Rename("oldDir", "newExistingDir") to os: Rename("oldDir", "newExistingDir") doesn't work on Windows Feb 26, 2016

@RezaSR

This comment has been minimized.

Show comment
Hide comment
@RezaSR

RezaSR commented Feb 26, 2016

Thanks

@ianlancetaylor

This comment has been minimized.

Show comment
Hide comment
@ianlancetaylor

ianlancetaylor Feb 26, 2016

Contributor

Note that on GNU/Linux the os.Rename will only succeed if the destination directory is empty. If the destination directory has any files it will fail with ENOTEMPTY. I think we should consider checking for and rejecting rename to an existing directory on non-Windows.

Contributor

ianlancetaylor commented Feb 26, 2016

Note that on GNU/Linux the os.Rename will only succeed if the destination directory is empty. If the destination directory has any files it will fail with ENOTEMPTY. I think we should consider checking for and rejecting rename to an existing directory on non-Windows.

@alexbrainman

This comment has been minimized.

Show comment
Hide comment
@alexbrainman

alexbrainman Feb 28, 2016

Member

@bradfitz, latest change (CL 6140) of windows os.Rename introduced use of MoveFileEx. Our code uses MOVEFILE_REPLACE_EXISTING flag, I guess, to make "overwriting of destination file" work. But according to the doco https://msdn.microsoft.com/en-us/library/windows/desktop/aa365240(v=vs.85).aspx:

MOVEFILE_REPLACE_EXISTING ... This value cannot be used if lpNewFileName or lpExistingFileName names a directory.

That is why, I guess, MoveFileEx return ERROR_ACCESS_DENIED in that scenario. (Running example above I see MoveFileEx return ERROR_ACCESS_DENIED).

I will let you decide what to do here.

Alex

Member

alexbrainman commented Feb 28, 2016

@bradfitz, latest change (CL 6140) of windows os.Rename introduced use of MoveFileEx. Our code uses MOVEFILE_REPLACE_EXISTING flag, I guess, to make "overwriting of destination file" work. But according to the doco https://msdn.microsoft.com/en-us/library/windows/desktop/aa365240(v=vs.85).aspx:

MOVEFILE_REPLACE_EXISTING ... This value cannot be used if lpNewFileName or lpExistingFileName names a directory.

That is why, I guess, MoveFileEx return ERROR_ACCESS_DENIED in that scenario. (Running example above I see MoveFileEx return ERROR_ACCESS_DENIED).

I will let you decide what to do here.

Alex

@alexbrand

This comment has been minimized.

Show comment
Hide comment
@alexbrand

alexbrand Aug 26, 2016

@alexbrainman is there anything blocking this? can I be of any help?

alexbrand commented Aug 26, 2016

@alexbrainman is there anything blocking this? can I be of any help?

@alexbrainman

This comment has been minimized.

Show comment
Hide comment
@alexbrainman

alexbrainman Aug 28, 2016

Member

is there anything blocking this?

No, not really. Someone needs to come up with solution to this problem. I am not looking at this.

can I be of any help?

Feel free to propose solution to this.

Alex

Member

alexbrainman commented Aug 28, 2016

is there anything blocking this?

No, not really. Someone needs to come up with solution to this problem. I am not looking at this.

can I be of any help?

Feel free to propose solution to this.

Alex

@quentinmit quentinmit added the NeedsFix label Oct 10, 2016

@gopherbot

This comment has been minimized.

Show comment
Hide comment
@gopherbot

gopherbot commented Oct 18, 2016

CL https://golang.org/cl/31358 mentions this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.