Removing readonly files on windows with std.file

[Vladimir Panteleev]

On Saturday, 7 March 2020 at 20:01:07 UTC, Jonathan Marler wrote:
> After dealing with annoying failures to remove directories on 
> windows with std.file.rmdirRecurse for too long, I finally 
> decided to look into it.
>
> It looks like rmdirRecurse fails because I'm trying to remove a 
> directory that contains a git repository in it.  It turns out 
> that git marks some files as READONLY to indicate that they 
> shouldn't be modified.  However, this will cause the DeleteFile 
> function (which is what rmdirRecurse will eventually call) to 
> fail.  The docs say that you must remove the READONLY attribute 
> before the file can be deleted.

This sounds right to me.

> Note that deleting a directory/file from Windows Explorer or 
> calling `del` from the command-line will remove READONLY files 
> no problem.

I don't think this should affect what Phobos does, as the above 
are interfaces for humans and Phobos is an OS API wrapper. 
(Though, in the case of rmdirRecurse, it's not as clear because 
it is an utility function.)

>  I could create a pull request to modify std.file to handle 
> this, however, I'm not sure what the right solution is.  We 
> could modify std.file.remove to be able to remove readonly 
> files, but maybe there's use cases where people want the 
> removal to fail if it is marked as readonly?

I think the function should do no more than the OS primitive API.

As far as I can see, the steps to create such a file from with in 
D would be:

1. Create the file.
2. Mark it read-only.

The inverse operation, again from D, would then be:

2. Unmark it as read-only.
1. Delete the file.

No problem here.

If you begin to do "magic" and "do what I mean" things, you open 
yourself up to questions like "why only the readonly attribute? 
why not also the ACL lists? and what about other platforms?" and 
it goes downhill from there. One similar example of this is that 
Tango's initial directory iteration primitive skipped hidden and 
system files (because that's what the shell does, duh), which as 
you can imagine caused a lot of pain should one such file ever 
come across your program.

I think the correct solution is to make sure that it is easy to 
recursively delete a directory (while clearing the readonly 
attribute if that's what the user wants) using only Phobos 
directory iteration primitives (dirEntries). If it's not then 
that is where the improvements ought to be done.

The current dirEntries API is definitely suboptimal in that error 
handling is difficult if you want to skip parts of the tree that 
fail to iterate, and that you can't selectively choose which 
directories to recurse into. I tried to address these with an 
alternative approach in my library 
(https://github.com/CyberShadow/ae/blob/6177cd442d9737b8e8141e30197fe124c5aaba18/sys/file.d#L176), which is also many times faster than dirEntries in many cases due to allocating less and not needing to stat unless absolutely necessary.

I also have a few "improved" rmdirRecurse variants which solve 
this problem as well:

https://github.com/CyberShadow/ae/blob/6177cd442d9737b8e8141e30197fe124c5aaba18/sys/file.d#L1002

However, I don't think they are suitable for Phobos, because 
there is just too much variation in what exactly "recursively 
delete a directory" should mean - so, in your case, a variant of 
rmdirRecurse which now works on git directories will fail due to 
ACLs, or not do what the user expects if the given path is a 
symlink, or other of many such questions.
Therefore, I think we should ensure that the primitives are there 
and easily accessible and programs should build their own utility 
functions to delete directory trees with whatever properties they 
need to care about.