GNU bug report logs - #13119
cp --no-preserve=mode exits 1 since coreutils 8.20

Previous Next

Package: coreutils;

Reported by: Florian Pritz <bluewind <at> xinu.at>

Date: Fri, 7 Dec 2012 21:24:02 UTC

Severity: normal

Done: Bernhard Voelker <mail <at> bernhard-voelker.de>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 13119 in the body.
You can then email your comments to 13119 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-coreutils <at> gnu.org:
bug#13119; Package coreutils. (Fri, 07 Dec 2012 21:24:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Florian Pritz <bluewind <at> xinu.at>:
New bug report received and forwarded. Copy sent to bug-coreutils <at> gnu.org. (Fri, 07 Dec 2012 21:24:02 GMT) Full text and rfc822 format available.

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

From: Florian Pritz <bluewind <at> xinu.at>
To: bug-coreutils <at> gnu.org
Subject: cp --no-preserve=mode exits 1 since coreutils 8.20
Date: Fri, 07 Dec 2012 21:43:47 +0100
[Message part 1 (text/plain, inline)]
Hi,

I've used the following code in an update script. I don't want cp to
change permission since the target directory has default ACLs in place
that will set the correct permissions, but I want to keep timestamps so
once upon a time I added "--preserve=timestamp
--nopreserve=mode,ownership". I've tested without mode now and it seems
permission are fine.

mkdir a
touch a/a
cp -rfT --preserve=timestamps --no-preserve=mode,ownership a b

If I remove mode from the arguments to --no-preserve it exits 0, but if
you have it it exits 1 and doesn't print anything. It worked in
coreutils 8.19 (exit code 0) even though it might not actually do
anything. If it's intended behaviour you should at least print some kind
of error message.

The cause of this change is commit
24ebca61a3a0f10d6cd2800b188b3c034d1c4755 but it doesn't say anything
about changing the exit code.

-- 
Florian Pritz

[signature.asc (application/pgp-signature, attachment)]

Information forwarded to bug-coreutils <at> gnu.org:
bug#13119; Package coreutils. (Sat, 08 Dec 2012 18:14:01 GMT) Full text and rfc822 format available.

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

From: Bernhard Voelker <mail <at> bernhard-voelker.de>
To: Florian Pritz <bluewind <at> xinu.at>
Cc: 13119 <at> debbugs.gnu.org
Subject: Re: bug#13119: cp --no-preserve=mode exits 1 since coreutils 8.20
Date: Sat, 08 Dec 2012 19:12:57 +0100
On 12/07/2012 09:43 PM, Florian Pritz wrote:
> Hi,
> 
> I've used the following code in an update script. I don't want cp to
> change permission since the target directory has default ACLs in place
> that will set the correct permissions, but I want to keep timestamps so
> once upon a time I added "--preserve=timestamp
> --nopreserve=mode,ownership". I've tested without mode now and it seems
> permission are fine.
> 
> mkdir a
> touch a/a
> cp -rfT --preserve=timestamps --no-preserve=mode,ownership a b
> 
> If I remove mode from the arguments to --no-preserve it exits 0, but if
> you have it it exits 1 and doesn't print anything. It worked in
> coreutils 8.19 (exit code 0) even though it might not actually do
> anything. If it's intended behaviour you should at least print some kind
> of error message.
> 
> The cause of this change is commit
> 24ebca61a3a0f10d6cd2800b188b3c034d1c4755 but it doesn't say anything
> about changing the exit code.

Thanks for the report.

Outch, yes, unlike in other places, return_val is set to false
unconditionally in copy_reg():

  +  else if (x->explicit_no_preserve_mode)
  +    {
  +      set_acl (dst_name, dest_desc, 0666 & ~cached_umask ());
  +      return_val = false;
  +    }

