[PATCH] libstdc++: Fix error handling for std::filesystem::equivalent [PR113250]

[PATCH] libstdc++: Fix error handling for std::filesystem::equivalent [PR113250]

[Ken Matsui]

This patch made std::filesystem::equivalent correctly throw an exception
when either path does not exist as per [fs.op.equivalent]/4.

libstdc++-v3/ChangeLog:

	* src/c++17/fs_ops.cc (equivalent): Use || instead of &&.
	* testsuite/27_io/filesystem/operations/equivalent.cc: Handle
	error codes.

Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org>
---
 libstdc++-v3/src/c++17/fs_ops.cc                              | 2 +-
 .../testsuite/27_io/filesystem/operations/equivalent.cc       | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc
index e0b308a37ea..61df19753ef 100644
--- a/libstdc++-v3/src/c++17/fs_ops.cc
+++ b/libstdc++-v3/src/c++17/fs_ops.cc
@@ -897,7 +897,7 @@ fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
       return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino;
 #endif
     }
-  else if (!exists(s1) && !exists(s2))
+  else if (!exists(s1) || !exists(s2))
     ec = std::make_error_code(std::errc::no_such_file_or_directory);
   else if (err)
     ec.assign(err, std::generic_category());
diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc b/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc
index 78f6e368204..68f32366d65 100644
--- a/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc
+++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc
@@ -34,13 +34,13 @@ test01()
   bool result;
 
   result = equivalent(p1, p2, ec);
-  VERIFY( ec );
+  VERIFY( ec == std::errc::no_such_file_or_directory );
   VERIFY( !result );
 
   __gnu_test::scoped_file f1(p1);
   ec = bad_ec;
   result = equivalent(p1, p2, ec);
-  VERIFY( !ec );
+  VERIFY( ec == std::errc::no_such_file_or_directory );
   VERIFY( !result );
 
   __gnu_test::scoped_file f2(p2);
-- 
2.43.0

[PATCH v2 1/2] libstdc++: Fix error handling in filesystem::equivalent [PR113250]

[Ken Matsui]

This patch made std::filesystem::equivalent correctly throw an exception
when either path does not exist as per [fs.op.equivalent]/4.

libstdc++-v3/ChangeLog:

	* src/c++17/fs_ops.cc (fs::equivalent): Use || instead of &&.
	* src/filesystem/ops.cc (fs::equivalent): Likewise.
	* testsuite/27_io/filesystem/operations/equivalent.cc: Handle
	error codes.
	* testsuite/experimental/filesystem/operations/equivalent.cc:
	Likewise.

Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org>
---
 libstdc++-v3/src/c++17/fs_ops.cc                              | 2 +-
 libstdc++-v3/src/filesystem/ops.cc                            | 2 +-
 .../testsuite/27_io/filesystem/operations/equivalent.cc       | 4 ++--
 .../experimental/filesystem/operations/equivalent.cc          | 4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc
index e0b308a37ea..61df19753ef 100644
--- a/libstdc++-v3/src/c++17/fs_ops.cc
+++ b/libstdc++-v3/src/c++17/fs_ops.cc
@@ -897,7 +897,7 @@ fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
       return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino;
 #endif
     }
-  else if (!exists(s1) && !exists(s2))
+  else if (!exists(s1) || !exists(s2))
     ec = std::make_error_code(std::errc::no_such_file_or_directory);
   else if (err)
     ec.assign(err, std::generic_category());
diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index eccdae3d910..4d23a804da0 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -765,7 +765,7 @@ fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
 	return false;
       return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino;
     }
-  else if (!exists(s1) && !exists(s2))
+  else if (!exists(s1) || !exists(s2))
     ec = std::make_error_code(std::errc::no_such_file_or_directory);
   else if (err)
     ec.assign(err, std::generic_category());
diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc b/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc
index 78f6e368204..68f32366d65 100644
--- a/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc
+++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc
@@ -34,13 +34,13 @@ test01()
   bool result;
 
   result = equivalent(p1, p2, ec);
-  VERIFY( ec );
+  VERIFY( ec == std::errc::no_such_file_or_directory );
   VERIFY( !result );
 
   __gnu_test::scoped_file f1(p1);
   ec = bad_ec;
   result = equivalent(p1, p2, ec);
-  VERIFY( !ec );
+  VERIFY( ec == std::errc::no_such_file_or_directory );
   VERIFY( !result );
 
   __gnu_test::scoped_file f2(p2);
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc
index 929a6ca8609..5bc477a31cd 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc
@@ -35,13 +35,13 @@ test01()
   bool result;
 
   result = equivalent(p1, p2, ec);
-  VERIFY( ec );
+  VERIFY( ec == std::errc::no_such_file_or_directory );
   VERIFY( !result );
   const auto bad_ec = ec;
 
   __gnu_test::scoped_file f1(p1);
   result = equivalent(p1, p2, ec);
-  VERIFY( !ec );
+  VERIFY( ec == std::errc::no_such_file_or_directory );
   VERIFY( !result );
 
   __gnu_test::scoped_file f2(p2);
-- 
2.43.0

[PATCH v2 2/2] libstdc++: Use using instead of typedef in opts-common.h

[Ken Matsui]

libstdc++-v3/ChangeLog:

	* src/filesystem/ops-common.h (stat_type): Use using.

Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org>
---
 libstdc++-v3/src/filesystem/ops-common.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libstdc++-v3/src/filesystem/ops-common.h b/libstdc++-v3/src/filesystem/ops-common.h
index d78a54754c2..e302d8caae6 100644
--- a/libstdc++-v3/src/filesystem/ops-common.h
+++ b/libstdc++-v3/src/filesystem/ops-common.h
@@ -118,7 +118,7 @@ namespace __gnu_posix
   inline int close(int fd)
   { return ::_close(fd); }
 
-  typedef struct ::__stat64 stat_type;
+  using stat_type = struct ::_stat64;
 
   inline int stat(const wchar_t* path, stat_type* buffer)
   { return ::_wstat64(path, buffer); }
@@ -184,7 +184,7 @@ namespace __gnu_posix
   using ::open;
   using ::close;
 # ifdef _GLIBCXX_HAVE_SYS_STAT_H
-  typedef struct ::stat stat_type;
+  using stat_type = struct ::stat;
   using ::stat;
 #  ifdef _GLIBCXX_USE_LSTAT
   using ::lstat;
-- 
2.43.0

Re: [PATCH v2 2/2] libstdc++: Use using instead of typedef in opts-common.h

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 1239 bytes --]

On Thu, 11 Jan 2024, 09:43 Ken Matsui, <kmatsui@gcc.gnu.org> wrote:

> libstdc++-v3/ChangeLog:
>
>         * src/filesystem/ops-common.h (stat_type): Use using.
>
> Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org>
> ---
>  libstdc++-v3/src/filesystem/ops-common.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libstdc++-v3/src/filesystem/ops-common.h
> b/libstdc++-v3/src/filesystem/ops-common.h
> index d78a54754c2..e302d8caae6 100644
> --- a/libstdc++-v3/src/filesystem/ops-common.h
> +++ b/libstdc++-v3/src/filesystem/ops-common.h
> @@ -118,7 +118,7 @@ namespace __gnu_posix
>    inline int close(int fd)
>    { return ::_close(fd); }
>
> -  typedef struct ::__stat64 stat_type;
> +  using stat_type = struct ::_stat64;
>

You've removed an underscore, is that intentional? Has this been compiled +
tested with mingw?


>    inline int stat(const wchar_t* path, stat_type* buffer)
>    { return ::_wstat64(path, buffer); }
> @@ -184,7 +184,7 @@ namespace __gnu_posix
>    using ::open;
>    using ::close;
>  # ifdef _GLIBCXX_HAVE_SYS_STAT_H
> -  typedef struct ::stat stat_type;
> +  using stat_type = struct ::stat;
>    using ::stat;
>  #  ifdef _GLIBCXX_USE_LSTAT
>    using ::lstat;
> --
> 2.43.0
>
>

