[Bug libstdc++/106932] New: Incorrect behavior of std::filesystem::copy() with overwrite_existing or update_existing options

[Bug libstdc++/106932] New: Incorrect behavior of std::filesystem::copy() with overwrite_existing or update_existing options

[thomas.allen at intel dot com]

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

            Bug ID: 106932
           Summary: Incorrect behavior of std::filesystem::copy() with
                    overwrite_existing or update_existing options
           Product: gcc
           Version: 11.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: thomas.allen at intel dot com
  Target Milestone: ---

When using std::filesystem::copy_options::overwrite_existing or
std::filesystem::copy_options::update_existing as part of a call to
std::filesystem::copy(), no destination directory is created, and the source
directory is not copied. In addition, no exception is thrown. If neither of
these options are specified, the behavior is as expected, creating the
destination directory and copying all regular files into it.

This appears to be contrary to bullet 4.7.4 in Section 29.11.14.3 of the C++20
spec, where for regular files in a source directory, the effect should be
equivalent to passing any options through to a copy_file() call on each file.

This bug occurs on SUSE Linux Enterprise Server 12 SP5, running on an Intel
Xeon Gold 6136 CPU.

Additionally, testing with GCC 10.2.0 and 12.2.0 shows the same issue.

The build configuration options used with GCC 11.3.0 specifically were:
/nfs/orto/proj/tapeout/cit_rep/ImagingToolsSupport/v2/bootstrap/build/sles12/gcc920/gcc/11.3.0/default/extract/gcc-11.3.0/configure
--prefix=/nfs/orto/proj/tapeout/cit_rep/ImagingToolsSupport/v2/bootstrap/install/sles12/gcc920/gcc/11.3.0/default
--with-specs='%{!static:%x{-rpath=/nfs/orto/proj/tapeout/cit_rep/ImagingToolsSupport/v2/bootstrap/install/sles12/gcc920/gcc/11.3.0/default/lib64:/nfs/orto/proj/tapeout/cit_rep/ImagingToolsSupport/v2/bootstrap/install/sles12/gcc920/gcc/11.3.0/default/lib}}'
--enable-lto

The command line which triggers this bug is:
g++ -std=c++20 -Wall -o dir_copy_test dir_copy_test.cpp

No errors or warnings are emitted by the compiler, and the source directory for
the test case is identical in structure to the one shown in the Notes section
at https://en.cppreference.com/w/cpp/filesystem/copy, i.e.

