On Sun, 24 Mar 2024 at 21:34, Björn Schäpers <g...@hazardy.de> wrote: > > From: Björn Schäpers <bjo...@hazardy.de> > > This fixes i.e. https://github.com/msys2/MSYS2-packages/issues/1937 > I don't know if I picked the right way to do it. > > When acceptable I think the declaration should be moved into > ops-common.h, since then we could use stat_type and also use that in the > commonly used function. > > Manually tested on i686-w64-mingw32. > > -- >8 -- > libstdc++: Fix overwriting files on windows > > The inodes have no meaning on windows, thus all files have an inode of > 0. Use a differenz approach to identify equivalent files. As a result > std::filesystem::copy_file did not honor > copy_options::overwrite_existing. Factored the method out of > std::filesystem::equivalent. > > libstdc++-v3/Changelog: > > * include/bits/fs_ops.h: Add declaration of > __detail::equivalent_win32. > * src/c++17/fs_ops.cc (__detail::equivalent_win32): Implement it > (fs::equivalent): Use __detail::equivalent_win32, factored the > old test out. > * src/filesystem/ops-common.h (_GLIBCXX_FILESYSTEM_IS_WINDOWS): > Use the function. > > Signed-off-by: Björn Schäpers <bjo...@hazardy.de> > --- > libstdc++-v3/include/bits/fs_ops.h | 8 +++ > libstdc++-v3/src/c++17/fs_ops.cc | 79 +++++++++++++----------- > libstdc++-v3/src/filesystem/ops-common.h | 10 ++- > 3 files changed, 60 insertions(+), 37 deletions(-) > > diff --git a/libstdc++-v3/include/bits/fs_ops.h > b/libstdc++-v3/include/bits/fs_ops.h > index 90650c47b46..d10b78a4bdd 100644 > --- a/libstdc++-v3/include/bits/fs_ops.h > +++ b/libstdc++-v3/include/bits/fs_ops.h > @@ -40,6 +40,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > > namespace filesystem > { > +#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS > +namespace __detail > +{ > + bool > + equivalent_win32(const wchar_t* p1, const wchar_t* p2, error_code& ec);
I don't think we want this declared in the public header, it should be internal to the library. Can it just be declared in ops-common.h instead? > +} // namespace __detail > +#endif //_GLIBCXX_FILESYSTEM_IS_WINDOWS > + > /** @addtogroup filesystem > * @{ > */ > diff --git a/libstdc++-v3/src/c++17/fs_ops.cc > b/libstdc++-v3/src/c++17/fs_ops.cc > index 61df19753ef..3cc87d45237 100644 > --- a/libstdc++-v3/src/c++17/fs_ops.cc > +++ b/libstdc++-v3/src/c++17/fs_ops.cc > @@ -67,6 +67,49 @@ > namespace fs = std::filesystem; > namespace posix = std::filesystem::__gnu_posix; > > +#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS > +bool > +fs::__detail::equivalent_win32(const wchar_t* p1, const wchar_t* p2, > + error_code& ec) > +{ > + struct auto_handle { > + explicit auto_handle(const path& p_) > + : handle(CreateFileW(p_.c_str(), 0, > + FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE, > + 0, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, 0)) > + { } > + > + ~auto_handle() > + { if (*this) CloseHandle(handle); } > + > + explicit operator bool() const > + { return handle != INVALID_HANDLE_VALUE; } > + > + bool get_info() > + { return GetFileInformationByHandle(handle, &info); } > + > + HANDLE handle; > + BY_HANDLE_FILE_INFORMATION info; > + }; > + auto_handle h1(p1); This creates a new filesystem::path, just to call c_str() on it immediately. The auto_handle ctor should be changed to just take const wchar_t* so we don't need to allocate and parse new path objects. > + auto_handle h2(p2); > + if (!h1 || !h2) > + { > + if (!h1 && !h2) > + ec = __last_system_error(); > + return false; > + } > + if (!h1.get_info() || !h2.get_info()) > + { > + ec = __last_system_error(); > + return false; > + } > + return h1.info.dwVolumeSerialNumber == h2.info.dwVolumeSerialNumber > + && h1.info.nFileIndexHigh == h2.info.nFileIndexHigh > + && h1.info.nFileIndexLow == h2.info.nFileIndexLow; > +} > +#endif //_GLIBCXX_FILESYSTEM_IS_WINDOWS > + > fs::path > fs::absolute(const path& p) > { > @@ -858,41 +901,7 @@ fs::equivalent(const path& p1, const path& p2, > error_code& ec) noexcept > if (st1.st_mode != st2.st_mode || st1.st_dev != st2.st_dev) > return false; > > - struct auto_handle { > - explicit auto_handle(const path& p_) > - : handle(CreateFileW(p_.c_str(), 0, > - FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE, > - 0, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, 0)) > - { } > - > - ~auto_handle() > - { if (*this) CloseHandle(handle); } > - > - explicit operator bool() const > - { return handle != INVALID_HANDLE_VALUE; } > - > - bool get_info() > - { return GetFileInformationByHandle(handle, &info); } > - > - HANDLE handle; > - BY_HANDLE_FILE_INFORMATION info; > - }; > - auto_handle h1(p1); > - auto_handle h2(p2); > - if (!h1 || !h2) > - { > - if (!h1 && !h2) > - ec = __last_system_error(); > - return false; > - } > - if (!h1.get_info() || !h2.get_info()) > - { > - ec = __last_system_error(); > - return false; > - } > - return h1.info.dwVolumeSerialNumber == h2.info.dwVolumeSerialNumber > - && h1.info.nFileIndexHigh == h2.info.nFileIndexHigh > - && h1.info.nFileIndexLow == h2.info.nFileIndexLow; > + return __detail::equivalent_win32(p1.c_str(), p2.c_str(), ec); > #else > return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino; > #endif > diff --git a/libstdc++-v3/src/filesystem/ops-common.h > b/libstdc++-v3/src/filesystem/ops-common.h > index d917fddbeb1..7e67286bd01 100644 > --- a/libstdc++-v3/src/filesystem/ops-common.h > +++ b/libstdc++-v3/src/filesystem/ops-common.h > @@ -489,8 +489,14 @@ _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM > return false; > } > > - if (to_st->st_dev == from_st->st_dev > - && to_st->st_ino == from_st->st_ino) > +#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS > + // st_ino is not set, so can't be used to distinguish files > + std::error_code not_used; > + if (st1.st_mode != st2.st_mode || st1.st_dev != st2.st_dev || > + fs::__detail::equivalent_win32(from, to, not_used)) > +#else > + if (st1.st_dev != st2.st_dev || st1.st_ino != st2.st_ino) > +#endif > { > ec = std::make_error_code(std::errc::file_exists); > return false; > -- > 2.44.0 >