Re: Re: [PATCH v2 2/2] libstdc++: Use using instead of typedef in opts-common.h

[Ken Matsui]

On Thu, 11 Jan 2024 at 09:55, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>    On Thu, 11 Jan 2024, 09:43 Ken Matsui, <[1]kmatsui@gcc.gnu.org> wrote:       
>                                                                                 
>    > libstdc++-v3/ChangeLog:                                                    
>                                                                                 
>    >         * src/filesystem/ops-common.h (stat_type): Use using.              
>                                                                                 
>    > Signed-off-by: Ken Matsui <[2]kmatsui@gcc.gnu.org>                         
>    > ---                                                                        
>    >  libstdc++-v3/src/filesystem/ops-common.h | 4 ++--                         
>    >  1 file changed, 2 insertions(+), 2 deletions(-)                           
>                                                                                 
>    > diff --git a/libstdc++-v3/src/filesystem/ops-common.h                      
>    > b/libstdc++-v3/src/filesystem/ops-common.h                                 
>    > index d78a54754c2..e302d8caae6 100644                                      
>    > --- a/libstdc++-v3/src/filesystem/ops-common.h                             
>    > +++ b/libstdc++-v3/src/filesystem/ops-common.h                             
>    > @@ -118,7 +118,7 @@ namespace __gnu_posix                                  
>    >    inline int close(int fd)                                                
>    >    { return ::_close(fd); }                                                
>                                                                                 
>    > -  typedef struct ::__stat64 stat_type;                                    
>    > +  using stat_type = struct ::_stat64;                                     
>                                                                                 
>    You've removed an underscore, is that intentional? Has this been compiled    
>    + tested with mingw?                                                         
This is totally unintentional.  I'll fix it.  I've tested it with Linux
and do not have a mingw environment to test it with.  Would you have any
suggestions on how to test it?

>                                                                                 
>    >    inline int stat(const wchar_t* path, stat_type* buffer)                 
>    >    { return ::_wstat64(path, buffer); }                                    
>    > @@ -184,7 +184,7 @@ namespace __gnu_posix                                  
>    >    using ::open;                                                           
>    >    using ::close;                                                          
>    >  # ifdef _GLIBCXX_HAVE_SYS_STAT_H                                          
>    > -  typedef struct ::stat stat_type;                                        
>    > +  using stat_type = struct ::stat;                                        
>    >    using ::stat;                                                           
>    >  #  ifdef _GLIBCXX_USE_LSTAT                                               
>    >    using ::lstat;                                                          
>    > --                                                                         
>    > 2.43.0                                                                     
> 
> References
> 
>    Visible links
>    1. mailto:kmatsui@gcc.gnu.org
>    2. mailto:kmatsui@gcc.gnu.org

-- 
Ken Matsui

Re: Re: [PATCH v2 2/2] libstdc++: Use using instead of typedef in opts-common.h

[Jonathan Wakely]

On Thu, 11 Jan 2024 at 10:12, Ken Matsui <kmatsui@cs.washington.edu> wrote:
>
> On Thu, 11 Jan 2024 at 09:55, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> >    On Thu, 11 Jan 2024, 09:43 Ken Matsui, <[1]kmatsui@gcc.gnu.org> wrote:
> >
> >    > libstdc++-v3/ChangeLog:
> >
> >    >         * src/filesystem/ops-common.h (stat_type): Use using.
> >
> >    > Signed-off-by: Ken Matsui <[2]kmatsui@gcc.gnu.org>
> >    > ---
> >    >  libstdc++-v3/src/filesystem/ops-common.h | 4 ++--
> >    >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> >    > diff --git a/libstdc++-v3/src/filesystem/ops-common.h
> >    > b/libstdc++-v3/src/filesystem/ops-common.h
> >    > index d78a54754c2..e302d8caae6 100644
> >    > --- a/libstdc++-v3/src/filesystem/ops-common.h
> >    > +++ b/libstdc++-v3/src/filesystem/ops-common.h
> >    > @@ -118,7 +118,7 @@ namespace __gnu_posix
> >    >    inline int close(int fd)
> >    >    { return ::_close(fd); }
> >
> >    > -  typedef struct ::__stat64 stat_type;
> >    > +  using stat_type = struct ::_stat64;
> >
> >    You've removed an underscore, is that intentional? Has this been compiled
> >    + tested with mingw?
> This is totally unintentional.  I'll fix it.  I've tested it with Linux
> and do not have a mingw environment to test it with.  Would you have any
> suggestions on how to test it?

If your linux distro has prebuilt packages for mingw-w64-g++ then it's
pretty easy to build a new compiler from trunk. I can give you a
simple recipe for doing it on Fedora.

In the meanwhile, I can test your updated patch for you.

Re: Re: Re: [PATCH v2 2/2] libstdc++: Use using instead of typedef in opts-common.h

[Ken Matsui]

On Thu, 11 Jan 2024 at 10:50, Jonathan Wakely <jwakely@redhat.com> wrote:
> On Thu, 11 Jan 2024 at 10:12, Ken Matsui <kmatsui@cs.washington.edu> wrote:
> >
> > On Thu, 11 Jan 2024 at 09:55, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> > >    On Thu, 11 Jan 2024, 09:43 Ken Matsui, <[1]kmatsui@gcc.gnu.org> wrote:
> > >
> > >    > libstdc++-v3/ChangeLog:
> > >
> > >    >         * src/filesystem/ops-common.h (stat_type): Use using.
> > >
> > >    > Signed-off-by: Ken Matsui <[2]kmatsui@gcc.gnu.org>
> > >    > ---
> > >    >  libstdc++-v3/src/filesystem/ops-common.h | 4 ++--
> > >    >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > >    > diff --git a/libstdc++-v3/src/filesystem/ops-common.h
> > >    > b/libstdc++-v3/src/filesystem/ops-common.h
> > >    > index d78a54754c2..e302d8caae6 100644
> > >    > --- a/libstdc++-v3/src/filesystem/ops-common.h
> > >    > +++ b/libstdc++-v3/src/filesystem/ops-common.h
> > >    > @@ -118,7 +118,7 @@ namespace __gnu_posix
> > >    >    inline int close(int fd)
> > >    >    { return ::_close(fd); }
> > >
> > >    > -  typedef struct ::__stat64 stat_type;
> > >    > +  using stat_type = struct ::_stat64;
> > >
> > >    You've removed an underscore, is that intentional? Has this been compiled
> > >    + tested with mingw?
> > This is totally unintentional.  I'll fix it.  I've tested it with Linux
> > and do not have a mingw environment to test it with.  Would you have any
> > suggestions on how to test it?
> 
> If your linux distro has prebuilt packages for mingw-w64-g++ then it's
> pretty easy to build a new compiler from trunk. I can give you a
> simple recipe for doing it on Fedora.
> 
> In the meanwhile, I can test your updated patch for you.
> 

I am using Ubuntu and was able to install mingw-w64-g++ with apt.  I
tried the following command to test the patch:

make check-target-libstdc++-v3 ALT_CXX_UNDER_TEST="$(which x86_64-w64-mingw32-g++)" RUNTESTFLAGS='conformance.exp=filesystem/*'

But it looks like the test is not using the mingw compiler.  Also,
when we want to test to build GCC with a different compiler, do we
need to rebuild the whole GCC again (i.e., make clean && export CXX=...
&& make)?

I really appreciate your recipe if it also works for Ubuntu.  Thank you!

-- 
Ken Matsui

Re: Re: Re: [PATCH v2 2/2] libstdc++: Use using instead of typedef in opts-common.h

[Jonathan Wakely]

