Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't remove readonly files on Windows, is it supported? #121

Closed
wang-zuxian opened this issue May 14, 2021 · 5 comments
Closed

Can't remove readonly files on Windows, is it supported? #121

wang-zuxian opened this issue May 14, 2021 · 5 comments
Assignees
Labels
available on master Fix is done on master branch, issue closed on next release bug Something isn't working Windows Windows platform is affected
Milestone

Comments

@wang-zuxian
Copy link

Describe the bug
Can't remove readonly files on Windows.

To Reproduce
Create an txt file and change the property to readonly, then use ghc::filesystem::remove() to delete this file, it can't be deleted.

Expected behavior
A clear and concise description of what you expected to happen.

Additional context

@wang-zuxian wang-zuxian changed the title Can't remove readonly files on Windows, is it support? Can't remove readonly files on Windows, is it supported? May 14, 2021
@wang-zuxian
Copy link
Author

attr &= ~FILE_ATTRIBUTE_READONLY;
SetFileAttributesW(cstr, attr);

It can work when add this code in ghc::filesystem::remove()

@wang-zuxian
Copy link
Author

GHC_INLINE bool remove(const path& p, std::error_code& ec) noexcept
{
... ...
DWORD attr = GetFileAttributesW(cstr);
if (attr == INVALID_FILE_ATTRIBUTES) {
auto error = ::GetLastError();
if (error == ERROR_FILE_NOT_FOUND || error == ERROR_PATH_NOT_FOUND) {
return false;
}
ec = detail::make_system_error(error);
}
if (!ec) {
attr &= ~FILE_ATTRIBUTE_READONLY;
SetFileAttributesW(cstr, attr);

    if (attr & FILE_ATTRIBUTE_DIRECTORY) {
        if (!RemoveDirectoryW(cstr)) {
            ec = detail::make_system_error();
        }
    }
    else {
        if (!DeleteFileW(cstr)) {
            ec = detail::make_system_error();
        }
    }
}

#else
... ...
}

@gulrak
Copy link
Owner

gulrak commented May 14, 2021

Thank you for reporting this. The current behavior is consistent with that of the official implementation from Microsoft, but it looks like this is listed as a Bug there too: microsoft/STL#1511 for about half a year and the PR that tries to fix it is open since beginning of this year.

I'm not sure how I want to tackle this, so I need to look into this more deeply. Thanks also, for suggesting a fix, but it comes at the price slowing down all deletions, even of non-readonly files and I have scenarios where hundreds of thousands of files are been deleted, so I would prefer to maybe solve it by reacting on the deletion not succeeding and retry with changed attributes.

I'll look into it.

@gulrak gulrak self-assigned this May 14, 2021
@gulrak gulrak added bug Something isn't working Windows Windows platform is affected labels May 14, 2021
@gulrak gulrak added this to the v1.5.6 milestone May 18, 2021
@gulrak
Copy link
Owner

gulrak commented May 23, 2021

In the end I followed your suggestion with some additional error handling. Thank you for the prepared work! I'll try to optimize fs::remove in a future change, but I want to have this fixed now, as I wan't to get v1.5.6 out with this fixed.

gulrak added a commit that referenced this issue May 23, 2021
@gulrak gulrak added the available on master Fix is done on master branch, issue closed on next release label May 23, 2021
@gulrak
Copy link
Owner

gulrak commented May 24, 2021

This is now part of release v1.5.6

@gulrak gulrak closed this as completed May 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
available on master Fix is done on master branch, issue closed on next release bug Something isn't working Windows Windows platform is affected
Projects
None yet
Development

No branches or pull requests

2 participants