[PATCH] Fix overwriting files with fs::copy_file on windows

[PATCH] Fix overwriting files with fs::copy_file on windows

[Björn Schäpers]

From: Björn Schäpers <bjoern@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 <bjoern@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);
+} // 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);
+  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

Re: [PATCH] Fix overwriting files with fs::copy_file on windows

[Björn Schäpers]

Am 24.03.2024 um 22:34 schrieb Björn Schäpers:
> From: Björn Schäpers <bjoern@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 <bjoern@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);
> +} // 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);
> +  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;

A friendly ping.

Re: [PATCH] Fix overwriting files with fs::copy_file on windows

[One of your IPs tried to hack me]

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 <bjoern@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 <bjoern@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);
>> +} // 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);
>> +  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;
> 
> A friendly ping.

Ping.

Re: [PATCH] Fix overwriting files with fs::copy_file on windows

[Jonathan Wakely]

On Sun, 24 Mar 2024 at 21:34, Björn Schäpers <gcc@hazardy.de> wrote:
>
> From: Björn Schäpers <bjoern@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 <bjoern@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
>

Re: [PATCH] Fix overwriting files with fs::copy_file on windows

[Jonathan Wakely]

On Sun, 24 Mar 2024 at 21:34, Björn Schäpers <gcc@hazardy.de> wrote:
>
> From: Björn Schäpers <bjoern@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 <bjoern@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);
> +} // 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);
> +  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

What's this code doing? You seem to have inverted the logic, so that
it now returns a file_exists error when the files are *not* the same.

Is this just a copy & paste error from the similar code in
fs::equivalent? Was this tested?