On Thu, 11 Jan 2024 at 11:28, Ken Matsui <kmatsui@gcc.gnu.org> wrote:
>
> On Thu, 11 Jan 2024 at 10:50, Jonathan Wakely <jwakely@redhat.com> wrote:
> > On Thu, 11 Jan 2024 at 10:12, Ken Matsui <kmatsui@cs.washington.edu> wrote:
> > >
> > > On Thu, 11 Jan 2024 at 09:55, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> > > >    On Thu, 11 Jan 2024, 09:43 Ken Matsui, <[1]kmatsui@gcc.gnu.org> wrote:
> > > >
> > > >    > libstdc++-v3/ChangeLog:
> > > >
> > > >    >         * src/filesystem/ops-common.h (stat_type): Use using.
> > > >
> > > >    > Signed-off-by: Ken Matsui <[2]kmatsui@gcc.gnu.org>
> > > >    > ---
> > > >    >  libstdc++-v3/src/filesystem/ops-common.h | 4 ++--
> > > >    >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > >    > diff --git a/libstdc++-v3/src/filesystem/ops-common.h
> > > >    > b/libstdc++-v3/src/filesystem/ops-common.h
> > > >    > index d78a54754c2..e302d8caae6 100644
> > > >    > --- a/libstdc++-v3/src/filesystem/ops-common.h
> > > >    > +++ b/libstdc++-v3/src/filesystem/ops-common.h
> > > >    > @@ -118,7 +118,7 @@ namespace __gnu_posix
> > > >    >    inline int close(int fd)
> > > >    >    { return ::_close(fd); }
> > > >
> > > >    > -  typedef struct ::__stat64 stat_type;
> > > >    > +  using stat_type = struct ::_stat64;
> > > >
> > > >    You've removed an underscore, is that intentional? Has this been compiled
> > > >    + tested with mingw?
> > > This is totally unintentional.  I'll fix it.  I've tested it with Linux
> > > and do not have a mingw environment to test it with.  Would you have any
> > > suggestions on how to test it?
> >
> > If your linux distro has prebuilt packages for mingw-w64-g++ then it's
> > pretty easy to build a new compiler from trunk. I can give you a
> > simple recipe for doing it on Fedora.
> >
> > In the meanwhile, I can test your updated patch for you.
> >
>
> I am using Ubuntu and was able to install mingw-w64-g++ with apt.  I
> tried the following command to test the patch:
>
> make check-target-libstdc++-v3 ALT_CXX_UNDER_TEST="$(which x86_64-w64-mingw32-g++)" RUNTESTFLAGS='conformance.exp=filesystem/*'
>
> But it looks like the test is not using the mingw compiler.  Also,
> when we want to test to build GCC with a different compiler, do we
> need to rebuild the whole GCC again (i.e., make clean && export CXX=...
> && make)?

Don't use 'make clean' with GCC, it doesn't work well. Just remove the
build directory.

Rebuilding with CXX set to your mingw compiler will not work. That
would try to compile gcc using the mingw compiler, producing a
compiler that would *run* on Windows. That's not what you want. You
need to configure gcc with --target=x86_64-w64-mingw32 to build a
compiler that *runs* on linux, but targets Windows.

Do this in a separate build directory, don't try to reuse the same
build directory that you already have for a native linux compiler.

>
> I really appreciate your recipe if it also works for Ubuntu.  Thank you!

Here's what I do on Fedora:
https://gist.github.com/jwakely/5579a4be12a86495d47cb5519f671b12#file-mingw-build-sh

To figure that command out I checked the output of x86_64-w64-mingw32-g++ -v
That will show you the sysroot used by the Ubuntu mingw compiler, and
the other options it was built with. Copy the relevant ones and add
those to your configure command for your mingw cross-compiler. On my
systems, the configure command is:

../gcc/configure --prefix=/home/jwakely/gcc/mingw --with-gnu-as
--with-as=/usr/bin/x86_64-w64-mingw32-as --with-gnu-ld
--with-ld=/usr/bin/x86_64-w64-mingw32-ld --without-newlib
--disable-multilib --disable-plugin --disable-libstdcxx-pch
--with-system-zlib --disable-nls --without-included-gettext
--disable-win32-registry --enable-threads=posix --enable-libgomp
--target=x86_64-w64-mingw32
--with-sysroot=/usr/x86_64-w64-mingw32/sys-root
target_alias=x86_64-w64-mingw32 --enable-libstdcxx-debug
--disable-libgomp --enable-libstdcxx-backtrace
--enable-languages=c,c++

The relevant parts that I copied from the output of the system
'x86_64-w64-mingw32-gcc -v' are:

--with-as=/usr/bin/x86_64-w64-mingw32-as
--with-ld=/usr/bin/x86_64-w64-mingw32-ld --without-newlib
--disable-multilib --disable-plugin
--disable-win32-registry
--enable-threads=posix
--target=x86_64-w64-mingw32
--with-sysroot=/usr/x86_64-w64-mingw32/sys-root

You might want the same options, but the paths to the mingw32-as,
mingw32-ld and the sys-root will probably be different on Ubuntu.

