[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#8370: RFC: cp --no-preserve=contents
From: |
Jim Meyering |
Subject: |
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 @address@hidden@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