On 08/07/2012 05:37 PM, Bernhard Voelker wrote:
On 08/07/2012 05:01 PM, Ondrej Oprala wrote:
Hi, I've renamed the variable to be more hinting of it's purpose
and --no-preserve=mode should now work properly with directories
as well.
Cheers,
Ondrej
Thanks, that looks good ... maybe the directory case deserves to
be added to the test. Minor nit: the change in copy_internal() is
missing in the commit log.
I'm sure that Padraig/Jim will add both when committing.
BTW: Interestingly, the TODO entry mentioned "--no-preserve=X"
to be buggy:
-cp --no-preserve=X should not attempt to preserve attribute X
- reported by Andreas Schwab
but in fact, all the modes (t,o,l,c, and x) but "mode"
have already been working. ;-)
Have a nice day,
Berny
Hi, I've amended the commit log and added a test for directories.
Cheers,
Ondrej.
From d52f8990353dac04bff141ff31b6601ba5112a18 Mon Sep 17 00:00:00 2001
From: Ondrej Oprala <oopr...@redhat.com>
Date: Tue, 7 Aug 2012 16:56:52 +0200
Subject: [PATCH] cp: Fix the --no-preserve=mode option
* NEWS: Mention the fix.
* TODO: Remove an entry.
* src/copy.c (copy_reg): Add a condition to properly
handle the --no-preserve=mode option for files
(copy_internal): Add a condition to properly handle the
--no-preserve=mode option for directories.
* src/copy.h (struct cp_options): Add a new boolean.
* src/cp.c (cp_option_init,decode_preserve_arg): Set the
new boolean value according to specified options.
* src/install.c (struct cp_options): Initialize the new boolean.
* src/mv.c (struct cp_options): Initialize the new boolean.
* tests/cp/preserve-mode.sh: Add a new test.
* tests/local.mk: Add the new test to the list.
---
NEWS | 3 +++
TODO | 3 ---
src/copy.c | 10 ++++++++++
src/copy.h | 1 +
src/cp.c | 2 ++
src/install.c | 1 +
src/mv.c | 1 +
tests/cp/preserve-mode.sh | 40 ++++++++++++++++++++++++++++++++++++++++
tests/local.mk | 1 +
9 files changed, 59 insertions(+), 3 deletions(-)
create mode 100755 tests/cp/preserve-mode.sh
diff --git a/NEWS b/NEWS
index f3874fd..c79292b 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,9 @@ GNU coreutils NEWS -*-
outline -*-
** Bug fixes
+ cp --no-preserve=mode now no longer preserves the original file's
+ permissions but correctly sets mode specified by 0666 & ~umask
+
du no longer emits a "disk-corrupted"-style diagnostic when it detects
a directory cycle that is due to a bind-mounted directory. Instead,
it detects this precise type of cycle, diagnoses it as such and
diff --git a/TODO b/TODO
index cd434e5..2dcaf08 100644
--- a/TODO
+++ b/TODO
@@ -47,9 +47,6 @@ doc/coreutils.texi:
ls: add --format=FORMAT option that controls how each line is printed.
-cp --no-preserve=X should not attempt to preserve attribute X
- reported by Andreas Schwab
-
copy.c: Address the FIXME-maybe comment in copy_internal.
And once that's done, add an exclusion so that 'cp --link'
no longer incurs the overhead of saving src. dev/ino and dest. filename
diff --git a/src/copy.c b/src/copy.c
index 2558fea..16aed03 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1151,6 +1151,11 @@ preserve_metadata:
if (set_acl (dst_name, dest_desc, x->mode) != 0)
return_val = false;
}
+ else if (x->explicit_no_preserve_mode)
+ {
+ set_acl (dst_name, dest_desc, 0666 & ~cached_umask ());
+ return_val = false;
+ }
else if (omitted_permissions)
{
omitted_permissions &= ~ cached_umask ();
@@ -2570,6 +2575,11 @@ copy_internal (char const *src_name, char const
*dst_name,
if (set_acl (dst_name, -1, x->mode) != 0)
return false;
}
+ else if (x->explicit_no_preserve_mode)
+ {
+ if (set_acl (dst_name, -1, 0777 & ~cached_umask ()) != 0)
+ return false;
+ }
else
{
if (omitted_permissions)
diff --git a/src/copy.h b/src/copy.h
index d70c09e..440d3bb 100644
--- a/src/copy.h
+++ b/src/copy.h
@@ -157,6 +157,7 @@ struct cp_options
bool preserve_ownership;
bool preserve_mode;
bool preserve_timestamps;
+ bool explicit_no_preserve_mode;
/* Enabled for mv, and for cp by the --preserve=links option.
If true, attempt to preserve in the destination files any
diff --git a/src/cp.c b/src/cp.c
index 6649af2..d63843c 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -783,6 +783,7 @@ cp_option_init (struct cp_options *x)
x->preserve_links = false;
x->preserve_mode = false;
x->preserve_timestamps = false;
+ x->explicit_no_preserve_mode = false;
x->preserve_security_context = false;
x->require_preserve_context = false;
x->preserve_xattr = false;
@@ -860,6 +861,7 @@ decode_preserve_arg (char const *arg, struct cp_options *x,
bool on_off)
{
case PRESERVE_MODE:
x->preserve_mode = on_off;
+ x->explicit_no_preserve_mode = !on_off;
break;
case PRESERVE_TIMESTAMPS:
diff --git a/src/install.c b/src/install.c
index 854436a..8ea5491 100644
--- a/src/install.c
+++ b/src/install.c
@@ -275,6 +275,7 @@ cp_option_init (struct cp_options *x)
x->preserve_links = false;
x->preserve_mode = false;
x->preserve_timestamps = false;
+ x->explicit_no_preserve_mode = false;
x->reduce_diagnostics=false;
x->data_copy_required = true;
x->require_preserve = false;
diff --git a/src/mv.c b/src/mv.c
index 4f5708e..5b08fdd 100644
--- a/src/mv.c
+++ b/src/mv.c
@@ -118,6 +118,7 @@ cp_option_init (struct cp_options *x)
x->preserve_links = true;
x->preserve_mode = true;
x->preserve_timestamps = true;
+ x->explicit_no_preserve_mode= false;
x->preserve_security_context = selinux_enabled;
x->reduce_diagnostics = false;
x->data_copy_required = true;
diff --git a/tests/cp/preserve-mode.sh b/tests/cp/preserve-mode.sh
new file mode 100755
index 0000000..1ef2e16
--- /dev/null
+++ b/tests/cp/preserve-mode.sh
@@ -0,0 +1,40 @@
+#!/bin/sh
+# ensure that cp's --no-preserve=mode works correctly
+
+# Copyright (C) 2002-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=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ cp
+
+rm -f a b c
+umask 0022
+touch a
+touch b
+chmod 600 b
+cp --no-preserve=mode b c
+mode_a=$(ls -l a | gawk '{print $1}')
+mode_c=$(ls -l c | gawk '{print $1}')
+test "$mode_a" = "$mode_c" || fail=1
+
+rm -rf d1 d2 d3
+mkdir d1 d2
+chmod 705 d2
+cp --no-preserve=mode -r d2 d3
+mode_d1=$(ls -l d1 | gawk '{print $1}')
+mode_d3=$(ls -l d3 | gawk '{print $1}')
+test "$mode_d1" = "$mode_d3" || fail=1
+
+Exit $fail
diff --git a/tests/local.mk b/tests/local.mk
index 2440667..41a214f 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -434,6 +434,7 @@ all_tests = \
tests/cp/perm.sh \
tests/cp/preserve-2.sh \
tests/cp/preserve-link.sh \
+ tests/cp/preserve-mode.sh \
tests/cp/preserve-slink-time.sh \
tests/cp/proc-short-read.sh \
tests/cp/proc-zero-len.sh \
--
1.7.11.4