Looking at the -v output on Ubuntu, you probably want the
--with-headers option and --enable-fully-dynamic-string (to be
compatible with the system's mingw compiler) and
--enable-threads=win32 as well as the --with-as and --with-ld options.
You'll want to add a --prefix too so that you install somewhere under
your home directory, not under /usr

You might need to experiment with configure options until you get
something to build that works.

Once I have a mingw compiler that runs on my linux box, I use this
script to test it:
https://gist.github.com/jwakely/5579a4be12a86495d47cb5519f671b12#file-mingw_tests-sh

Like so:
find ~/src/gcc/gcc/libstdc++-v3/testsuite/27_io/filesystem -type f
-name '*.cc' | ~/src/gcc/scripts/mingw_tests.sh

That compiles all the 27_io filesystem tests using my mingw compiler
and then runs them using Wine.

Re: Re: Re: [PATCH v2 2/2] libstdc++: Use using instead of typedef in opts-common.h

[Ken Matsui]

On Thu, Jan 11, 2024 at 4:01 AM Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On Thu, 11 Jan 2024 at 11:28, Ken Matsui <kmatsui@gcc.gnu.org> wrote:
> >
> > On Thu, 11 Jan 2024 at 10:50, Jonathan Wakely <jwakely@redhat.com> wrote:
> > > On Thu, 11 Jan 2024 at 10:12, Ken Matsui <kmatsui@cs.washington.edu> wrote:
> > > >
> > > > On Thu, 11 Jan 2024 at 09:55, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> > > > >    On Thu, 11 Jan 2024, 09:43 Ken Matsui, <[1]kmatsui@gcc.gnu.org> wrote:
> > > > >
> > > > >    > libstdc++-v3/ChangeLog:
> > > > >
> > > > >    >         * src/filesystem/ops-common.h (stat_type): Use using.
> > > > >
> > > > >    > Signed-off-by: Ken Matsui <[2]kmatsui@gcc.gnu.org>
> > > > >    > ---
> > > > >    >  libstdc++-v3/src/filesystem/ops-common.h | 4 ++--
> > > > >    >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > >
> > > > >    > diff --git a/libstdc++-v3/src/filesystem/ops-common.h
> > > > >    > b/libstdc++-v3/src/filesystem/ops-common.h
> > > > >    > index d78a54754c2..e302d8caae6 100644
> > > > >    > --- a/libstdc++-v3/src/filesystem/ops-common.h
> > > > >    > +++ b/libstdc++-v3/src/filesystem/ops-common.h
> > > > >    > @@ -118,7 +118,7 @@ namespace __gnu_posix
> > > > >    >    inline int close(int fd)
> > > > >    >    { return ::_close(fd); }
> > > > >
> > > > >    > -  typedef struct ::__stat64 stat_type;
> > > > >    > +  using stat_type = struct ::_stat64;
> > > > >
> > > > >    You've removed an underscore, is that intentional? Has this been compiled
> > > > >    + tested with mingw?
> > > > This is totally unintentional.  I'll fix it.  I've tested it with Linux
> > > > and do not have a mingw environment to test it with.  Would you have any
> > > > suggestions on how to test it?
> > >
> > > If your linux distro has prebuilt packages for mingw-w64-g++ then it's
> > > pretty easy to build a new compiler from trunk. I can give you a
> > > simple recipe for doing it on Fedora.
> > >
> > > In the meanwhile, I can test your updated patch for you.
> > >
> >
> > I am using Ubuntu and was able to install mingw-w64-g++ with apt.  I
> > tried the following command to test the patch:
> >
> > make check-target-libstdc++-v3 ALT_CXX_UNDER_TEST="$(which x86_64-w64-mingw32-g++)" RUNTESTFLAGS='conformance.exp=filesystem/*'
> >
> > But it looks like the test is not using the mingw compiler.  Also,
> > when we want to test to build GCC with a different compiler, do we
> > need to rebuild the whole GCC again (i.e., make clean && export CXX=...
> > && make)?
>
> Don't use 'make clean' with GCC, it doesn't work well. Just remove the
> build directory.
>
> Rebuilding with CXX set to your mingw compiler will not work. That
> would try to compile gcc using the mingw compiler, producing a
> compiler that would *run* on Windows. That's not what you want. You
> need to configure gcc with --target=x86_64-w64-mingw32 to build a
> compiler that *runs* on linux, but targets Windows.
>
> Do this in a separate build directory, don't try to reuse the same
> build directory that you already have for a native linux compiler.
>
> >
> > I really appreciate your recipe if it also works for Ubuntu.  Thank you!
>
> Here's what I do on Fedora:
> https://gist.github.com/jwakely/5579a4be12a86495d47cb5519f671b12#file-mingw-build-sh
>
> To figure that command out I checked the output of x86_64-w64-mingw32-g++ -v
> That will show you the sysroot used by the Ubuntu mingw compiler, and
> the other options it was built with. Copy the relevant ones and add
> those to your configure command for your mingw cross-compiler. On my
> systems, the configure command is:
>
> ../gcc/configure --prefix=/home/jwakely/gcc/mingw --with-gnu-as
> --with-as=/usr/bin/x86_64-w64-mingw32-as --with-gnu-ld
> --with-ld=/usr/bin/x86_64-w64-mingw32-ld --without-newlib
> --disable-multilib --disable-plugin --disable-libstdcxx-pch
> --with-system-zlib --disable-nls --without-included-gettext
> --disable-win32-registry --enable-threads=posix --enable-libgomp
> --target=x86_64-w64-mingw32
> --with-sysroot=/usr/x86_64-w64-mingw32/sys-root
> target_alias=x86_64-w64-mingw32 --enable-libstdcxx-debug
> --disable-libgomp --enable-libstdcxx-backtrace
> --enable-languages=c,c++
>
> The relevant parts that I copied from the output of the system
> 'x86_64-w64-mingw32-gcc -v' are:
>
> --with-as=/usr/bin/x86_64-w64-mingw32-as
> --with-ld=/usr/bin/x86_64-w64-mingw32-ld --without-newlib
> --disable-multilib --disable-plugin
> --disable-win32-registry
> --enable-threads=posix
> --target=x86_64-w64-mingw32
> --with-sysroot=/usr/x86_64-w64-mingw32/sys-root
>
> You might want the same options, but the paths to the mingw32-as,
> mingw32-ld and the sys-root will probably be different on Ubuntu.
>
> Looking at the -v output on Ubuntu, you probably want the
> --with-headers option and --enable-fully-dynamic-string (to be
> compatible with the system's mingw compiler) and
> --enable-threads=win32 as well as the --with-as and --with-ld options.
> You'll want to add a --prefix too so that you install somewhere under
> your home directory, not under /usr
>
> You might need to experiment with configure options until you get
> something to build that works.
>
> Once I have a mingw compiler that runs on my linux box, I use this
> script to test it:
> https://gist.github.com/jwakely/5579a4be12a86495d47cb5519f671b12#file-mingw_tests-sh
>
> Like so:
> find ~/src/gcc/gcc/libstdc++-v3/testsuite/27_io/filesystem -type f
> -name '*.cc' | ~/src/gcc/scripts/mingw_tests.sh
>
> That compiles all the 27_io filesystem tests using my mingw compiler
> and then runs them using Wine.
>

Thank you so much!  I will try it!

Re: Re: [PATCH v2 2/2] libstdc++: Use using instead of typedef in opts-common.h

[Jonathan Wakely]

On Thu, 11 Jan 2024 at 10:50, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On Thu, 11 Jan 2024 at 10:12, Ken Matsui <kmatsui@cs.washington.edu> wrote:
> >
> > On Thu, 11 Jan 2024 at 09:55, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> > >    On Thu, 11 Jan 2024, 09:43 Ken Matsui, <[1]kmatsui@gcc.gnu.org> wrote:
> > >
> > >    > libstdc++-v3/ChangeLog:
> > >
> > >    >         * src/filesystem/ops-common.h (stat_type): Use using.
> > >
> > >    > Signed-off-by: Ken Matsui <[2]kmatsui@gcc.gnu.org>
> > >    > ---
> > >    >  libstdc++-v3/src/filesystem/ops-common.h | 4 ++--
> > >    >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > >    > diff --git a/libstdc++-v3/src/filesystem/ops-common.h
> > >    > b/libstdc++-v3/src/filesystem/ops-common.h
> > >    > index d78a54754c2..e302d8caae6 100644
> > >    > --- a/libstdc++-v3/src/filesystem/ops-common.h
> > >    > +++ b/libstdc++-v3/src/filesystem/ops-common.h
> > >    > @@ -118,7 +118,7 @@ namespace __gnu_posix
> > >    >    inline int close(int fd)
> > >    >    { return ::_close(fd); }
> > >
> > >    > -  typedef struct ::__stat64 stat_type;
> > >    > +  using stat_type = struct ::_stat64;
> > >
> > >    You've removed an underscore, is that intentional? Has this been compiled
> > >    + tested with mingw?
> > This is totally unintentional.  I'll fix it.  I've tested it with Linux
> > and do not have a mingw environment to test it with.  Would you have any
> > suggestions on how to test it?
>
> If your linux distro has prebuilt packages for mingw-w64-g++ then it's
> pretty easy to build a new compiler from trunk. I can give you a
> simple recipe for doing it on Fedora.
>
> In the meanwhile, I can test your updated patch for you.

The patch works on both Windows (well, Wine) and Linux. OK for trunk, thanks.

Re: Re: [PATCH v2 2/2] libstdc++: Use using instead of typedef in opts-common.h

[Ken Matsui]

On Thu, Jan 11, 2024 at 4:02 AM Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On Thu, 11 Jan 2024 at 10:50, Jonathan Wakely <jwakely@redhat.com> wrote:
> >
> > On Thu, 11 Jan 2024 at 10:12, Ken Matsui <kmatsui@cs.washington.edu> wrote:
> > >
> > > On Thu, 11 Jan 2024 at 09:55, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> > > >    On Thu, 11 Jan 2024, 09:43 Ken Matsui, <[1]kmatsui@gcc.gnu.org> wrote:
> > > >
> > > >    > libstdc++-v3/ChangeLog:
> > > >
> > > >    >         * src/filesystem/ops-common.h (stat_type): Use using.
> > > >
> > > >    > Signed-off-by: Ken Matsui <[2]kmatsui@gcc.gnu.org>
> > > >    > ---
> > > >    >  libstdc++-v3/src/filesystem/ops-common.h | 4 ++--
> > > >    >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > >    > diff --git a/libstdc++-v3/src/filesystem/ops-common.h
> > > >    > b/libstdc++-v3/src/filesystem/ops-common.h
> > > >    > index d78a54754c2..e302d8caae6 100644
> > > >    > --- a/libstdc++-v3/src/filesystem/ops-common.h
> > > >    > +++ b/libstdc++-v3/src/filesystem/ops-common.h
> > > >    > @@ -118,7 +118,7 @@ namespace __gnu_posix
> > > >    >    inline int close(int fd)
> > > >    >    { return ::_close(fd); }
> > > >
> > > >    > -  typedef struct ::__stat64 stat_type;
> > > >    > +  using stat_type = struct ::_stat64;
> > > >
> > > >    You've removed an underscore, is that intentional? Has this been compiled
> > > >    + tested with mingw?
> > > This is totally unintentional.  I'll fix it.  I've tested it with Linux
> > > and do not have a mingw environment to test it with.  Would you have any
> > > suggestions on how to test it?
> >
> > If your linux distro has prebuilt packages for mingw-w64-g++ then it's
> > pretty easy to build a new compiler from trunk. I can give you a
> > simple recipe for doing it on Fedora.
> >
> > In the meanwhile, I can test your updated patch for you.
>
> The patch works on both Windows (well, Wine) and Linux. OK for trunk, thanks.
>
Thank you!

[PATCH v3 2/2] libstdc++: Use using instead of typedef in opts-common.h

[Ken Matsui]

libstdc++-v3/ChangeLog:

	* src/filesystem/ops-common.h (stat_type): Use using.

Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org>
---
 libstdc++-v3/src/filesystem/ops-common.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libstdc++-v3/src/filesystem/ops-common.h b/libstdc++-v3/src/filesystem/ops-common.h
index d78a54754c2..d917fddbeb1 100644
--- a/libstdc++-v3/src/filesystem/ops-common.h
+++ b/libstdc++-v3/src/filesystem/ops-common.h
@@ -118,7 +118,7 @@ namespace __gnu_posix
   inline int close(int fd)
   { return ::_close(fd); }
 
-  typedef struct ::__stat64 stat_type;
+  using stat_type = struct ::__stat64;
 
   inline int stat(const wchar_t* path, stat_type* buffer)
   { return ::_wstat64(path, buffer); }
@@ -184,7 +184,7 @@ namespace __gnu_posix
   using ::open;
   using ::close;
 # ifdef _GLIBCXX_HAVE_SYS_STAT_H
-  typedef struct ::stat stat_type;
+  using stat_type = struct ::stat;
   using ::stat;
 #  ifdef _GLIBCXX_USE_LSTAT
   using ::lstat;
-- 
2.43.0

Re: [PATCH v2 1/2] libstdc++: Fix error handling in filesystem::equivalent [PR113250]

[Jonathan Wakely]

On Thu, 11 Jan 2024 at 09:43, Ken Matsui <kmatsui@gcc.gnu.org> wrote:
>
> This patch made std::filesystem::equivalent correctly throw an exception
> when either path does not exist as per [fs.op.equivalent]/4.

Thanks, OK for trunk and all active branches (let me know if you need
help backporting it).


>
> libstdc++-v3/ChangeLog:
>
>         * src/c++17/fs_ops.cc (fs::equivalent): Use || instead of &&.
>         * src/filesystem/ops.cc (fs::equivalent): Likewise.
>         * testsuite/27_io/filesystem/operations/equivalent.cc: Handle
>         error codes.
>         * testsuite/experimental/filesystem/operations/equivalent.cc:
>         Likewise.
>
> Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org>
> ---
>  libstdc++-v3/src/c++17/fs_ops.cc                              | 2 +-
>  libstdc++-v3/src/filesystem/ops.cc                            | 2 +-
>  .../testsuite/27_io/filesystem/operations/equivalent.cc       | 4 ++--
>  .../experimental/filesystem/operations/equivalent.cc          | 4 ++--
>  4 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc
> index e0b308a37ea..61df19753ef 100644
> --- a/libstdc++-v3/src/c++17/fs_ops.cc
> +++ b/libstdc++-v3/src/c++17/fs_ops.cc
> @@ -897,7 +897,7 @@ fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
>        return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino;
>  #endif
>      }
> -  else if (!exists(s1) && !exists(s2))
> +  else if (!exists(s1) || !exists(s2))
>      ec = std::make_error_code(std::errc::no_such_file_or_directory);
>    else if (err)
>      ec.assign(err, std::generic_category());
> diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
> index eccdae3d910..4d23a804da0 100644
> --- a/libstdc++-v3/src/filesystem/ops.cc
> +++ b/libstdc++-v3/src/filesystem/ops.cc
> @@ -765,7 +765,7 @@ fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
>         return false;
>        return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino;
>      }
> -  else if (!exists(s1) && !exists(s2))
> +  else if (!exists(s1) || !exists(s2))
>      ec = std::make_error_code(std::errc::no_such_file_or_directory);
>    else if (err)
>      ec.assign(err, std::generic_category());
> diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc b/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc
> index 78f6e368204..68f32366d65 100644
> --- a/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc
> +++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc
> @@ -34,13 +34,13 @@ test01()
>    bool result;
>
>    result = equivalent(p1, p2, ec);
> -  VERIFY( ec );
> +  VERIFY( ec == std::errc::no_such_file_or_directory );
>    VERIFY( !result );
>
>    __gnu_test::scoped_file f1(p1);
>    ec = bad_ec;
>    result = equivalent(p1, p2, ec);
> -  VERIFY( !ec );
> +  VERIFY( ec == std::errc::no_such_file_or_directory );
>    VERIFY( !result );
>
>    __gnu_test::scoped_file f2(p2);
> diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc
> index 929a6ca8609..5bc477a31cd 100644
> --- a/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc
> +++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc
> @@ -35,13 +35,13 @@ test01()
>    bool result;
>
>    result = equivalent(p1, p2, ec);
> -  VERIFY( ec );
> +  VERIFY( ec == std::errc::no_such_file_or_directory );
>    VERIFY( !result );
>    const auto bad_ec = ec;
>
>    __gnu_test::scoped_file f1(p1);
>    result = equivalent(p1, p2, ec);
> -  VERIFY( !ec );
> +  VERIFY( ec == std::errc::no_such_file_or_directory );
>    VERIFY( !result );
>
>    __gnu_test::scoped_file f2(p2);
> --
> 2.43.0
>

