Title
Correct copy_options handling
Status
open
Section
[fs.op.copy]
Submitter
Davis Herring

Created on 2018-01-29.00:00:00 last changed 66 months ago

Messages

Date: 2018-11-14.19:07:53

Proposed resolution:

This wording is relative to N4750.

  1. Modify Table 115 — "Enum class copy_options" as indicated:

    Option group controlling copy and copy_file function effects for existing target files
    Constant Meaning
    […] […]
  2. Modify [fs.op.copy] as indicated:

    Rationale:

    POSIX.1-2008 allows the implementation to create hard links "to" symbolic links, and provides linkat() to choose between the symlink and its target.

    [fs.op.copy]/4.9 is redundant given [fs.err.report]/3.1.

    void copy(const path& from, const path& to, copy_options options);
    void copy(const path& from, const path& to, copy_options options,
              error_code& ec) noexcept;
    

    -3- Requires: At most one element from each option group ([fs.enum.copy.opts]) is set in options.

    -4- Effects: Before the first use of f and t:

    1. (4.1) — If […]

    2. […]

    3. (4.10) — Otherwise, no effects.

    If each is needed below,
    auto linkf = (options & (copy_options::copy_symlinks |
                             copy_options::skip_symlinks)) != copy_options::none;
    auto f = linkf ? symlink_status(from) : status(from), t = status(to);
    auto to2 = !is_directory(f) && is_directory(t) ? to/from.filename() : to.
    bool linkt = (options & (copy_options::create_symlinks |
                             copy_options::create_hard_links)) != copy_options::none ||
                is_symlink(f);
    auto t2 = linkt ? symlink_status(to2) : status(to2);
    

    [Drafting note: copy_options::create_symlinks is intentionally omitted for linkf; it may simply have been a typo for copy_options::copy_symlinks (which was added by LWG 2681) since at least N3940.]

    Effects are then as follows:
    1. (?.?) — If f.type() or t.type() is an implementation-defined file type [fs.enum.file_type], then the effects are implementation-defined.

      [Drafting note: the text between the previous drafting note and this one is the only unchanged text under /4.]

    2. (?.?) — Otherwise, if exists(f) is false, report an error as specified in [fs.err.report].

    3. (?.?) — Otherwise, do nothing if

      1. (?.?.?) — (options & copy_options::directories_only) != copy_options::none and is_directory(f) is false, or

      2. (?.?.?) — (options & copy_options::skip_symlinks) != copy_options::none and is_symlink(f) is true, or

      3. (?.?.?) — (options & copy_options::skip_existing) != copy_options::none and exists(t2) is true.

    4. (?.?) — Otherwise, report an error as specified in [fs.err.report] if:

      1. (?.?.?) — is_other(f) || is_other(t2) is true, or

      2. (?.?.?) — exists(t2) && exists(from) == exists(to2) && equivalent(from, to) is true.

    5. (?.?) — Otherwise, if is_directory(f) is true, then:

      1. (?.?.?) — create_directory(to, from).

      2. (?.?.?) — If (options & copy_options::recursive) != copy_options::none or if (options & copy_options::directories_only) == copy_options::none, iterate over the files in from, as if by

        for (const directory_entry& x : directory_iterator(from))
          if ((options & copy_options::recursive) != copy_options::none ||
             !is_directory(linkf ? symlink_status(x.path()) : status(x.path())))
               copy(x.path(), to/x.path().filename(), options);
        
    6. (?.?) — Otherwise, do nothing if (options & copy_options::update_existing) != copy_options::none, exists(to2) is true, and from is not more recent than to2, determined as if by use of the last_write_time function ([fs.op.last_write_time]).

    7. (?.?) — Otherwise, report an error as specified in [fs.err.report] if:

      1. (?.?.?) — is_directory(t2) is true, or

      2. (?.?.?) — (options & (copy_options::overwrite_existing | copy_options::update_existing)) == copy_options::none and exists(t2) is true.

    8. (?.?) — Otherwise, if linkt is true, then:

      1. (?.?.?) — remove(to2) if an existing to2 would prevent the following link creation.

      2. (?.?.?) — If (options & copy_options::create_symlinks) != copy_options::none, create_symlink(from, to2). [Note: If from is a symbolic link, it is not followed. — end note]

      3. (?.?.?) — Otherwise, if (options & copy_options::create_hard_links) != copy_options::none, then create a hard link to from, if linkf is true, or else to the file that from resolves to. [Note: Not all file systems that support hard links and symbolic links support creating hard links to symbolic links. — end note]

      4. (?.?.?) — Otherwise, copy_symlink(from, to2).

    9. (?.?) — Otherwise, copy_file(from, to2, options).

    -5- Throws: As specified in [fs.err.report].

    -6- Remarks: For the signature with argument ec, any library functions called by the implementation shall have an error_code argument if applicable. If any such function fails, copy returns immediately without (further) modifying ec.

