Subject: bug#5775: [PATCH] doc: fix info to say cp -a does _not_ diagnose xattr failures

From: Pádraig Brady <[email-masked]>
Date: Mon, 12 Apr 2010 12:08:07 +0100

On 11/04/10 19:31, Jim Meyering wrote:
> Pádraig Brady wrote:
> ...
>> Well the patch _reduces_ warnings so that they're only
>> shown in case 3 above. They should probably also
>> be output in case 5 also which currently doesn't happen
>> as explained above.
> 
> Yes, but I'm convinced that those warnings have some
> value at least some of the time, so I'm very leery of
> suppressing them, even if the rules for when they're
> emitted are inconsistent and hard to describe.

OK fair enough, I'll push this doc clarification soon
which attempts to better describe what's happening.

cheers,
Pádraig.

commit c420cfef77b9173a754806dc49deacabb436044f
Author: Pádraig Brady <[email-masked]>
Date:   Mon Apr 12 08:46:37 2010 +0100

    doc: clarify when cp and mv output xattr warnings

    The 2010-03-26 commit, 4c38625e, "doc: fix info on cp --preserve..."
    was not entirely correct as cp --preserve=all does produce some
    xattr warnings.

    * src/copy.h: Update and clarify the comments for reduce_diagnostics
    and require_preserve_{xattr,context}.
    * doc/coreutils.texi (cp invocation): Update the -a and
    --preserve=xattr,context options to say when and which
    xattr warnings are output.
    (mv invocation): Mention that some warnings are output
    when preserving xattrs.
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index f549989..7561f2a 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7344,7 +7344,7 @@ directory structure; i.e., @samp{ls -U} may list the entries in a copied
 directory in a different order).
 Try to preserve SELinux security context and extended attributes (xattr),
 but ignore any failure to do that and print no corresponding diagnostic.
-Equivalent to @option{-dR --preserve=all}.
+Equivalent to @option{-dR --preserve=all} with the reduced diagnostics.

 @item -b
 @itemx @w{@kbd{--backup}[=@var{method}]}
@@ -7507,18 +7507,18 @@ $ mkdir b c; (cd b; : > a; ln -s a b); cp -aL b c; ls -i1 c/b
 @end smallexample

 @itemx context
-Preserve SELinux security context of the file. @command{cp} will fail
-if the preserving of SELinux security context is not succesful.
+Preserve SELinux security context of the file, or fail with full diagnostics.
 @itemx xattr
-Preserve extended attributes if @command{cp} is built with xattr support,
-and xattrs are supported and enabled on your file system.
+Preserve extended attributes of the file, or fail with full diagnostics.
+If @command{cp} is built without xattr support, ignore this option.
 If SELinux context, ACLs or Capabilities are implemented using xattrs,
 they are preserved by this option as well.
 @itemx all
 Preserve all file attributes.
 Equivalent to specifying all of the above, but with the difference
 that failure to preserve SELinux security context or extended attributes
-does not change @command{cp}'s exit status, or produce error messages.
+does not change @command{cp}'s exit status.  In contrast to @option{-a},
+all but @samp{Operation not supported} warnings are output.
 @end table

 Using @option{--preserve} with no @var{attribute_list} is equivalent
@@ -8276,6 +8276,7 @@ original partition.
 @cindex extended attributes, xattr
 @command{mv} always tries to copy extended attributes (xattr), which may
 include SELinux context, ACLs or Capabilities.
+Upon failure all but @samp{Operation not supported} warnings are output.

 @cindex prompting, and @command{mv}
 If a destination file exists but is normally
diff --git a/src/copy.h b/src/copy.h
index bd7359f..59e29f5 100644
--- a/src/copy.h
+++ b/src/copy.h
@@ -180,15 +180,13 @@ struct cp_options
      Set this only if the kernel is SELinux enabled.  */
   bool preserve_security_context;

-  /* Useful only when preserve_security_context is true.
-     If true, a failed attempt to preserve a file's security context
-     propagates failure "out" to the caller.  If false, a failure to
-     preserve a file's security context does not change the invoking
-     application's exit status.  Give diagnostics for failed syscalls
-     regardless of this setting.  For example, with "cp --preserve=context"
-     this flag is "true", while with "cp -a", it is false.  That means
-     "cp -a" attempts to preserve any security context, but does not
-     fail if it is unable to do so.  */
+  /* Useful only when preserve_context is true.
+     If true, a failed attempt to preserve file's security context
+     propagates failure "out" to the caller, along with full diagnostics.
+     If false, a failure to preserve file's security context does not
+     change the invoking application's exit status, but may output diagnostics.
+     For example, with `cp --preserve=context` this flag is "true",
+     while with `cp --preserve=all` or `cp -a`, it is "false". */
   bool require_preserve_context;

   /* If true, attempt to preserve extended attributes using libattr.
@@ -197,16 +195,19 @@ struct cp_options

   /* Useful only when preserve_xattr is true.
      If true, a failed attempt to preserve file's extended attributes
-     propagates failure "out" to the caller.  If false, a failure to
-     preserve file's extended attributes does not change the invoking
-     application's exit status.  Give diagnostics for failed syscalls
-     regardless of this setting.  For example, with "cp --preserve=xattr"
-     this flag is "true", while with "cp --preserve=all", it is false. */
+     propagates failure "out" to the caller, along with full diagnostics.
+     If false, a failure to preserve file's extended attributes does not
+     change the invoking application's exit status, but may output diagnostics.
+     For example, with `cp --preserve=xattr` this flag is "true",
+     while with `cp --preserve=all` or `cp -a`, it is "false". */
   bool require_preserve_xattr;

-  /* Used as difference boolean between cp -a and cp -dR --preserve=all.
-     If true, non-mandatory failure diagnostics are not displayed. This
-     should prevent poluting cp -a output.
+  /* This allows us to output warnings in cases 2 and 4 below,
+     while being quiet for case 1 (when reduce_diagnostics is true).
+       1. cp -a                       try to copy xattrs with no errors
+       2. cp --preserve=all           copy xattrs with all but ENOTSUP warnings
+       3. cp --preserve=xattr,context copy xattrs with all errors
+       4. mv                          copy xattrs with all but ENOTSUP warnings
    */
   bool reduce_diagnostics;