Re: Re: [PATCH v2 1/2] libstdc++: Fix error handling in filesystem::equivalent [PR113250]

[Ken Matsui]

On Thu, 11 Jan 2024 at 10:46, Jonathan Wakely <jwakely@redhat.com> wrote:
> On Thu, 11 Jan 2024 at 09:43, Ken Matsui <kmatsui@gcc.gnu.org> wrote:
> >
> > This patch made std::filesystem::equivalent correctly throw an exception
> > when either path does not exist as per [fs.op.equivalent]/4.
> 
> Thanks, OK for trunk and all active branches (let me know if you need
> help backporting it).
> 

Thank you for your review as always!  I do not know how to backport this
to the active branches.  I think the following page is explaining it,
but I am not sure how I can know all the active branches.

https://gcc.gnu.org/wiki/GitCookbook#backport

Do we basically want to git checkout & gcc-backport for each branch
after this patch is committed to the trunk?

> 
> >
> > libstdc++-v3/ChangeLog:
> >
> >         * src/c++17/fs_ops.cc (fs::equivalent): Use || instead of &&.
> >         * src/filesystem/ops.cc (fs::equivalent): Likewise.
> >         * testsuite/27_io/filesystem/operations/equivalent.cc: Handle
> >         error codes.
> >         * testsuite/experimental/filesystem/operations/equivalent.cc:
> >         Likewise.
> >
> > Signed-off-by: Ken Matsui <kmatsui@gcc.gnu.org>
> > ---
> >  libstdc++-v3/src/c++17/fs_ops.cc                              | 2 +-
> >  libstdc++-v3/src/filesystem/ops.cc                            | 2 +-
> >  .../testsuite/27_io/filesystem/operations/equivalent.cc       | 4 ++--
> >  .../experimental/filesystem/operations/equivalent.cc          | 4 ++--
> >  4 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc
> > index e0b308a37ea..61df19753ef 100644
> > --- a/libstdc++-v3/src/c++17/fs_ops.cc
> > +++ b/libstdc++-v3/src/c++17/fs_ops.cc
> > @@ -897,7 +897,7 @@ fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
> >        return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino;
> >  #endif
> >      }
> > -  else if (!exists(s1) && !exists(s2))
> > +  else if (!exists(s1) || !exists(s2))
> >      ec = std::make_error_code(std::errc::no_such_file_or_directory);
> >    else if (err)
> >      ec.assign(err, std::generic_category());
> > diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
> > index eccdae3d910..4d23a804da0 100644
> > --- a/libstdc++-v3/src/filesystem/ops.cc
> > +++ b/libstdc++-v3/src/filesystem/ops.cc
> > @@ -765,7 +765,7 @@ fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
> >         return false;
> >        return st1.st_dev == st2.st_dev && st1.st_ino == st2.st_ino;
> >      }
> > -  else if (!exists(s1) && !exists(s2))
> > +  else if (!exists(s1) || !exists(s2))
> >      ec = std::make_error_code(std::errc::no_such_file_or_directory);
> >    else if (err)
> >      ec.assign(err, std::generic_category());
> > diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc b/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc
> > index 78f6e368204..68f32366d65 100644
> > --- a/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc
> > +++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/equivalent.cc
> > @@ -34,13 +34,13 @@ test01()
> >    bool result;
> >
> >    result = equivalent(p1, p2, ec);
> > -  VERIFY( ec );
> > +  VERIFY( ec == std::errc::no_such_file_or_directory );
> >    VERIFY( !result );
> >
> >    __gnu_test::scoped_file f1(p1);
> >    ec = bad_ec;
> >    result = equivalent(p1, p2, ec);
> > -  VERIFY( !ec );
> > +  VERIFY( ec == std::errc::no_such_file_or_directory );
> >    VERIFY( !result );
> >
> >    __gnu_test::scoped_file f2(p2);
> > diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc
> > index 929a6ca8609..5bc477a31cd 100644
> > --- a/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc
> > +++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/equivalent.cc
> > @@ -35,13 +35,13 @@ test01()
> >    bool result;
> >
> >    result = equivalent(p1, p2, ec);
> > -  VERIFY( ec );
> > +  VERIFY( ec == std::errc::no_such_file_or_directory );
> >    VERIFY( !result );
> >    const auto bad_ec = ec;
> >
> >    __gnu_test::scoped_file f1(p1);
> >    result = equivalent(p1, p2, ec);
> > -  VERIFY( !ec );
> > +  VERIFY( ec == std::errc::no_such_file_or_directory );
> >    VERIFY( !result );
> >
> >    __gnu_test::scoped_file f2(p2);
> > --
> > 2.43.0
> >
> 