copy_test/
`-- source
    |-- file1
    |-- file3
    `-- subdir
        `-- file2

To produce the issue:
cd copy_test
dir_copy_test source dest

The preprocessor output for the test code is attached.

[Bug libstdc++/106932] Incorrect behavior of std::filesystem::copy() with overwrite_existing or update_existing options

[thomas.allen at intel dot com]

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

--- Comment #1 from Tom Allen <thomas.allen at intel dot com> ---
Created attachment 53573
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53573&action=edit
Preprocessor output for minimal testcase reproducing this issue.

[Bug libstdc++/106932] Incorrect behavior of std::filesystem::copy() with overwrite_existing or update_existing options

[redi at gcc dot gnu.org]

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

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I think this is the correct behaviour according to the standard.

Where f is status("source") and t is status("dest").

Effects are then as follows:

- If f.type() or t.type() is an implementation-defined file type ...

[they're not]

- Otherwise, an error is reported as specified in 31.12.5 if:

[list of conditions that are not true]

- Otherwise, if is_symlink(f), then:

[it's not]

- Otherwise, if is_regular_file(f), then:

[it's not]

- Otherwise, if
  is_directory(f) &&
  (options & copy_options::create_symlinks) != copy_options::none

[create_symlinks is not set in the options]

- Otherwise, if
  is_directory(f) &&
  ((options & copy_options::recursive) != copy_options::none ||
    options == copy_options::none)

[this is the case we want to hit, but the condition is false because recursive
is not set and options != none]

- Otherwise, for the signature with argument ec, ec.clear().

[You didn't pass an erroc_code]

- Otherwise, no effects.

[Bingo]

So you need to use copy_options::recursive to get the effects you want.

[Bug libstdc++/106932] Incorrect behavior of std::filesystem::copy() with overwrite_existing or update_existing options

[redi at gcc dot gnu.org]

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

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Tom Allen from comment #0)
> This appears to be contrary to bullet 4.7.4 in Section 29.11.14.3 of the
> C++20 spec, where for regular files in a source directory, the effect should
> be equivalent to passing any options through to a copy_file() call on each
> file.

No, because f is not a regular file, it's the directory "source".

[Bug libstdc++/106932] Incorrect behavior of std::filesystem::copy() with overwrite_existing or update_existing options

[thomas.allen at intel dot com]

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

--- Comment #4 from Tom Allen <thomas.allen at intel dot com> ---
(In reply to Jonathan Wakely from comment #2)
> I think this is the correct behaviour according to the standard.
> 
> Where f is status("source") and t is status("dest").
> 
> Effects are then as follows:
> 
> - If f.type() or t.type() is an implementation-defined file type ...
> 
> [they're not]
> 
> - Otherwise, an error is reported as specified in 31.12.5 if:
> 
> [list of conditions that are not true]
> 
> - Otherwise, if is_symlink(f), then:
> 
> [it's not]
> 
> - Otherwise, if is_regular_file(f), then:
> 
> [it's not]
> 
> - Otherwise, if
>   is_directory(f) &&
>   (options & copy_options::create_symlinks) != copy_options::none
> 
> [create_symlinks is not set in the options]
> 
> - Otherwise, if
>   is_directory(f) &&
>   ((options & copy_options::recursive) != copy_options::none ||
>     options == copy_options::none)
> 
> [this is the case we want to hit, but the condition is false because
> recursive is not set and options != none]
> 
> - Otherwise, for the signature with argument ec, ec.clear().
> 
> [You didn't pass an erroc_code]
> 
> - Otherwise, no effects.
> 
> [Bingo]
> 
> So you need to use copy_options::recursive to get the effects you want.

If this is the case, then when I have subdirectories which I explicitly do not
want to copy, but files at the same level in the source directory which I do
want to overwrite in their destinations, there is no way to perform the
operation unless I manually iterate and check file types, then call
copy_file(). That seems like a strange asymmetry compared to the recursive
call.

[Bug libstdc++/106932] Incorrect behavior of std::filesystem::copy() with overwrite_existing or update_existing options

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Tom Allen from comment #4)
> If this is the case, then when I have subdirectories which I explicitly do
> not want to copy, but files at the same level in the source directory which
> I do want to overwrite in their destinations, there is no way to perform the
> operation unless I manually iterate and check file types, then call
> copy_file().

I don't think that's quite true. You can still use filesystem::copy, you just
need to use copy_options specific to the file type. So iterate over the
directory contents and for directories, call filesystem::copy with no
arguments, and for files, call filesystem::copy with update_existing /
overwrite_existing.

> That seems like a strange asymmetry compared to the recursive
> call.

For a recursive call, passing copy_options that only make sense for files is
useful, because there could be files somewhere in a sub-dir that you want to
copy. Passing those when copying just a directory, without recursing, isn't
useful.

For a shallow, non-recursive copy, I think it makes sense that you would have
to call copy for each file individually, and if you're already doing that, it's
not such a stretch to use arguments specific to the file type.

In any case, that's what the standard says, so it's not a GCC bug.

If you think this is a defect in the standard then report it to the committee
instead: https://cplusplus.github.io/LWG/lwg-active.html#submit_issue

[Bug libstdc++/106932] Incorrect behavior of std::filesystem::copy() with overwrite_existing or update_existing options

[thomas.allen at intel dot com]

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

--- Comment #6 from Tom Allen <thomas.allen at intel dot com> ---
(In reply to Jonathan Wakely from comment #5)
> (In reply to Tom Allen from comment #4)
> > If this is the case, then when I have subdirectories which I explicitly do
> > not want to copy, but files at the same level in the source directory which
> > I do want to overwrite in their destinations, there is no way to perform the
> > operation unless I manually iterate and check file types, then call
> > copy_file().
> 
> I don't think that's quite true. You can still use filesystem::copy, you
> just need to use copy_options specific to the file type. So iterate over the
> directory contents and for directories, call filesystem::copy with no
> arguments, and for files, call filesystem::copy with update_existing /
> overwrite_existing.
> 
> > That seems like a strange asymmetry compared to the recursive
> > call.
> 
> For a recursive call, passing copy_options that only make sense for files is
> useful, because there could be files somewhere in a sub-dir that you want to
> copy. Passing those when copying just a directory, without recursing, isn't
> useful.
> 
> For a shallow, non-recursive copy, I think it makes sense that you would
> have to call copy for each file individually, and if you're already doing
> that, it's not such a stretch to use arguments specific to the file type.
> 
> In any case, that's what the standard says, so it's not a GCC bug.
> 
> If you think this is a defect in the standard then report it to the
> committee instead:
> https://cplusplus.github.io/LWG/lwg-active.html#submit_issue

In this case, would it be possible to add a warning to the compiler for this
usage? Even with -Wall -Wextra it's silent, and since it functions as a no-op,
it's somewhat confusing to track down.

[Bug libstdc++/106932] Incorrect behavior of std::filesystem::copy() with overwrite_existing or update_existing options

[redi at gcc dot gnu.org]

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

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
How would that work? The compile has no idea whether "source" is a regular file
or directory. Should it just suggest adding 'recursive' to the options whenever
a non-empty set of options is used?

[Bug libstdc++/106932] Incorrect behavior of std::filesystem::copy() with overwrite_existing or update_existing options

[rs2740 at gmail dot com]

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

TC <rs2740 at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rs2740 at gmail dot com

--- Comment #8 from TC <rs2740 at gmail dot com> ---
https://cplusplus.github.io/LWG/issue3057?

[Bug libstdc++/106932] [DR 3057] Incorrect behavior of std::filesystem::copy() with overwrite_existing or update_existing options

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
            Summary|Incorrect behavior of       |[DR 3057] Incorrect
                   |std::filesystem::copy()     |behavior of
                   |with overwrite_existing or  |std::filesystem::copy()
                   |update_existing options     |with overwrite_existing or
                   |                            |update_existing options
             Status|RESOLVED                    |SUSPENDED
   Last reconfirmed|                            |2022-09-16
         Resolution|INVALID                     |---

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Ah yes, thanks. I thought we'd resolved Davis's filesystem enum issues, but not
that one.
Let's suspend this bug then, pending a resolution from the committee.