code review 4984051: os: os.RemoveAll has to check for 2 error codes on Wind...

os: os.RemoveAll has to check for 2 error codes on Windows. ENOENT is not enough.
os.Lstat can return ENOTDIR as well.

[jp, 2011-09-06 08:35:17]

Hello golang-dev@googlegroups.com (cc: alex.brainman@gmail.com,
golang-dev@googlegroups.com, rsc@golang.org),

I'd like you to review this change to
https://go.googlecode.com/hg/

[r2, 2011-09-06 17:17:25]

what's the difference between ENOENT and
Errno(syscall.ERROR_FILE_NOT_FOUND)? it looks like the windows syscall
code is making a distinction without a difference and should be
changed.

-rob

[rsc, 2011-09-07 18:11:29]

ERROR_PATH_NOT_FOUND and ERROR_FILE_NOT_FOUND
should turn into ENOENT in package syscall, so that higher
level packages like os don't need to know about all the
different spellings.

Thanks.
Russ

[jp, 2011-09-07 18:39:40]

Sounds like a right solution.

Although it can be a bit tricky.
ERROR_PATH_NOT_FOUND turns into ENOTDIR in os.OpenFile, but expected to turn
into ENOENT for os.Lstat.
And to which of them (or something third?) should it turn for other functions:
os.StartProcess(), etc ?
It requires some investigation.

On 2011/09/07 18:11:29, rsc wrote:
> ERROR_PATH_NOT_FOUND and ERROR_FILE_NOT_FOUND
> should turn into ENOENT in package syscall, so that higher
> level packages like os don't need to know about all the
> different spellings.
> 
> Thanks.
> Russ

[brainman, 2011-09-08 01:34:20]

On 2011/09/07 18:39:40, jp wrote:
> 
> Although it can be a bit tricky.
> ERROR_PATH_NOT_FOUND turns into ENOTDIR in os.OpenFile, but expected to turn
> into ENOENT for os.Lstat.

I agree, it starts to get messy. But let's keep going with it for now. Please,
change windows os.Stat/os.Lstat to return ENOENT, as os.removeAll expects. Thank
you.

Alex

[jp, 2011-09-08 06:57:11]

Alex, PTAL

First, os.Lstat can return ENOTDIR even on Linux, so os.RemoveAll has to check
for 2 error codes anyway.
Second, I think ENOTDIR should correspond to ERROR_PATH_NOT_FOUND, not to
ERROR_DIRECTORY.
ERROR_DIRECTORY is a rare error and it does not mean "dir not found".
It raises when you try to CreateDirectory with a forbidden name like "LPT1"

On 2011/09/08 01:34:20, brainman wrote:
> On 2011/09/07 18:39:40, jp wrote:
> > 
> > Although it can be a bit tricky.
> > ERROR_PATH_NOT_FOUND turns into ENOTDIR in os.OpenFile, but expected to turn
> > into ENOENT for os.Lstat.
> 
> I agree, it starts to get messy. But let's keep going with it for now. Please,
> change windows os.Stat/os.Lstat to return ENOENT, as os.removeAll expects.
Thank
> you.
> 
> Alex

[brainman, 2011-09-08 07:25:24]

LGTM.

I am always happy with less code. Thank you.

[brainman, 2011-09-08 07:27:49]

*** Submitted as http://code.google.com/p/go/source/detail?r=60d5fcd03b3b ***

os: os.RemoveAll has to check for 2 error codes on Windows. ENOENT is not
enough.
os.Lstat can return ENOTDIR as well.

R=golang-dev, r, alex.brainman
CC=golang-dev, rsc
http://codereview.appspot.com/4984051

Committer: Alex Brainman <alex.brainman@gmail.com>

[jp, 2011-09-08 18:43:01]

I forget to upload ztypes_windows.go with ERROR_DIRECTORY removed

On 2011/09/08 07:27:49, brainman wrote:
> *** Submitted as http://code.google.com/p/go/source/detail?r=60d5fcd03b3b ***
> 
> os: os.RemoveAll has to check for 2 error codes on Windows. ENOENT is not
> enough.
> os.Lstat can return ENOTDIR as well.
> 
> R=golang-dev, r, alex.brainman
> CC=golang-dev, rsc
> http://codereview.appspot.com/4984051
> 
> Committer: Alex Brainman <mailto:alex.brainman@gmail.com>