-- 
Ken Matsui

Backporting [was Re: [PATCH v2 1/2] libstdc++: Fix error handling in filesystem::equivalent [PR113250]]

[Jonathan Wakely]

On Thu, 11 Jan 2024 at 10:56, Ken Matsui <kmatsui@gcc.gnu.org> wrote:
>
> On Thu, 11 Jan 2024 at 10:46, Jonathan Wakely <jwakely@redhat.com> wrote:
> > On Thu, 11 Jan 2024 at 09:43, Ken Matsui <kmatsui@gcc.gnu.org> wrote:
> > >
> > > This patch made std::filesystem::equivalent correctly throw an exception
> > > when either path does not exist as per [fs.op.equivalent]/4.
> >
> > Thanks, OK for trunk and all active branches (let me know if you need
> > help backporting it).
> >
>
> Thank you for your review as always!  I do not know how to backport this
> to the active branches.  I think the following page is explaining it,
> but I am not sure how I can know all the active branches.
>
> https://gcc.gnu.org/wiki/GitCookbook#backport

Supported releases are listed on the front page at gcc.gnu.org, the
active branches are currently releases/gcc-11, releases/gcc-12 and
releases/gcc-13.

>
> Do we basically want to git checkout & gcc-backport for each branch
> after this patch is committed to the trunk?

Almost. I use gcc-backport for the newest release branch
(releases/gcc-13) and then I just use 'git cherry-pick' to cherry-pick
the gcc-13 commit onto gcc-12, and then cherry-pick the gcc-12 commit
onto gcc-11.

The reason for this is that there might be some changes needed on a
branch, either to resolve conflicts, or because of other differences
on the branch. e.g. when I did 'git gcc-backport 74a0dab18292be' to
backport that to gcc-13 I had to remove the changes to
include/bits/version.* and edit include/std/version instead (because
we do feature test macros differently on trunk).

If I then wanted to backport it to gcc-12 and I just did 'git
gcc-backport 74a0dab18292be' again in the gcc-12 branch, I would have
to resolve the same conflicts again. If I do 'git cherry-pick
c5ef02e5629f8c' instead (using the hash of the commit on the gcc-13
branch) then it will apply cleanly to gcc-12, because I'm using the
commit that already has the conflicts resolved.

Then if I want to backport to gcc-11 as well, use cherry-pick with the
hash from the gcc-12 branch.

This way any fixes that were needed for branch N-1 will get backported
to N-2 as well. Sometimes this doesn't matter, e.g. the trunk commit
might apply cleanly to every branch. But sometimes the commit needs
slightly more massaging to apply to each older branch, so doing it
trunk->13 then 13->12 then 12->11 tends to work better.

The reason I use cherry-pick after the first backport (instead of
gcc-backport every time) is because I don't want a second "(cherry
picked from commit ...)" line to be added to the commit message.
That's added by gcc-backport (by using cherry-pick -x) but we only
need to add it once to be able to track the provenance of the
backport, to know which trunk patch was backported.

If cherry picking a backport fails and creates a mess of conflicts and
you just want to give up and start again, 'git cherry-pick --abort'
will undo the changes and leave the working tree clean again. This
works whether you use gcc-backport or cherry-pick (because
gcc-backport just uses cherry-pick).

Re: Backporting [was Re: [PATCH v2 1/2] libstdc++: Fix error handling in filesystem::equivalent [PR113250]]

[Ken Matsui]

On Thu, 11 Jan 2024 at 11:14, Jonathan Wakely <jwakely@redhat.com> wrote:
> On Thu, 11 Jan 2024 at 10:56, Ken Matsui <kmatsui@gcc.gnu.org> wrote:
> >
> > On Thu, 11 Jan 2024 at 10:46, Jonathan Wakely <jwakely@redhat.com> wrote:
> > > On Thu, 11 Jan 2024 at 09:43, Ken Matsui <kmatsui@gcc.gnu.org> wrote:
> > > >
> > > > This patch made std::filesystem::equivalent correctly throw an exception
> > > > when either path does not exist as per [fs.op.equivalent]/4.
> > >
> > > Thanks, OK for trunk and all active branches (let me know if you need
> > > help backporting it).
> > >
> >
> > Thank you for your review as always!  I do not know how to backport this
> > to the active branches.  I think the following page is explaining it,
> > but I am not sure how I can know all the active branches.
> >
> > https://gcc.gnu.org/wiki/GitCookbook#backport
> 
> Supported releases are listed on the front page at gcc.gnu.org, the
> active branches are currently releases/gcc-11, releases/gcc-12 and
> releases/gcc-13.
> 
> >
> > Do we basically want to git checkout & gcc-backport for each branch
> > after this patch is committed to the trunk?
> 
> Almost. I use gcc-backport for the newest release branch
> (releases/gcc-13) and then I just use 'git cherry-pick' to cherry-pick
> the gcc-13 commit onto gcc-12, and then cherry-pick the gcc-12 commit
> onto gcc-11.
> 
> The reason for this is that there might be some changes needed on a
> branch, either to resolve conflicts, or because of other differences
> on the branch. e.g. when I did 'git gcc-backport 74a0dab18292be' to
> backport that to gcc-13 I had to remove the changes to
> include/bits/version.* and edit include/std/version instead (because
> we do feature test macros differently on trunk).
> 
> If I then wanted to backport it to gcc-12 and I just did 'git
> gcc-backport 74a0dab18292be' again in the gcc-12 branch, I would have
> to resolve the same conflicts again. If I do 'git cherry-pick
> c5ef02e5629f8c' instead (using the hash of the commit on the gcc-13
> branch) then it will apply cleanly to gcc-12, because I'm using the
> commit that already has the conflicts resolved.
> 
> Then if I want to backport to gcc-11 as well, use cherry-pick with the
> hash from the gcc-12 branch.
> 
> This way any fixes that were needed for branch N-1 will get backported
> to N-2 as well. Sometimes this doesn't matter, e.g. the trunk commit
> might apply cleanly to every branch. But sometimes the commit needs
> slightly more massaging to apply to each older branch, so doing it
> trunk->13 then 13->12 then 12->11 tends to work better.
> 
> The reason I use cherry-pick after the first backport (instead of
> gcc-backport every time) is because I don't want a second "(cherry
> picked from commit ...)" line to be added to the commit message.
> That's added by gcc-backport (by using cherry-pick -x) but we only
> need to add it once to be able to track the provenance of the
> backport, to know which trunk patch was backported.
> 
> If cherry picking a backport fails and creates a mess of conflicts and
> you just want to give up and start again, 'git cherry-pick --abort'
> will undo the changes and leave the working tree clean again. This
> works whether you use gcc-backport or cherry-pick (because
> gcc-backport just uses cherry-pick).
> 

