bug#39929: coreutils-8.32 fails to build on aarch64

2020-03-07 Thread Paul Eggert 

On 3/5/20 11:36 PM, Bernhard Voelker wrote:

s/emits/shall not emit/

P.S. Also the check for $host_triplet containing 'linux' in test is:
a) no longer needed, ...


Thanks for catching those; I installed the attached further patch.
>From ab149bd415daf1cb8ecde0b948bc0a2663611a61 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 7 Mar 2020 10:29:51 -0800
Subject: [PATCH] ls: improve removed-directory test

* tests/ls/removed-directory.sh: Remove host_triplet test.
Skip this test if one cannot remove the working directory.
>From a suggestion by Bernhard Voelker (Bug#39929).
---
 tests/ls/removed-directory.sh | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/tests/ls/removed-directory.sh b/tests/ls/removed-directory.sh
index fe8f929a1..63b209dee 100755
--- a/tests/ls/removed-directory.sh
+++ b/tests/ls/removed-directory.sh
@@ -1,7 +1,7 @@
 #!/bin/sh
-# If ls is asked to list a removed directory (e.g. the parent process's
-# current working directory that has been removed by another process), it
-# emits an error message.
+# If ls is asked to list a removed directory (e.g., the parent process's
+# current working directory has been removed by another process), it
+# should not emit an error message merely because the directory is removed.
 
 # Copyright (C) 2020 Free Software Foundation, Inc.
 
@@ -21,15 +21,10 @@
 . "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
 print_ver_ ls
 
-case $host_triplet in
-  *linux*) ;;
-  *) skip_ 'non linux kernel' ;;
-esac
-
 cwd=$(pwd)
 mkdir d || framework_failure_
 cd d || framework_failure_
-rmdir ../d || framework_failure_
+rmdir ../d || skip_ "can't remove working directory on this platform"
 
 ls >../out 2>../err || fail=1
 cd "$cwd" || framework_failure_
-- 
2.17.1

bug#39929: coreutils-8.32 fails to build on aarch64

2020-03-06 Thread Pádraig Brady 

On 05/03/2020 21:43, Paul Eggert wrote:

On 3/5/20 9:39 AM, Pádraig Brady wrote:

Ah well.
Does the attached address this for you.


Eeeuw.

Why is this code even there at all? If readdir(3) says that the current
directory has no entries, shouldn't 'ls' just say that? Why should ls
report an error simply because the current directory isn't reachable
from the filesystem? Whether the current directory is unreachable has
nothing to do with ls's job, which is to report whether the current
directory has entries.


I'm not very attached to the new behavior so feel free to apply this.
As per the original discussion, the change was made to distinguish
unreachable directories from empty directories.
Unreachable dirs are not common, but it seems useful for the user
to know they're in one

cheers,
Pádraig

bug#39929: coreutils-8.32 fails to build on aarch64

2020-03-06 Thread Kamil Dudka 
On Thursday, March 5, 2020 6:39:23 PM CET Pádraig Brady wrote:
> On 05/03/2020 16:21, Kamil Dudka wrote:
> > While trying to build coreutils-8.32 for Fedora on aarch64, I got the
> > following compilation failure:
> > 
> > ../src/ls.c: In function 'print_dir':
> > ../src/ls.c:3026:24: error: 'SYS_getdents' undeclared (first use in this
> > function); did you mean 'SYS_getdents64'?> 
> >   3026 |   if (syscall (SYS_getdents, dirfd (dirp), NULL, 0) == -1
> >   
> >|^~~~
> >|SYS_getdents64
> > 
> > ../src/ls.c:3026:24: note: each undeclared identifier is reported only
> > once for each function it appears in
> > 
> > 
> > Sorry for reporting it late.  I tried to build 8.31.90-cc4c and
> > 8.31.99-f2034 in Fedora copr but forgot to enable aarch64, so these
> > builds succeeded  :-/
> Ah well.
> Does the attached address this for you.

Thanks!  I confirm that the ls/removed-directory test passes with the patch
on all architectures that Fedora builds for:

https://koji.fedoraproject.org/koji/taskinfo?taskID=42242960

Kamil

bug#39929: coreutils-8.32 fails to build on aarch64

2020-03-05 Thread Bernhard Voelker 
On 2020-03-06 08:31, Bernhard Voelker wrote:
> On 2020-03-06 02:27, Paul Eggert wrote:
>> On 3/5/20 1:43 PM, Paul Eggert wrote:
>>
>>> Why is this code even there at all? If readdir(3) says that the current
>>> directory has no entries, shouldn't 'ls' just say that? Why should ls
>>> report an error simply because the current directory isn't reachable
>>> from the filesystem? Whether the current directory is unreachable has
>>> nothing to do with ls's job, which is to report whether the current
>>> directory has entries.
>>
>> Attached is a proposed patch to fix this.
> 
> I tend to agree (now): returning an error when there was none seems at least
> unlucky.  Sorry I didn't comment in the original discussion.
> 
>> diff --git a/tests/ls/removed-directory.sh b/tests/ls/removed-directory.sh
> 
> That test was also added in commit 05a99f7d7f8e, so the the description
> at the top does not match after reverting:
> 
>   #!/bin/sh
>   # If ls is asked to list a removed directory (e.g. the parent process's
>   # current working directory that has been removed by another process), it
>   # emits an error message.
> 
> s/emits/shall not emit/

P.S. Also the check for $host_triplet containing 'linux' in test is:
a) no longer needed, and
b) looks like a good argument to revert 05a99f7d7f8e, because it introduced
different (and user-visible) behavior just because of the platform.

