digitalmars.D - Removing readonly files on windows with std.file
- Jonathan Marler (25/25) Mar 07 2020 After dealing with annoying failures to remove directories on
- Dominikus Dittes Scherkl (7/10) Mar 07 2020 I always thought "readonly" was intended to protect from
- Patrick Schluter (11/24) Mar 08 2020 Microsoft says [1]
- Les De Ridder (3/6) Mar 08 2020 Such inconsistencies are sadly fairly common in Win32 and other
- Mann (3/12) Mar 08 2020 Perhaps a small addendum to rmdirRecurse's documentation
- Vladimir Panteleev (47/65) Mar 07 2020 I don't think this should affect what Phobos does, as the above
- Kagamin (2/5) Mar 07 2020 del gives me "Access denied" error.
- WebFreak001 (8/33) Mar 08 2020 I made a package for this solving this exact issue I had too with
- notna (6/13) Mar 08 2020 On Sunday, 8 March 2020 at 16:38:37 UTC, WebFreak001 wrote:
- Walter Bright (4/5) Mar 09 2020 This is definitely the wrong approach for the default behavior.
- Vino (22/29) Mar 10 2020 I am fine with whatever method this forum decides in implementing
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 2020
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 2020
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: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-constantsWe 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 08 2020
Les De Ridder <les lesderid.net> 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 2020
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-constantsPerhaps a small addendum to rmdirRecurse's documentation concerning this behavior would be appropriate?
Mar 08 2020
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 2020
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 2020
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 2020
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 :pHmm... How about https://code.dlang.org/packages/scriptlike? That's were I would have looked for such an "extension"..
Mar 08 2020
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 2020
On Monday, 9 March 2020 at 22:55:10 UTC, Walter Bright wrote:On 3/7/2020 12:01 PM, Jonathan Marler wrote: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, VinoWe 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 10 2020