Thank you for the detailed explanation!  I think I was able to backport
the patch to the active branches.

-- 
Ken Matsui

Re: Backporting [was Re: [PATCH v2 1/2] libstdc++: Fix error handling in filesystem::equivalent [PR113250]]

[Ken Matsui]

On Thu, Jan 11, 2024 at 3:45 AM Ken Matsui <kmatsui@cs.washington.edu> wrote:
>
> On Thu, 11 Jan 2024 at 11:14, Jonathan Wakely <jwakely@redhat.com> wrote:
> > On Thu, 11 Jan 2024 at 10:56, Ken Matsui <kmatsui@gcc.gnu.org> wrote:
> > >
> > > On Thu, 11 Jan 2024 at 10:46, Jonathan Wakely <jwakely@redhat.com> wrote:
> > > > On Thu, 11 Jan 2024 at 09:43, Ken Matsui <kmatsui@gcc.gnu.org> wrote:
> > > > >
> > > > > This patch made std::filesystem::equivalent correctly throw an exception
> > > > > when either path does not exist as per [fs.op.equivalent]/4.
> > > >
> > > > Thanks, OK for trunk and all active branches (let me know if you need
> > > > help backporting it).
> > > >
> > >
> > > Thank you for your review as always!  I do not know how to backport this
> > > to the active branches.  I think the following page is explaining it,
> > > but I am not sure how I can know all the active branches.
> > >
> > > https://gcc.gnu.org/wiki/GitCookbook#backport
> >
> > Supported releases are listed on the front page at gcc.gnu.org, the
> > active branches are currently releases/gcc-11, releases/gcc-12 and
> > releases/gcc-13.
> >
> > >
> > > Do we basically want to git checkout & gcc-backport for each branch
> > > after this patch is committed to the trunk?
> >
> > Almost. I use gcc-backport for the newest release branch
> > (releases/gcc-13) and then I just use 'git cherry-pick' to cherry-pick
> > the gcc-13 commit onto gcc-12, and then cherry-pick the gcc-12 commit
> > onto gcc-11.
> >
> > The reason for this is that there might be some changes needed on a
> > branch, either to resolve conflicts, or because of other differences
> > on the branch. e.g. when I did 'git gcc-backport 74a0dab18292be' to
> > backport that to gcc-13 I had to remove the changes to
> > include/bits/version.* and edit include/std/version instead (because
> > we do feature test macros differently on trunk).
> >
> > If I then wanted to backport it to gcc-12 and I just did 'git
> > gcc-backport 74a0dab18292be' again in the gcc-12 branch, I would have
> > to resolve the same conflicts again. If I do 'git cherry-pick
> > c5ef02e5629f8c' instead (using the hash of the commit on the gcc-13
> > branch) then it will apply cleanly to gcc-12, because I'm using the
> > commit that already has the conflicts resolved.
> >
> > Then if I want to backport to gcc-11 as well, use cherry-pick with the
> > hash from the gcc-12 branch.
> >
> > This way any fixes that were needed for branch N-1 will get backported
> > to N-2 as well. Sometimes this doesn't matter, e.g. the trunk commit
> > might apply cleanly to every branch. But sometimes the commit needs
> > slightly more massaging to apply to each older branch, so doing it
> > trunk->13 then 13->12 then 12->11 tends to work better.
> >
> > The reason I use cherry-pick after the first backport (instead of
> > gcc-backport every time) is because I don't want a second "(cherry
> > picked from commit ...)" line to be added to the commit message.
> > That's added by gcc-backport (by using cherry-pick -x) but we only
> > need to add it once to be able to track the provenance of the
> > backport, to know which trunk patch was backported.
> >
> > If cherry picking a backport fails and creates a mess of conflicts and
> > you just want to give up and start again, 'git cherry-pick --abort'
> > will undo the changes and leave the working tree clean again. This
> > works whether you use gcc-backport or cherry-pick (because
> > gcc-backport just uses cherry-pick).
> >
>
> Thank you for the detailed explanation!  I think I was able to backport
> the patch to the active branches.
>

For the Bugzilla issue, should I update the status to RESOLVED?  Or
does someone else handle this?  Also, are there other things I should
do about this issue?

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113250

> --
> Ken Matsui

Re: Backporting [was Re: [PATCH v2 1/2] libstdc++: Fix error handling in filesystem::equivalent [PR113250]]

[Jonathan Wakely]

