File::setReadOnly does not appear to work as expected on windows. It’s implemented by attempting to set the FILE_ATTRIBUTE_READONLY flag using the SetFileAttributes function. However, per the Windows API documentation:
Applications can read the file, but cannot write to it or delete it. This attribute is not honored on directories.
As such, the assert in the following code sample will trigger on windows:
File dir('path/to/file')
bool created = dir.createDirectory(); // created == true
bool set_readonly = dir.setReadOnly(true); // set_readonly == true
bool is_readonly = !dir.hasWriteAccess(); // is_readonly == false
// assert triggers - parent still has write access!
assert(is_readonly);
This appears to be a bug as the JUCE documentation states plainly that the function “Changes the write-permission of a file or directory.” Because this method fails silently, I think it should return false in the event it’s attempted on a windows directory. That’d have to be added as a special case, as for whatever reason, attempting to set FILE_ATTRIBUTE_READONLY on a directory still causes SetFileAttributes to return TRUE even though the attribute is ignored.
As far as I can tell, on Windows a folder itself doesn’t have a read only property. If you open the Properties window of a folder in Explorer the Read-only checkbox will always display an indeterminate state, and the note next to it says “Only applies to files in a folder”. So what the Windows API documentation suggests to me, is that this property is not applicable to directories.
If you call setReadOnly (true, true) asking for the contained files to be made read only that seems to work as expected.
The documentation for File::hasWriteAccess() specifically mentions only files, so we can’t expect the above assertion to hold for a directory.
Since the property is not applicable to folders I think it might be better to return a result indicating whether a failure occurred during the operation. If we always returned false on Windows then developers might need to add conditionals to always ignore the return value on Windows, which just sounds like boilerplate.
That’s fair, I didn’t realize that File::hasWriteAccess() doesn’t explicitly mention directories. However, File::setReadOnly is stated to:
[return] true if it manages to change the file’s permissions.
Given that setReadOnly(true) does not succeed in changing the permissions for any directory on Windows, I believe that constitutes a violation of what the documentation states the return value represents. Another similar example is the File::setExecutePermission call, which is hard-coded to return false on Windows, representing that the change in executable status does not occur.
As such, if you’re against returning false for these calls on Windows, I’d appreciate a caveat along these lines of the following in the documentation:
Calling file.setReadOnly(true, false) on Windows is undefined behavior if file.isDirectory() == true.
Otherwise, without knowing about this caveat about readonly flags on Windows directories, users could easily assume that file.setReadOnly(true, false) == true means that the directory has been made read-only, and then write buggy code based on that fair assumption.
