GNU bug report logs - #8370
RFC: cp --no-preserve=contents

Previous Next

Full log

Message #17 received at 8370 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 8370 <at> debbugs.gnu.org, Eric Blake <eblake <at> redhat.com>
Subject: Re: bug#8370: RFC: cp --no-preserve=contents
Date: Thu, 12 Apr 2012 16:04:36 +0200

Pádraig Brady wrote:
...
> 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.

Thanks for doing this.
I prefer the new non-truncating behavior, too.

> 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.

Perhaps "... no longer truncates any existing destination file, ..." ?


> 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.

Similarly here.
IMHO, it's the file that is being truncated, not its data.
Maybe say something like "the data in a destination file is not modified"?

> 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));

What do you think about separating the flag definition
and open separate?

         /* We don't truncate with --attributes-only to support
            copying attributes to an existing file.  */
         int flags = (O_WRONLY | O_BINARY
                      | (x->data_copy_required ? O_TRUNC : 0));
         dest_desc = open (dst_name, flags);


> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.