Have a nice day,
Berny

bug#39929: coreutils-8.32 fails to build on aarch64

2020-03-05 Thread Bernhard Voelker 
On 2020-03-06 02:27, Paul Eggert wrote:
> On 3/5/20 1:43 PM, Paul Eggert wrote:
>
>> Why is this code even there at all? If readdir(3) says that the current
>> directory has no entries, shouldn't 'ls' just say that? Why should ls
>> report an error simply because the current directory isn't reachable
>> from the filesystem? Whether the current directory is unreachable has
>> nothing to do with ls's job, which is to report whether the current
>> directory has entries.
>
> Attached is a proposed patch to fix this.

I tend to agree (now): returning an error when there was none seems at least
unlucky.  Sorry I didn't comment in the original discussion.

> diff --git a/tests/ls/removed-directory.sh b/tests/ls/removed-directory.sh

That test was also added in commit 05a99f7d7f8e, so the the description
at the top does not match after reverting:

  #!/bin/sh
  # If ls is asked to list a removed directory (e.g. the parent process's
  # current working directory that has been removed by another process), it
  # emits an error message.

s/emits/shall not emit/

Thanks & have a nice day,
Berny

bug#39929: coreutils-8.32 fails to build on aarch64

2020-03-05 Thread Paul Eggert 

On 3/5/20 1:43 PM, Paul Eggert wrote:

Why is this code even there at all? If readdir(3) says that the current 
directory has no entries, shouldn't 'ls' just say that? Why should ls 
report an error simply because the current directory isn't reachable 
from the filesystem? Whether the current directory is unreachable has 
nothing to do with ls's job, which is to report whether the current 
directory has entries.


Attached is a proposed patch to fix this.
>From 511d0c323bc90a0ab7e8f3672b07a1144885a9e8 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Thu, 5 Mar 2020 17:25:29 -0800
Subject: [PATCH] ls: restore 8.31 behavior on removed directories

* NEWS: Mention this.
* src/ls.c: Do not include 
(print_dir): Don't worry about whether the directory is removed.
* tests/ls/removed-directory.sh: Adjust to match new (i.e., old)
behavior.
---
 NEWS  |  6 ++
 src/ls.c  | 22 --
 tests/ls/removed-directory.sh | 10 ++
 3 files changed, 8 insertions(+), 30 deletions(-)

diff --git a/NEWS b/NEWS
index fdc8bf5db..653e7178b 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,12 @@ GNU coreutils NEWS-*- outline -*-
 
 * Noteworthy changes in release ?.? (-??-??) [?]
 
+** Changes in behavior
+
+  On GNU/Linux systems, ls no longer issues an error message on
+  directory merely because it was removed.  This reverts a change
+  that was made in release 8.32.
+
 
 * Noteworthy changes in release 8.32 (2020-03-05) [stable]
 
diff --git a/src/ls.c b/src/ls.c
index 24b983287..4acf5f44d 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -49,10 +49,6 @@
 # include 
 #endif
 
-#ifdef __linux__
-# include 
-#endif
-
 #include 
 #include 
 #include 
@@ -2896,7 +2892,6 @@ print_dir (char const *name, char const *realname, bool command_line_arg)
   struct dirent *next;
   uintmax_t total_blocks = 0;
   static bool first = true;
-  bool found_any_entries = false;
 
   errno = 0;
   dirp = opendir (name);
@@ -2972,7 +2967,6 @@ print_dir (char const *name, char const *realname, bool command_line_arg)
   next = readdir (dirp);
   if (next)
 {
-  found_any_entries = true;
   if (! file_ignored (next->d_name))
 {
   enum filetype type = unknown;
@@ -3018,22 +3012,6 @@ print_dir (char const *name, char const *realname, bool command_line_arg)
   if (errno != EOVERFLOW)
 break;
 }
-#ifdef __linux__
-  else if (! found_any_entries)
-{
-  /* If readdir finds no directory entries at all, not even "." or
- "..", then double check that the directory exists.  */
-  if (syscall (SYS_getdents, dirfd (dirp), NULL, 0) == -1
-  && errno != EINVAL)
-{
-  /* We exclude EINVAL as that pertains to buffer handling,
- and we've passed NULL as the buffer for simplicity.
- ENOENT is returned if appropriate before buffer handling.  */
-  file_failure (command_line_arg, _("reading directory %s"), name);
-}
-  break;
-}
-#endif
   else
 break;
 
diff --git a/tests/ls/removed-directory.sh b/tests/ls/removed-directory.sh
index e8c835dab..fe8f929a1 100755
--- a/tests/ls/removed-directory.sh
+++ b/tests/ls/removed-directory.sh
@@ -26,20 +26,14 @@ case $host_triplet in
   *) skip_ 'non linux kernel' ;;
 esac
 