>           {
>             ec = std::make_error_code(std::errc::file_exists);
>             return false;
> --
> 2.44.0
>

[committed] libstdc++: Fix overwriting files with fs::copy_file on Windows

[Jonathan Wakely]

I've pushed this for https://github.com/msys2/MSYS2-packages/issues/1937
but I'm taking a slightly different approach to Björn's original patch.

Instead of adding __detail::equivalent_win32 I'm adding fs::equiv_files
to do the check for both POSIX and Windows. The logic in do_copy_file
should be correct here, and fs::copy also needed an update to fix the
case of overwriting existing directories. I've also added tests.

Tested x86_64-linux and x86_64-w64-mingw32. Pushed to trunk. I'll
backport this after some time on trunk.

-- >8 --

There are no inode numbers on Windows filesystems, so stat_type::st_ino
is always zero and the check for equivalent files in do_copy_file was
incorrectly identifying distinct files as equivalent. This caused
copy_file to incorrectly report errors when trying to overwrite existing
files.

The fs::equivalent function already does the right thing on Windows, so
factor that logic out into a new function that can be reused by
fs::copy_file.

The tests for fs::copy_file were quite inadequate, so this also adds
checks for that function's error conditions.

libstdc++-v3/ChangeLog:

	* src/c++17/fs_ops.cc (auto_win_file_handle): Change constructor
	parameter from const path& to const wchar_t*.
	(fs::equiv_files): New function.
	(fs::equivalent): Use equiv_files.
	* src/filesystem/ops-common.h (fs::equiv_files): Declare.
	(do_copy_file): Use equiv_files.
	* src/filesystem/ops.cc (fs::equiv_files): Define.
	(fs::copy, fs::equivalent): Use equiv_files.
	* testsuite/27_io/filesystem/operations/copy.cc: Test
	overwriting directory contents recursively.
	* testsuite/27_io/filesystem/operations/copy_file.cc: Test
	overwriting existing files.
---
 libstdc++-v3/src/c++17/fs_ops.cc              |  71 ++++++----
 libstdc++-v3/src/filesystem/ops-common.h      |  12 +-
 libstdc++-v3/src/filesystem/ops.cc            |  18 ++-
 .../27_io/filesystem/operations/copy.cc       |   9 ++
 .../27_io/filesystem/operations/copy_file.cc  | 122 ++++++++++++++++++
 5 files changed, 199 insertions(+), 33 deletions(-)

diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc
index 81227c49dfd..7ffdce67782 100644
--- a/libstdc++-v3/src/c++17/fs_ops.cc
+++ b/libstdc++-v3/src/c++17/fs_ops.cc
@@ -350,7 +350,7 @@ fs::copy(const path& from, const path& to, copy_options options,
   f = make_file_status(from_st);
 
   if (exists(t) && !is_other(t) && !is_other(f)
-      && to_st.st_dev == from_st.st_dev && to_st.st_ino == from_st.st_ino)
+      && fs::equiv_files(from.c_str(), from_st, to.c_str(), to_st, ec))
     {
       ec = std::make_error_code(std::errc::file_exists);
       return;
@@ -829,8 +829,8 @@ namespace
   struct auto_win_file_handle
   {
     explicit
-    auto_win_file_handle(const fs::path& p_)
-    : handle(CreateFileW(p_.c_str(), 0,
+    auto_win_file_handle(const wchar_t* p)
+    : handle(CreateFileW(p, 0,
 			 FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,
 			 0, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, 0))
     { }
@@ -850,6 +850,44 @@ namespace
 }
 #endif
 
+#ifdef _GLIBCXX_HAVE_SYS_STAT_H
+#ifdef NEED_DO_COPY_FILE // Only define this once, not in cow-ops.o too
+bool
+fs::equiv_files([[maybe_unused]] const char_type* p1, const stat_type& st1,
+		[[maybe_unused]] const char_type* p2, const stat_type& st2,
+		[[maybe_unused]] error_code& ec)
+{
+#if ! _GLIBCXX_FILESYSTEM_IS_WINDOWS
+  // For POSIX the device ID and inode number uniquely identify a file.
+  return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino;
+#else
+  // For Windows st_ino is not set, so can't be used to distinguish files.
+  // We can compare modes and device IDs as a cheap initial check:
+  if (st1.st_mode != st2.st_mode || st1.st_dev != st2.st_dev)
+    return false;
+
+  // Need to use GetFileInformationByHandle to get more info about the files.
+  auto_win_file_handle h1(p1);
+  auto_win_file_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
+}
+#endif // NEED_DO_COPY_FILE
+#endif // _GLIBCXX_HAVE_SYS_STAT_H
+
 bool
 fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
 {
@@ -881,30 +919,7 @@ fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
       ec.clear();
       if (is_other(s1) || is_other(s2))
 	return false;
-#if _GLIBCXX_FILESYSTEM_IS_WINDOWS
-      // st_ino is not set, so can't be used to distinguish files
-      if (st1.st_mode != st2.st_mode || st1.st_dev != st2.st_dev)
-	return false;
-
-      auto_win_file_handle h1(p1);
-      auto_win_file_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;
-#else
-      return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino;
-#endif
+      return fs::equiv_files(p1.c_str(), st1, p2.c_str(), st2, ec);
     }
   else if (!exists(s1) || !exists(s2))
     ec = std::make_error_code(std::errc::no_such_file_or_directory);
@@ -992,7 +1007,7 @@ std::uintmax_t
 fs::hard_link_count(const path& p, error_code& ec) noexcept
 {
 #if _GLIBCXX_FILESYSTEM_IS_WINDOWS
-  auto_win_file_handle h(p);
+  auto_win_file_handle h(p.c_str());
   if (h && h.get_info())
     return static_cast<uintmax_t>(h.info.nNumberOfLinks);
   ec = __last_system_error();
diff --git a/libstdc++-v3/src/filesystem/ops-common.h b/libstdc++-v3/src/filesystem/ops-common.h
index d917fddbeb1..1be8d754d6f 100644
--- a/libstdc++-v3/src/filesystem/ops-common.h
+++ b/libstdc++-v3/src/filesystem/ops-common.h
@@ -303,7 +303,6 @@ namespace __gnu_posix
   {
     bool skip, update, overwrite;
   };
-
 #endif // _GLIBCXX_HAVE_SYS_STAT_H
 
 } // namespace filesystem
@@ -327,6 +326,12 @@ _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM
 	   std::error_code&);
 
 
+  // Test whether two files are the same file.
+  bool
+  equiv_files(const char_type*, const stat_type&,
+	      const char_type*, const stat_type&,
+	      error_code&);
+
   inline file_type
   make_file_type(const stat_type& st) noexcept
   {
@@ -400,6 +405,7 @@ _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM
     return true;
   }
 #endif
+
 #if defined _GLIBCXX_USE_SENDFILE && ! defined _GLIBCXX_FILESYSTEM_IS_WINDOWS
   bool
   copy_file_sendfile(int fd_in, int fd_out, size_t length) noexcept
@@ -428,6 +434,7 @@ _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM
     return true;
   }
 #endif
+
   bool
   do_copy_file(const char_type* from, const char_type* to,
 	       std::filesystem::copy_options_existing_file options,
@@ -489,8 +496,7 @@ _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM
 	    return false;
 	  }
 
-	if (to_st->st_dev == from_st->st_dev
-	    && to_st->st_ino == from_st->st_ino)
+	if (equiv_files(from, *from_st, to, *to_st, ec))
 	  {
 	    ec = std::make_error_code(std::errc::file_exists);
 	    return false;
diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index 4d23a804da0..c430f58c47a 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -257,6 +257,20 @@ namespace
 
 } // namespace
 
+#ifdef _GLIBCXX_HAVE_SYS_STAT_H
+#ifdef NEED_DO_COPY_FILE // Only define this once, not in cow-ops.o too
+bool
+fs::equiv_files([[maybe_unused]] const char_type* p1, const stat_type& st1,
+		[[maybe_unused]] const char_type* p2, const stat_type& st2,
+		[[maybe_unused]] error_code& ec)
+{
+  // For POSIX the device ID and inode number uniquely identify a file.
+  // This doesn't work on Windows (see equiv_files in src/c++17/fs_ops.cc).
+  return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino;
+}
+#endif
+#endif
+
 void
 fs::copy(const path& from, const path& to, copy_options options,
 	 error_code& ec) noexcept
@@ -293,7 +307,7 @@ fs::copy(const path& from, const path& to, copy_options options,
   f = make_file_status(from_st);
 
   if (exists(t) && !is_other(t) && !is_other(f)
-      && to_st.st_dev == from_st.st_dev && to_st.st_ino == from_st.st_ino)
+      && fs::equiv_files(from.c_str(), from_st, to.c_str(), to_st, ec))
     {
       ec = std::make_error_code(std::errc::file_exists);
       return;
@@ -763,7 +777,7 @@ fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
       ec.clear();
       if (is_other(s1) || is_other(s2))
 	return false;
-      return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino;
+      return fs::equiv_files(p1.c_str(), st1, p2.c_str(), st2, ec);
     }
   else if (!exists(s1) || !exists(s2))
     ec = std::make_error_code(std::errc::no_such_file_or_directory);
diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/copy.cc b/libstdc++-v3/testsuite/27_io/filesystem/operations/copy.cc
index 777a9b8402e..893f32297ae 100644
--- a/libstdc++-v3/testsuite/27_io/filesystem/operations/copy.cc
+++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/copy.cc
@@ -216,6 +216,15 @@ test_pr99290()
   }
 #endif
 
+  // https://github.com/msys2/MSYS2-packages/issues/1937
+  auto copy_opts
+    = fs::copy_options::overwrite_existing | fs::copy_options::recursive;
+  copy(source, dest, copy_opts, ec);
+  VERIFY( !ec );
+
+  auto ch = std::ifstream{dest/"file"}.get();
+  VERIFY( ch == 'a' );
+
   remove_all(dir);
 }
 
diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/copy_file.cc b/libstdc++-v3/testsuite/27_io/filesystem/operations/copy_file.cc
index e3a1219fa6e..af643151044 100644
--- a/libstdc++-v3/testsuite/27_io/filesystem/operations/copy_file.cc
+++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/copy_file.cc
@@ -76,8 +76,130 @@ test01()
   remove(to);
 }
 
+void
+test_directory()
+{
+  std::error_code ec;
+
+  auto dir = __gnu_test::nonexistent_path();
+  auto file = __gnu_test::nonexistent_path();
+
+  // error condition: is_regular_file(from) is false
+  create_directory(dir);
+  bool b = copy_file(dir, file, ec);
+  VERIFY( !b );
+  VERIFY( ec == std::errc::invalid_argument );
+  VERIFY( !exists(file) );
+  VERIFY( is_directory(dir) );
+
+  ec = std::make_error_code(std::errc::address_in_use);
+  // error condition: exists(to) is true and is_regular_file(to) is false
+  std::ofstream{file} << "regular file";
+  b = copy_file(file, dir, ec);
+  VERIFY( !b );
+  VERIFY( ec == std::errc::invalid_argument );
+  VERIFY( exists(file) );
+  VERIFY( is_directory(dir) );
+
+  remove(dir);
+  remove(file);
+}
+
+void
+test_existing()
+{
+  using std::filesystem::copy_options;
+  std::error_code ec;
+
+  auto from = __gnu_test::nonexistent_path();
+  auto to = from;
+
+  // error condition: exists(to) is true and equivalent(from, to) is true
+  std::ofstream{from} << "source";
+  bool b = copy_file(from, to, ec);
+  VERIFY( !b );
+  VERIFY( ec == std::errc::file_exists );
+
+  to = __gnu_test::nonexistent_path();
+  std::ofstream{to} << "overwrite me";
+
+  // error condition: exists(to) is true and options == copy_options::none
+  b = copy_file(from, to, ec);
+  VERIFY( !b );
+  VERIFY( ec == std::errc::file_exists );
+  VERIFY( file_size(to) != file_size(from) );
+
+#if __cpp_exceptions
+  try
+  {
+    (void) copy_file(from, to);
+    VERIFY(false);
+  }
+  catch (const std::filesystem::filesystem_error& e)
+  {
+    std::string_view msg = e.what();
+    VERIFY( msg.find(from.string()) != msg.npos );
+    VERIFY( msg.find(to.string()) != msg.npos );
+    VERIFY( e.code() == std::errc::file_exists );
+  }
+  VERIFY( file_size(to) == 12 );
+#endif
+
+  b = copy_file(from, to, copy_options::skip_existing, ec);
+  VERIFY( !b );
+  VERIFY( !ec );
+  VERIFY( file_size(to) != file_size(from) );
+
+  b = copy_file(from, to, copy_options::skip_existing); // doesn't throw
+  VERIFY( !b );
+  VERIFY( file_size(to) != file_size(from) );
+
+  b = copy_file(from, to, copy_options::overwrite_existing, ec);
+  VERIFY( b );
+  VERIFY( !ec );
+  VERIFY( file_size(to) == file_size(from) );
+
+  std::ofstream{to} << "overwrite me again";
+  b = copy_file(from, to, copy_options::overwrite_existing); // doesn't throw
+  VERIFY( b );
+  VERIFY( file_size(to) == file_size(from) );
+
+  ec = std::make_error_code(std::errc::address_in_use);
+  auto time = last_write_time(from);
+  std::ofstream{to} << "touched";
+  last_write_time(to, time + std::chrono::seconds(60));
+  b = copy_file(from, to, copy_options::update_existing, ec);
+  VERIFY( !b );
+  VERIFY( !ec );
+  VERIFY( file_size(to) != file_size(from) );
+
+  b = copy_file(from, to, copy_options::update_existing); // doesn't throw
+  VERIFY( !b );
+  VERIFY( file_size(to) != file_size(from) );
+
+  ec = std::make_error_code(std::errc::address_in_use);
+  time = last_write_time(to);
+  std::ofstream{from} << "touched more recently";
+  last_write_time(from, time + std::chrono::seconds(60));
+  b = copy_file(from, to, copy_options::update_existing, ec);
+  VERIFY( b );
+  VERIFY( !ec );
+  VERIFY( file_size(to) == file_size(from) );
+
+  time = last_write_time(to);
+  std::ofstream{from} << "touched even more recently";
+  last_write_time(from, time + std::chrono::seconds(60));
+  b = copy_file(from, to, copy_options::update_existing); // doesn't throw
+  VERIFY( b );
+  VERIFY( file_size(to) == file_size(from) );
+
+  remove(from);
+  remove(to);
+}
+
 int
 main()
 {
   test01();
+  test_existing();
 }