And furthermore, the test preserve-mode.sh which would have
detected this error, don't check cp's exit code ;-(

Here's a proposed patch.

Have a nice day,
Berny


From 62543570d72b756a3b04ca9d1ebec6f4dd2eea4b Mon Sep 17 00:00:00 2001
From: Bernhard Voelker <mail <at> bernhard-voelker.de>
Date: Sat, 8 Dec 2012 19:09:19 +0100
Subject: [PATCH] cp: fix --no-preserve=mode to not exit 1

cp --no-preserve=mode exited 1 unconditionally.  Furthermore,
the tests which would have detected this error - namely
link-preserve.sh and reserve-mode.sh - failed to test
cp's exit code.

* src/copy.c (copy_reg): In the case x->explicit_no_preserve_mode,
do only set return_val to false iff the previous set_acl ()
failed.
* tests/cp/link-preserve.sh: Check cp's exit code.
* tests/cp/link-symlink.sh: Likewise.
* tests/cp/preserve-mode.sh: Likewise.
* NEWS: Mention the fix.

Bug introduced in commit v8.19-145-g24ebca6.

Reported by Florian Pritz in http://bugs.gnu.org/13119.
---
 NEWS                      |    3 +++
 src/copy.c                |    4 ++--
 tests/cp/link-preserve.sh |   12 ++++++------
 tests/cp/link-symlink.sh  |    2 +-
 tests/cp/preserve-mode.sh |    6 +++---
 5 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/NEWS b/NEWS
index 0e1414c..6576b50 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,9 @@ GNU coreutils NEWS                                    -*- outline -*-

 ** Bug fixes

+  cp --no-preserve=mode now no longer exits non-zero.
+  [bug introduced in coreutils-8.20]
+
   cut no longer accepts the invalid range 0-, which made it print empty lines.
   Instead, cut now fails and emits an appropriate diagnostic.
   [This bug was present in "the beginning".]
diff --git a/src/copy.c b/src/copy.c
index 7a35414..0decf9f 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1153,8 +1153,8 @@ preserve_metadata:
     }
   else if (x->explicit_no_preserve_mode)
     {
-      set_acl (dst_name, dest_desc, 0666 & ~cached_umask ());
-      return_val = false;
+      if (set_acl (dst_name, dest_desc, 0666 & ~cached_umask ()) != 0)
+        return_val = false;
     }
   else if (omitted_permissions)
     {
diff --git a/tests/cp/link-preserve.sh b/tests/cp/link-preserve.sh
index bb3b244..44768e4 100755
--- a/tests/cp/link-preserve.sh
+++ b/tests/cp/link-preserve.sh
@@ -37,7 +37,7 @@ rm -rf a b c
 touch a
 ln -s a b
 mkdir c
-cp --preserve=links -R -H a b c
+cp --preserve=links -R -H a b c || fail=1
 a_inode=$(ls -i c/a|sed 's,c/.*,,')
 b_inode=$(ls -i c/b|sed 's,c/.*,,')
 test "$a_inode" = "$b_inode" || fail=1
@@ -46,7 +46,7 @@ test "$a_inode" = "$b_inode" || fail=1
 # Ensure that -L makes cp follow the b->a symlink
 # and translates to hard-linked a and b in the destination dir.
 rm -rf a b c d; mkdir d; (cd d; touch a; ln -s a b)
-cp --preserve=links -R -L d c
+cp --preserve=links -R -L d c || fail=1
 a_inode=$(ls -i c/a|sed 's,c/.*,,')
 b_inode=$(ls -i c/b|sed 's,c/.*,,')
 test "$a_inode" = "$b_inode" || fail=1
@@ -54,7 +54,7 @@ test "$a_inode" = "$b_inode" || fail=1

 # Same as above, but starting with a/b hard linked.
 rm -rf a b c d; mkdir d; (cd d; touch a; ln a b)
-cp --preserve=links -R -L d c
+cp --preserve=links -R -L d c || fail=1
 a_inode=$(ls -i c/a|sed 's,c/.*,,')
 b_inode=$(ls -i c/b|sed 's,c/.*,,')
 test "$a_inode" = "$b_inode" || fail=1
@@ -62,7 +62,7 @@ test "$a_inode" = "$b_inode" || fail=1

 # Ensure that --no-preserve=links works.
 rm -rf a b c d; mkdir d; (cd d; touch a; ln a b)
-cp -dR --no-preserve=links d c
+cp -dR --no-preserve=links d c || fail=1
 a_inode=$(ls -i c/a|sed 's,c/.*,,')
 b_inode=$(ls -i c/b|sed 's,c/.*,,')
 test "$a_inode" = "$b_inode" && fail=1
@@ -72,7 +72,7 @@ test "$a_inode" = "$b_inode" && fail=1
 rm -rf a b c d
 touch a; ln a b
 mkdir c
-cp -d a b c
+cp -d a b c || fail=1
 a_inode=$(ls -i c/a|sed 's,c/.*,,')
 b_inode=$(ls -i c/b|sed 's,c/.*,,')
 test "$a_inode" = "$b_inode" || fail=1
@@ -82,7 +82,7 @@ test "$a_inode" = "$b_inode" || fail=1
 rm -rf a b c d
 touch a; chmod 731 a
 umask 077
-cp -a --no-preserve=mode a b
+cp -a --no-preserve=mode a b || fail=1
 mode=$(ls -l b|cut -b-10)
 test "$mode" = "-rw-------" || fail=1
 umask 022
diff --git a/tests/cp/link-symlink.sh b/tests/cp/link-symlink.sh
index 5186887..0b7fb6e 100755
--- a/tests/cp/link-symlink.sh
+++ b/tests/cp/link-symlink.sh
@@ -32,7 +32,7 @@ esac

 # link.cp is probably a hardlink, but may also be a symlink
 # In either case the timestamp should match the original.
-cp -al link link.cp
+cp -al link link.cp || fail=1
 case $(stat --format=%y link.cp) in
   2011-01-01*) ;;
   *) fail=1 ;;
