www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Removing readonly files on windows with std.file

reply Jonathan Marler <johnnymarler gmail.com> writes:
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.

Note that deleting a directory/file from Windows Explorer or 
calling `del` from the command-line will remove READONLY files no 
problem.  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?  Although, the 
counter-argument to this would be that the standard mechanisms to 
delete files ignore the READONLY attribute and delete them 
anyway, so maybe D's default behavior should match?

We could also enhance the API to allow the user to specify the 
behavior.  By either defining new functions like 
"removeForce/rmdirForceRecurse", or adding an options argument 
remove(file, options), rmdirRecurse(dir, options).

What do people think is the right solution here?
Mar 07
next sibling parent reply Dominikus Dittes Scherkl <dominikus scherkl.de> writes:
On Saturday, 7 March 2020 at 20:01:07 UTC, Jonathan Marler wrote:
 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 always thought "readonly" was intended to protect from "modification" not from "deletion", so simply making the function consistent with the explorer would be the right thing to do. It's like a "const" variable: you cannot change its value, but of course you should be able to free the memory it occupies if it goes out of scope or is otherwise detectable not used anymore.
Mar 07
parent reply Patrick Schluter <Patrick.Schluter bbox.fr> writes:
On Saturday, 7 March 2020 at 22:51:46 UTC, Dominikus Dittes 
Scherkl wrote:
 On Saturday, 7 March 2020 at 20:01:07 UTC, Jonathan Marler 
 wrote:
 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 always thought "readonly" was intended to protect from "modification" not from "deletion", so simply making the function consistent with the explorer would be the right thing to do. It's like a "const" variable: you cannot change its value, but of course you should be able to free the memory it occupies if it goes out of scope or is otherwise detectable not used anymore.
Microsoft says [1] FILE_ATTRIBUTE_READONLY A file that is read-only. Applications can read the file, but cannot write to it or delete it. This attribute is not honored on directories. So the behaviour of del or the explorer is violating theur own definition. [1]: https://docs.microsoft.com/en-us/windows/win32/fileio/file-attribute-constants
Mar 08
next sibling parent Les De Ridder <les lesderid.net> writes:
On Sunday, 8 March 2020 at 09:04:34 UTC, Patrick Schluter wrote:
 [...]

 So the behaviour of del or the explorer is violating theur own 
 definition.
Such inconsistencies are sadly fairly common in Win32 and other Microsoft docs.
Mar 08
prev sibling parent Mann <m a.nn> writes:
On Sunday, 8 March 2020 at 09:04:34 UTC, Patrick Schluter wrote:
 Microsoft says [1]
 FILE_ATTRIBUTE_READONLY
 A file that is read-only. Applications can read the file, but 
 cannot write to it or delete it. This attribute is not honored 
 on directories.

 So the behaviour of del or the explorer is violating theur own 
 definition.


 [1]: 
 https://docs.microsoft.com/en-us/windows/win32/fileio/file-attribute-constants
Perhaps a small addendum to rmdirRecurse's documentation concerning this behavior would be appropriate?
Mar 08
prev sibling next sibling parent Vladimir Panteleev <thecybershadow.lists gmail.com> writes:
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/6177cd442d9737b8e8141e30197fe124c5aaba 8/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.
Mar 07
prev sibling next sibling parent Kagamin <spam here.lot> writes:
On Saturday, 7 March 2020 at 20:01:07 UTC, Jonathan Marler wrote:
 Note that deleting a directory/file from Windows Explorer or 
 calling `del` from the command-line will remove READONLY files 
 no problem.
del gives me "Access denied" error.
Mar 07
prev sibling next sibling parent reply WebFreak001 <d.forum webfreak.org> writes:
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.

 Note that deleting a directory/file from Windows Explorer or 
 calling `del` from the command-line will remove READONLY files 
 no problem.  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?  Although, the 
 counter-argument to this would be that the standard mechanisms 
 to delete files ignore the READONLY attribute and delete them 
 anyway, so maybe D's default behavior should match?

 We could also enhance the API to allow the user to specify the 
 behavior.  By either defining new functions like 
 "removeForce/rmdirForceRecurse", or adding an options argument 
 remove(file, options), rmdirRecurse(dir, options).

 What do people think is the right solution here?
I made a package for this solving this exact issue I had too with the .git folder! https://code.dlang.org/packages/rm-rf https://github.com/WebFreak001/rm-rf/blob/master/source/rm/rf.d It's public domain so you can just look over the few lines it has and decide if you want to copy paste the file into your project or depend on it on dub :p
Mar 08
parent notna <notna.remove.this ist-einmalig.de> writes:
On Sunday, 8 March 2020 at 16:38:37 UTC, WebFreak001 wrote:
.
.
.

 I made a package for this solving this exact issue I had too 
 with the .git folder!

 https://code.dlang.org/packages/rm-rf
 https://github.com/WebFreak001/rm-rf/blob/master/source/rm/rf.d

 It's public domain so you can just look over the few lines it 
 has and decide if you want to copy paste the file into your 
 project or depend on it on dub :p
Hmm... How about https://code.dlang.org/packages/scriptlike? That's were I would have looked for such an "extension"..
Mar 08
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 3/7/2020 12:01 PM, Jonathan Marler wrote:
 We could modify std.file.remove to be able to remove readonly files,
This is definitely the wrong approach for the default behavior. It should fail and the app then remove the readonly attribute and try again, or the remove() should have a "force" non-default option.
Mar 09
parent Vino <akashvino79 gmail.com> writes:
On Monday, 9 March 2020 at 22:55:10 UTC, Walter Bright wrote:
 On 3/7/2020 12:01 PM, Jonathan Marler wrote:
 We could modify std.file.remove to be able to remove readonly 
 files,
This is definitely the wrong approach for the default behavior. It should fail and the app then remove the readonly attribute and try again, or the remove() should have a "force" non-default option.
I am fine with whatever method this forum decides in implementing this function, at present I have been using the below code to remove the read-only attribute and then delete the file foreach(d; parallel(dFiles[],1)) { foreach(f; dirEntries(join([`\\?\`,d[0]]), SpanMode.depth, false)) { if (f.isFile) { setAttributes(f, 0x80); remove(f); } else { rmdir(f); } } rmdirRecurse(d[0]); } } } I have been using this code for one of my old project since past 2 year’s and below are my observation. The above code works for 90% of the time. Meaning we execute this script via task scheduler on daily basis, it works for few weeks and error out stating, “Access Denied”. The execution time is very costly as the above function deletes TB’s of file daily So I request you to look into the above 2 point while implementing this solution From, Vino
Mar 10