On 15/05/18 17:11, Paul Eggert wrote: > On 05/15/2018 10:05 AM, Pádraig Brady wrote: >>> It's also what I would expect, if I use -f, I expect cp will do everything >>> it can to force the operation and succeed if all possible. >> Maybe, though that's worth further consideration. > > POSIX is arguably ambiguous about what 'cp -f S D' should do if D is a > symlink to itself. POSIX says that if D "exists", then cp must try to > unlink and then re-create D; and that if D does not "exist", cp must > fail. So, if one considers a self-symlink as "existing", then GNU cp > doesn't conform to POSIX; if one considers such a symlink as not > "existing", then GNU cp conforms. Unfortunately, as far as I can tell > POSIX never exactly defines what "exists" means in this context.
Well `test -e self-symlink` => does not exist > That being said, POSIX uses nearly the same wording for 'ln -f' that it > does for 'cp -f', which implies that cp should be consistent with ln, > and GNU ln (like most ln implementations) treat self-symlinks as > "existing" in this case. Also, other implementations of cp seem act like > ln does here, so they interpret the ambiguity in POSIX the opposite way > that GNU cp does. Furthermore, I think that users by and large expect > the non-GNU interpretation where 'cp -f' is like 'ln -f'. For all these > reasons, I'm inclined to think that GNU cp should fall into line here. Agreed. I can't think of a case where replacing a self symlink is not what you'd want to happen. Proposed patch is attached. cheers, Pádraig
From f3cf3716c70e4680f9b3ed2180c38de632c06010 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]> Date: Tue, 15 May 2018 23:41:36 -0700 Subject: [PATCH] cp: with --force; replace self referential symlinks * src/copy.c (copy_internal): Don't fail immediately upon getting ELOOP when running stat() on the destination, rather proceeding if -f specified, allowing the link to be removed. If the loop is not in the final component of the destination path, we still fail but at the subsequent unlink() stage. * doc/coreutils.texi (cp invocation): Adjust wording to say that --force doesn't work with dangling links, rather than all links that can't be traversed. * tests/cp/thru-dangling.sh: Add a test case. * NEWS: Mention the change in behavior. Discussed in https://bugs.gnu.org/31335 --- NEWS | 4 ++++ doc/coreutils.texi | 2 +- src/copy.c | 7 +++++-- tests/cp/thru-dangling.sh | 13 +++++++++++-- 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/NEWS b/NEWS index f981596..4c63b6d 100644 --- a/NEWS +++ b/NEWS @@ -39,6 +39,10 @@ GNU coreutils NEWS -*- outline -*- now silently does nothing if A exists. [bug introduced with coreutils-7.1] +** Changes in behavior + + 'cp --force file symlink' now removes the symlink even if + it is self referential. ** Improvements diff --git a/doc/coreutils.texi b/doc/coreutils.texi index c8d9bd9..c28b8d0 100644 --- a/doc/coreutils.texi +++ b/doc/coreutils.texi @@ -8535,7 +8535,7 @@ When copying without this option and an existing destination file cannot be opened for writing, the copy fails. However, with @option{--force}, when a destination file cannot be opened, @command{cp} then tries to recreate the file by first removing it. Note @option{--force} -alone will not remove symlinks that can't be traversed. +alone will not remove dangling symlinks. When this option is combined with @option{--link} (@option{-l}) or @option{--symbolic-link} (@option{-s}), the destination link is replaced, and unless diff --git a/src/copy.c b/src/copy.c index 0407c56..f4c92d7 100644 --- a/src/copy.c +++ b/src/copy.c @@ -1949,12 +1949,15 @@ copy_internal (char const *src_name, char const *dst_name, } else { - if (errno != ENOENT) + if (errno == ELOOP && x->unlink_dest_after_failed_open) + /* leave new_dst=false so we unlink later. */; + else if (errno != ENOENT) { error (0, errno, _("cannot stat %s"), quoteaf (dst_name)); return false; } - new_dst = true; + else + new_dst = true; } } diff --git a/tests/cp/thru-dangling.sh b/tests/cp/thru-dangling.sh index 8fd452e..e433525 100755 --- a/tests/cp/thru-dangling.sh +++ b/tests/cp/thru-dangling.sh @@ -28,15 +28,24 @@ echo "cp: not writing through dangling symlink 'dangle'" \ # Starting with 6.9.90, this usage fails, by default: for opt in '' '-f'; do - cp $opt f dangle > err 2>&1 && fail=1 + returns_ 1 cp $opt f dangle > err 2>&1 || fail=1 compare exp-err err || fail=1 test -f no-such && fail=1 done + # But you can set POSIXLY_CORRECT to get the historical behavior. env POSIXLY_CORRECT=1 cp f dangle > out 2>&1 || fail=1 cat no-such >> out || fail=1 - compare exp out || fail=1 + +# Starting with 8.30 we treat ELOOP as existing and so +# remove the symlink +ln -s loop loop || framework_failure_ +cp -f f loop > err 2>&1 || fail=1 +compare /dev/null err || fail=1 +compare exp loop || fail=1 +test -f loop || fail=1 + Exit $fail -- 2.9.3
