gulrak / filesystem

An implementation of C++17 std::filesystem for C++11 /C++14/C++17/C++20 on Windows, macOS, Linux and FreeBSD.
MIT License
1.34k stars 173 forks source link

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

Closed wang-zuxian closed 3 years ago

wang-zuxian commented 3 years ago

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 commented 3 years ago

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

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

wang-zuxian commented 3 years ago

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 commented 3 years ago

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: https://github.com/microsoft/STL/issues/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 commented 3 years ago

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 commented 3 years ago

This is now part of release v1.5.6