bug#31472: tsort reporting false loop in input. unix2dos fixes the problem.

[Bernhard Voelker]

On 05/16/2018 11:23 AM, Ivan Ivanov wrote:
> Hi all,
> 
> tsort is reporting loop in my input file, but the loop doesn't exist
> really. 
> 
> I have checked this manually, examining the contents of the file,
> related to the reported loop. 
> 
> Further more, if I run the input file trhough unix2dos – it works, so I
> suspect some strange problem with the unix newlines. 
> 
> The input file is generated with python script on the Ubuntu 16.04.3 so
> all newlines should be the same.
> 
> Test environments (acting the same way):
> ---
> 
> Ubuntu 16.04.3 LTS
> tsort (coreutils) 8.25
> 
> and
> 
> Debian GNU/Linux 9 (stretch)
> tsort (coreutils) 8.26
> 
> 
> Reproduction steps:
> ---
> $ unxz for-tsort-bug-example.txt.xz
> $ tsort for-tsort-bug-example.txt > /dev/null
> 
> the above command should produce:
> tsort: for-tsort-bug-example.txt: input contains a loop:
> tsort: 15731101
> tsort: 15731102

The problematic lines are:

$ grep -E '15731101|15731102' for-tsort-bug-example.txt
15731102 16019755
15731102 15731104
15731102 15731101
15731102 15731105
15731102 15731103
15731101
15731102

Therefore your example reduces to:

$ grep -E '15731101|15731102|16019755|15731104|15731105|15731103' 
for-tsort-bug-example.txt
15731102 16019755
15731102 15731104
15731102 15731101
15731102 15731105
15731102 15731103
15731101
15731102

> $ cat for-tsort-bug-example.txt | unix2dos | tsort > /dev/null

This replaces "\n" by "\r\n", and of course changes the way tsort works.
If your suspicion was that the file has Windows-style line-endings, then
you would have had to use 'dos2unix'.

Have a nice day,
Berny

bug#31472: tsort reporting false loop in input. unix2dos fixes the problem.

[Ivan Ivanov]

Hi again,

I've digged more into this, and I found that unix2dos doesn't fix
the problem, but just masks the tsort behaviour.

If you point the tsort result into a file, you will notice, that tsort
breaks some of the newline chars. The input file seems to be ok. 

I am still looking into this, but I am now a little bit suspicious if
this is really a tsort bug.

If I find something useful, I will share it.

Ivan Ivanov

На Wed, 16 May 2018 12:23:12 +0300
Ivan Ivanov  написа:

> Hi all,
> 
> tsort is reporting loop in my input file, but the loop doesn't exist
> really. 
> 
> I have checked this manually, examining the contents of the file,
> related to the reported loop. 
> 
> Further more, if I run the input file trhough unix2dos – it works, so
> I suspect some strange problem with the unix newlines. 
> 
> The input file is generated with python script on the Ubuntu 16.04.3
> so all newlines should be the same.
> 
> Test environments (acting the same way):
> ---
> 
> Ubuntu 16.04.3 LTS
> tsort (coreutils) 8.25
> 
> and
> 
> Debian GNU/Linux 9 (stretch)
> tsort (coreutils) 8.26
> 
> 
> Reproduction steps:
> ---
> $ unxz for-tsort-bug-example.txt.xz
> $ tsort for-tsort-bug-example.txt > /dev/null
> 
> the above command should produce:
> tsort: for-tsort-bug-example.txt: input contains a loop:
> tsort: 15731101
> tsort: 15731102
> 
> $ cat for-tsort-bug-example.txt | unix2dos | tsort > /dev/null
> 
> should exit properly.
> 
> You may choose to first convert the file and than call tsort and it
> will exit properly again:
> $ unix2dos for-tsort-bug-example.txt
> $ tsort for-tsort-bug-example.txt > /dev/null
> 
> If you have further questions to investigate the issue, feel free to
> write back!
> 
> Best wishes,
> 
> Ivan Ivanov

bug#31335: Fwd: bug#31335: unexpected cp -f behavior

[Pádraig Brady]

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?= 
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 >>