Re: [PATCH] Fix overwriting files with fs::copy_file on windows
On Sun, 24 Mar 2024 at 21:34, Björn Schäpers wrote: > > From: Björn Schäpers > > 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 > --- > 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, ); } > + > +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, ); } > - > - 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(); > -
Re: [PATCH] Fix overwriting files with fs::copy_file on windows
Am 25.04.2024 um 22:16 schrieb Björn Schäpers: Am 24.03.2024 um 22:34 schrieb Björn Schäpers: From: Björn Schäpers 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 --- 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); +} // 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, ); } + + 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; +} +#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, ); } - - 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 +++
Re: [PATCH] Fix overwriting files with fs::copy_file on windows
Am 24.03.2024 um 22:34 schrieb Björn Schäpers: From: Björn Schäpers 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 --- 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); +} // 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, ); } + +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; +} +#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, ); } - - 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
[PATCH] Fix overwriting files with fs::copy_file on windows
From: Björn Schäpers 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 --- 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); +} // 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, ); } + +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; +} +#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, ); } - - 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 ---