diff --git a/tests/cp/preserve-mode.sh b/tests/cp/preserve-mode.sh
index dc97cba..e1096f5 100755
--- a/tests/cp/preserve-mode.sh
+++ b/tests/cp/preserve-mode.sh
@@ -26,7 +26,7 @@ touch b
 chmod 600 b

 #regular file test
-cp --no-preserve=mode b c
+cp --no-preserve=mode b c || fail=1
 mode_a=$(ls -l a | gawk '{print $1}')
 mode_c=$(ls -l c | gawk '{print $1}')
 test "$mode_a" = "$mode_c" || fail=1
@@ -36,7 +36,7 @@ mkdir d1 d2
 chmod 705 d2

 #directory test
-cp --no-preserve=mode -r d2 d3
+cp --no-preserve=mode -r d2 d3 || fail=1
 mode_d1=$(ls -l d1 | gawk '{print $1}')
 mode_d3=$(ls -l d3 | gawk '{print $1}')
 test "$mode_d1" = "$mode_d3" || fail=1
@@ -46,7 +46,7 @@ touch a
 chmod 600 a

 #contradicting options test
-cp --no-preserve=mode --preserve=all a b
+cp --no-preserve=mode --preserve=all a b || fail=1
 mode_a=$(ls -l a | gawk '{print $1}')
 mode_b=$(ls -l b | gawk '{print $1}')
 test "$mode_a" = "$mode_b" || fail=1
-- 
1.7.7





Information forwarded to bug-coreutils <at> gnu.org:
bug#13119; Package coreutils. (Sun, 09 Dec 2012 12:50:02 GMT) Full text and rfc822 format available.

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

From: Pádraig Brady <P <at> draigBrady.com>
To: Bernhard Voelker <mail <at> bernhard-voelker.de>
Cc: 13119 <at> debbugs.gnu.org, Florian Pritz <bluewind <at> xinu.at>
Subject: Re: bug#13119: cp --no-preserve=mode exits 1 since coreutils 8.20
Date: Sun, 09 Dec 2012 12:48:43 +0000
On 12/08/2012 06:12 PM, Bernhard Voelker wrote:
> On 12/07/2012 09:43 PM, Florian Pritz wrote:
>> Hi,
>>
>> I've used the following code in an update script. I don't want cp to
>> change permission since the target directory has default ACLs in place
>> that will set the correct permissions, but I want to keep timestamps so
>> once upon a time I added "--preserve=timestamp
>> --nopreserve=mode,ownership". I've tested without mode now and it seems
>> permission are fine.
>>
>> mkdir a
>> touch a/a
>> cp -rfT --preserve=timestamps --no-preserve=mode,ownership a b
>>
>> If I remove mode from the arguments to --no-preserve it exits 0, but if
>> you have it it exits 1 and doesn't print anything. It worked in
>> coreutils 8.19 (exit code 0) even though it might not actually do
>> anything. If it's intended behaviour you should at least print some kind
>> of error message.
>>
>> The cause of this change is commit
>> 24ebca61a3a0f10d6cd2800b188b3c034d1c4755 but it doesn't say anything
>> about changing the exit code.
>
> Thanks for the report.
>
> Outch, yes, unlike in other places, return_val is set to false
> unconditionally in copy_reg():
>
>    +  else if (x->explicit_no_preserve_mode)
>    +    {
>    +      set_acl (dst_name, dest_desc, 0666 & ~cached_umask ());
>    +      return_val = false;
>    +    }
>
> And furthermore, the test preserve-mode.sh which would have
> detected this error, don't check cp's exit code ;-(
>
> Here's a proposed patch.

Ouch indeed.
The patch looks perfect.

thanks!
Pádraig.




Reply sent to Bernhard Voelker <mail <at> bernhard-voelker.de>:
You have taken responsibility. (Sun, 09 Dec 2012 16:33:02 GMT) Full text and rfc822 format available.

Notification sent to Florian Pritz <bluewind <at> xinu.at>:
bug acknowledged by developer. (Sun, 09 Dec 2012 16:33:02 GMT) Full text and rfc822 format available.

Message #16 received at 13119-done <at> debbugs.gnu.org (full text, mbox):

From: Bernhard Voelker <mail <at> bernhard-voelker.de>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 13119-done <at> debbugs.gnu.org, Florian Pritz <bluewind <at> xinu.at>
Subject: Re: bug#13119: cp --no-preserve=mode exits 1 since coreutils 8.20
Date: Sun, 09 Dec 2012 17:32:08 +0100
On 12/09/2012 01:48 PM, Pádraig Brady wrote:
> The patch looks perfect.

Thanks for the review. Pushed:
http://git.sv.gnu.org/cgit/coreutils.git/commit/?id=62543570

Marking the bug as "done".

Have a nice day,
Berny




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 07 Jan 2013 12:24:45 GMT) Full text and rfc822 format available.

This bug report was last modified 11 years and 84 days ago.

Previous Next


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