bug#62572: Bug#1058752: bug#62572: cp --no-clobber behavior has changed

[Pádraig Brady]

On 17/12/2023 14:46, Pádraig Brady wrote:
> On 16/12/2023 21:46, Bernhard Voelker wrote:
> > On 12/15/23 21:13, Michael Stone wrote:
> > > On Fri, Dec 15, 2023 at 11:21:06AM -0800, Paul Eggert wrote:
> > > > Stlll, Pádraig gave a reasonable summary of why the change was made,
>
> To clarify my summary a little, there I said that -n now _immediately_ fails.
> I should have said _silently_ fails.  I.e. the complete copy operation
> proceeds as before, and only the exit status is at issue here.
>
> > > > despite its incompatibility with previous behavior. (One thing I'd add
> > > > is that the FreeBSD behavior is inherently less race-prone.)
> >
> > Whether the implementation is race-prone or not is an internal thing.
> > I think we're currently discussing more on a user-perspective level.
> >
> > IIUC then the question is whether `cp -n` should continue to behave like
> > the (new) `cp --update=none` which returns EXIT_SUCCESS.
> >
> > Regardless what other implementations do, when reading the -n description
> > from a user's point of view:
> >
> >      -n, --no-clobber             do not overwrite an existing file (overrides a
> >                                     -u or previous -i option). See also --update
> >
> > then I'd expect the tool to just skip existing files like `rsync 
> > --ignore-existing`
> > does.  In that regard I would be surprised if skipping files would result in an 
> > error.
> > Well, I would understand if there'd be a '--no-clobber=fail' option.
>
> Agreed we should improve the docs a bit for this option.
> I'll apply this at least:
>
> diff --git a/doc/coreutils.texi b/doc/coreutils.texi
> index 1f8b356d1..bf0f424d3 100644
> --- a/doc/coreutils.texi
> +++ b/doc/coreutils.texi
> @@ -9057,6 +9057,8 @@ Do not overwrite an existing file; silently fail instead.
>    This option overrides a previous
>    @option{-i} option.  This option is mutually exclusive with @option{-b} or
>    @option{--backup} option.
> +See also the @option{--update=none} option which will
> +skip existing files but not fail.
>
>    @item -P
>    @itemx --no-dereference
> diff --git a/src/cp.c b/src/cp.c
> index 04a5cbee3..3ccc4c4e6 100644
> --- a/src/cp.c
> +++ b/src/cp.c
> @@ -192,8 +192,8 @@ Copy SOURCE to DEST, or multiple SOURCE(s) to DIRECTORY.\n\
>      -L, --dereference            always follow symbolic links in SOURCE\n\
>    "), stdout);
>          fputs (_("\
> -  -n, --no-clobber             do not overwrite an existing file (overrides 
> a\n\
> -                                 -u or previous -i option). See also 
> --update\n\
> +  -n, --no-clobber             ensure no existing files overwritten, and 
> fail\n\
> +                                 silently instead. See also --update\n\
>    "), stdout);
>          fputs (_("\
>      -P, --no-dereference         never follow symbolic links in SOURCE\n\
>
>
> > As Kamil added the option in 2009, I'd assume that the same patch was already
> > active in RHEL versions for quite some longer time.
> > Now changing the exit code feels kind of rough.
>
> Well RHEL 6 came out a bit after (2010), and had the --no-clobber change,
> while RHEL 5 before that did not.
>
> Taking about distros, it's worth noting that the change is Fedora 39
> which has been released for a month now.
> We'll keep a close eye on issues, but haven't heard much as
> of yet at least.
>
> > Therefore, from a pure user's perspective and regarding many years of 
> > precedence,
> > I am 80:20 for reverting the exit code change.
>
> Thanks for your thoughts,
> appreciated as always.

We were undecided how to proceed as of the above discussion,
with some hoping to consolidate -n behavior across all systems,
with others preferring to keep compat with original Linux behavior.

Things have changed in Debian I see, so that cp -n now aggressively warns like:
https://sources.debian.org/patches/coreutils/9.4-3/cp-n.diff/

  $ cp -n /bin/true tmp
  cp: warning: behavior of -n is non-portable and may change in future; use 
--update=none instead

This is problematic as:

  - It's noisy
  - There is no way to get the behavior of indicating failure if existing files 
present
  - The --update=none advice is only portable to newer coreutils

To summarise existing use cases of -n:

  1. User expected -n to exit failure if existing files
  2. User didn't care about existing status
  3. User expected -n to exit success if existing files

Use case 3 is the problematic case we changed behavior for,
but also I think that's the minority of the 3 use cases
and could have been fixed forward.

The debian advice forces use case 2 to change,
and provides bad advice for use case 1,
and in fact there is no solution now for use case 1.

At this stage it seems best for us go back to the original Linux behiavor (use 
case 3),
and to silently deprecate -n in docs to document the portability issues with it.
We should also provide --update=noclobber for use case 1.
Having the control on the --update option, allows use to more clearly deprecate 
-n.

I do realise folks may expect use case 1 now with -n,
especially on releases like Fedora 39, but given the feedback
that would be the least bad option.

thanks,
Pádraig