-LS_FAILURE=2
-
-cat <<\EOF >exp-err || framework_failure_
-ls: reading directory '.': No such file or directory
-EOF
-
 cwd=$(pwd)
 mkdir d || framework_failure_
 cd d || framework_failure_
 rmdir ../d || framework_failure_
 
-returns_ $LS_FAILURE ls >../out 2>../err || fail=1
+ls >../out 2>../err || fail=1
 cd "$cwd" || framework_failure_
 compare /dev/null out || fail=1
-compare exp-err err || fail=1
+compare /dev/null err || fail=1
 
 Exit $fail
-- 
2.24.1

bug#39929: coreutils-8.32 fails to build on aarch64

2020-03-05 Thread Paul Eggert 

On 3/5/20 9:39 AM, Pádraig Brady wrote:

Ah well.
Does the attached address this for you.


Eeeuw.

Why is this code even there at all? If readdir(3) says that the current 
directory has no entries, shouldn't 'ls' just say that? Why should ls 
report an error simply because the current directory isn't reachable 
from the filesystem? Whether the current directory is unreachable has 
nothing to do with ls's job, which is to report whether the current 
directory has entries.

bug#39929: coreutils-8.32 fails to build on aarch64

2020-03-05 Thread Pádraig Brady 

On 05/03/2020 16:21, Kamil Dudka wrote:

While trying to build coreutils-8.32 for Fedora on aarch64, I got the following
compilation failure:

../src/ls.c: In function 'print_dir':
../src/ls.c:3026:24: error: 'SYS_getdents' undeclared (first use in this 
function); did you mean 'SYS_getdents64'?
  3026 |   if (syscall (SYS_getdents, dirfd (dirp), NULL, 0) == -1
   |^~~~
   |SYS_getdents64
../src/ls.c:3026:24: note: each undeclared identifier is reported only once for 
each function it appears in


Sorry for reporting it late.  I tried to build 8.31.90-cc4c and 8.31.99-f2034
in Fedora copr but forgot to enable aarch64, so these builds succeeded  :-/


Ah well.
Does the attached address this for you.
>From 8012c07aae89f9f855b7c0663158b24464248a3d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= 
Date: Thu, 5 Mar 2020 17:34:33 +
Subject: [PATCH] build: fix build failure on linux systems without getdents

* src/ls.c (print_dir): aarch64 doesn't define the getdents
system call, only providing the getdents64 variant.
This is available enough to use instead.
---
 src/ls.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/ls.c b/src/ls.c
index 24b983287..29462bfee 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -3018,12 +3018,12 @@ print_dir (char const *name, char const *realname, bool command_line_arg)
   if (errno != EOVERFLOW)
 break;
 }
-#ifdef __linux__
   else if (! found_any_entries)
 {
+#if defined __linux__ && defined SYS_getdents64
   /* If readdir finds no directory entries at all, not even "." or
  "..", then double check that the directory exists.  */
-  if (syscall (SYS_getdents, dirfd (dirp), NULL, 0) == -1
+  if (syscall (SYS_getdents64, dirfd (dirp), NULL, 0) == -1
   && errno != EINVAL)
 {
   /* We exclude EINVAL as that pertains to buffer handling,
@@ -3031,9 +3031,9 @@ print_dir (char const *name, char const *realname, bool command_line_arg)
  ENOENT is returned if appropriate before buffer handling.  */
   file_failure (command_line_arg, _("reading directory %s"), name);
 }
+#endif
   break;
 }
-#endif
   else
 break;
 
-- 
2.24.1

bug#39929: coreutils-8.32 fails to build on aarch64

2020-03-05 Thread Kamil Dudka 
While trying to build coreutils-8.32 for Fedora on aarch64, I got the following
compilation failure:

../src/ls.c: In function 'print_dir':
../src/ls.c:3026:24: error: 'SYS_getdents' undeclared (first use in this 
function); did you mean 'SYS_getdents64'?
 3026 |   if (syscall (SYS_getdents, dirfd (dirp), NULL, 0) == -1
  |^~~~
  |SYS_getdents64
../src/ls.c:3026:24: note: each undeclared identifier is reported only once for 
each function it appears in


Sorry for reporting it late.  I tried to build 8.31.90-cc4c and 8.31.99-f2034
in Fedora copr but forgot to enable aarch64, so these builds succeeded  :-/

Kamil