On Thu, 11 Jan 2024 at 12:23, Ken Matsui <kmatsui@gcc.gnu.org> wrote:
>
> On Thu, Jan 11, 2024 at 3:45 AM Ken Matsui <kmatsui@cs.washington.edu> wrote:
> >
> > On Thu, 11 Jan 2024 at 11:14, Jonathan Wakely <jwakely@redhat.com> wrote:
> > > On Thu, 11 Jan 2024 at 10:56, Ken Matsui <kmatsui@gcc.gnu.org> wrote:
> > > >
> > > > On Thu, 11 Jan 2024 at 10:46, Jonathan Wakely <jwakely@redhat.com> wrote:
> > > > > On Thu, 11 Jan 2024 at 09:43, Ken Matsui <kmatsui@gcc.gnu.org> wrote:
> > > > > >
> > > > > > This patch made std::filesystem::equivalent correctly throw an exception
> > > > > > when either path does not exist as per [fs.op.equivalent]/4.
> > > > >
> > > > > Thanks, OK for trunk and all active branches (let me know if you need
> > > > > help backporting it).
> > > > >
> > > >
> > > > Thank you for your review as always!  I do not know how to backport this
> > > > to the active branches.  I think the following page is explaining it,
> > > > but I am not sure how I can know all the active branches.
> > > >
> > > > https://gcc.gnu.org/wiki/GitCookbook#backport
> > >
> > > Supported releases are listed on the front page at gcc.gnu.org, the
> > > active branches are currently releases/gcc-11, releases/gcc-12 and
> > > releases/gcc-13.
> > >
> > > >
> > > > Do we basically want to git checkout & gcc-backport for each branch
> > > > after this patch is committed to the trunk?
> > >
> > > Almost. I use gcc-backport for the newest release branch
> > > (releases/gcc-13) and then I just use 'git cherry-pick' to cherry-pick
> > > the gcc-13 commit onto gcc-12, and then cherry-pick the gcc-12 commit
> > > onto gcc-11.
> > >
> > > The reason for this is that there might be some changes needed on a
> > > branch, either to resolve conflicts, or because of other differences
> > > on the branch. e.g. when I did 'git gcc-backport 74a0dab18292be' to
> > > backport that to gcc-13 I had to remove the changes to
> > > include/bits/version.* and edit include/std/version instead (because
> > > we do feature test macros differently on trunk).
> > >
> > > If I then wanted to backport it to gcc-12 and I just did 'git
> > > gcc-backport 74a0dab18292be' again in the gcc-12 branch, I would have
> > > to resolve the same conflicts again. If I do 'git cherry-pick
> > > c5ef02e5629f8c' instead (using the hash of the commit on the gcc-13
> > > branch) then it will apply cleanly to gcc-12, because I'm using the
> > > commit that already has the conflicts resolved.
> > >
> > > Then if I want to backport to gcc-11 as well, use cherry-pick with the
> > > hash from the gcc-12 branch.
> > >
> > > This way any fixes that were needed for branch N-1 will get backported
> > > to N-2 as well. Sometimes this doesn't matter, e.g. the trunk commit
> > > might apply cleanly to every branch. But sometimes the commit needs
> > > slightly more massaging to apply to each older branch, so doing it
> > > trunk->13 then 13->12 then 12->11 tends to work better.
> > >
> > > The reason I use cherry-pick after the first backport (instead of
> > > gcc-backport every time) is because I don't want a second "(cherry
> > > picked from commit ...)" line to be added to the commit message.
> > > That's added by gcc-backport (by using cherry-pick -x) but we only
> > > need to add it once to be able to track the provenance of the
> > > backport, to know which trunk patch was backported.
> > >
> > > If cherry picking a backport fails and creates a mess of conflicts and
> > > you just want to give up and start again, 'git cherry-pick --abort'
> > > will undo the changes and leave the working tree clean again. This
> > > works whether you use gcc-backport or cherry-pick (because
> > > gcc-backport just uses cherry-pick).
> > >
> >
> > Thank you for the detailed explanation!  I think I was able to backport
> > the patch to the active branches.
> >
>
> For the Bugzilla issue, should I update the status to RESOLVED?  Or
> does someone else handle this?  Also, are there other things I should
> do about this issue?
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113250

Yes, please close as RESOLVED FIXED and set the target milestone to
the first release with the fix, which will be 11.5

In cases like this where the fix is backported to multiple release
branches I also add a comment to say "fixed in 11.5, 12.4, 13.3"
because otherwise just setting the target milestone to 11.5 doesn't
tell you about the other branches.

It's also nice to thank the reporter for filing it, especially when
they identify the root cause and suggest a fix.

And thank you for taking care of it!

Re: Backporting [was Re: [PATCH v2 1/2] libstdc++: Fix error handling in filesystem::equivalent [PR113250]]

[Ken Matsui]

On Thu, Jan 11, 2024 at 4:27 AM Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On Thu, 11 Jan 2024 at 12:23, Ken Matsui <kmatsui@gcc.gnu.org> wrote:
> >
> > On Thu, Jan 11, 2024 at 3:45 AM Ken Matsui <kmatsui@cs.washington.edu> wrote:
> > >
> > > On Thu, 11 Jan 2024 at 11:14, Jonathan Wakely <jwakely@redhat.com> wrote:
> > > > On Thu, 11 Jan 2024 at 10:56, Ken Matsui <kmatsui@gcc.gnu.org> wrote:
> > > > >
> > > > > On Thu, 11 Jan 2024 at 10:46, Jonathan Wakely <jwakely@redhat.com> wrote:
> > > > > > On Thu, 11 Jan 2024 at 09:43, Ken Matsui <kmatsui@gcc.gnu.org> wrote:
> > > > > > >
> > > > > > > This patch made std::filesystem::equivalent correctly throw an exception
> > > > > > > when either path does not exist as per [fs.op.equivalent]/4.
> > > > > >
> > > > > > Thanks, OK for trunk and all active branches (let me know if you need
> > > > > > help backporting it).
> > > > > >
> > > > >
> > > > > Thank you for your review as always!  I do not know how to backport this
> > > > > to the active branches.  I think the following page is explaining it,
> > > > > but I am not sure how I can know all the active branches.
> > > > >
> > > > > https://gcc.gnu.org/wiki/GitCookbook#backport
> > > >
> > > > Supported releases are listed on the front page at gcc.gnu.org, the
> > > > active branches are currently releases/gcc-11, releases/gcc-12 and
> > > > releases/gcc-13.
> > > >
> > > > >
> > > > > Do we basically want to git checkout & gcc-backport for each branch
> > > > > after this patch is committed to the trunk?
> > > >
> > > > Almost. I use gcc-backport for the newest release branch
> > > > (releases/gcc-13) and then I just use 'git cherry-pick' to cherry-pick
> > > > the gcc-13 commit onto gcc-12, and then cherry-pick the gcc-12 commit
> > > > onto gcc-11.
> > > >
> > > > The reason for this is that there might be some changes needed on a
> > > > branch, either to resolve conflicts, or because of other differences
> > > > on the branch. e.g. when I did 'git gcc-backport 74a0dab18292be' to
> > > > backport that to gcc-13 I had to remove the changes to
> > > > include/bits/version.* and edit include/std/version instead (because
> > > > we do feature test macros differently on trunk).
> > > >
> > > > If I then wanted to backport it to gcc-12 and I just did 'git
> > > > gcc-backport 74a0dab18292be' again in the gcc-12 branch, I would have
> > > > to resolve the same conflicts again. If I do 'git cherry-pick
> > > > c5ef02e5629f8c' instead (using the hash of the commit on the gcc-13
> > > > branch) then it will apply cleanly to gcc-12, because I'm using the
> > > > commit that already has the conflicts resolved.
> > > >
> > > > Then if I want to backport to gcc-11 as well, use cherry-pick with the
> > > > hash from the gcc-12 branch.
> > > >
> > > > This way any fixes that were needed for branch N-1 will get backported
> > > > to N-2 as well. Sometimes this doesn't matter, e.g. the trunk commit
> > > > might apply cleanly to every branch. But sometimes the commit needs
> > > > slightly more massaging to apply to each older branch, so doing it
> > > > trunk->13 then 13->12 then 12->11 tends to work better.
> > > >
> > > > The reason I use cherry-pick after the first backport (instead of
> > > > gcc-backport every time) is because I don't want a second "(cherry
> > > > picked from commit ...)" line to be added to the commit message.
> > > > That's added by gcc-backport (by using cherry-pick -x) but we only
> > > > need to add it once to be able to track the provenance of the
> > > > backport, to know which trunk patch was backported.
> > > >
> > > > If cherry picking a backport fails and creates a mess of conflicts and
> > > > you just want to give up and start again, 'git cherry-pick --abort'
> > > > will undo the changes and leave the working tree clean again. This
> > > > works whether you use gcc-backport or cherry-pick (because
> > > > gcc-backport just uses cherry-pick).
> > > >
> > >
> > > Thank you for the detailed explanation!  I think I was able to backport
> > > the patch to the active branches.
> > >
> >
> > For the Bugzilla issue, should I update the status to RESOLVED?  Or
> > does someone else handle this?  Also, are there other things I should
> > do about this issue?
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113250
>
> Yes, please close as RESOLVED FIXED and set the target milestone to
> the first release with the fix, which will be 11.5
>
> In cases like this where the fix is backported to multiple release
> branches I also add a comment to say "fixed in 11.5, 12.4, 13.3"
> because otherwise just setting the target milestone to 11.5 doesn't
> tell you about the other branches.
>
> It's also nice to thank the reporter for filing it, especially when
> they identify the root cause and suggest a fix.
>
> And thank you for taking care of it!
>
Done!  I truly appreciate your kind step-by-step guidance!