Date: 2018-11-15.00:00:00

[ 2018-11-13; Billy O'Neal comments ]

I (Billy O'Neal) prefer Davis' solution to LWG 3057, as I think the wording follows the meaning of the individual enum values more closely, and enables more scenarios to function correctly instead of reporting such cases as errors.

However, I don't want to adopt that wording as is because it requires my implementation to detect errors in places that force us to do a bunch of extra system calls, and I don't believe those specific ways error handling happens is relevant to what the copy API wants to do.

Ideally, the wording would be structured such that it said "here's a list of error conditions, if they happen we aren't going to tell you when exactly they are detected" and then listed the behavior irrespective of when errors happen. That way implementations can do the error checks when it makes sense according to what their system APIs report. For example, anything that requires symlink resolution is very expensive on my platform so I'd want to be able to defer anything related to status (rather than symlink_status) to after I've detected that there's actually a symlink (or junction) involved.

Date: 2018-11-12.05:21:03

[ 2018-11 San Diego Thursday night issue processing ]

Need to gather implementation experience; revisit in Kona. Status to Open.

Date: 2018-06-07.21:24:29

[ 2018-06; Rapperswil Wednesday evening, discussing LWG 2682 ]

JW: can we use the words we are shipping already since two years?
BO: what we got is better than what we had before
no objection to moving to Ready
ACTION move to Ready
ACTION link LWG 2682 and LWG 3057 and set a priority 2 and look at 3057 in San Diego

Date: 2018-01-29.00:00:00

(The resolution of #3 resolves part of C++17 NB comment US 36.)

The handling of several options for filesystem::copy() is wrong:

  1. Single-level directory copying is silently suppressed by any flag other than copy_options::recursive (even copy_options::directories_only). Single-level directory copying operates via using some unspecified flag to trigger this misfeature.

  2. copy_options::create_symlinks and copy_options::skip_symlinks affect the interpretation of the destination name; the latter shouldn't ever, and the former should affect only broken symlinks (since it would want to replace them).

  3. The copy_options groups for existing target files and the form of copying are consulted only for creating regular files.

  4. copy("file", "dir") creates dir/file, but copy("symlink", "dir", copy_options::copy_symlinks) fails.

  5. If a symlink is encountered with copy_options::copy_symlinks and copy_options::create_symlinks, the latter flag is ignored (but its otherwise sensible restriction to absolute paths applies) rather than the former.

  6. copy_options::create_symlinks without copy_options::copy_symlinks fails if it encounters a symlink. (This is particularly damaging for recursive operation.)

This issue, since it replaces so much text, also addresses two error-handling concerns in passing:

  1. The significance of equivalent(from, to) failing is unspecified. (Ignoring such failures entirely would make dangerous those operations that replace the target with a link.)

  2. Copying a directory involves several operations. When an error_code is being used, the process continues past errors and (because successful functions call ec.clear()) may suppress them.

This expands on the resolution for LWG 2681.

This also addresses the same issue as LWG 2682, but has a different result (based on the fact that the Example successfully copies directories to new, non-existent names).

History
Date User Action Args
2018-11-14 19:07:53adminsetmessages: + msg10226
2018-11-12 05:21:03adminsetmessages: + msg10216
2018-11-12 05:21:03adminsetstatus: new -> open
2018-06-07 21:24:29adminsetmessages: + msg9876
2018-01-30 21:56:15adminsetmessages: + msg9668
2018-01-29 00:00:00admincreate