Subject: bug#8370: RFC: cp --no-preserve=contents

From: Pádraig Brady <[email-masked]>
Date: Thu, 12 Apr 2012 13:56:57 +0100

On 04/12/2012 01:41 AM, Pádraig Brady wrote:
> On 03/28/2011 10:56 PM, Pádraig Brady wrote:
>> On 28/03/11 21:55, Eric Blake wrote:
>>> cp --attributes-only is great for preserving all metadata attributes
>>> without corrupting contents, but what if I want to preserve only some of
>>> the metadata (for example, copying SELinux context but _not_ timestamps
>>> or content)?  It seems like --attributes-only would be a great synonym
>>> for '--preserve=all  --no-preserve=contents', and that by adding the
>>> 'contents' category to --preserve (and defaulting it to on unless turned
>>> off explicitly), that you expose finer-grained tuning to what metadata
>>> gets copied.
>>
>> --attr and --preserve can be combined already. Is that enough?
>> Here are the info docs:
>>
>> `--attributes-only'
>>      Preserve the specified attributes of the original files in the
>>      copy, but do not copy any data.  See the `--preserve' option for
>>      controlling which attributes to copy.
> 
> Cross referencing a related bug report:
> https://bugzilla.redhat.com/show_bug.cgi?id=811746#c1

So thinking a bit more about this,
and given the confusion expressed in the above bug report,
perhaps it's best to change --attributes-only to
_not_ truncate existing files?

I think scripts relying on the truncation behavior
of this relative new feature would be very rare,
and the non truncating behavior is more generally useful.

Patch to implement this change is attached.

cheers,
Pádraig.
From f41ca5d0ce5304136613ddac91adf23cd84cad2d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Thu, 12 Apr 2012 12:47:30 +0100
Subject: [PATCH] cp: change --attributes-only to not truncate existing files

* src/copy.c (copy_reg): Don't truncate an existing file,
to support copying attributes between existing files.
The original use case only considered creating new files,
and it would be a very unusual use case to be relying
on the truncating behavior.
* doc/coreutils.texi (cp invocation): Mention the non
truncating behavior.
* tests/cp/attr-existing: A new test to ensure O_TRUNC skipped.
* tests/Makefile.am: Reference the new test.
* NEWS: Mention the change in behavior.
---
 NEWS                   |    5 +++++
 doc/coreutils.texi     |    5 +++--
 src/copy.c             |    5 ++++-
 tests/Makefile.am      |    1 +
 tests/cp/attr-existing |   29 +++++++++++++++++++++++++++++
 5 files changed, 42 insertions(+), 3 deletions(-)
 create mode 100755 tests/cp/attr-existing

diff --git a/NEWS b/NEWS
index c8d2bbc..92a2ec4 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,11 @@ GNU coreutils NEWS                                    -*- outline -*-
 
 * Noteworthy changes in release ?.? (????-??-??) [?]
 
+** Changes in behavior
+
+  cp --attributes-only no longer truncates the data in existing files,
+  allowing for more general copying of attributes from one file to another.
+
 
 * Noteworthy changes in release 8.16 (2012-03-26) [stable]
 
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 510abb9..9f1d7b9 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7650,8 +7650,9 @@ Equivalent to @option{-dR --preserve=all} with the reduced diagnostics.
 @itemx --attributes-only
 @opindex --attributes-only
 Preserve the specified attributes of the original files in the copy,
-but do not copy any data.  See the @option{--preserve} option for
-controlling which attributes to copy.
+but do not copy any data.  Data in existing destination files is not
+truncated.  See the @option{--preserve} option for controlling which
+attributes to copy.
 
 @item -b
 @itemx @w{@kbd{--backup}[=@var{method}]}
diff --git a/src/copy.c b/src/copy.c
index f63a726..03d68db 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -826,7 +826,10 @@ copy_reg (char const *src_name, char const *dst_name,
      by the specs for both cp and mv.  */
   if (! *new_dst)
     {
-      dest_desc = open (dst_name, O_WRONLY | O_TRUNC | O_BINARY);
+      /* We don't truncate with --attributes-only to support
+         copying attributes to an existing file.  */
+      dest_desc = open (dst_name, O_WRONLY | O_BINARY
+                        | (x->data_copy_required ? O_TRUNC : 0));
       dest_errno = errno;
 
       /* When using cp --preserve=context to copy to an existing destination,
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 011051a..4d73a92 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -320,6 +320,7 @@ TESTS =						\
   chown/separator				\
   cp/abuse					\
   cp/acl					\
+  cp/attr-existing				\
   cp/backup-1					\
   cp/backup-dir					\
   cp/backup-is-src				\
diff --git a/tests/cp/attr-existing b/tests/cp/attr-existing
new file mode 100755
index 0000000..9cf0ffc
--- /dev/null
+++ b/tests/cp/attr-existing
@@ -0,0 +1,29 @@
+#!/bin/sh
+# Make sure cp --attributes-only doesn't truncate existing data
+
+# Copyright 2012 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+print_ver_ cp
+
+printf '1' > file1
+printf '2' > file2
+printf '2' > file2.exp
+
+cp --attributes-only file1 file2 || fail=1
+cmp file2 file2.exp || fail=1
+
+Exit $fail
-- 
1.7.6.4