-- 
2.45.2

[committed] libstdc++: Fix name of source file in comment

[Jonathan Wakely]

Pushed to trunk.

-- >8 --

libstdc++-v3/ChangeLog:

	* src/c++17/fs_ops.cc: Fix file name in comment.
---
 libstdc++-v3/src/c++17/fs_ops.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc
index 7ffdce67782..9606afa9f1f 100644
--- a/libstdc++-v3/src/c++17/fs_ops.cc
+++ b/libstdc++-v3/src/c++17/fs_ops.cc
@@ -851,7 +851,7 @@ namespace
 #endif
 
 #ifdef _GLIBCXX_HAVE_SYS_STAT_H
-#ifdef NEED_DO_COPY_FILE // Only define this once, not in cow-ops.o too
+#ifdef NEED_DO_COPY_FILE // Only define this once, not in cow-fs_ops.o too
 bool
 fs::equiv_files([[maybe_unused]] const char_type* p1, const stat_type& st1,
 		[[maybe_unused]] const char_type* p2, const stat_type& st2,
-- 
2.45.2

Re: [PATCH] Fix overwriting files with fs::copy_file on windows

[Björn Schäpers]

Am 30.07.2024 um 11:13 schrieb Jonathan Wakely:
> On Sun, 24 Mar 2024 at 21:34, Björn Schäpers <gcc@hazardy.de> wrote:
>>
>> From: Björn Schäpers <bjoern@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 <bjoern@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);
>> +} // 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);
>> +  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
> 
> What's this code doing? You seem to have inverted the logic, so that
> it now returns a file_exists error when the files are *not* the same.
> 
> Is this just a copy & paste error from the similar code in
> fs::equivalent? Was this tested?
> 
>>            {
>>              ec = std::make_error_code(std::errc::file_exists);
>>              return false;
>> --
>> 2.44.0
>>
> 

It was tested on windows and no other platform. You already sent some feedback, 
but I've not yet found the time to work on that, since I already deployed a 
work-around.

But I see you fixed it yourself, thanks for that.

Re: [PATCH] Fix overwriting files with fs::copy_file on windows

[Jonathan Wakely]

On Wed, 31 Jul 2024 at 15:42, Björn Schäpers <gcc@hazardy.de> wrote:
>
> Am 30.07.2024 um 11:13 schrieb Jonathan Wakely:
> > On Sun, 24 Mar 2024 at 21:34, Björn Schäpers <gcc@hazardy.de> wrote:
> >>
> >> From: Björn Schäpers <bjoern@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 <bjoern@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);
> >> +} // 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);
> >> +  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
> >
> > What's this code doing? You seem to have inverted the logic, so that
> > it now returns a file_exists error when the files are *not* the same.
> >
> > Is this just a copy & paste error from the similar code in
> > fs::equivalent? Was this tested?
> >
> >>            {
> >>              ec = std::make_error_code(std::errc::file_exists);
> >>              return false;
> >> --
> >> 2.44.0
> >>
> >
>
> It was tested on windows and no other platform. You already sent some feedback,
> but I've not yet found the time to work on that, since I already deployed a
> work-around.

I think this logic would have caused a spurious error when copying a
file over an existing file on a different device:

> >> +       if (st1.st_mode != st2.st_mode || st1.st_dev != st2.st_dev ||
> >> +           fs::__detail::equivalent_win32(from, to, not_used))

Which probably wouldn't show up for most testing, and certainly isn't
exercised in our testsuite.


> But I see you fixed it yourself, thanks for that.

Yup, it should be fixed now. If you notice any remaining issues,
please let me know.

Thanks for identifying where the problem was and how to fix it!