bug#70214: 'install' fails to copy regular file to autofs/cifs, due to ACL or xattr handling

[Pádraig Brady]

On 13/04/2024 18:42, Pádraig Brady wrote:

On 13/04/2024 17:39, Pádraig Brady wrote:

install(1) defaults to mode 600 for new files, and uses set_acl() with that
(since 2007 https://github.com/coreutils/coreutils/commit/f634e8844 )
The psuedo code that install(1) uses is:

copy_reg()
 if (x->set_mode) /* install */
   set_acl(dest, x->mode /* 600 */)
 ctx->acl = acl_from_mode ( /* 600 */)
 acl_set_fd (ctx->acl) /* fails EACCES */
 if (! acls_set)
must_chmod = true;
 if (must_chmod)
   saved_errno = EACCES;
   chmod (ctx->mode /* 600 */)
   if (save_errno)
 return -1;

This issue only only seems to be on CIFS.
I'm seeing lot of weird behavior with ACLs there:

 acl_set_fd (acl_from_mode (600)) -> EACCES
 acl_set_fd (acl_from_mode (755)) -> EINVAL
 getxattr ("system.posix_acl_access") -> EOPNOTSUPP

Note we ignore EINVAL and EOPNOTSUPP errors in set_acl(),
and it's just the EACCES that's problematic.
Note this is quite similar to https://debbugs.gnu.org/65599
where Paul also noticed EACCES with fsetxattr() (and others) on CIFS.

The attached is a potential solution which I tested as working
on the same matoro system that Bruno used.

I think I'll apply that after thinking a bit more about it.


Actually that probably isn't appropriate,
as fsetxattr() can validly return EACESS
even though not documented in the man page at least.
The following demonstrates that:
.
#include 
#include 
#include 
#include 
#include 
#include 
#include 

int main(void)
{
  int wfd;
  /* Note S_IWUSR is not set.  */
  if ((wfd=open("writable", O_CREAT|O_WRONLY|O_EXCL, S_IRUSR)) == -1)
  fprintf(stderr, "open('writable') error [%m]\n");
  if (write(wfd, "data", 1) == -1)
  fprintf(stderr, "write() error [%m]\n");
  if (fsetxattr(wfd, "user.test", "test", 4, 0) == -1)
  fprintf(stderr, "fsetxattr() error [%m]\n");
}

Another solution might be to file_has_acl() in copy.c
and skip if "not supported" or nothing is returned.
The following would do that, but I'm not sure about this
(as I'm not sure about ACLs in general TBH).
Note listxattr() returns 0 on CIFS here,
while getxattr ("system.posix_acl_access") returns EOPNOTSUPP,
and file_has_acl() uses listxattr() first.

diff --git a/src/copy.c b/src/copy.c
index d584a27eb..2145d89d5 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1673,8 +1673,13 @@ set_dest_mode:
   }
 else if (x->set_mode)
   {
-  if (set_acl (dst_name, dest_desc, x->mode) != 0)
-return_val = false;
+  errno = 0;
+  int n = file_has_acl (dst_name, );
+  if (0 < n || (errno && ! (is_ENOTSUP (errno) || errno == ENOSYS)))
+{
+  if (set_acl (dst_name, dest_desc, x->mode) != 0)
+return_val = false;
+}
   }


BTW I'm surprised this wasn't reported previously for CIFS,
so I due to this bug and https://debbugs.gnu.org/65599
I suspect a recentish change in CIFS wrt EACCES.


Thinking more about this, the solution presented above wouldn't work,
and I think this needs to be addressed in CIFS.
I.e. we may still need to reset the mode even if the file has no ACLs,
as generally only dirs get default ACLs copied, as demonstrated below:

$ mkdir acl$ setfacl -d -m o::rw acl
$ getfacl acl
# file: acl
# owner: padraig
# group: padraig
user::rwx
group::r-x
other::r-x
default:user::rwx
default:group::r-x
default:other::rw-

$ touch acl/file
$ ls -l acl/file
-rw-r--rw-. 1 padraig padraig 0 May  9 13:11 acl/file
$ getfacl acl/file
# file: acl/file
# owner: padraig
# group: padraig
user::rw-
group::r--
other::rw-

cheers,
Pádraig.

bug#70814: pwd(1) man page: Unclear "avoid" wording

[Pádraig Brady]

On 07/05/2024 05:19, Bruce Jerrick wrote:

This wording in the pwd(1) man page is unclear:

     -P, --physical
     avoid all symlinks

"resolve all symlinks" would be much better wording.


Pushed that in your name.

Marking this as done.

thanks,
Pádraig

bug#70814: pwd(1) man page: Unclear "avoid" wording

[Bruce Jerrick]

This wording in the pwd(1) man page is unclear:

    -P, --physical
    avoid all symlinks

"resolve all symlinks" would be much better wording.

-- Bruce Jerrick

bug#70801: Weird behaviour when standard input is closed

[Pádraig Brady]

On 06/05/2024 08:38, Bernard Burette wrote:

Hi,



If I try:

$ cat <&-

cat: -: Bad file descriptor

cat: closing standard input: Bad file descriptor

$



The error on stdin beign closed is displayed twice plus "-" is for a FILE 
argument to replace standard input, It would make more sense to me to have someting like:

$ cat <&-

cat: standard input: Bad file descriptor

$



This is the "git diff" of my proposed change:

diff --git a/src/cat.c b/src/cat.c

index b33faeb35..4be189d85 100644

--- a/src/cat.c

+++ b/src/cat.c

@@ -50,6 +50,14 @@

/* Name of input file.  May be "-".  */

static char const *infile;



+/* Pretty name of input file */

+static char const *

+quotef_infile (void)

+{

+  if (STREQ (infile, "-")) return _("standard input");

+  return quotef (infile);

+}

+

/* Descriptor on which input file is open.  */

static int input_desc;



@@ -164,7 +172,7 @@ simple_cat (char *buf, idx_t bufsize)

    size_t n_read = safe_read (input_desc, buf, bufsize);

    if (n_read == SAFE_READ_ERROR)

  {

-  error (0, errno, "%s", quotef (infile));

+  error (0, errno, "%s", quotef_infile());

    return false;

  }



@@ -313,7 +321,7 @@ cat (char *inbuf, idx_t insize, char *outbuf, idx_t outsize,

    size_t n_read = safe_read (input_desc, inbuf, insize);

    if (n_read == SAFE_READ_ERROR)

  {

-  error (0, errno, "%s", quotef (infile));

+  error (0, errno, "%s", quotef_infile());

    write_pending (outbuf, );

    newlines2 = newlines;

    return false;

@@ -526,7 +534,7 @@ copy_cat (void)

  || errno == EBADF || errno == EXDEV || errno == ETXTBSY

  || errno == EPERM)

    return 0;

-    error (0, errno, "%s", quotef (infile));

+    error (0, errno, "%s", quotef_infile());

  return -1;

    }

}

@@ -684,7 +692,7 @@ main (int argc, char **argv)

    input_desc = open (infile, file_open_mode);

    if (input_desc < 0)

  {

-  error (0, errno, "%s", quotef (infile));

+  error (0, errno, "%s", quotef_infile());

    ok = false;

    continue;

  }

@@ -692,7 +700,7 @@ main (int argc, char **argv)



    if (fstat (input_desc, _buf) < 0)

  {

-  error (0, errno, "%s", quotef (infile));

+  error (0, errno, "%s", quotef_infile());

    ok = false;

    goto contin;

  }

@@ -719,7 +727,7 @@ main (int argc, char **argv)

  }

    if (exhausting)

  {

-  error (0, 0, _("%s: input file is output file"), quotef 
(infile));

+  error (0, 0, _("%s: input file is output file"), 
quotef_infile());

    ok = false;

    goto contin;

  }

@@ -794,7 +802,7 @@ main (int argc, char **argv)

  contin:

    if (!reading_stdin && close (input_desc) < 0)

  {

-  error (0, errno, "%s", quotef (infile));

+  error (0, errno, "%s", quotef_infile());

    ok = false;

  }

  }

@@ -807,7 +815,8 @@ main (int argc, char **argv)

  }



    if (have_read_stdin && close (STDIN_FILENO) < 0)

-    error (EXIT_FAILURE, errno, _("closing standard input"));

+    if (ok)

+  error (EXIT_FAILURE, errno, _("closing standard input"));



    return ok ? EXIT_SUCCESS : EXIT_FAILURE;

}



There are a lot more of coreutil utilities that (could) behave weird on a 
closed standard input, here is my first list:

b2sum base32 base64 cat cksum comm csplit cut expand factor fmt fold head join 
md5sum nl nohup numfmt od paste pr ptx sha1sum sha224sum sha256sum sha384sum 
sha512sum shred shuf sort split stdbuf stty sum tac tail tee tr tsort unexpand 
uniq wc





I would like to work on those as well but it would help to have:

1) some kind of consensus on "this is a better way of displaying the error", 
for example other tools also go wild like:

  grep: (standard input): Bad file descriptor

  - why parentheses?

  sed: read error on stdin: Bad file descriptor

  - stdin is the C name for standard input, documented?

2) a better way of offering changes than sending a diff patch in a e-mail.


Yes it would be good to fix up at least the duplicated error.
We did the same on the write side of things with:
https://github.com/coreutils/coreutils/commit/0b2ff7637
Also we generally checked that readers diagnosed errors in:
https://github.com/coreutils/coreutils/blob/master/tests/misc/read-errors.sh

So something similar could probably be done here.

cheers,
Pádraig

bug#70801: Weird behaviour when standard input is closed

[Bernard Burette]

Hi,



If I try:

$ cat <&-

cat: -: Bad file descriptor

cat: closing standard input: Bad file descriptor

$



The error on stdin beign closed is displayed twice plus "-" is for a FILE 
argument to replace standard input, It would make more sense to me to have 
someting like:

$ cat <&-

cat: standard input: Bad file descriptor

$



This is the "git diff" of my proposed change:

diff --git a/src/cat.c b/src/cat.c

index b33faeb35..4be189d85 100644

--- a/src/cat.c

+++ b/src/cat.c

@@ -50,6 +50,14 @@

/* Name of input file.  May be "-".  */

static char const *infile;



+/* Pretty name of input file */

+static char const *

+quotef_infile (void)

+{

+  if (STREQ (infile, "-")) return _("standard input");

+  return quotef (infile);

+}

+

/* Descriptor on which input file is open.  */

static int input_desc;



@@ -164,7 +172,7 @@ simple_cat (char *buf, idx_t bufsize)

   size_t n_read = safe_read (input_desc, buf, bufsize);

   if (n_read == SAFE_READ_ERROR)

 {

-  error (0, errno, "%s", quotef (infile));

+  error (0, errno, "%s", quotef_infile());

   return false;

 }



@@ -313,7 +321,7 @@ cat (char *inbuf, idx_t insize, char *outbuf, idx_t outsize,

   size_t n_read = safe_read (input_desc, inbuf, insize);

   if (n_read == SAFE_READ_ERROR)

 {

-  error (0, errno, "%s", quotef (infile));

+  error (0, errno, "%s", quotef_infile());

   write_pending (outbuf, );

   newlines2 = newlines;

   return false;

@@ -526,7 +534,7 @@ copy_cat (void)

 || errno == EBADF || errno == EXDEV || errno == ETXTBSY

 || errno == EPERM)

   return 0;

-    error (0, errno, "%s", quotef (infile));

+    error (0, errno, "%s", quotef_infile());

 return -1;

   }

}

@@ -684,7 +692,7 @@ main (int argc, char **argv)

   input_desc = open (infile, file_open_mode);

   if (input_desc < 0)

 {

-  error (0, errno, "%s", quotef (infile));

+  error (0, errno, "%s", quotef_infile());

   ok = false;

   continue;

 }

@@ -692,7 +700,7 @@ main (int argc, char **argv)



   if (fstat (input_desc, _buf) < 0)

 {

-  error (0, errno, "%s", quotef (infile));

+  error (0, errno, "%s", quotef_infile());

   ok = false;

   goto contin;

 }

@@ -719,7 +727,7 @@ main (int argc, char **argv)

 }

   if (exhausting)

 {

-  error (0, 0, _("%s: input file is output file"), quotef 
(infile));

+  error (0, 0, _("%s: input file is output file"), 
quotef_infile());

   ok = false;

   goto contin;

 }

@@ -794,7 +802,7 @@ main (int argc, char **argv)

 contin:

   if (!reading_stdin && close (input_desc) < 0)

 {

-  error (0, errno, "%s", quotef (infile));

+  error (0, errno, "%s", quotef_infile());

   ok = false;

 }

 }

@@ -807,7 +815,8 @@ main (int argc, char **argv)

 }



   if (have_read_stdin && close (STDIN_FILENO) < 0)

-    error (EXIT_FAILURE, errno, _("closing standard input"));

+    if (ok)

+  error (EXIT_FAILURE, errno, _("closing standard input"));



   return ok ? EXIT_SUCCESS : EXIT_FAILURE;

}



There are a lot more of coreutil utilities that (could) behave weird on a 
closed standard input, here is my first list:

b2sum base32 base64 cat cksum comm csplit cut expand factor fmt fold head join 
md5sum nl nohup numfmt od paste pr ptx sha1sum sha224sum sha256sum sha384sum 
sha512sum shred shuf sort split stdbuf stty sum tac tail tee tr tsort unexpand 
uniq wc





I would like to work on those as well but it would help to have:

1) some kind of consensus on "this is a better way of displaying the error", 
for example other tools also go wild like:

 grep: (standard input): Bad file descriptor

 - why parentheses?

 sed: read error on stdin: Bad file descriptor

 - stdin is the C name for standard input, documented?

2) a better way of offering changes than sending a diff patch in a e-mail.



Thanks,

Regards,

bug#70727: cp doesn't recognize the new --update=none-fail option

[Pádraig Brady]

On 03/05/2024 05:12, Attila Fidan via GNU coreutils Bug Reports wrote:

Hi,

I wanted to use the new cp --update=none-fail option introduced in 9.5,
but it said "invalid argument ‘none-fail’ for ‘--update’". It turns out
that the commit (49912bac286eb3c0ef7d1567ae790193ad5eb2e8) adding it
forgot to add the new operation to update_type[] and
update_type_string[] in cp.c like it did for mv.c. After patching
coreutils locally the functionality works as expected.

It seems like the test suite didn't catch this because there's no
cp/update.sh test like there is for mv. There's a test for if using
--backup and --update=none-fail are mutually exclusive by checking if cp
returns 1, but an invalid argument also makes cp return 1 :)

I didn't include a patch in case a change to the test suite is wanted,
but the proposed change is tiny and rather obvious.


Well that's embarrassing.
I implemented in cp first, tested that manually,
then must have messed up that hunk when rebasing.

The attached should fix this.

Marking this as done.

thanks,
Pádraig.From de49e993ea8b6dcdf6cada3c0f44a6371514f952 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= 
Date: Fri, 3 May 2024 10:18:50 +0100
Subject: [PATCH] cp: actually support --update=none-fail

* src/cp.c: Add the entries for the --update=none-fail option.
* tests/mv/update.sh: Add a test case.
* NEWS: Mention the bug fix.
Fixes https://bugs.gnu.org/70727
---
 NEWS   |  4 
 src/cp.c   |  4 ++--
 tests/mv/update.sh | 11 +++
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 389f72516..7e8ccb34f 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,10 @@ GNU coreutils NEWS-*- outline -*-
 
 ** Bug fixes
 
+  cp fixes support for --update=none-fail, which would have been
+  rejected as an invalid option.
+  [bug introduced in coreutils-9.5]
+
   ls and printf fix shell quoted output in the edge case of escaped
   first and last characters, and single quotes in the string.
   [bug introduced in coreutils-8.26]
diff --git a/src/cp.c b/src/cp.c
index 28b0217db..06dbad155 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -104,11 +104,11 @@ ARGMATCH_VERIFY (reflink_type_string, reflink_type);
 
 static char const *const update_type_string[] =
 {
-  "all", "none", "older", nullptr
+  "all", "none", "none-fail", "older", nullptr
 };
 static enum Update_type const update_type[] =
 {
-  UPDATE_ALL, UPDATE_NONE, UPDATE_OLDER,
+  UPDATE_ALL, UPDATE_NONE, UPDATE_NONE_FAIL, UPDATE_OLDER,
 };
 ARGMATCH_VERIFY (update_type_string, update_type);
 
diff --git a/tests/mv/update.sh b/tests/mv/update.sh
index 164357803..39ff677b9 100755
--- a/tests/mv/update.sh
+++ b/tests/mv/update.sh
@@ -38,6 +38,17 @@ for interactive in '' -i; do
   done
 done
 
+# These should accept all options
+for update_option in '--update' '--update=older' '--update=all' \
+ '--update=none' '--update=none-fail'; do
+
+  touch file1 || framework_failure_
+  mv $update_option file1 file2 || fail=1
+  test -f file1 && fail=1
+  cp $update_option file2 file1 || fail=1
+  rm file1 file2 || framework_failure_
+done
+
 # These should perform the rename / copy
 for update_option in '--update' '--update=older' '--update=all' \
  '--update=none --update=all'; do
-- 
2.44.0

bug#70727: cp doesn't recognize the new --update=none-fail option

[Attila Fidan via GNU coreutils Bug Reports]

Hi,

I wanted to use the new cp --update=none-fail option introduced in 9.5,
but it said "invalid argument ‘none-fail’ for ‘--update’". It turns out
that the commit (49912bac286eb3c0ef7d1567ae790193ad5eb2e8) adding it
forgot to add the new operation to update_type[] and
update_type_string[] in cp.c like it did for mv.c. After patching
coreutils locally the functionality works as expected.

It seems like the test suite didn't catch this because there's no
cp/update.sh test like there is for mv. There's a test for if using
--backup and --update=none-fail are mutually exclusive by checking if cp
returns 1, but an invalid argument also makes cp return 1 :)

I didn't include a patch in case a change to the test suite is wanted,
but the proposed change is tiny and rather obvious.

Thanks!

bug#70714: realpath no error for unreadable-symlink

[Pádraig Brady]

On 02/05/2024 07:16, Nineteendo INC wrote:

coreutils version: stable 9.5 (bottled)
OS version: macOS 13.6.6 (22G630)

`realpath` doesn’t behave correctly for unreadable symlinks:

wannes@Stefans-iMac ~ % ln -s . src
wannes@Stefans-iMac ~ % grealpath -e src/..
/Users
wannes@Stefans-iMac ~ % chmod -h 000 src
wannes@Stefans-iMac ~ % grealpath -e src/..
/Users/wannes

Expected behaviour:

wannes@Stefans-iMac ~ % grealpath -e src/..
grealpath: src/..: Permission denied


Right, looks like we'll have to cater for EACCES on darwin.
I'll have a look

bug#70714: realpath no error for unreadable-symlink

[Nineteendo INC]

coreutils version: stable 9.5 (bottled)
OS version: macOS 13.6.6 (22G630)

`realpath` doesn’t behave correctly for unreadable symlinks:

wannes@Stefans-iMac ~ % ln -s . src
wannes@Stefans-iMac ~ % grealpath -e src/..
/Users
wannes@Stefans-iMac ~ % chmod -h 000 src
wannes@Stefans-iMac ~ % grealpath -e src/..
/Users/wannes

Expected behaviour:

wannes@Stefans-iMac ~ % grealpath -e src/..
grealpath: src/..: Permission denied

bug#70699: Possible bug in /usr/bin/paste

[Paul Eggert]

On 2024-05-01 09:32, Art Shelest via GNU coreutils Bug Reports wrote:

Yes, this is the bug.

Will you please fix it?


I don't see a bug here. In the POSIX world, CR is just another character 
and 'paste' is required to paste it like it would paste any other character.


As you mentioned, a simple way to fix it is to remove the CRs first, 
e.g., with the command "tr -d '\r'". In Bash:


paste <(tr -d '\r' letters.txt) <(tr -d '\r' numbers.txt)

bug#70699: Possible bug in /usr/bin/paste

[Art Shelest via GNU coreutils Bug Reports]

Yes, this is the bug.

Will you please fix it?


-Original Message-
From: Pádraig Brady  On Behalf Of Pádraig Brady
Sent: Wednesday, May 1, 2024 12:25 PM
To: Art Shelest ; 70...@debbugs.gnu.org
Subject: [External] Re: bug#70699: Possible bug in /usr/bin/paste

On 01/05/2024 15:28, Art Shelest via GNU coreutils Bug Reports wrote:
> Good morning,
> 
> I am seeing an aberrant behavior from the /usr/bin/paste utility when working 
> with Windows-style CR/LF text files.
> The repro is for Mint Mate (Virginia).
> 
> If I change the line endings in the first file to Unix format (LF), it works 
> as expected.
> If I change the line endings to Max (CR), it breaks even worse.
> 
> $ hexdump -C letters.txt
>   61 61 09 41 41 0d 0a 62  62 09 42 42 0d 0a|aa.AA..bb.BB..|
> 
> $ cat letters.txt
> aa   AA
> bb   BB
> $ cat numbers.txt
> 1
> 2
> $ paste letters.txt numbers.txt
> aa   1A
> bb   2B
> $
> 
> 
> Expected:
> $ paste letters.txt numbers.txt
> aa   AA   1
> bb   BB   2
> 
> Thank you.
> 

paste(1) is treating the CR like a standard character, and when outputting that 
back to the terminal it "messes up" the expected output.
I suggest you convert any such files to unix format before processing.

thanks,
Pádraig

bug#70699: Possible bug in /usr/bin/paste

[Pádraig Brady]

On 01/05/2024 15:28, Art Shelest via GNU coreutils Bug Reports wrote:

Good morning,

I am seeing an aberrant behavior from the /usr/bin/paste utility when working 
with Windows-style CR/LF text files.
The repro is for Mint Mate (Virginia).

If I change the line endings in the first file to Unix format (LF), it works as 
expected.
If I change the line endings to Max (CR), it breaks even worse.

$ hexdump -C letters.txt
  61 61 09 41 41 0d 0a 62  62 09 42 42 0d 0a|aa.AA..bb.BB..|

$ cat letters.txt
aa   AA
bb   BB
$ cat numbers.txt
1
2
$ paste letters.txt numbers.txt
aa   1A
bb   2B
$


Expected:
$ paste letters.txt numbers.txt
aa   AA   1
bb   BB   2

Thank you.



paste(1) is treating the CR like a standard character,
and when outputting that back to the terminal it "messes up" the expected 
output.
I suggest you convert any such files to unix format before processing.

thanks,
Pádraig

bug#70699: Possible bug in /usr/bin/paste

[Art Shelest via GNU coreutils Bug Reports]

Good morning,

I am seeing an aberrant behavior from the /usr/bin/paste utility when working 
with Windows-style CR/LF text files.
The repro is for Mint Mate (Virginia).

If I change the line endings in the first file to Unix format (LF), it works as 
expected.
If I change the line endings to Max (CR), it breaks even worse.

$ hexdump -C letters.txt
  61 61 09 41 41 0d 0a 62  62 09 42 42 0d 0a|aa.AA..bb.BB..|

$ cat letters.txt
aa   AA
bb   BB
$ cat numbers.txt
1
2
$ paste letters.txt numbers.txt
aa   1A
bb   2B
$


Expected:
$ paste letters.txt numbers.txt
aa   AA   1
bb   BB   2

Thank you.

bug#70639: Bug or Misleading Description of realpath command

[Collin Funk]

On 4/28/24 4:37 PM, Hazel Cooney wrote:
> To reproduce this bug, create a directory with some files in it. Then, go
> to the directory that contains the one you just made. Type ls new_directory
> | xargs realpath.

Using 'build-aux' for my directory instead of music_copy. We have this
for build-aux:

ls build-aux
announce-genconfig.libpathdepcomp 

>From the 'realpath' info page we have:

   By default, all but the last component of the specified files must
   exist.

Then when we perform the 'ls build-aux | xargs realpath', we don't
change our current working directory. So that is why we get:

# Notice 'build-aux' is missing after gnulib.
$ ls build-aux | xargs realpath
/home/collin/.local/src/gnulib/announce-gen
/home/collin/.local/src/gnulib/ar-lib
/home/collin/.local/src/gnulib/bootstrap

There might be a better way to write it, but I would do this:

$ for file in build-aux/*; do echo $file; done | xargs realpath
/home/collin/.local/src/gnulib/build-aux/announce-gen
/home/collin/.local/src/gnulib/build-aux/ar-lib
/home/collin/.local/src/gnulib/build-aux/bootstrap

Since unlike 'ls' that loop does not remove the directory name:

   $ for file in build-aux/*; do echo $file; done
   build-aux/announce-gen
   build-aux/ar-lib
   build-aux/bootstrap

Does that explination help? Or I misunderstanding your issue?

Collin

bug#17553: du unit suggestion

[Reuben Thomas via GNU coreutils Bug Reports]

On Fri, 23 May 2014 at 15:02, Reuben Thomas  wrote:

> On 23 May 2014 13:12, Pádraig Brady  wrote:
>
>> tl;dr
>>
>>   You can get what you want currently by doing:
>>
>> du() { env du -B1 "$@" | numfmt --to=iec-i --suffix=B; }
>>
>
> Thanks very much, that's certainly good enough for me.
>

Today I found that this doesn't cope with filenames that contain newlines,
but it's easy to fix:

du() { env du "$@" --block-size=1 --null | numfmt --zero-terminated
--from=auto --to=iec-i --suffix=B | tr '\0' '\n'; }

-- 
https://rrt.sc3d.org

bug#70532: sort: Mention counting fields from the end

[Dan Jacobson]

> "PB" == Pádraig Brady  writes:
PB> All good suggestions. I'll at least add an example along the lines of:

PB>   awk '{print $NF, $0}' | sort -k1,1 | cut -f2- -d' '

OK, also say what it's doing. Not everybody knows awk.

Also join(1) needs a tip added. Users might want to join on
e.g., the second to last field on a variable-number-of-fields file.

bug#70601: join(1) info page -o: too hazy at first

[Dan Jacobson]

The join main page is really clear,

   -o FORMAT
  obey FORMAT while constructing output line

A base hit on the first pitch. The info page on the other hand,

‘-o FIELD-LIST’
‘-o auto’
 If the keyword ‘auto’ is specified, infer the output format from
 the first line in each file.  This is the same as the default
 output format but also ensures the same number of fields are output
 for each line.  Missing fields are replaced with the ‘-e’ option
 and extra fields are discarded.

even after reading down to here, hasn't tipped the user off that it is
talking about the output format! It's its turn up to bat but it's still
drinking coffee in the bullpen.

In fact FIELD-LIST vs FORMAT make the user double check that they are
actually looking at the same commands' documentation.

bug#70600: trailing whitespace spotted in join info pages

[Dan Jacobson]

The join info pages have tons of trailing whitespace.
sed s/$/$/ reveals:

‘sort -u file1 file2’Union of unsorted files$
 $
‘sort file1 file2 | uniq -d’ Intersection of unsorted files$
 $
‘sort file1 file1 file2 | uniq -u’   Difference of unsorted files$
 $
‘sort file1 file2 | uniq -u’ Symmetric Difference of unsorted$
 files$

Or in emacs
(progn
  (setq-default show-trailing-whitespace t)
  (info "(coreutils) Set operations"))
will drive home the point.
Same with some other pages in that manual.

Why bad? It says that whatever typesetting program you are using, is
like a broken Xerox machine that is spitting out extra blank pages,
wasting paper. Even if we're rich so who cares.

bug#70599: join vs. numeric order

[Dan Jacobson]

(info "(coreutils) Sorting files for join") needs to talk about numeric
order.

$ seq 111|join --check-order - /dev/null
join: -:10: is not sorted: 10

So the info manual needs to mention 'Even though your files might be in
perfect "sort --numeric-sort" order, you need to make them into plain
"sort" order first. Sorry. At least you'll get the same number of joins.'

Or, add a new join -n option. The join man page could now say:
'-n: use numeric comparisons. Note sort order also needs to be "sort -n" order.'

And / or mention how the user might tinker with LC_NUMERIC and / or
LC_COLLATE to somehow achieve numeric sorting...

bug#70586: cp walks dir differently than rm and is hitting "File name too long" where this could be avoided

[Arkadiusz Miśkiewicz via GNU coreutils Bug Reports]

Hello.

rm -r while deleting a directory that's longer than PATH_MAX walks it in 
a way to avoid hitting max limit


$ (for i in `seq 1 2000`; do mkdir 
1234567890123456789012345678901234567890; cd 
1234567890123456789012345678901234567890; done)

$ rm -r 1234567890123456789012345678901234567890


but cp doesn't do that:

$ (for i in `seq 1 2000`; do mkdir 
1234567890123456789012345678901234567890; cd 
1234567890123456789012345678901234567890; done)

$ cp -a 1234567890123456789012345678901234567890 2

cp: cannot stat 
'1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890/1234567890123456789012345678901234567890':
 File name too long


I wonder (+ report as a enhancement request) why cp isn't made to do the 
same smart thing and avoid hitting ENAMETOOLONG?


$ cp --version
cp (GNU coreutils) 9.5

--
Arkadiusz Miśkiewicz, arekm / ( maven.pl | pld-linux.org )

bug#70561: ln -sfr stats the wrong file

[David Leonard]

(resend)

When analyzing file accesses with strace, I noticed that ln stats an
unrelated file. This only happens with the opions -sfr and when the dest
symlink already exists.

Here's a reproduction script:

# setup
mkdir -p a b
> b/x
ln -fs anything a/x

# ln -sfr b/x a/x
strace -o trace.out --decode-fds=path -e %file \
src/ln -sfr b/x a/x

test $(readlink a/x) = ../b/x
grep stat trace.out

The invalid stat that happens is:

newfstatat(AT_FDCWD, "../b/x", 0x7ffdb94a0270, 0) = -1 ENOENT
(No such file or directory)

The cause seems to be in src/ln.c's do_link() where the `source` "b/x" is
converted to "../b/x" by

if (relative)
  source = rel_source = convert_abs_rel (source, dest);

and then the test to see if deleting the destination file would delete
the source works by checking hard links on the source.

if (source_status != 0)
source_status = stat (source, _stats);

Except that -r made source relative to the dest directory.

bug#70558: ln -sfr stats the wrong file

[David Leonard]

When analyzing file accesses with strace, I noticed that ln stats an
unrelated file. This only happens with the opions -sfr and when the dest
symlink already exists.

Here's a reproduction script:

# setup
mkdir -p a b
> b/x
ln -fs anything a/x

# ln -sfr b/x a/x
strace -o trace.out --decode-fds=path -e %file \
src/ln -sfr b/x a/x

test $(readlink a/x) = ../b/x
grep stat trace.out

The invalid stat that happens is:

newfstatat(AT_FDCWD, "../b/x", 0x7ffdb94a0270, 0) = -1 ENOENT 
(No such file or directory)

The cause seems to be in src/ln.c's do_link() where the `source` "b/x" is
converted to "../b/x" by

if (relative)
  source = rel_source = convert_abs_rel (source, dest);

and then the test to see if deleting the destination file would delete
the source works by checking hard links on the source.

if (source_status != 0)
source_status = stat (source, _stats);

Except that -r made source relative to the dest directory.

bug#70532: sort: Mention counting fields from the end

[Pádraig Brady]

On 23/04/2024 11:14, Dan Jacobson wrote:

In (info "(coreutils) sort invocation") be sure to add an example of a
way or workaround for counting fields from the end of the line. E.g., we
want to sort on the last field, but don't know for sure how many fields
a line might contain. E.g., sort by surname, when lines consist of First
[Middle...] Surname. perl -a uses $F[-1]. so maybe sort(1) could also
use a negative field number. Same for character number.


All good suggestions. I'll at least add an example along the lines of:

  awk '{print $NF, $0}' | sort -k1,1 | cut -f2- -d' '

cheers,
Pádraig.

bug#70532: sort: Mention counting fields from the end

[Dan Jacobson]

In (info "(coreutils) sort invocation") be sure to add an example of a
way or workaround for counting fields from the end of the line. E.g., we
want to sort on the last field, but don't know for sure how many fields
a line might contain. E.g., sort by surname, when lines consist of First
[Middle...] Surname. perl -a uses $F[-1]. so maybe sort(1) could also
use a negative field number. Same for character number.

sort (GNU coreutils) 9.4

bug#70477: tail command does not hang on /dev/random

[Paul Eggert]

On 2024-04-19 11:40, Ionut Nicula wrote:

The following command hangs indefinitely, as expected:

 tail -c 4096 /dev/random

The following command immediately returns 4097 bytes, unexpectedly:

 tail -c 4097 /dev/random


Thanks for the bug report. Although the two commands should behave 
similarly, neither should loop: they should both output the requested 
number of random bytes. Similarly for /dev/zero. I installed the 
attached patch to do that.From fb543b6b82c1f3a20ff88f44cc3ed367bfe811b6 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Fri, 19 Apr 2024 21:44:32 -0700
Subject: [PATCH] tail: avoid infloop with -c on /dev/zero

Problem reported by Ionut Nicula in:
https://bugs.gnu.org/70477
* src/tail.c (tail_bytes): Do not loop forever on commands
like 'tail -c 4096 /dev/zero'.
* tests/tail/tail-c.sh: Test this fix.
---
 NEWS |  3 +++
 src/tail.c   | 24 +++-
 tests/tail/tail-c.sh | 10 ++
 3 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/NEWS b/NEWS
index 43ce84d7e..389f72516 100644
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,9 @@ GNU coreutils NEWS-*- outline -*-
   have exited with a "Function not implemented" error.
   [bug introduced in coreutils-8.28]
 
+  'tail -c 4096 /dev/zero' no longer loops forever.
+  [This bug was present in "the beginning".]
+
 ** Changes in behavior
 
   ls's -f option now simply acts like -aU, instead of also ignoring
diff --git a/src/tail.c b/src/tail.c
index 52c081031..a3b46ca2d 100644
--- a/src/tail.c
+++ b/src/tail.c
@@ -760,7 +760,8 @@ free_lbuffers:
   return ok;
 }
 
-/* Print the last N_BYTES characters from the end of pipe FD.
+/* Print the last N_BYTES characters from the end of FD.
+   Work even if the input is a pipe.
This is a stripped down version of pipe_lines.
Return true if successful.  */
 
@@ -1875,15 +1876,28 @@ tail_bytes (char const *pretty_filename, int fd, uintmax_t n_bytes,
 {
   off_t end_pos = -1;
   off_t current_pos = -1;
+  bool copy_from_current_pos = false;
 
   if (! presume_input_pipe && n_bytes <= OFF_T_MAX)
 {
   if (usable_st_size ())
-end_pos = stats.st_size;
-  else if ((current_pos = lseek (fd, -n_bytes, SEEK_END)) != -1)
-end_pos = current_pos + n_bytes;
+{
+  /* Use st_size only if it's so large that this is
+ probably not a /proc or similar file, where st_size
+ is notional.  */
+  end_pos = stats.st_size;
+  off_t smallish_size = STP_BLKSIZE ();
+  copy_from_current_pos = smallish_size < end_pos;
+}
+  else
+{
+  current_pos = lseek (fd, -n_bytes, SEEK_END);
+  copy_from_current_pos = current_pos != -1;
+  if (copy_from_current_pos)
+end_pos = current_pos + n_bytes;
+}
 }
-  if (end_pos <= (off_t) STP_BLKSIZE ())
+  if (! copy_from_current_pos)
 return pipe_bytes (pretty_filename, fd, n_bytes, read_pos);
   if (current_pos == -1)
 current_pos = xlseek (fd, 0, SEEK_CUR, pretty_filename);
diff --git a/tests/tail/tail-c.sh b/tests/tail/tail-c.sh
index f518e5b21..a9f2bc2d1 100755
--- a/tests/tail/tail-c.sh
+++ b/tests/tail/tail-c.sh
@@ -35,4 +35,14 @@ printf '123456' | tail -c3 > out || fail=1
 printf '456' > exp || framework_failure_
 compare exp out || fail=1
 
+# Any part of /dev/zero should be valid for tail -c.
+head -c 4096 /dev/zero >exp || fail=1
+tail -c 4096 /dev/zero >out || fail=1
+compare exp out || fail=1
+
+# Any part of /dev/urandom, if it exists, should be valid for tail -c.
+if test -r /dev/urandom; then
+  timeout --verbose 1 tail -c 4096 /dev/urandom >/dev/null || fail=1
+fi
+
 Exit $fail
-- 
2.40.1

bug#70477: tail command does not hang on /dev/random

[Ionut Nicula]

The following command hangs indefinitely, as expected:

tail -c 4096 /dev/random

The following command immediately returns 4097 bytes, unexpectedly:

tail -c 4097 /dev/random

Details:
- Source code obtained from: git://git.sv.gnu.org/coreutils
- Commit id: 9ab1f9db94479eca4dd2c29b4e21550ff2bcd5bb
- System:

$ uname -a
Linux zinc 6.1.0-20-amd64 #1 SMP PREEMPT_DYNAMIC Debian
6.1.85-1 (2024-04-11) x86_64 GNU/Linux

bug#70411: [bug] install(1) fails to read /dev/stdin on Darwin

[Sergei Trofimovich]

On Fri, 19 Apr 2024 00:33:52 -0700
Paul Eggert  wrote:

> On 2024-04-18 14:52, Sergei Trofimovich wrote:
> > $ clang simple.c -o simple && echo 42 | ./simple
> > 1: ino=3009428657538693161
> > 2: ino=3009428657538693161
> > 3: ino=1568241705
> > 
> > Note how stat() and fstat() don't agree on inode.
> > 
> > Apparently it's documented in
> > https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fstat.2.html
> > as
> > 
> >BUGS
> >   Applying fstat to a socket (and thus to a pipe) returns a zero'd 
> > buffer,
> >   except for the blocksize field, and a unique device and inode number. 
> >  
> 
> The BUGS note simply means that a pipe has a unique inode number, which 
> is what we want. So that's not indicating any problem.
> 
> 
> Oh, I see the problem now.  For a socket or pipe, macOS fstat returns 
> the full 64-bit inode number, whereas macOS stat returns only the low 
> order 32 bits.  In your example, 3009428657538693161 % (2**32) == 
> 1568241705.
> 
> This is a kernel bug in macOS. Can you report it or otherwise arrange to 
> have the kernel bug fixed? I expect that you have better connections 
> with Apple than I do. A proposed patch (relative to xnu-10063.101.15) is 
> attached; I have not tested it as I don't use macOS. Thanks.

I reported it via https://www.apple.com/feedback/macos.html

> Also, I am documenting this macOS bug in Gnulib by installing the second 
> attached patch to Gnulib, and am cc'ing this email to bug-gnulib.

Thank you, Paul!

-- 

  Sergei

bug#70411: [bug] install(1) fails to read /dev/stdin on Darwin

[Paul Eggert]

On 2024-04-18 14:52, Sergei Trofimovich wrote:

$ clang simple.c -o simple && echo 42 | ./simple
1: ino=3009428657538693161
2: ino=3009428657538693161
3: ino=1568241705

Note how stat() and fstat() don't agree on inode.

Apparently it's documented in
https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fstat.2.html
as

   BUGS
  Applying fstat to a socket (and thus to a pipe) returns a zero'd buffer,
  except for the blocksize field, and a unique device and inode number.


The BUGS note simply means that a pipe has a unique inode number, which 
is what we want. So that's not indicating any problem.



Oh, I see the problem now.  For a socket or pipe, macOS fstat returns 
the full 64-bit inode number, whereas macOS stat returns only the low 
order 32 bits.  In your example, 3009428657538693161 % (2**32) == 
1568241705.


This is a kernel bug in macOS. Can you report it or otherwise arrange to 
have the kernel bug fixed? I expect that you have better connections 
with Apple than I do. A proposed patch (relative to xnu-10063.101.15) is 
attached; I have not tested it as I don't use macOS. Thanks.


Also, I am documenting this macOS bug in Gnulib by installing the second 
attached patch to Gnulib, and am cc'ing this email to bug-gnulib.From 29345117a4cf85aceb88e3901758b19a4867062e Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Fri, 19 Apr 2024 00:12:50 -0700
Subject: [PATCH] Fix bug with stat truncating pipe/socket st_ino

Problem reported by Sergei Trofimovich in:
https://bugs.gnu.org/70411
https://github.com/NixOS/nixpkgs/pull/300797
* bsd/miscfs/devfs/devfs_fdesc_support.c (fdesc_attr):
Do not truncate inode numbers to 32 bits.
---
 bsd/miscfs/devfs/devfs_fdesc_support.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bsd/miscfs/devfs/devfs_fdesc_support.c b/bsd/miscfs/devfs/devfs_fdesc_support.c
index a17c6992..b4a55103 100644
--- a/bsd/miscfs/devfs/devfs_fdesc_support.c
+++ b/bsd/miscfs/devfs/devfs_fdesc_support.c
@@ -437,10 +437,10 @@ fdesc_attr(int fd, struct vnode_attr *vap, vfs_context_t a_context)
 	case DTYPE_PIPE:
 #if SOCKETS
 		if (FILEGLOB_DTYPE(fp->fp_glob) == DTYPE_SOCKET) {
-			error = soo_stat((struct socket *)fp_get_data(fp), (void *), 0);
+			error = soo_stat((struct socket *)fp_get_data(fp), (void *), 1);
 		} else
 #endif /* SOCKETS */
-		error = pipe_stat((struct pipe *)fp_get_data(fp), (void *), 0);
+		error = pipe_stat((struct pipe *)fp_get_data(fp), (void *), 1);
 
 		if (error == 0) {
 			if (FILEGLOB_DTYPE(fp->fp_glob) == DTYPE_SOCKET) {
-- 
2.44.0

From c2174a623d33096b52f4d7fd2963f76acb3e301f Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Fri, 19 Apr 2024 00:29:32 -0700
Subject: [PATCH] fstatat, stat: document macOS st_ino pipe bug

* doc/posix-functions/fstatat.texi (fstatat):
* doc/posix-functions/stat.texi (stat):
Document macOS bug (see ).
---
 ChangeLog| 7 +++
 doc/posix-functions/fstatat.texi | 5 +
 doc/posix-functions/stat.texi| 5 +
 3 files changed, 17 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index 7ce75a98a9..1667f90c55 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2024-04-19  Paul Eggert  
+
+	fstatat, stat: document macOS st_ino pipe bug
+	* doc/posix-functions/fstatat.texi (fstatat):
+	* doc/posix-functions/stat.texi (stat):
+	Document macOS bug (see ).
+
 2024-04-18  Bruno Haible  
 
 	totalordermagl: Add tests.
diff --git a/doc/posix-functions/fstatat.texi b/doc/posix-functions/fstatat.texi
index e959a5cc73..90884e2eb1 100644
--- a/doc/posix-functions/fstatat.texi
+++ b/doc/posix-functions/fstatat.texi
@@ -40,5 +40,10 @@ This function does not fail when the second argument is an empty string
 on some platforms, even when @code{AT_EMPTY_PATH} is not used:
 glibc 2.7, Linux 2.6.38.
 @item
+This function sets @code{st_ino} only to the low-order 32 bits of
+the inode number of a socket or pipe, which thus can disagree
+with the @code{st_ino} obtained by @code{fstat}:
+macOS 14.
+@item
 @xref{sys/stat.h}, for general portability problems with @code{struct stat}.
 @end itemize
diff --git a/doc/posix-functions/stat.texi b/doc/posix-functions/stat.texi
index f655451392..8afd3b17bb 100644
--- a/doc/posix-functions/stat.texi
+++ b/doc/posix-functions/stat.texi
@@ -50,6 +50,11 @@ Portability problems not fixed by Gnulib:
 Cygwin's @code{stat} function sometimes sets @code{errno} to @code{EACCES} when
 @code{ENOENT} would be more appropriate.
 @item
+This function sets @code{st_ino} only to the low-order 32 bits of
+the inode number of a socket or pipe, which thus can disagree
+with the @code{st_ino} obtained by @code{fstat}:
+macOS 14.
+@item
 Because of the definition of @code{struct stat}, it is not possible to
 portably replace @code{stat} via an object-like macro.  Therefore,
 expressions such as @code{(islnk ? lstat : stat) (name, buf)} are not
-- 
2.40.1

bug#70411: [bug] install(1) fails to read /dev/stdin on Darwin

[Paul Eggert]

On 4/18/24 14:52, Sergei Trofimovich wrote:


https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fstat.2.html
as

   BUGS
  Applying fstat to a socket (and thus to a pipe) returns a zero'd buffer,
  except for the blocksize field, and a unique device and inode number.


How is a macOS program supposed to tell whether a file has been changed 
between stat and open? Is it impossible?

bug#70411: [bug] install(1) fails to read /dev/stdin on Darwin

[Alejandro Colomar]

Hi Sergei,

On Thu, Apr 18, 2024 at 10:52:32PM +0100, Sergei Trofimovich wrote:
> > The system I have locally is this one:
> > 
> > $ uname -a
> > Darwin tests-iMac.local 18.7.0 Darwin Kernel Version 18.7.0: Tue Jun 22 
> > 19:37:08 PDT 2021; root:xnu-4903.278.70~1/RELEASE_X86_64 x86_64
> 
> I debugged it a bit more locally and it looks like it's a known
> limitation of fstat()/stat() mismatch on FIFOs on Darwin. Example
> program:

Thanks!

> $ cat simple.c
> #include 
> #include 
> #include 
> #include 
> #include 
> 
> int main() {
>   struct stat s;
>   int fd;
>   int r;
> 
>   fd = open("/dev/fd/0", O_RDONLY);
>   assert(fd > 0);
>   r = fstat(fd, );
>   assert(r == 0);
>   fprintf(stderr, "1: ino=%llu\n", s.st_ino);
>   close(fd);
> 
>   fd = open("/dev/fd/0", O_RDONLY);
>   assert(fd > 0);
>   r = fstat(fd, );
>   assert(r == 0);
>   fprintf(stderr, "2: ino=%llu\n", s.st_ino);
>   close(fd);
> 
>   r = stat("/dev/fd/0", );
>   assert(r == 0);
>   fprintf(stderr, "3: ino=%llu\n", s.st_ino);
>   close(fd);
> }
> 
> Running:
> 
> $ clang simple.c -o simple && echo 42 | ./simple
> 1: ino=3009428657538693161
> 2: ino=3009428657538693161
> 3: ino=1568241705
> 
> Note how stat() and fstat() don't agree on inode.
> 
> Apparently it's documented in
> https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fstat.2.html
> as
> 
>   BUGS
>  Applying fstat to a socket (and thus to a pipe) returns a zero'd buffer,
>  except for the blocksize field, and a unique device and inode number.
> 
> Perhaps coreutils should avoid inode checks on FIFOs and sockets. They
> are already volatile by nature.

I disagree.  I think coreutils should not try to work on broken systems;
especially when the checks are done for security reasons.  Let the
system die.

Have a lovely night!
Alex

-- 



signature.asc
Description: PGP signature

bug#70411: [bug] install(1) fails to read /dev/stdin on Darwin

[Sergei Trofimovich]

On Tue, 16 Apr 2024 22:37:58 +0100
Sergei Trofimovich  wrote:

> On Tue, 16 Apr 2024 12:33:44 +0100
> Pádraig Brady  wrote:
> 
> > On 16/04/2024 01:19, Alejandro Colomar wrote:  
> > > Hi!
> > > 
> > > I don't own a Darwin system, so I can't help much reproduce.  However,
> > > I've received a bug report to the Linux man-pages, that our build
> > > system (GNUmakefile-based), which ends up calling
> > > 
> > >   ... | install /dev/stdin $@
> > > 
> > > doesn't work on Darwin.  Here's the original bug report:
> > > .
> > > 
> > > Here are the reported error messages:
> > > 
> > > ...
> > > INSTALL 
> > > /nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/addseverity.3
> > > INSTALL 
> > > /nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/adjtime.3
> > > install: skipping file '/dev/stdin', as it was replaced while being copied
> > > make: *** [share/mk/install/man.mk:54: 
> > > /nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/addmntent.3]
> > >  Error 1
> > > make: *** Waiting for unfinished jobs
> > > install: skipping file '/dev/stdin', as it was replaced while being copied
> > > make: *** [share/mk/install/man.mk:54: 
> > > /nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/acosh.3]
> > >  Error 1
> > > install: skipping file '/dev/stdin', as it was replaced while being copied
> > > install: skipping file '/dev/stdin', as it was replaced while being copied
> > > install: skipping file '/dev/stdin', as it was replaced while being copied
> > > ...
> > > 
> > > I don't see why install(1) should fail to read /dev/stdin under any
> > > POSIX system
> > 
> > What version of darwin is this? I can't repro on Darwin 21.6.0 (MacOSX 
> > 12.6).
> > The issue seems to be that /dev/stdin returns a varying inode which 
> > install(1) doesn't like currently  
> 
> The system I have locally is this one:
> 
> $ uname -a
> Darwin tests-iMac.local 18.7.0 Darwin Kernel Version 18.7.0: Tue Jun 22 
> 19:37:08 PDT 2021; root:xnu-4903.278.70~1/RELEASE_X86_64 x86_64

I debugged it a bit more locally and it looks like it's a known
limitation of fstat()/stat() mismatch on FIFOs on Darwin. Example
program:

$ cat simple.c
#include 
#include 
#include 
#include 
#include 

int main() {
  struct stat s;
  int fd;
  int r;

  fd = open("/dev/fd/0", O_RDONLY);
  assert(fd > 0);
  r = fstat(fd, );
  assert(r == 0);
  fprintf(stderr, "1: ino=%llu\n", s.st_ino);
  close(fd);

  fd = open("/dev/fd/0", O_RDONLY);
  assert(fd > 0);
  r = fstat(fd, );
  assert(r == 0);
  fprintf(stderr, "2: ino=%llu\n", s.st_ino);
  close(fd);

  r = stat("/dev/fd/0", );
  assert(r == 0);
  fprintf(stderr, "3: ino=%llu\n", s.st_ino);
  close(fd);
}

Running:

$ clang simple.c -o simple && echo 42 | ./simple
1: ino=3009428657538693161
2: ino=3009428657538693161
3: ino=1568241705

Note how stat() and fstat() don't agree on inode.

Apparently it's documented in
https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/fstat.2.html
as

  BUGS
 Applying fstat to a socket (and thus to a pipe) returns a zero'd buffer,
 except for the blocksize field, and a unique device and inode number.

Perhaps coreutils should avoid inode checks on FIFOs and sockets. They
are already volatile by nature.

-- 

  Sergei

bug#70418: ls bug

[Paul Eggert]

On 4/17/24 03:19, Pádraig Brady wrote:


Patch looks good, and conforms to POSIX.


Thanks for the review. I installed the patch and am closing this bug report.

bug#70418: ls bug

[Pádraig Brady]

On 16/04/2024 23:17, Paul Eggert wrote:

On 4/16/24 14:30, Toby Kelsey wrote:

The man page doesn't explain this format conflict, while the info page
(info '(coreutils) ls invocation' or 'info ls') claims '-f' implies '-1'
which is also incorrect: 'ls -1f' gives different output to to 'ls -f'.


Yes, this area of GNU 'ls' a mess. Option order should not matter here.

Option order didn't matter in 7th Edition Unix, where -f overrode -l
regardless of whether -f came before or after -l. And option order
doesn't matter in FreeBSD, where -f and -l are orthogonal. GNU ls is an
odd hybrid of 7th Edition and FreeBSD and messes this up.

Rather than document the hybrid mess, let's bite the bullet and fix it.
FreeBSD behavior makes more sense, so let's do that. Proposed patch
attached.


+1

Related to this is the recent adjustment of usage() for -f:
https://bugs.gnu.org/67765

Patch looks good, and conforms to POSIX.

thanks!
Pádraig

bug#70418: ls bug

[Chris Elvidge]

Thanks for the clarification and patch

On 16/04/2024 at 23:17, Paul Eggert wrote:

On 4/16/24 14:30, Toby Kelsey wrote:
The man page doesn't explain this format conflict, while the info page 
(info '(coreutils) ls invocation' or 'info ls') claims '-f' implies 
'-1' which is also incorrect: 'ls -1f' gives different output to to 
'ls -f'.


Yes, this area of GNU 'ls' a mess. Option order should not matter here.

Option order didn't matter in 7th Edition Unix, where -f overrode -l 
regardless of whether -f came before or after -l. And option order 
doesn't matter in FreeBSD, where -f and -l are orthogonal. GNU ls is an 
odd hybrid of 7th Edition and FreeBSD and messes this up.


Rather than document the hybrid mess, let's bite the bullet and fix it. 
FreeBSD behavior makes more sense, so let's do that. Proposed patch 
attached.



--

Chris Elvidge

bug#70418: ls bug

[Paul Eggert]

On 4/16/24 14:30, Toby Kelsey wrote:
The man page doesn't explain this format conflict, while the info page 
(info '(coreutils) ls invocation' or 'info ls') claims '-f' implies '-1' 
which is also incorrect: 'ls -1f' gives different output to to 'ls -f'.


Yes, this area of GNU 'ls' a mess. Option order should not matter here.

Option order didn't matter in 7th Edition Unix, where -f overrode -l 
regardless of whether -f came before or after -l. And option order 
doesn't matter in FreeBSD, where -f and -l are orthogonal. GNU ls is an 
odd hybrid of 7th Edition and FreeBSD and messes this up.


Rather than document the hybrid mess, let's bite the bullet and fix it. 
FreeBSD behavior makes more sense, so let's do that. Proposed patch 
attached.From 674a6c262e594102b0485c5ded854c1be5902777 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Tue, 16 Apr 2024 15:08:51 -0700
Subject: [PATCH] ls: -f now means -a -U

Problem reported by Toby Kelsey .
* src/ls.c (decode_switches): -f now simply means -a -U.
(usage): Adjust to match.
---
 NEWS   |  7 +++
 doc/coreutils.texi | 14 +++---
 src/ls.c   |  7 +--
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/NEWS b/NEWS
index 21ed3435b..43ce84d7e 100644
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,13 @@ GNU coreutils NEWS-*- outline -*-
   have exited with a "Function not implemented" error.
   [bug introduced in coreutils-8.28]
 
+** Changes in behavior
+
+  ls's -f option now simply acts like -aU, instead of also ignoring
+  some earlier options.  For example 'ls -fl' and 'ls -lf' are now
+  equivalent because -f no longer ignores an earlier -l.  The new
+  behavior is more orthogonal and is compatible with FreeBSD.
+
 ** Improvements
 
   sort operates more efficiently when used on pseudo files with
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 8f7f43ea0..2c6c8b68b 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -8150,11 +8150,9 @@ sort according to the ctime.  @xref{File timestamps}.
 @opindex -f
 @cindex unsorted directory listing
 @cindex directory order, listing by
-Produce an unsorted directory listing.
-This is like @option{--sort=none} (@option{-U}),
-but also enable @option{--all} (@option{-a}),
-while also disabling any previous use of @option{-l}, @option{--color}
-@option{--size}, or @option{--hyperlink}.
+Produce an unsorted listing of all directory entries.
+This is like @option{--all} (@option{-a}) combined
+with @option{--sort=none} (@option{-U}).
 
 @item -r
 @itemx --reverse
@@ -8224,10 +8222,12 @@ sort according to the birth time.
 @opindex --sort
 @opindex none@r{, sorting option for @command{ls}}
 Do not sort; list the files in whatever order they are
-stored in the directory.  (Do not do any of the other unrelated things
-that @option{-f} does.)  This can be useful when listing large
+stored in the directory.  This can be useful when listing large
 directories, where sorting can take some time.
 
+Unlike @option{-f}, this option does not imply @option{--all}
+(@option{-a}).
+
 @item -v
 @itemx --sort=version
 @opindex -v
diff --git a/src/ls.c b/src/ls.c
index f5ac98df7..916d7c050 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -1952,11 +1952,6 @@ decode_switches (int argc, char **argv)
 case 'f':
   ignore_mode = IGNORE_MINIMAL; /* enable -a */
   sort_opt = sort_none; /* enable -U */
-  if (format_opt == long_format)
-format_opt = -1;/* disable -l */
-  print_with_color = false; /* disable --color */
-  print_hyperlink = false;  /* disable --hyperlink */
-  print_block_size = false; /* disable -s */
   break;
 
 case FILE_TYPE_INDICATOR_OPTION: /* --file-type */
@@ -5466,7 +5461,7 @@ Sort entries alphabetically if none of -cftuvSUX nor --sort is specified.\n\
   -D, --diredgenerate output designed for Emacs' dired mode\n\
 "), stdout);
   fputs (_("\
-  -f do not sort, enable -aU, disable -ls --color\n\
+  -f same as -a -U\n\
   -F, --classify[=WHEN]  append indicator (one of */=>@|) to entries WHEN\n\
   --file-typelikewise, except do not append '*'\n\
 "), stdout);
-- 
2.44.0

bug#70411: [bug] install(1) fails to read /dev/stdin on Darwin

[Sergei Trofimovich]

On Tue, 16 Apr 2024 12:33:44 +0100
Pádraig Brady  wrote:

> On 16/04/2024 01:19, Alejandro Colomar wrote:
> > Hi!
> > 
> > I don't own a Darwin system, so I can't help much reproduce.  However,
> > I've received a bug report to the Linux man-pages, that our build
> > system (GNUmakefile-based), which ends up calling
> > 
> > ... | install /dev/stdin $@
> > 
> > doesn't work on Darwin.  Here's the original bug report:
> > .
> > 
> > Here are the reported error messages:
> > 
> > ...
> > INSTALL 
> > /nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/addseverity.3
> > INSTALL 
> > /nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/adjtime.3
> > install: skipping file '/dev/stdin', as it was replaced while being copied
> > make: *** [share/mk/install/man.mk:54: 
> > /nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/addmntent.3]
> >  Error 1
> > make: *** Waiting for unfinished jobs
> > install: skipping file '/dev/stdin', as it was replaced while being copied
> > make: *** [share/mk/install/man.mk:54: 
> > /nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/acosh.3]
> >  Error 1
> > install: skipping file '/dev/stdin', as it was replaced while being copied
> > install: skipping file '/dev/stdin', as it was replaced while being copied
> > install: skipping file '/dev/stdin', as it was replaced while being copied
> > ...
> > 
> > I don't see why install(1) should fail to read /dev/stdin under any
> > POSIX system  
> 
> What version of darwin is this? I can't repro on Darwin 21.6.0 (MacOSX 12.6).
> The issue seems to be that /dev/stdin returns a varying inode which 
> install(1) doesn't like currently

The system I have locally is this one:

$ uname -a
Darwin tests-iMac.local 18.7.0 Darwin Kernel Version 18.7.0: Tue Jun 22 
19:37:08 PDT 2021; root:xnu-4903.278.70~1/RELEASE_X86_64 x86_64

-- 

  Sergei

bug#70418: ls bug

[Toby Kelsey]

The man page doesn't explain this format conflict, while the info page (info '(coreutils) ls invocation' or 'info ls') 
claims '-f' implies '-1' which is also incorrect: 'ls -1f' gives different output to to 'ls -f'.


I am using coreutils 9.4-3 in Manjaro and the online documentation for 9.5 
 seems to to be only document 
consistent with actual 9.4 behavior ('-f'  implies '-aU' and disables '-l'), Technically then these are errors in the 
man and info pages, but I think it is misleading to have what is described as a 'sort option'  determining output 
format. If the option was described correctly in the man and info documents installed with the software that would be 
some improvement. Hopefully 9.5 fixes these bugs.


Toby

On 16/04/2024 16:53, Chris Elvidge wrote:



On 16/04/2024 at 16:07, Toby Kelsey wrote:
There appears to be a bug with 'ls' when using the '-f' and '-l' options, or at least an inconsistency with the man 
page. The man page says '-f' list entries in directory order and does not mention any incompatibilities with other 
options, however I find using it with the '-l' option gives different ouputs depending on the order of options. See 
the following script log for an example:




It really says -f = do not sort, enable -aU. -U = do not sort

It also says -f = disable -ls --color

So -l -f = -f takes precedence, -f -l = -l takes precedence.

bug#70411: [bug] install(1) fails to read /dev/stdin on Darwin

[Paul Eggert]

On 4/16/24 12:44, Pádraig Brady wrote:

A related suggestion was from Marc Chantreux (CC'd)
to support '-' to imply stdin, which would be more portable.
There is some merit to that suggestion too.


I see that merit too, as when 'install' reads from stdin it needn't do 
the inode check. However, POSIX requires 'cp - X' to copy a file named 
'-' to X; cp cannot copy from stdin. Although POSIX doesn't specify 
'install', there is some value to having 'install' behave like 'cp'.


I suppose we could have install special-case "/dev/stdin", on platforms 
where /dev/stdin is known to mean standard input. Seems like a bit much 
to work around a Darwin kernel bug, though.

bug#70411: [bug] install(1) fails to read /dev/stdin on Darwin

[Alejandro Colomar]

Hi,

On Tue, Apr 16, 2024 at 08:44:06PM +0100, Pádraig Brady wrote:
> I agree if it's older Darwin only, we can ignore.
> The version I tested on is 3 years old now though,
> so I'm not sure whether the issue is on newer or older.
> 
> Note we had similar issue on Solaris,
> where we used an fstat() wrapper to adjust things:
> https://bugs.gnu.org/35713
> 
> A related suggestion was from Marc Chantreux (CC'd)
> to support '-' to imply stdin, which would be more portable.
> There is some merit to that suggestion too.

I think /dev/stdin is a good name for stdin.  It should work everywhere,
and if not the system is broken, in which case, I'd rather fix the
system than adding a slightly different special case for - in every
program.  Of course there are systems one can't fix, like Solaris...

Commands that special-case - I always need to remember, and check the
docs.  But it's a personal opinion, and I don't feel too strongly about
it.

Have a lovely night!
Alex

-- 



signature.asc
Description: PGP signature

bug#70411: [bug] install(1) fails to read /dev/stdin on Darwin

[Paul Eggert]

On 4/16/24 07:47, Alejandro Colomar wrote:

Since you couldn't reprodude it in a recent Darwin, maybe it's
just a bug in an old Darwin.


It'd have to be pretty old. As near as I can see from 
xnu/bsd/kern/sys_pipe.c, the st_ino field was zero (i.e., not random) 
even in xnu-792 dated 2005.


I'd be leery of changing this part of coreutils/src/copy.c without 
knowing more about exactly why bug occurs on (older?) Darwin, as 
weakening the check would make GNU 'install' easier to attack.

bug#70411: [bug] install(1) fails to read /dev/stdin on Darwin

[Pádraig Brady]

On 16/04/2024 15:47, Alejandro Colomar wrote:

Hi Pádraig,

On Tue, Apr 16, 2024 at 03:25:22PM +0100, Pádraig Brady wrote:

What version of darwin is this? I can't repro on Darwin 21.6.0 (MacOSX 12.6).
The issue seems to be that /dev/stdin returns a varying inode which install(1) 
doesn't like currently


I don't know.  The reporter didn't tell.  I see you also asked on the
Github original report.


Note that check originally came from:
https://github.com/coreutils/coreutils/commit/dea4262fa

I suppose we could relax the check as follows, for files of apparent size 0
which would cater for this, and others that may also have unstable inodes.


H.  Since you couldn't reprodude it in a recent Darwin, maybe it's
just a bug in an old Darwin.  And since noone else seems to have met
this Darwin's bug, maybe we can just ignore it.  (And if it were a
regression in a more recent Darwin, hopefully they should fix their
kernel.)

I'm not happy relaxing a security check, without making sure that there
are no implications at all.

I vote for claiming only limited support to such a Darwin system.  I
already workarounded it in the Linux man-pages, by not piping to
install(1) in a common task; and nobody else seems to be affected.

Unless you feel confident that it's perfectly fine to do it.  But I have
no sympathy for workarounding Darwin bugs here.


I agree if it's older Darwin only, we can ignore.
The version I tested on is 3 years old now though,
so I'm not sure whether the issue is on newer or older.

Note we had similar issue on Solaris,
where we used an fstat() wrapper to adjust things:
https://bugs.gnu.org/35713

A related suggestion was from Marc Chantreux (CC'd)
to support '-' to imply stdin, which would be more portable.
There is some merit to that suggestion too.

cheers,
Pádraig

bug#70418: ls bug

[Chris Elvidge]

On 16/04/2024 at 16:07, Toby Kelsey wrote:
There appears to be a bug with 'ls' when using the '-f' and '-l' 
options, or at least an inconsistency with the man page. The man page 
says '-f' list entries in directory order and does not mention any 
incompatibilities with other options, however I find using it with the 
'-l' option gives different ouputs depending on the order of options. 
See the following script log for an example:




It really says -f = do not sort, enable -aU. -U = do not sort

It also says -f = disable -ls --color

So -l -f = -f takes precedence, -f -l = -l takes precedence.

--

Chris Elvidge

bug#70418: ls bug

[Toby Kelsey]

There appears to be a bug with 'ls' when using the '-f' and '-l' options, or at least an inconsistency with the man 
page. The man page says '-f' list entries in directory order and does not mention any incompatibilities with other 
options, however I find using it with the '-l' option gives different ouputs depending on the order of options. See the 
following script log for an example:



toby@toby-pc:/tmp$ mkdir test
toby@toby-pc:/tmp$ cd test
toby@toby-pc:/tmp/test$ touch c b a foo bar
toby@toby-pc:/tmp/test$ ls -lf
.  ..  c  b  a  foo  bar
toby@toby-pc:/tmp/test$ ls -fl
total 0
drwxr-xr-x  2 toby toby 140 Apr 16 15:50 .
drwxrwxrwt 18 root root 560 Apr 16 15:50 ..
-rw-r--r--  1 toby toby   0 Apr 16 15:50 c
-rw-r--r--  1 toby toby   0 Apr 16 15:50 b
-rw-r--r--  1 toby toby   0 Apr 16 15:50 a
-rw-r--r--  1 toby toby   0 Apr 16 15:50 foo
-rw-r--r--  1 toby toby   0 Apr 16 15:50 bar
toby@toby-pc:/tmp/test$ ls -l -f
.  ..  c  b  a  foo  bar
toby@toby-pc:/tmp/test$ ls -f -l
total 0
drwxr-xr-x  2 toby toby 140 Apr 16 15:50 .
drwxrwxrwt 18 root root 600 Apr 16 15:57 ..
-rw-r--r--  1 toby toby   0 Apr 16 15:50 c
-rw-r--r--  1 toby toby   0 Apr 16 15:50 b
-rw-r--r--  1 toby toby   0 Apr 16 15:50 a
-rw-r--r--  1 toby toby   0 Apr 16 15:50 foo
-rw-r--r--  1 toby toby   0 Apr 16 15:50 bar
toby@toby-pc:/tmp/test$ type ls
ls is /usr/bin/ls

toby@toby-pc:/tmp/test$ ls --version
ls (GNU coreutils) 9.4

Copyright (C) 2023 Free Software Foundation, Inc.

License GPLv3+: GNU GPL version 3 or later .

This is free software: you are free to change and redistribute it.

There is NO WARRANTY, to the extent permitted by law.



Written by Richard M. Stallman and David MacKenzie.


Regards,

Toby

bug#70411: [bug] install(1) fails to read /dev/stdin on Darwin

[Alejandro Colomar]

Hi Pádraig,

On Tue, Apr 16, 2024 at 03:25:22PM +0100, Pádraig Brady wrote:
> > What version of darwin is this? I can't repro on Darwin 21.6.0 (MacOSX 
> > 12.6).
> > The issue seems to be that /dev/stdin returns a varying inode which 
> > install(1) doesn't like currently

I don't know.  The reporter didn't tell.  I see you also asked on the
Github original report.

> Note that check originally came from:
> https://github.com/coreutils/coreutils/commit/dea4262fa
> 
> I suppose we could relax the check as follows, for files of apparent size 0
> which would cater for this, and others that may also have unstable inodes.

H.  Since you couldn't reprodude it in a recent Darwin, maybe it's
just a bug in an old Darwin.  And since noone else seems to have met
this Darwin's bug, maybe we can just ignore it.  (And if it were a
regression in a more recent Darwin, hopefully they should fix their
kernel.)

I'm not happy relaxing a security check, without making sure that there
are no implications at all.

I vote for claiming only limited support to such a Darwin system.  I
already workarounded it in the Linux man-pages, by not piping to
install(1) in a common task; and nobody else seems to be affected.

Unless you feel confident that it's perfectly fine to do it.  But I have
no sympathy for workarounding Darwin bugs here.

Have a lovely day!
Alex

> 
> cheers,
> Pádraig.
> 
> diff --git a/src/copy.c b/src/copy.c
> index 2145d89d5..fb5f0a1a0 100644
> --- a/src/copy.c
> +++ b/src/copy.c
> @@ -1263,8 +1263,11 @@ copy_reg (char const *src_name, char const *dst_name,
>  }
> 
>/* Compare the source dev/ino from the open file to the incoming,
> - saved ones obtained via a previous call to stat.  */
> -  if (! psame_inode (src_sb, _open_sb))
> + saved ones obtained via a previous call to stat.  Restrict
> + the check to files with an apparent size, to support "files"
> + with unstable inodes, like /dev/stdin on macOS.  */
> +  if (! psame_inode (src_sb, _open_sb)
> +  && (src_sb->st_size || src_open_sb.st_size))
>  {
>error (0, 0,
>   _("skipping file %s, as it was replaced while being copied"),
> 
> 

-- 



signature.asc
Description: PGP signature

bug#70411: [bug] install(1) fails to read /dev/stdin on Darwin

[Pádraig Brady]

On 16/04/2024 12:33, Pádraig Brady wrote:

On 16/04/2024 01:19, Alejandro Colomar wrote:

Hi!

I don't own a Darwin system, so I can't help much reproduce.  However,
I've received a bug report to the Linux man-pages, that our build
system (GNUmakefile-based), which ends up calling

... | install /dev/stdin $@

doesn't work on Darwin.  Here's the original bug report:
.

Here are the reported error messages:

...
INSTALL 
/nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/addseverity.3
INSTALL 
/nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/adjtime.3
install: skipping file '/dev/stdin', as it was replaced while being copied
make: *** [share/mk/install/man.mk:54: 
/nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/addmntent.3]
 Error 1
make: *** Waiting for unfinished jobs
install: skipping file '/dev/stdin', as it was replaced while being copied
make: *** [share/mk/install/man.mk:54: 
/nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/acosh.3]
 Error 1
install: skipping file '/dev/stdin', as it was replaced while being copied
install: skipping file '/dev/stdin', as it was replaced while being copied
install: skipping file '/dev/stdin', as it was replaced while being copied
...

I don't see why install(1) should fail to read /dev/stdin under any
POSIX system


What version of darwin is this? I can't repro on Darwin 21.6.0 (MacOSX 12.6).
The issue seems to be that /dev/stdin returns a varying inode which install(1) 
doesn't like currently


Note that check originally came from:
https://github.com/coreutils/coreutils/commit/dea4262fa

I suppose we could relax the check as follows, for files of apparent size 0
which would cater for this, and others that may also have unstable inodes.

cheers,
Pádraig.

diff --git a/src/copy.c b/src/copy.c
index 2145d89d5..fb5f0a1a0 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1263,8 +1263,11 @@ copy_reg (char const *src_name, char const *dst_name,
 }

   /* Compare the source dev/ino from the open file to the incoming,
- saved ones obtained via a previous call to stat.  */
-  if (! psame_inode (src_sb, _open_sb))
+ saved ones obtained via a previous call to stat.  Restrict
+ the check to files with an apparent size, to support "files"
+ with unstable inodes, like /dev/stdin on macOS.  */
+  if (! psame_inode (src_sb, _open_sb)
+  && (src_sb->st_size || src_open_sb.st_size))
 {
   error (0, 0,
  _("skipping file %s, as it was replaced while being copied"),

bug#70411: [bug] install(1) fails to read /dev/stdin on Darwin

[Pádraig Brady]

On 16/04/2024 01:19, Alejandro Colomar wrote:

Hi!

I don't own a Darwin system, so I can't help much reproduce.  However,
I've received a bug report to the Linux man-pages, that our build
system (GNUmakefile-based), which ends up calling

... | install /dev/stdin $@

doesn't work on Darwin.  Here's the original bug report:
.

Here are the reported error messages:

...
INSTALL 
/nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/addseverity.3
INSTALL 
/nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/adjtime.3
install: skipping file '/dev/stdin', as it was replaced while being copied
make: *** [share/mk/install/man.mk:54: 
/nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/addmntent.3]
 Error 1
make: *** Waiting for unfinished jobs
install: skipping file '/dev/stdin', as it was replaced while being copied
make: *** [share/mk/install/man.mk:54: 
/nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/acosh.3]
 Error 1
install: skipping file '/dev/stdin', as it was replaced while being copied
install: skipping file '/dev/stdin', as it was replaced while being copied
install: skipping file '/dev/stdin', as it was replaced while being copied
...

I don't see why install(1) should fail to read /dev/stdin under any
POSIX system


What version of darwin is this? I can't repro on Darwin 21.6.0 (MacOSX 12.6).
The issue seems to be that /dev/stdin returns a varying inode which install(1) 
doesn't like currently

cheers,
Pádraig

bug#70411: [bug] install(1) fails to read /dev/stdin on Darwin

[Alejandro Colomar]

Hi!

I don't own a Darwin system, so I can't help much reproduce.  However,
I've received a bug report to the Linux man-pages, that our build
system (GNUmakefile-based), which ends up calling

... | install /dev/stdin $@

doesn't work on Darwin.  Here's the original bug report:
.

Here are the reported error messages:

...
INSTALL 
/nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/addseverity.3
INSTALL 
/nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/adjtime.3
install: skipping file '/dev/stdin', as it was replaced while being copied
make: *** [share/mk/install/man.mk:54: 
/nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/addmntent.3]
 Error 1
make: *** Waiting for unfinished jobs
install: skipping file '/dev/stdin', as it was replaced while being copied
make: *** [share/mk/install/man.mk:54: 
/nix/store/3s28l9ijlkmsq8256zdxjvl173gkn37c-man-pages-6.7/share/man/man3/acosh.3]
 Error 1
install: skipping file '/dev/stdin', as it was replaced while being copied
install: skipping file '/dev/stdin', as it was replaced while being copied
install: skipping file '/dev/stdin', as it was replaced while being copied
...

I don't see why install(1) should fail to read /dev/stdin under any
POSIX system.

Reported-by: Mario Rodas 
Reported-by: Sergei Trofimovich 
Reported-by: Alejandro Colomar 
Cc: Eli Schwartz 

Have a lovely day!
Alex

-- 



signature.asc
Description: PGP signature

bug#70214: 'install' fails to copy regular file to autofs/cifs, due to ACL or xattr handling

[Pádraig Brady]

On 15/04/2024 15:37, Andreas Grünbacher wrote:

Hello,

Am So., 14. Apr. 2024 um 00:43 Uhr schrieb Pádraig Brady :

On 13/04/2024 20:29, Bruno Haible wrote:

Hi Pádraig,

I wrote:

5) The same thing with 'cp -a' succeeds:

$ build-sparc64/src/cp -a /var/tmp/foo3941 $HOME/foo3941; echo $?
0
$ build-sparc64-no-acl/src/cp -a /var/tmp/foo3941 $HOME/foo3941; echo $?
0


You wrote:

The psuedo code that install(1) uses is:

copy_reg()
 if (x->set_mode) /* install */
   set_acl(dest, x->mode /* 600 */)
 ctx->acl = acl_from_mode ( /* 600 */)
 acl_set_fd (ctx->acl) /* fails EACCES */
 if (! acls_set)
must_chmod = true;
 if (must_chmod)
   saved_errno = EACCES;
   chmod (ctx->mode /* 600 */)
   if (save_errno)
 return -1;


And, for comparison, what is the pseudo-code that 'cp -a' uses?
I would guess that there must be a relevant difference between both.


The cp pseudo code is:

copy_reg()
if (preserve_xattr)
  copy_attr()
ret = attr_copy_fd()
if (ret == -1 && require_preserve_xattr /*false*/)
  return failure;
if (preserve_mode)
  copy_acl()
qcopy_acl()
  #if USE_XATTR /* true */
fchmod() /* chmod before setting ACLs as doing after may reset */
return attr_copy_fd() /* successful if no ACLs in source */
  #endif

If however you add ACLs in the source, you induce a similar failure:

$ setfacl -m u:nobody:r /var/tmp/foo3942
$ src/cp -a /var/tmp/foo3942 foo3942; echo $?
src/cp: preserving permissions for ‘foo3942’: Permission denied
1

The corresponding strace is:

fchmod(4, 0100640)  = 0
flistxattr(3, NULL, 0)  = 24
flistxattr(3, "system.posix_acl_access\0", 24) = 24
fgetxattr(3, "system.posix_acl_access", NULL, 0) = 44
fgetxattr(3, "system.posix_acl_access", "\2\0...\4", 44) = 44
fsetxattr(4, "system.posix_acl_access", "\2\0...\4", 44, 0) = -1 EACCES 
(Permission denied)


Why does CIFS think EACCES is an appropriate error to return here? The
fchmod() succeeds, so changing the file permissions via fsetxattr()
should really succeed as well.


Right, it seems like a CIFS bug (already CC'd)
Even if it returned ENOTSUP (like getxattr(...posix...) does) it would be ok as 
we handle that.
It would be good to avoid it though.

You confirmed privately to me that the set_acl() is to clear any default ACLs
that may have been added to the newly created file, so the posted solution in
https://bugs.gnu.org/70214#11 that only does the set_acl() iff file_has_acl()
should avoid the CIFS issue.  It would be a bit less efficient if there were
ACLs, but a bit more efficient in the common case where there weren't ACLs.

cheers,
Pádraig

bug#70214: 'install' fails to copy regular file to autofs/cifs, due to ACL or xattr handling

[Andreas Grünbacher]

Hello,

Am So., 14. Apr. 2024 um 00:43 Uhr schrieb Pádraig Brady :
> On 13/04/2024 20:29, Bruno Haible wrote:
> > Hi Pádraig,
> >
> > I wrote:
> >>> 5) The same thing with 'cp -a' succeeds:
> >>>
> >>> $ build-sparc64/src/cp -a /var/tmp/foo3941 $HOME/foo3941; echo $?
> >>> 0
> >>> $ build-sparc64-no-acl/src/cp -a /var/tmp/foo3941 $HOME/foo3941; echo $?
> >>> 0
> >
> > You wrote:
> >> The psuedo code that install(1) uses is:
> >>
> >> copy_reg()
> >> if (x->set_mode) /* install */
> >>   set_acl(dest, x->mode /* 600 */)
> >> ctx->acl = acl_from_mode ( /* 600 */)
> >> acl_set_fd (ctx->acl) /* fails EACCES */
> >> if (! acls_set)
> >>must_chmod = true;
> >> if (must_chmod)
> >>   saved_errno = EACCES;
> >>   chmod (ctx->mode /* 600 */)
> >>   if (save_errno)
> >> return -1;
> >
> > And, for comparison, what is the pseudo-code that 'cp -a' uses?
> > I would guess that there must be a relevant difference between both.
>
> The cp pseudo code is:
>
> copy_reg()
>if (preserve_xattr)
>  copy_attr()
>ret = attr_copy_fd()
>if (ret == -1 && require_preserve_xattr /*false*/)
>  return failure;
>if (preserve_mode)
>  copy_acl()
>qcopy_acl()
>  #if USE_XATTR /* true */
>fchmod() /* chmod before setting ACLs as doing after may reset */
>return attr_copy_fd() /* successful if no ACLs in source */
>  #endif
>
> If however you add ACLs in the source, you induce a similar failure:
>
> $ setfacl -m u:nobody:r /var/tmp/foo3942
> $ src/cp -a /var/tmp/foo3942 foo3942; echo $?
> src/cp: preserving permissions for ‘foo3942’: Permission denied
> 1
>
> The corresponding strace is:
>
> fchmod(4, 0100640)  = 0
> flistxattr(3, NULL, 0)  = 24
> flistxattr(3, "system.posix_acl_access\0", 24) = 24
> fgetxattr(3, "system.posix_acl_access", NULL, 0) = 44
> fgetxattr(3, "system.posix_acl_access", "\2\0...\4", 44) = 44
> fsetxattr(4, "system.posix_acl_access", "\2\0...\4", 44, 0) = -1 EACCES 
> (Permission denied)

Why does CIFS think EACCES is an appropriate error to return here? The
fchmod() succeeds, so changing the file permissions via fsetxattr()
should really succeed as well.

Thanks,
Andreas

bug#70214: 'install' fails to copy regular file to autofs/cifs, due to ACL or xattr handling

[Pádraig Brady]

On 13/04/2024 20:29, Bruno Haible wrote:

Hi Pádraig,

I wrote:

5) The same thing with 'cp -a' succeeds:

$ build-sparc64/src/cp -a /var/tmp/foo3941 $HOME/foo3941; echo $?
0
$ build-sparc64-no-acl/src/cp -a /var/tmp/foo3941 $HOME/foo3941; echo $?
0


You wrote:

The psuedo code that install(1) uses is:

copy_reg()
if (x->set_mode) /* install */
  set_acl(dest, x->mode /* 600 */)
ctx->acl = acl_from_mode ( /* 600 */)
acl_set_fd (ctx->acl) /* fails EACCES */
if (! acls_set)
   must_chmod = true;
if (must_chmod)
  saved_errno = EACCES;
  chmod (ctx->mode /* 600 */)
  if (save_errno)
return -1;


And, for comparison, what is the pseudo-code that 'cp -a' uses?
I would guess that there must be a relevant difference between both.


The cp pseudo code is:

copy_reg()
  if (preserve_xattr)
copy_attr()
  ret = attr_copy_fd()
  if (ret == -1 && require_preserve_xattr /*false*/)
return failure;
  if (preserve_mode)
copy_acl()
  qcopy_acl()
#if USE_XATTR /* true */
  fchmod() /* chmod before setting ACLs as doing after may reset */
  return attr_copy_fd() /* successful if no ACLs in source */
#endif

If however you add ACLs in the source, you induce a similar failure:

$ setfacl -m u:nobody:r /var/tmp/foo3942
$ src/cp -a /var/tmp/foo3942 foo3942; echo $?
src/cp: preserving permissions for ‘foo3942’: Permission denied
1

The corresponding strace is:

fchmod(4, 0100640)  = 0
flistxattr(3, NULL, 0)  = 24
flistxattr(3, "system.posix_acl_access\0", 24) = 24
fgetxattr(3, "system.posix_acl_access", NULL, 0) = 44
fgetxattr(3, "system.posix_acl_access", "\2\0...\4", 44) = 44
fsetxattr(4, "system.posix_acl_access", "\2\0...\4", 44, 0) = -1 EACCES 
(Permission denied)

BTW I was wondering about the need for install(1) to set_acl() at all,
rather than just using chmod.
The following comment in lib/set-permissions.c may be pertinent:

/* If we can't set an acl which we expect to be able to set, try setting
   the permissions to ctx->mode. Due to possible inherited permissions,
   we cannot simply chmod */

BTW this is all under kernel version:

$ uname -r
6.8.5-gentoo-sparc64

With these cifs options:

$ mount | grep cifs
//syslog.matoro.tk/guest-pixelbeat on /media/guest-homedirs/pixelbeat type cifs
(rw,nosuid,relatime,vers=1.0,cache=strict,username=nobody,uid=30017,forceuid,
gid=30017,forcegid,addr=fd05:::::::0001,
soft,unix,posixpaths,serverino,mapposix,acl,
rsize=1048576,wsize=65536,bsize=1048576,retrans=1,echo_interval=60,actimeo=1,closetimeo=1)

cheers,
Pádraig

bug#70214: 'install' fails to copy regular file to autofs/cifs, due to ACL or xattr handling

[Bruno Haible]

Hi Pádraig,

I wrote:
> > 5) The same thing with 'cp -a' succeeds:
> > 
> > $ build-sparc64/src/cp -a /var/tmp/foo3941 $HOME/foo3941; echo $?
> > 0
> > $ build-sparc64-no-acl/src/cp -a /var/tmp/foo3941 $HOME/foo3941; echo $?
> > 0

You wrote:
> The psuedo code that install(1) uses is:
> 
> copy_reg()
>if (x->set_mode) /* install */
>  set_acl(dest, x->mode /* 600 */)
>ctx->acl = acl_from_mode ( /* 600 */)
>acl_set_fd (ctx->acl) /* fails EACCES */
>if (! acls_set)
>   must_chmod = true;
>if (must_chmod)
>  saved_errno = EACCES;
>  chmod (ctx->mode /* 600 */)
>  if (save_errno)
>return -1;

And, for comparison, what is the pseudo-code that 'cp -a' uses?
I would guess that there must be a relevant difference between both.

Bruno

bug#70214: 'install' fails to copy regular file to autofs/cifs, due to ACL or xattr handling

[Pádraig Brady]

On 13/04/2024 17:39, Pádraig Brady wrote:

install(1) defaults to mode 600 for new files, and uses set_acl() with that
(since 2007 https://github.com/coreutils/coreutils/commit/f634e8844 )
The psuedo code that install(1) uses is:

copy_reg()
if (x->set_mode) /* install */
  set_acl(dest, x->mode /* 600 */)
ctx->acl = acl_from_mode ( /* 600 */)
acl_set_fd (ctx->acl) /* fails EACCES */
if (! acls_set)
   must_chmod = true;
if (must_chmod)
  saved_errno = EACCES;
  chmod (ctx->mode /* 600 */)
  if (save_errno)
return -1;

This issue only only seems to be on CIFS.
I'm seeing lot of weird behavior with ACLs there:

acl_set_fd (acl_from_mode (600)) -> EACCES
acl_set_fd (acl_from_mode (755)) -> EINVAL
getxattr ("system.posix_acl_access") -> EOPNOTSUPP

Note we ignore EINVAL and EOPNOTSUPP errors in set_acl(),
and it's just the EACCES that's problematic.
Note this is quite similar to https://debbugs.gnu.org/65599
where Paul also noticed EACCES with fsetxattr() (and others) on CIFS.

The attached is a potential solution which I tested as working
on the same matoro system that Bruno used.

I think I'll apply that after thinking a bit more about it.


Actually that probably isn't appropriate,
as fsetxattr() can validly return EACESS
even though not documented in the man page at least.
The following demonstrates that:
.
#include 
#include 
#include 
#include 
#include 
#include 
#include 

int main(void)
{
int wfd;
/* Note S_IWUSR is not set.  */
if ((wfd=open("writable", O_CREAT|O_WRONLY|O_EXCL, S_IRUSR)) == -1)
fprintf(stderr, "open('writable') error [%m]\n");
if (write(wfd, "data", 1) == -1)
fprintf(stderr, "write() error [%m]\n");
if (fsetxattr(wfd, "user.test", "test", 4, 0) == -1)
fprintf(stderr, "fsetxattr() error [%m]\n");
}

Another solution might be to file_has_acl() in copy.c
and skip if "not supported" or nothing is returned.
The following would do that, but I'm not sure about this
(as I'm not sure about ACLs in general TBH).
Note listxattr() returns 0 on CIFS here,
while getxattr ("system.posix_acl_access") returns EOPNOTSUPP,
and file_has_acl() uses listxattr() first.

diff --git a/src/copy.c b/src/copy.c
index d584a27eb..2145d89d5 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1673,8 +1673,13 @@ set_dest_mode:
 }
   else if (x->set_mode)
 {
-  if (set_acl (dst_name, dest_desc, x->mode) != 0)
-return_val = false;
+  errno = 0;
+  int n = file_has_acl (dst_name, );
+  if (0 < n || (errno && ! (is_ENOTSUP (errno) || errno == ENOSYS)))
+{
+  if (set_acl (dst_name, dest_desc, x->mode) != 0)
+return_val = false;
+}
 }


BTW I'm surprised this wasn't reported previously for CIFS,
so I due to this bug and https://debbugs.gnu.org/65599
I suspect a recentish change in CIFS wrt EACCES.

cheers,
Pádraig

bug#70214: 'install' fails to copy regular file to autofs/cifs, due to ACL or xattr handling

[Pádraig Brady]

On 05/04/2024 10:48, Bruno Haible wrote:

Hi,

The 'install' program from coreutils-9.5 fails to copy a regular file
from an ext4 mount to an autofs/cifs mount.

The same operation, with 'cp -a', works fine.

Also, it works fine when coreutils was built with the configure options
"--disable-acl --disable-xattr".

How to reproduce


1) On the machine sparcdev.matoro.tk (Linux 6.8.2), I built coreutils-9.5
from source,
   - once with default options, in build-sparc64/,
   - once with "--disable-acl --disable-xattr", in build-sparc64-no-acl/.

2) Create a regular file on an ext4 mount:

$ echo hi > /var/tmp/foo3941
$ ls -lZ /var/tmp/foo3941
-rw-r- 1 g-haible g-haible ? 3 Apr  4 13:29 /var/tmp/foo3941
$ getfacl /var/tmp/foo3941
getfacl: Removing leading '/' from absolute path names
# file: var/tmp/foo3941
# owner: g-haible
# group: g-haible
user::rw-
group::r--
other::---
$ df -m /var/tmp/
Filesystem 1M-blocks   Used Available Use% Mounted on
/dev/root 560245 123140408574  24% /
$ mount | grep ' / '
/dev/sda2 on / type ext4 (rw,noatime)

3) Details about the destination directory:

$ echo $HOME
/media/guest-homedirs/haible
$ mount | grep /media/guest-homedirs/haible
/etc/autofs/auto.guest-homedirs on /media/guest-homedirs/haible type autofs 
(rw,relatime,fd=7,pgrp=2325,timeout=60,minproto=5,maxproto=5,direct,pipe_ino=46092)
//syslog.matoro.tk/guest-haible on /media/guest-homedirs/haible type cifs 
(rw,nosuid,relatime,vers=1.0,cache=strict,username=nobody,uid=30014,forceuid,gid=30014,forcegid,addr=fd05:::::::0001,soft,unix,posixpaths,serverino,mapposix,acl,rsize=1048576,wsize=65536,bsize=1048576,retrans=1,echo_interval=60,actimeo=1,closetimeo=1)

4) The operation that fails:

$ build-sparc64/src/ginstall -c /var/tmp/foo3941 $HOME/foo3941; echo $?
build-sparc64/src/ginstall: setting permissions for 
'/media/guest-homedirs/haible/foo3941': Permission denied
1
$ build-sparc64-no-acl/src/ginstall -c /var/tmp/foo3941 $HOME/foo3941; echo $?
0

5) The same thing with 'cp -a' succeeds:

$ build-sparc64/src/cp -a /var/tmp/foo3941 $HOME/foo3941; echo $?
0
$ build-sparc64-no-acl/src/cp -a /var/tmp/foo3941 $HOME/foo3941; echo $?
0

6) 'strace' shows a failing call to fsetxattr:

$ strace build-sparc64/src/ginstall -c /var/tmp/foo3941 $HOME/foo3941



fsetxattr(4, "system.posix_acl_access", 
"\2\0\0\0\1\0\6\0\377\377\377\377\4\0\0\0\377\377\377\377 \0\0\0\377\377\377\377", 28, 0) 
= -1 EACCES (Permission denied)
fchmod(4, 0600) = 0



Notes
=

The 'cp' program does *not* use fsetxattr() calls on the destination file
descriptor and therefore does not fail:

$ strace build-sparc64/src/cp -a /var/tmp/foo3941 $HOME/foo3941



flistxattr(3, NULL, 0)  = 0
flistxattr(3, 0x7feff9860a0, 0) = 0
fchmod(4, 0100640)  = 0
flistxattr(3, NULL, 0)  = 0
flistxattr(3, 0x7feff9860c0, 0) = 0



As you can see, it uses 4 flistxattr() calls on the source file descriptor,
apparently detecting that it's a regular file without ACLs, and proceeds to
do a simple fchmod() call on the destination file descriptor.

Probably the same logic is needed in the 'install' program.


install(1) defaults to mode 600 for new files, and uses set_acl() with that
(since 2007 https://github.com/coreutils/coreutils/commit/f634e8844 )
The psuedo code that install(1) uses is:

copy_reg()
  if (x->set_mode) /* install */
set_acl(dest, x->mode /* 600 */)
  ctx->acl = acl_from_mode ( /* 600 */)
  acl_set_fd (ctx->acl) /* fails EACCES */
  if (! acls_set)
 must_chmod = true;
  if (must_chmod)
saved_errno = EACCES;
chmod (ctx->mode /* 600 */)
if (save_errno)
  return -1;

This issue only only seems to be on CIFS.
I'm seeing lot of weird behavior with ACLs there:

  acl_set_fd (acl_from_mode (600)) -> EACCES
  acl_set_fd (acl_from_mode (755)) -> EINVAL
  getxattr ("system.posix_acl_access") -> EOPNOTSUPP

Note we ignore EINVAL and EOPNOTSUPP errors in set_acl(),
and it's just the EACCES that's problematic.
Note this is quite similar to https://debbugs.gnu.org/65599
where Paul also noticed EACCES with fsetxattr() (and others) on CIFS.

The attached is a potential solution which I tested as working
on the same matoro system that Bruno used.

I think I'll apply that after thinking a bit more about it.

cheers,
Pádraig.From d828d9656c3bd1ddf0fcddb578ddb2ed9a4d3701 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= 
Date: Sat, 13 Apr 2024 17:13:02 +0100
Subject: [PATCH] acl-permissions: avoid erroneous failure on CIFS

* lib/set-permissions.c (set_acls): On Linux also discount
EACESS as a valid errno with FD operations, as CIFS was seen to
return that erroneously in some cases.
---
 ChangeLog | 7 +++
 lib/set-permissions.c | 8 +++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 

bug#35247: [PATCH] Add alacritty to terminal list supporting colors

[Pádraig Brady]

Things have changed a little since the original request.
alacritty sets $COLORTERM, and dircolors now auto accepts that since:
https://github.com/coreutils/coreutils/commit/75c9fc674

There are some complications with remote shells, but
they should boil down to setting up ssh to send/accept COLORTERM.

For some background for setting colors based on env vars
from the Fedora side of things at least, see:
https://fedoraproject.org/wiki/Features/256_Color_Terminals
https://fedoraproject.org/wiki/Changes/Drop_256term.sh

cheers,
Pádraig.

bug#70298: uname -i returns unknown since fedora 38

[Bernhard Voelker]

On 4/9/24 2:21 PM, Pádraig Brady wrote:

Thanks for looking at this.
 From the Fedora side, they dropped a Fedora specific patch for Fedora 38.

https://bugzilla.redhat.com/show_bug.cgi?id=548834
https://bugzilla.redhat.com/show_bug.cgi?id=2208048


FWIW: OpenSUSE still has that patch which was kept in sync with Fedora for 
years:

  
https://build.opensuse.org/projects/openSUSE:Factory/packages/coreutils/files/coreutils-sysinfo.patch?expand=1

I have no strong opinion about it, but I guess some (build?) scripts rely on 
this.

Have a nice day,
Berny

bug#35247: [PATCH] Add alacritty to terminal list supporting colors

[Eike Gebauer]

I am not trying to spam this mailing list, but I received several DMARC 
reports after sending the previous mail (never happened to me before, no 
idea why).
So I am trying again from a different domain, in case the email wasn't 
received at all.


Sorry for the inconvenience.
Regards
Eike Gebauer

On 28/03/2024 17:58, Eike Gebauer wrote:
By now alacritty has been packaged for Fedora for quite a while 
(https://packages.fedoraproject.org/pkgs/rust-alacritty/alacritty/) 
and is used by a relatively large community (>50k Stars on Github).


The corresponding issue on the alacritty side has sadly been closed as 
"wontfix" (https://github.com/alacritty/alacritty/issues/2210), so I 
would like to ask you to add `TERM alacritty` to dircolors.hin 
instead, as originally requested by Doron Behar.


Regards,

Eike

PS
I did not find a way to reply to the old thread from the mailing list 
archive with the correct `In-Reply-To` header, so this might not be 
threaded correctly with the original messages from 2019.


On Sat, 11 May 2019 16:57:40 +0300, Doron Behar wrote:
> Well I'm not a developer of alacritty, just a humble user. I had only
> taken the effort to investigate why I dircolors wouldn't define colors
> for me when in this terminal.
>
> Under it's installation section of the README, listed the supported
> packages for the various distros:
>
> https://github.com/jwilm/alacritty#installation
>
> On Fri, May 10, 2019 at 08:24:09AM -0700, Pádraig Brady wrote:On 
28/03/2024 17:34, bug-coreutils-requ...@gnu.org wrote:

> > On 12/04/19 12:20, address@hidden wrote:
> > > From: Doron Behar 
> > >
> > > https://github.com/jwilm/alacritty
> > > ---
> > >  src/dircolors.hin | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/src/dircolors.hin b/src/dircolors.hin
> > > index 3aa445bfa..424ed5b19 100644
> > > --- a/src/dircolors.hin
> > > +++ b/src/dircolors.hin
> > > @@ -12,6 +12,7 @@
> > >  # against the TERM environment variable to determine if it is 
colorizable.

> > >  TERM Eterm
> > >  TERM ansi
> > > +TERM alacritty
> > >  TERM *color*
> > >  TERM con[0-9]*x[0-9]*
> > >  TERM cons25
> > >
> >
> > I don't see this terminal packaged for Feodra at least.
> > Why does it need its own TERM ?
> > If it does, could one add 'color' to get the existing matching?
> >
> > thanks,
> > Pádraig

bug#70298: uname -i returns unknown since fedora 38

[Pádraig Brady]

On 09/04/2024 10:17, Collin Funk wrote:

On 4/9/24 12:57 AM, Paul Eggert wrote:

Indeed there is, and I merged your bug report into that old one. It'd be nice 
if someone could get to the bottom of that bug.


I decided to look into this a bit, since I also have an unknown
'uname -i' and 'uname -p'.

It seems that this option is a Solaris thing. I found the commit that
it was introduced [1]. It also adds some other Solaris compatibility
stuff, so everything seems to add up so far.

The first function it tries is sysinfo (SI_PLATFORM, ...) which seems
to be a Solaris thing [2]. I think Linux has sysinfo but not
SI_PLATFORM.

Then it tries sysctl (...) with a UNAME_HARDWARE_PLATFORM macro to deal
with the BSDs. I think OpenBSD might be missing that definition in the
#ifdef, but I have no way of testing it at the moment [3].

I'm not sure if 'uname -i', 'uname -p', or 'uname -m' are supposed to
be any different from each other. Maybe someone who knows Solaris can
help more?

Illumios defines 'sysinfo' as an alias to 'systeminfo' [4]. There it
returns an 'extern char *platform' in the given buffer when passed
SI_PLATFORM [5]. I've found the definition of the variable, but I
don't know where it is actually set to something useful [6]. Hopefully
that information helps someone...

[1] 
https://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=aaeb7a61c4662fd28cf2bc161b740352711538d2
[2] 
https://github.com/illumos/illumos-gate/blob/cf618897f43ea305e3a426f93bbcef4e8106829c/usr/src/uts/common/sys/systeminfo.h#L86
[3] https://git.savannah.gnu.org/cgit/coreutils.git/tree/src/uname.c#n36
[4] 
https://github.com/illumos/illumos-gate/blob/master/usr/src/lib/libc/common/sys/sysinfo.S
[5] 
https://github.com/illumos/illumos-gate/blob/cf618897f43ea305e3a426f93bbcef4e8106829c/usr/src/uts/common/syscall/systeminfo.c#L124
[6] 
https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/conf/param.c#L533


Thanks for looking at this.
From the Fedora side, they dropped a Fedora specific patch for Fedora 38.

https://bugzilla.redhat.com/show_bug.cgi?id=548834
https://bugzilla.redhat.com/show_bug.cgi?id=2208048

cheers,
Pádraig.

bug#70298: uname -i returns unknown since fedora 38

[Collin Funk]

On 4/9/24 12:57 AM, Paul Eggert wrote:
> Indeed there is, and I merged your bug report into that old one. It'd be nice 
> if someone could get to the bottom of that bug.

I decided to look into this a bit, since I also have an unknown
'uname -i' and 'uname -p'.

It seems that this option is a Solaris thing. I found the commit that
it was introduced [1]. It also adds some other Solaris compatibility
stuff, so everything seems to add up so far.

The first function it tries is sysinfo (SI_PLATFORM, ...) which seems
to be a Solaris thing [2]. I think Linux has sysinfo but not
SI_PLATFORM.

Then it tries sysctl (...) with a UNAME_HARDWARE_PLATFORM macro to deal
with the BSDs. I think OpenBSD might be missing that definition in the
#ifdef, but I have no way of testing it at the moment [3].

I'm not sure if 'uname -i', 'uname -p', or 'uname -m' are supposed to
be any different from each other. Maybe someone who knows Solaris can
help more?

Illumios defines 'sysinfo' as an alias to 'systeminfo' [4]. There it
returns an 'extern char *platform' in the given buffer when passed
SI_PLATFORM [5]. I've found the definition of the variable, but I
don't know where it is actually set to something useful [6]. Hopefully
that information helps someone...

[1] 
https://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=aaeb7a61c4662fd28cf2bc161b740352711538d2
[2] 
https://github.com/illumos/illumos-gate/blob/cf618897f43ea305e3a426f93bbcef4e8106829c/usr/src/uts/common/sys/systeminfo.h#L86
[3] https://git.savannah.gnu.org/cgit/coreutils.git/tree/src/uname.c#n36
[4] 
https://github.com/illumos/illumos-gate/blob/master/usr/src/lib/libc/common/sys/sysinfo.S
[5] 
https://github.com/illumos/illumos-gate/blob/cf618897f43ea305e3a426f93bbcef4e8106829c/usr/src/uts/common/syscall/systeminfo.c#L124
[6] 
https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/conf/param.c#L533

Collin

bug#70298: uname -i returns unknown since fedora 38

[Paul Eggert]

On 2024-04-08 10:59, Ondrej Mejzlik wrote:

The command uname returns "unknown" on Fedora 38,39 and probably 40.
There is a similar very old bug here which has not been fixed
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=34905


Indeed there is, and I merged your bug report into that old one. It'd be 
nice if someone could get to the bottom of that bug.

bug#70298: uname -i returns unknown since fedora 38

[Ondrej Mejzlik]

The command uname returns "unknown" on Fedora 38,39 and probably 40.
There is a similar very old bug here which has not been fixed
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=34905

F38:
[root@vm~]# uname -a
Linux vmcom 6.8.4-100.fc38.x86_64 #1 SMP PREEMPT_DYNAMIC Thu Apr  4
20:40:57 UTC 2024 x86_64 GNU/Linux
[root@vm~]# uname -i
unknown

Here is the strace output on F38
# strace uname -pi
execve("/usr/bin/uname", ["uname", "-pi"], 0x74989c48 /* 32 vars */) = 0
brk(NULL)   = 0x55fae2442000
arch_prctl(0x3001 /* ARCH_??? */, 0x7ffd9202bd60) = -1 EINVAL (Invalid
argument)
access("/etc/ld.so.preload", R_OK)  = -1 ENOENT (No such file or
directory)
openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=18475, ...},
AT_EMPTY_PATH) = 0
mmap(NULL, 18475, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f6491b8f000
close(3)= 0
openat(AT_FDCWD, "/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\1\1\3\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0`}\2\0\0\0\0\0"...,
832) = 832
pread64(3, "\6\0\0\0\4\0\0\0@\0\0\0\0\0\0\0@\0\0\0\0\0\0\0@\0\0\0\0\0\0\0"...,
784, 64) = 784
newfstatat(3, "", {st_mode=S_IFREG|0755, st_size=2448096, ...},
AT_EMPTY_PATH) = 0
mmap(NULL, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x7f6491b8d000
pread64(3, "\6\0\0\0\4\0\0\0@\0\0\0\0\0\0\0@\0\0\0\0\0\0\0@\0\0\0\0\0\0\0"...,
784, 64) = 784
mmap(NULL, 1957168, PROT_READ, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) =
0x7f64919af000
mmap(0x7f64919d5000, 1429504, PROT_READ|PROT_EXEC,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x26000) = 0x7f64919d5000
mmap(0x7f6491b32000, 315392, PROT_READ,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x183000) = 0x7f6491b32000
mmap(0x7f6491b7f000, 24576, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1d) = 0x7f6491b7f000
mmap(0x7f6491b85000, 32048, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f6491b85000
close(3)= 0
mmap(NULL, 12288, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0x7f64919ac000
arch_prctl(ARCH_SET_FS, 0x7f64919ac740) = 0
set_tid_address(0x7f64919aca10) = 1320
set_robust_list(0x7f64919aca20, 24) = 0
rseq(0x7f64919ad060, 0x20, 0, 0x53053053) = 0
mprotect(0x7f6491b7f000, 16384, PROT_READ) = 0
mprotect(0x55fae0ff3000, 4096, PROT_READ) = 0
mprotect(0x7f6491bc6000, 8192, PROT_READ) = 0
prlimit64(0, RLIMIT_STACK, NULL, {rlim_cur=8192*1024,
rlim_max=RLIM64_INFINITY}) = 0
munmap(0x7f6491b8f000, 18475)   = 0
getrandom("\x1b\x52\xa1\xbf\xb0\x7e\x23\x6b", 8, GRND_NONBLOCK) = 8
brk(NULL)   = 0x55fae2442000
brk(0x55fae2463000) = 0x55fae2463000
openat(AT_FDCWD, "/usr/lib/locale/locale-archive", O_RDONLY|O_CLOEXEC) = -1
ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/share/locale/locale.alias", O_RDONLY|O_CLOEXEC) = 3
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=2998, ...}, AT_EMPTY_PATH)
= 0
read(3, "# Locale name alias data base.\n#"..., 4096) = 2998
read(3, "", 4096)   = 0
close(3)= 0
openat(AT_FDCWD, "/usr/lib/locale/en_US.UTF-8/LC_IDENTIFICATION",
O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/lib/locale/en_US.utf8/LC_IDENTIFICATION",
O_RDONLY|O_CLOEXEC) = 3
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=369, ...}, AT_EMPTY_PATH)
= 0
mmap(NULL, 369, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f6491b93000
close(3)= 0
openat(AT_FDCWD, "/usr/lib64/gconv/gconv-modules.cache", O_RDONLY) = 3
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=27012, ...},
AT_EMPTY_PATH) = 0
mmap(NULL, 27012, PROT_READ, MAP_SHARED, 3, 0) = 0x7f64919a5000
close(3)= 0
futex(0x7f6491b84a6c, FUTEX_WAKE_PRIVATE, 2147483647) = 0
openat(AT_FDCWD, "/usr/lib/locale/en_US.UTF-8/LC_MEASUREMENT",
O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/lib/locale/en_US.utf8/LC_MEASUREMENT",
O_RDONLY|O_CLOEXEC) = 3
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=23, ...}, AT_EMPTY_PATH) =
0
mmap(NULL, 23, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f6491b92000
close(3)= 0
openat(AT_FDCWD, "/usr/lib/locale/en_US.UTF-8/LC_TELEPHONE",
O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/lib/locale/en_US.utf8/LC_TELEPHONE",
O_RDONLY|O_CLOEXEC) = 3
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=59, ...}, AT_EMPTY_PATH) =
0
mmap(NULL, 59, PROT_READ, MAP_PRIVATE, 3, 0) = 0x7f6491b91000
close(3)= 0
openat(AT_FDCWD, "/usr/lib/locale/en_US.UTF-8/LC_ADDRESS",
O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/usr/lib/locale/en_US.utf8/LC_ADDRESS",
O_RDONLY|O_CLOEXEC) = 3
newfstatat(3, "", {st_mode=S_IFREG|0644, st_size=167, ...}, AT_EMPTY_PATH)
= 0
mmap(NULL, 167, PROT_READ, MAP_PRIVATE, 3, 0) = 

bug#70231: Performance issue on sort with zero-sized pseudo files

[Pádraig Brady]

On 06/04/2024 23:22, Paul Eggert wrote:

On 2024-04-06 03:09, Pádraig Brady wrote:

I'll apply this.


Heh, I beat you to it by looking for similar errors elsewhere and
applying the attached patches to fix the issues I found. None of them
look like serious bugs.


Cool. I thought the sort(1) change worthy of a NEWS entry so pushed one.
Marking this as done.


BTW we should improve sort buffer handling in general


Oh yes.

PS. My current little task is to get i18n to work better with 'sort'.
Among other things I want Unicode-style full case folding.


Excellent, that will help keep the related uniq(1) and join(1)
commands more aligned in their ordering.

cheers,
Pádraig

bug#70231: Performance issue on sort with zero-sized pseudo files

[Paul Eggert]

On 2024-04-06 03:09, Pádraig Brady wrote:

I'll apply this.


Heh, I beat you to it by looking for similar errors elsewhere and 
applying the attached patches to fix the issues I found. None of them 
look like serious bugs.



BTW we should improve sort buffer handling in general


Oh yes.

PS. My current little task is to get i18n to work better with 'sort'. 
Among other things I want Unicode-style full case folding.From 225cb8d7473eadb481a4884e929bf23589d4bd82 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 6 Apr 2024 15:13:23 -0700
Subject: [PATCH 1/4] =?UTF-8?q?cat:=20don=E2=80=99t=20trust=20st=5Fsize=20?=
 =?UTF-8?q?on=20/proc=20files?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* src/cat.c (main):
Improve test for when copying will exhaust the output device.
Do not rely on st_size, which is unreliable in /proc.
Use lseek instead; this is good enough here.
* tests/cat/cat-self.sh: Test the relaxation of the heuristic
for self-copying.
---
 src/cat.c | 31 +--
 tests/cat/cat-self.sh | 20 
 2 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/src/cat.c b/src/cat.c
index 4ed404363..b33faeb35 100644
--- a/src/cat.c
+++ b/src/cat.c
@@ -645,9 +645,10 @@ main (int argc, char **argv)
   /* Optimal size of i/o operations of output.  */
   idx_t outsize = io_blksize (_buf);
 
-  /* Device and I-node number of the output.  */
+  /* Device, I-node number and lazily-acquired flags of the output.  */
   dev_t out_dev = stat_buf.st_dev;
   ino_t out_ino = stat_buf.st_ino;
+  int out_flags = -2;
 
   /* True if the output is a regular file.  */
   bool out_isreg = S_ISREG (stat_buf.st_mode) != 0;
@@ -701,17 +702,27 @@ main (int argc, char **argv)
 
   fdadvise (input_desc, 0, 0, FADVISE_SEQUENTIAL);
 
-  /* Don't copy a nonempty regular file to itself, as that would
- merely exhaust the output device.  It's better to catch this
- error earlier rather than later.  */
+  /* Don't copy a file to itself if that would merely exhaust the
+ output device.  It's better to catch this error earlier
+ rather than later.  */
 
-  if (out_isreg
-  && stat_buf.st_dev == out_dev && stat_buf.st_ino == out_ino
-  && lseek (input_desc, 0, SEEK_CUR) < stat_buf.st_size)
+  if (stat_buf.st_dev == out_dev && stat_buf.st_ino == out_ino)
 {
-  error (0, 0, _("%s: input file is output file"), quotef (infile));
-  ok = false;
-  goto contin;
+  if (out_flags < -1)
+out_flags = fcntl (STDOUT_FILENO, F_GETFL);
+  bool exhausting = 0 <= out_flags && out_flags & O_APPEND;
+  if (!exhausting)
+{
+  off_t in_pos = lseek (input_desc, 0, SEEK_CUR);
+  if (0 <= in_pos)
+exhausting = in_pos < lseek (STDOUT_FILENO, 0, SEEK_CUR);
+}
+  if (exhausting)
+{
+  error (0, 0, _("%s: input file is output file"), quotef (infile));
+  ok = false;
+  goto contin;
+}
 }
 
   /* Pointer to the input buffer.  */
diff --git a/tests/cat/cat-self.sh b/tests/cat/cat-self.sh
index e0f6455c0..854825def 100755
--- a/tests/cat/cat-self.sh
+++ b/tests/cat/cat-self.sh
@@ -30,4 +30,24 @@ echo y >doc.end || framework_failure_
 cat doc doc.end >doc || fail=1
 compare doc doc.end || fail=1
 
+# This terminates even though it copies a file to itself.
+# Coreutils 9.5 and earlier rejected this.
+echo x >fx || framework_failure_
+echo y >fy || framework_failure_
+cat fx fy >fxy || fail=1
+for i in 1 2; do
+  cat fx >fxy$i || fail=1
+done
+for i in 3 4 5 6; do
+  cat fx >fx$i || fail=1
+done
+cat - fy fxy1 || fail=1
+compare fxy fxy1 || fail=1
+cat fxy2 fy 1<>fxy2 || fail=1
+compare fxy fxy2 || fail=1
+returns_ 1 cat fx fx3 1<>fx3 || fail=1
+returns_ 1 cat - fx4 fx4 || fail=1
+returns_ 1 cat fx5 >>fx5 || fail=1
+returns_ 1 cat >fx6 || fail=1
+
 Exit $fail
-- 
2.40.1

From ac6b8d8224de140f5a6f2ca66e6ce279604a37e6 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 6 Apr 2024 15:15:04 -0700
Subject: [PATCH 2/4] =?UTF-8?q?dd:=20don=E2=80=99t=20trust=20st=5Fsize=20o?=
 =?UTF-8?q?n=20/proc/files?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* src/dd.c (skip): Don’t trust st_size == 0.
---
 src/dd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/dd.c b/src/dd.c
index 370a42c97..6e0d725da 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -1809,7 +1809,7 @@ skip (int fdesc, char const *file, intmax_t records, idx_t blocksize,
struct stat st;
if (ifstat (STDIN_FILENO, ) != 0)
  error (EXIT_FAILURE, errno, _("cannot fstat %s"), quoteaf (file));
-   if (usable_st_size () && 0 <= input_offset
+   if (usable_st_size () && 0 < st.st_size && 0 <= input_offset
&& 

bug#70219: Bug/Issue with timeout and signals

[Pádraig Brady]

tag 70219 notabug
close 70219
stop

On 06/04/2024 16:50, Branden R. Williams via GNU coreutils Bug Reports wrote:

   -k, --kill-after=DURATION
  also send a KILL signal if COMMAND is still running
this long after the initial signal was sent


If you read the above carefully, please note the words _also_ and _initial_.
I.e. -k will cause _another_ signal to be sent, iff the first doesn't terminate 
the command.

Hopefully this example with all pertinent options explains things.
I.e. the first signal sent after 1s is ignored,
and the second kill signal sent after another 2s forcefully kills the command.

  $ date +%s; timeout -k 2s -s0 1s sleep inf; date +%s
  1712427916
  Killed
  1712427919

cheers,
Pádraig

bug#70219: Bug/Issue with timeout and signals

[Branden R. Williams]

Working from the code published here: 
https://github.com/coreutils/coreutils/blob/master/src/timeout.c

If you look at the code, you can see the kill_after variable set on line 486 if 
the switch is there. In the cleanup() function on line 202, it looks for that 
variable to be st on line 212, then sets the kill signal to be SIGKILL and sets 
the timeout. But on line 578, the timeout is overridden anyway by the timeout 
variable obtained from the argv on line 520. So as I stated previously, if the 
goal of the -k is to send SIGKILL, it should not be a kill_after or require any 
seconds to be sent. What is happening on line 485 is it is capturing what 
SHOULD be the duration passed to “timeout” to kill_after, and then the timeout 
that is attempted to be grabbed in line 520 is literally the command you are 
wanting to run the timeout against.

It does not appear to matter what you pass into -k as it is never used in 
practice. -k does not do this:

  -k, --kill-after=DURATION
 also send a KILL signal if COMMAND is still running
   this long after the initial signal was sent

It’s a logic bug. The -k flag should not take a duration, but just set the kill 
signal to SIGKILL. See this example where the 2s is captured on line 486 
leaving nothing for line 520.

$ timeout -k 2s ping 8.8.8.8
timeout: invalid time interval ‘ping’
Try 'timeout --help' for more information.

See these examples of passing garbage into the -k flag:

$ timeout -k 2s 2s ping 8.8.8.8
PING 8.8.8.8 (8.8.8.8) 56(84) bytes of data.
64 bytes from 8.8.8.8: icmp_seq=1 ttl=114 time=2.02 ms
64 bytes from 8.8.8.8: icmp_seq=2 ttl=114 time=1.92 ms

$ timeout -k 32s 2s ping 8.8.8.8
PING 8.8.8.8 (8.8.8.8) 56(84) bytes of data.
64 bytes from 8.8.8.8: icmp_seq=1 ttl=114 time=2.37 ms
64 bytes from 8.8.8.8: icmp_seq=2 ttl=114 time=1.94 ms

$ timeout -k 92s 2s ping 8.8.8.8
PING 8.8.8.8 (8.8.8.8) 56(84) bytes of data.
64 bytes from 8.8.8.8: icmp_seq=1 ttl=114 time=1.94 ms
64 bytes from 8.8.8.8: icmp_seq=2 ttl=114 time=8.38 ms

$ timeout -k 9265535s 2s ping 8.8.8.8
PING 8.8.8.8 (8.8.8.8) 56(84) bytes of data.
64 bytes from 8.8.8.8: icmp_seq=1 ttl=114 time=8.77 ms
64 bytes from 8.8.8.8: icmp_seq=2 ttl=114 time=11.8 ms

$ timeout 2s ping 8.8.8.8
PING 8.8.8.8 (8.8.8.8) 56(84) bytes of data.
64 bytes from 8.8.8.8: icmp_seq=1 ttl=114 time=1.94 ms
64 bytes from 8.8.8.8: icmp_seq=2 ttl=114 time=2.00 ms


Regards,

B

> On Apr 6, 2024, at 9:44 AM, Chris Elvidge  wrote:
> 
> Branden,
> 
> Then how come timeout -k 10s 20s ping 8.8.8.8 works?
> 10s is DEFINITELY NOT an integer. 20s in the timeout value.
> 
> I still think your reasoning is suspect.
> 
> 
> On 06/04/2024 at 13:43, Branden R. Williams wrote:
>> I understand this, but the manpage and the help file do not explain the 
>> functionally this way. The manpage suggests that the following should work:
>> $ timeout -k 10s sleep 10
>> It does not because the first argument after -k MUST be the an integer value 
>> of the signal you want to send, not the duration that the manpage and --help 
>> tell you to pass.
>> Regards,
>> B
>>> On Apr 6, 2024, at 4:06 AM, Andreas Schwab  wrote:
>>> 
>>> On Apr 05 2024, "Branden R. Williams" via GNU coreutils Bug Reports wrote:
>>> 
 That’s not an accurate representation of what the command actually does. 
 The argument after -k MUST be the kill signal code, without the code the 
 command fails. The manpage and help document agree with what you are 
 saying but the execution of the program fails.
>>> 
>>> $ timeout -k USR1 1s sleep 10
>>> timeout: invalid time interval ‘USR1’
>>> Try 'timeout --help' for more information.
>>> $ timeout -s KILL 1s sleep 10
>>> Killed
>>> 
>>> -- 
>>> Andreas Schwab, sch...@linux-m68k.org
>>> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
>>> "And now for something completely different."
> 
> 
> -- 
> 
> Chris Elvidge
> 
> 5 Ebor Park, Appleton Roebuck, York, Nor

bug#70219: Bug/Issue with timeout and signals

[Branden R. Williams]

I understand this, but the manpage and the help file do not explain the 
functionally this way. The manpage suggests that the following should work:

$ timeout -k 10s sleep 10

It does not because the first argument after -k MUST be the an integer value of 
the signal you want to send, not the duration that the manpage and --help tell 
you to pass.

Regards, 

B



> On Apr 6, 2024, at 4:06 AM, Andreas Schwab  wrote:
> 
> On Apr 05 2024, "Branden R. Williams" via GNU coreutils Bug Reports wrote:
> 
>> That’s not an accurate representation of what the command actually does. The 
>> argument after -k MUST be the kill signal code, without the code the command 
>> fails. The manpage and help document agree with what you are saying but the 
>> execution of the program fails.
> 
> $ timeout -k USR1 1s sleep 10
> timeout: invalid time interval ‘USR1’
> Try 'timeout --help' for more information.
> $ timeout -s KILL 1s sleep 10
> Killed
> 
> -- 
> Andreas Schwab, sch...@linux-m68k.org
> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
> "And now for something completely different."

bug#70219: Bug/Issue with timeout and signals

[Chris Elvidge]

Branden,

Then how come timeout -k 10s 20s ping 8.8.8.8 works?
10s is DEFINITELY NOT an integer. 20s in the timeout value.

I still think your reasoning is suspect.


On 06/04/2024 at 13:43, Branden R. Williams wrote:
I understand this, but the manpage and the help file do not explain the 
functionally this way. The manpage suggests that the following should work:


$ timeout -k 10s sleep 10

It does not because the first argument after -k MUST be the an integer 
value of the signal you want to send, not the duration that the manpage 
and --help tell you to pass.


Regards,

B




On Apr 6, 2024, at 4:06 AM, Andreas Schwab  wrote:

On Apr 05 2024, "Branden R. Williams" via GNU coreutils Bug Reports wrote:

That’s not an accurate representation of what the command actually 
does. The argument after -k MUST be the kill signal code, without the 
code the command fails. The manpage and help document agree with what 
you are saying but the execution of the program fails.


$ timeout -k USR1 1s sleep 10
timeout: invalid time interval ‘USR1’
Try 'timeout --help' for more information.
$ timeout -s KILL 1s sleep 10
Killed

--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."





--

Chris Elvidge

5 Ebor Park, Appleton Roebuck, York, Nor

bug#70231: Performance issue on sort with zero-sized pseudo files

[Pádraig Brady]

On 06/04/2024 03:52, Takashi Kusumi wrote:

Hi,

I have found a performance issue with the sort command when used on
pseudo files with zero size. For instance, sorting `/proc/kallsyms`, as
demonstrated below, takes significantly longer than executing with
`cat`, generating numerous temporary files. I confirmed this issue on
v8.32 as well as on commit 8f3989d in the master branch.

$ time cat /proc/kallsyms | sort > /dev/null
real0m0.954s
user0m0.873s
sys 0m0.096s

$ time sort /proc/kallsyms > /dev/null
real0m8.555s
user0m3.367s
sys 0m5.064s

$ strace -e trace=openat sort /proc/kallsyms 2>&1 > /dev/null \
  | grep /tmp/sort | head -100
...
openat(AT_FDCWD, "/tmp/sortM6Y6Y1", ...
openat(AT_FDCWD, "/tmp/sortPrHKMG", ...

$ strace -e trace=openat -c sort /proc/kallsyms > /dev/null
% time seconds  usecs/call callserrors syscall
-- --- --- - - 
100.006.419777  19333258 8 openat
-- --- --- - - 
100.006.419777  19333258 8 total

It appears that the buffer size allocated for pseudo files with zero
size is insufficient, likely because it is based on their file size,
which is zero. As seen in the attached patch, I think using
`INPUT_FILE_SIZE_GUESS` to calculate the buffer size when the file size
is zero would resolve this issue.


I'll apply this.

BTW we should improve sort buffer handling in general. From my TODO...

0. Have sort --debug output memory buffer sizes and space avail at $TMPDIR(s)
1. auto increase buffer when reading from pipe or zero sized files.
This will be more efficient and more importantly enable parallel operation.
See http://superuser.com/questions/938558/sort-parallel-isnt-parallelizing/
At least your more appropriate default buffer sizes in this case.
I.e. bigger mins and probably smaller maxs as half avail mem is too aggressive.
2. check() should not need full buffer size?
only merge buffer size or something small at least.
3. Look at minimizing the amount of mem used by default.
Hmm, sort auto adjusts down to avail mem in initbuf() (Test with ulimit -v)
4. Careful with too small buffers as that may initiate
an extra merge step (see section above).

If anyone wants to look at the above give me a heads up,
or I'll get to it sometime in the next release cycle.

thanks!
Pádraig.

bug#70219: Bug/Issue with timeout and signals

[Andreas Schwab]

On Apr 05 2024, "Branden R. Williams" via GNU coreutils Bug Reports wrote:

> That’s not an accurate representation of what the command actually does. The 
> argument after -k MUST be the kill signal code, without the code the command 
> fails. The manpage and help document agree with what you are saying but the 
> execution of the program fails. 

$ timeout -k USR1 1s sleep 10
timeout: invalid time interval ‘USR1’
Try 'timeout --help' for more information.
$ timeout -s KILL 1s sleep 10
Killed

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#70231: Performance issue on sort with zero-sized pseudo files

[Takashi Kusumi]

Hi,

I have found a performance issue with the sort command when used on
pseudo files with zero size. For instance, sorting `/proc/kallsyms`, as
demonstrated below, takes significantly longer than executing with
`cat`, generating numerous temporary files. I confirmed this issue on
v8.32 as well as on commit 8f3989d in the master branch.

  $ time cat /proc/kallsyms | sort > /dev/null
  real0m0.954s
  user0m0.873s
  sys 0m0.096s

  $ time sort /proc/kallsyms > /dev/null
  real0m8.555s
  user0m3.367s
  sys 0m5.064s

  $ strace -e trace=openat sort /proc/kallsyms 2>&1 > /dev/null \
| grep /tmp/sort | head -100
  ...
  openat(AT_FDCWD, "/tmp/sortM6Y6Y1", ...
  openat(AT_FDCWD, "/tmp/sortPrHKMG", ...

  $ strace -e trace=openat -c sort /proc/kallsyms > /dev/null
  % time seconds  usecs/call callserrors syscall
  -- --- --- - - 
  100.006.419777  19333258 8 openat
  -- --- --- - - 
  100.006.419777  19333258 8 total

It appears that the buffer size allocated for pseudo files with zero
size is insufficient, likely because it is based on their file size,
which is zero. As seen in the attached patch, I think using
`INPUT_FILE_SIZE_GUESS` to calculate the buffer size when the file size
is zero would resolve this issue.

Best regards,
Takashi KusumiFrom 9f759cc72014ab66da0e14318d4aa0c72e9311d9 Mon Sep 17 00:00:00 2001
From: Takashi Kusumi 
Date: Fri, 5 Apr 2024 12:03:42 +0900
Subject: [PATCH] sort: fix performance issue on zero-sized pseudo files

Previously, an insufficient buffer size was chosen for zero-sized pseudo
files (e.g., /proc/kallsyms). Now, the buffer size is calculated using
INPUT_FILE_SIZE_GUESS when the file size is zero.
---
 src/sort.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/sort.c b/src/sort.c
index 329ed45dc..8d757da55 100644
--- a/src/sort.c
+++ b/src/sort.c
@@ -1538,7 +1538,7 @@ sort_buffer_size (FILE *const *fps, size_t nfps,
   != 0)
 sort_die (_("stat failed"), files[i]);
 
-  if (S_ISREG (st.st_mode))
+  if (S_ISREG (st.st_mode) && st.st_size != 0)
 file_size = st.st_size;
   else
 {
-- 
2.41.0

bug#70219: Bug/Issue with timeout and signals

[Branden R. Williams]

That’s not an accurate representation of what the command actually does. The 
argument after -k MUST be the kill signal code, without the code the command 
fails. The manpage and help document agree with what you are saying but the 
execution of the program fails. 

That functionality is not represented in the manpage or --help. If you used -k 
10 10s it would send a SIGTERM vs SIGKILL. The point is that the argument 
immediately after the -k or --kill-after= is the kill signal code, not the 
duration. The manpage and help file suggest that it should be the duration.

That’s why I am suggesting using a default SIGTERM if the -k or --kill-after= 
is passed so that the duration is the actual argument that follows. If you 
wanted to alter the signal, that’s where the -s flag comes in.


> On Apr 5, 2024, at 4:59 PM, Chris Elvidge  wrote:
> 
> 
> 
> On 05/04/2024 at 16:19, Branden R. Williams via GNU coreutils Bug Reports 
> wrote:
>> I was integrating the timeout command into a shell script and realized the 
>> manpage & the --help docs do not accurately describe how the tool works. In 
>> addition, there appears to be a bug related to arguments passed. I am 
>> running version 9.1.
>> According to the help screen, this command should work:
>>  timeout -k 10s ping 
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__example.com=DwICaQ=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM=1o61i4CUhS9OzzVMraHh20b966TBpqSO4_0GGRNaLWA=aJDfsufl90pc4lE-vjk1Oo94O1RIrEP39t_G7X5-pvl_5ZXtR-2jcnl0a5OxJgGu=VjTsmLfaTKmXMJL0S6OXC7PHTK-rxRwmG7skx4FheqY=
>>  
>> 
>> It fails, however, because the next argument after invoking -k is the kill 
>> signal you want to send. The command (to send a SIGKILL) must be:
> 
> As I understand it, the argument after -k (or --kill-after=) is the time to 
> wait and send the kill signal if the timeout fails.
> So this line:
> 
>>  timeout -k 9 10s ping 
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__example.com=DwICaQ=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM=1o61i4CUhS9OzzVMraHh20b966TBpqSO4_0GGRNaLWA=aJDfsufl90pc4lE-vjk1Oo94O1RIrEP39t_G7X5-pvl_5ZXtR-2jcnl0a5OxJgGu=VjTsmLfaTKmXMJL0S6OXC7PHTK-rxRwmG7skx4FheqY=
>>  
>> 
> 
> sends a kill signal 9 seconds after the timeout, if the timeout hasn't worked.
> 
> The -s option is to change the signal sent (as below).
> 
>> I believe the kill after functionality and docs should be modified to send a 
>> default signal of SIGTERM without an additional argument so the first 
>> iteration above would work. Then you could explain that if you wanted to 
>> send a DIFFERENT signal, use the -s flag that is there to pass one. But 
>> according to the docs, the first command SHOULD work. Even using the 
>> --kill-after= syntax passes in the kill signal into that argument, not the 
>> actual time.
>> Thank you for your consideration and for maintaining such a critical set of 
>> tools!
>> Regards,
>> B
> 
> 
> -- 
> 
> Chris Elvidge
> 

bug#70219: Bug/Issue with timeout and signals

[Chris Elvidge]

On 05/04/2024 at 16:19, Branden R. Williams via GNU coreutils Bug 
Reports wrote:

I was integrating the timeout command into a shell script and realized the manpage 
& the --help docs do not accurately describe how the tool works. In addition, 
there appears to be a bug related to arguments passed. I am running version 9.1.

According to the help screen, this command should work:

timeout -k 10s ping example.com 

It fails, however, because the next argument after invoking -k is the kill 
signal you want to send. The command (to send a SIGKILL) must be:



As I understand it, the argument after -k (or --kill-after=) is the time 
to wait and send the kill signal if the timeout fails.

So this line:


timeout -k 9 10s ping example.com 



sends a kill signal 9 seconds after the timeout, if the timeout hasn't 
worked.


The -s option is to change the signal sent (as below).


I believe the kill after functionality and docs should be modified to send a 
default signal of SIGTERM without an additional argument so the first iteration 
above would work. Then you could explain that if you wanted to send a DIFFERENT 
signal, use the -s flag that is there to pass one. But according to the docs, 
the first command SHOULD work. Even using the --kill-after= syntax passes in 
the kill signal into that argument, not the actual time.

Thank you for your consideration and for maintaining such a critical set of 
tools!

Regards,

B





--

Chris Elvidge

bug#70219: Bug/Issue with timeout and signals

[Branden R. Williams]

I was integrating the timeout command into a shell script and realized the 
manpage & the --help docs do not accurately describe how the tool works. In 
addition, there appears to be a bug related to arguments passed. I am running 
version 9.1.

According to the help screen, this command should work:

timeout -k 10s ping example.com 

It fails, however, because the next argument after invoking -k is the kill 
signal you want to send. The command (to send a SIGKILL) must be:

timeout -k 9 10s ping example.com 

I believe the kill after functionality and docs should be modified to send a 
default signal of SIGTERM without an additional argument so the first iteration 
above would work. Then you could explain that if you wanted to send a DIFFERENT 
signal, use the -s flag that is there to pass one. But according to the docs, 
the first command SHOULD work. Even using the --kill-after= syntax passes in 
the kill signal into that argument, not the actual time.

Thank you for your consideration and for maintaining such a critical set of 
tools! 

Regards,

B

bug#70214: 'install' fails to copy regular file to autofs/cifs, due to ACL or xattr handling

[Bruno Haible]

Hi,

The 'install' program from coreutils-9.5 fails to copy a regular file
from an ext4 mount to an autofs/cifs mount.

The same operation, with 'cp -a', works fine.

Also, it works fine when coreutils was built with the configure options
"--disable-acl --disable-xattr".

How to reproduce


1) On the machine sparcdev.matoro.tk (Linux 6.8.2), I built coreutils-9.5
from source,
  - once with default options, in build-sparc64/,
  - once with "--disable-acl --disable-xattr", in build-sparc64-no-acl/.

2) Create a regular file on an ext4 mount:

$ echo hi > /var/tmp/foo3941
$ ls -lZ /var/tmp/foo3941
-rw-r- 1 g-haible g-haible ? 3 Apr  4 13:29 /var/tmp/foo3941
$ getfacl /var/tmp/foo3941
getfacl: Removing leading '/' from absolute path names
# file: var/tmp/foo3941
# owner: g-haible
# group: g-haible
user::rw-
group::r--
other::---
$ df -m /var/tmp/
Filesystem 1M-blocks   Used Available Use% Mounted on
/dev/root 560245 123140408574  24% /
$ mount | grep ' / '
/dev/sda2 on / type ext4 (rw,noatime)

3) Details about the destination directory:

$ echo $HOME
/media/guest-homedirs/haible
$ mount | grep /media/guest-homedirs/haible
/etc/autofs/auto.guest-homedirs on /media/guest-homedirs/haible type autofs 
(rw,relatime,fd=7,pgrp=2325,timeout=60,minproto=5,maxproto=5,direct,pipe_ino=46092)
//syslog.matoro.tk/guest-haible on /media/guest-homedirs/haible type cifs 
(rw,nosuid,relatime,vers=1.0,cache=strict,username=nobody,uid=30014,forceuid,gid=30014,forcegid,addr=fd05:::::::0001,soft,unix,posixpaths,serverino,mapposix,acl,rsize=1048576,wsize=65536,bsize=1048576,retrans=1,echo_interval=60,actimeo=1,closetimeo=1)

4) The operation that fails:

$ build-sparc64/src/ginstall -c /var/tmp/foo3941 $HOME/foo3941; echo $?
build-sparc64/src/ginstall: setting permissions for 
'/media/guest-homedirs/haible/foo3941': Permission denied
1
$ build-sparc64-no-acl/src/ginstall -c /var/tmp/foo3941 $HOME/foo3941; echo $?
0

5) The same thing with 'cp -a' succeeds:

$ build-sparc64/src/cp -a /var/tmp/foo3941 $HOME/foo3941; echo $?
0
$ build-sparc64-no-acl/src/cp -a /var/tmp/foo3941 $HOME/foo3941; echo $?
0

6) 'strace' shows a failing call to fsetxattr:

$ strace build-sparc64/src/ginstall -c /var/tmp/foo3941 $HOME/foo3941
execve("build-sparc64/src/ginstall", ["build-sparc64/src/ginstall", "-c", 
"/var/tmp/foo3941", "/media/guest-homedirs/haible/foo"...], 0x7fece28 /* 43 
vars */) = 0
brk(NULL)   = 0x1204000
access("/etc/ld.so.preload", R_OK)  = -1 ENOENT (No such file or directory)
openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 3
fstat64(3, {st_mode=S_IFREG|0644, st_size=41866, ...}) = 0
mmap(NULL, 41866, PROT_READ, MAP_PRIVATE, 3, 0) = 0xfff8000100028000
close(3)= 0
openat(AT_FDCWD, "/usr/lib64/libacl.so.1", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\2\1\0\0\0\0\0\0\0\0\0\0\3\0+\0\0\0\1\0\0\0\0\0\0\0\0"..., 
832) = 832
fstat64(3, {st_mode=S_IFREG|0755, st_size=1052312, ...}) = 0
mmap(NULL, 16384, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 
0xfff8000100034000
mmap(NULL, 3147696, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_DENYWRITE, -1, 0) 
= 0xfff8000100206000
mmap(0xfff800010030, 2099120, PROT_READ|PROT_EXEC, 
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0) = 0xfff800010030
munmap(0xfff8000100206000, 1024000) = 0
munmap(0xfff8000100502000, 18352)   = 0
mprotect(0xfff8000100308000, 2056192, PROT_NONE) = 0
mmap(0xfff80001004fe000, 16384, PROT_READ|PROT_WRITE|PROT_EXEC, 
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0xfe000) = 0xfff80001004fe000
close(3)= 0
openat(AT_FDCWD, "/usr/lib64/libattr.so.1", O_RDONLY|O_CLOEXEC) = 3
read(3, "\177ELF\2\2\1\0\0\0\0\0\0\0\0\0\0\3\0+\0\0\0\1\0\0\0\0\0\0\0\0"..., 
832) = 832
fstat64(3, {st_mode=S_IFREG|0755, st_size=1051928, ...}) = 0
mmap(NULL, 3147288, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_DENYWRITE, -1, 0) 
= 0xfff8000100502000
mmap(0xfff800010060, 2098712, PROT_READ|PROT_EXEC, 
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0) = 0xfff800010060
munmap(0xfff8000100502000, 1040384) = 0
munmap(0xfff8000100802000, 1560)= 0
mprotect(0xfff8000100604000, 2072576, PROT_NONE) = 0
mmap(0xfff80001007fe000, 16384, PROT_READ|PROT_WRITE|PROT_EXEC, 
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0xfe000) = 0xfff80001007fe000
close(3)= 0
openat(AT_FDCWD, "/usr/lib64/libc.so.6", O_RDONLY|O_CLOEXEC) = 3
read(3, 
"\177ELF\2\2\1\3\0\0\0\0\0\0\0\0\0\3\0+\0\0\0\1\0\0\0\0\0\2\305\240"..., 832) = 
832
fstat64(3, {st_mode=S_IFREG|0755, st_size=2110256, ...}) = 0
mmap(NULL, 4233760, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_DENYWRITE, -1, 0) 
= 0xfff8000100802000
mmap(0xfff800010090, 3185184, PROT_READ|PROT_EXEC, 
MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0) = 0xfff800010090
munmap(0xfff8000100802000, 1040384) = 0
munmap(0xfff8000100c0a000, 6688)= 0
mprotect(0xfff8000100a8e000, 

bug#70126: Missing a full stop in a coreutils-9.5-pre2 message

[Pádraig Brady]

On 01/04/2024 16:30, Petr Pisar wrote:

Hello,

while translating coreutils-9.5-pre2 I noticed this message:

#: src/chown.c:123
msgid ""
"  --reference=RFILE  use RFILE's ownership rather than specifying values\n"
" RFILE is always dereferenced if a symbolic link.\n"

I believe that there is missing a full stop after "values" on the first line.


Fixed.
Marking this as done.

thanks!
Pádraig

bug#70126: Missing a full stop in a coreutils-9.5-pre2 message

[Petr Pisar]

Hello,

while translating coreutils-9.5-pre2 I noticed this message:

#: src/chown.c:123
msgid ""
"  --reference=RFILE  use RFILE's ownership rather than specifying values\n"
" RFILE is always dereferenced if a symbolic link.\n"

I believe that there is missing a full stop after "values" on the first line.

-- Petr


signature.asc
Description: PGP signature

bug#70104: "FAIL: test-canonicalize" coreutils v9.5 on musl libc

[Paul Eggert]

On 2024-03-31 12:45, Bruno Haible wrote:

I think you must ask this to yourself:
   - What caused you to change the unit tests on 2023-09-04?
   - How is the musl version that you used on that date configured?
 What I use is Alpine Linux, as I said in versions 3.9, 3.14, 3.19.
   - Did you work with gnulib testdirs at that time, or did you use
 a package in which some modules are --avoid ed from import?


Thanks for fixing the problem. I cannot now remember why I put that musl 
stuff in. I suspect that I got it from some tests in some other package, 
but don't recall which ones. I don't use musl myself.

bug#70104: "FAIL: test-canonicalize" coreutils v9.5 on musl libc

[Bruno Haible]

Adept's Lab wrote:
> >> test-canonicalize.c:411: assertion 'strcmp (result1, "//") == 0' failed

Thanks for the report. I reproduce it with gnulib testdirs
  $ rm -rf ../testdir1; ./gnulib-tool --create-testdir --dir=../testdir1 
--single-configure canonicalize-lgpl
  $ rm -rf ../testdir2; ./gnulib-tool --create-testdir --dir=../testdir2 
--single-configure canonicalize
when run on Alpine Linux 3.9, 3.14, and 3.19.

History of these failures:
  - For many years, one of the tests was failing on Alpine Linux.
  - On 2021-01-17, I added a fix, that consists of a realpath() override [1].
  - On 2022-07-31, the bug was reported again [2], against the sed-4.8 release
which predates the 2021-01-17 fix [3].
  - On 2023-09-04, Paul changed the behaviour of the unit tests on musl libc,
without giving an explanation [4].
  - Now, the unit tests fail again, as reported above.

I'm therefore partially reverting Paul's change from 2023-09-04 (attached).

Paul Eggert wrote:
> Does this behavior (of whether / and // are the same directory) depend 
> on musl version, or on how musl is configured?

I think you must ask this to yourself:
  - What caused you to change the unit tests on 2023-09-04?
  - How is the musl version that you used on that date configured?
What I use is Alpine Linux, as I said in versions 3.9, 3.14, 3.19.
  - Did you work with gnulib testdirs at that time, or did you use
a package in which some modules are --avoid ed from import?

Bruno

[1] https://lists.gnu.org/archive/html/bug-gnulib/2021-01/msg00215.html
[2] https://lists.gnu.org/archive/html/bug-sed/2022-07/msg3.html
[3] https://ftp.gnu.org/gnu/sed/
[4] https://lists.gnu.org/archive/html/bug-gnulib/2023-09/msg00011.html
From 1defda6356c29c7f731bddb9e9231f594e01d9c9 Mon Sep 17 00:00:00 2001
From: Bruno Haible 
Date: Sun, 31 Mar 2024 21:31:34 +0200
Subject: [PATCH] canonicalize[-lgpl] tests: Fix test failure on musl libc.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Reported by Adept's Lab  via Pádraig Brady at
.

* tests/test-canonicalize-lgpl.c (main): Don't special-case "//"
handling for musl libc.
* tests/test-canonicalize.c (main): Likewise.
* modules/canonicalize-lgpl-tests (Files): Remove m4/musl.m4.
(configure.ac): Don't invoke gl_MUSL_LIBC.
---
 ChangeLog   | 11 +++
 modules/canonicalize-lgpl-tests |  2 --
 tests/test-canonicalize-lgpl.c  |  6 +++---
 tests/test-canonicalize.c   |  6 +++---
 4 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 2f05098a08..ffae5513ae 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2024-03-31  Bruno Haible  
+
+	canonicalize[-lgpl] tests: Fix test failure on musl libc.
+	Reported by Adept's Lab  via Pádraig Brady at
+	.
+	* tests/test-canonicalize-lgpl.c (main): Don't special-case "//"
+	handling for musl libc.
+	* tests/test-canonicalize.c (main): Likewise.
+	* modules/canonicalize-lgpl-tests (Files): Remove m4/musl.m4.
+	(configure.ac): Don't invoke gl_MUSL_LIBC.
+
 2024-03-31  Collin Funk  
 
 	gnulib-tool.py: Fix output of 'po/LINGUAS'.
diff --git a/modules/canonicalize-lgpl-tests b/modules/canonicalize-lgpl-tests
index 409ee7225b..73a0e32abb 100644
--- a/modules/canonicalize-lgpl-tests
+++ b/modules/canonicalize-lgpl-tests
@@ -1,5 +1,4 @@
 Files:
-m4/musl.m4
 tests/test-canonicalize-lgpl.c
 tests/signature.h
 tests/null-ptr.h
@@ -11,7 +10,6 @@ same-inode
 symlink
 
 configure.ac:
-gl_MUSL_LIBC
 
 Makefile.am:
 TESTS += test-canonicalize-lgpl
diff --git a/tests/test-canonicalize-lgpl.c b/tests/test-canonicalize-lgpl.c
index 1b2ad98f51..21211e5eac 100644
--- a/tests/test-canonicalize-lgpl.c
+++ b/tests/test-canonicalize-lgpl.c
@@ -262,9 +262,9 @@ main (void)
 ASSERT (stat ("/", ) == 0);
 ASSERT (stat ("//", ) == 0);
 bool same = psame_inode (, );
-#if defined __MVS__ || defined MUSL_LIBC
-/* On IBM z/OS and musl libc, "/" and "//" both canonicalize to
-   themselves, yet they both have st_dev == st_ino == 1.  */
+#if defined __MVS__
+/* On IBM z/OS, "/" and "//" both canonicalize to themselves, yet they both
+   have st_dev == st_ino == 1.  */
 same = false;
 #endif
 if (same)
diff --git a/tests/test-canonicalize.c b/tests/test-canonicalize.c
index 6763a525c9..5d19285c00 100644
--- a/tests/test-canonicalize.c
+++ b/tests/test-canonicalize.c
@@ -394,9 +394,9 @@ main (void)
 ASSERT (stat ("/", ) == 0);
 ASSERT (stat ("//", ) == 0);
 bool same = psame_inode (, );
-#if defined __MVS__ || defined MUSL_LIBC
-/* On IBM z/OS and musl libc, "/" and "//" both canonicalize to
-   themselves, yet they both have st_dev == st_ino == 1.  */
+#if defined __MVS__
+/* On IBM z/OS, "/" and "//" both canonicalize to themselves, yet they both
+   have st_dev == st_ino == 1.  */
 

bug#70104: "FAIL: test-canonicalize" coreutils v9.5 on musl libc

[Paul Eggert]

On 3/31/24 05:22, Pádraig Brady wrote:

On 31/03/2024 10:02, Adept's Lab wrote:

test-canonicalize.c:411: assertion 'strcmp (result1, "//") == 0' failed

^ the only error log message I get. Fail was not presented with previous
stable versions.


This is on musl 1.1.24 as detailed at:
https://github.com/coreutils/coreutils/issues/83T
Does this behavior (of whether / and // are the same directory) depend 
on musl version, or on how musl is configured?

bug#70104: "FAIL: test-canonicalize" coreutils v9.5 on musl libc

[Pádraig Brady]

On 31/03/2024 10:02, Adept's Lab wrote:

test-canonicalize.c:411: assertion 'strcmp (result1, "//") == 0' failed

^ the only error log message I get. Fail was not presented with previous
stable versions.


This is on musl 1.1.24 as detailed at:
https://github.com/coreutils/coreutils/issues/83

CC'ing bug-gnulib

cheers,
Pádraig

bug#70104: "FAIL: test-canonicalize" coreutils v9.5 on musl libc

[Adept's Lab]

test-canonicalize.c:411: assertion 'strcmp (result1, "//") == 0' failed

^ the only error log message I get. Fail was not presented with previous
stable versions.

bug#70070: FAIL: test-localtime_r

[Paul Eggert]

On 3/29/24 08:15, Andreas Schwab wrote:

On Mär 29 2024, Bruno Haible wrote:


Yes. And make sure that it has a time zone database installed at all.


Why? That doesn't make any sense.


Although Andreas is not clear, perhaps he is alluding to the fact that 
Gnulib's localtime_r tests assume that TZ='Europe/Paris' stands for 
Paris time. POSIX doesn't guarantee this, so the implication is that the 
tests should be skipped when Europe/Paris does not work.


I installed the attached to do that. I don't know whether this will fix 
Andreas's problem, whatever it is.From f130f5426ecd4edd5596797e0a5721b927f80126 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Sat, 30 Mar 2024 13:28:01 -0600
Subject: [PATCH] time_r-tests: skip French tests if no Europe/Paris

* tests/test-localtime_r.c (main):
* tests/test-localtime_r-mt.c (main):
If TZ='Europe/Paris' does not work, skip these tests.
---
 ChangeLog   |  7 +++
 tests/test-localtime_r-mt.c | 21 +
 tests/test-localtime_r.c| 21 +
 3 files changed, 49 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index eab98607a2..ecb4632196 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2024-03-30  Paul Eggert  
+
+	time_r-tests: skip French tests if no Europe/Paris
+	* tests/test-localtime_r.c (main):
+	* tests/test-localtime_r-mt.c (main):
+	If TZ='Europe/Paris' does not work, skip these tests.
+
 2024-03-29  Paul Eggert  
 
 	intprops: pacify GCC < 10 -Wsign-compare
diff --git a/tests/test-localtime_r-mt.c b/tests/test-localtime_r-mt.c
index ae371a8946..b09e86854f 100644
--- a/tests/test-localtime_r-mt.c
+++ b/tests/test-localtime_r-mt.c
@@ -107,6 +107,27 @@ main (int argc, char *argv[])
 {
   setenv ("TZ", FRENCH_TZ, 1);
 
+  /* Check that this TZ works.  */
+  {
+time_t t = 0; /* 1970-01-01 01:00:00 */
+struct tm *result = localtime ();
+if (! (result
+   && result->tm_sec == 0
+   && result->tm_min == 0
+   && result->tm_hour == 1
+   && result->tm_mday == 1
+   && result->tm_mon == 1 - 1
+   && result->tm_year == 1970 - 1900
+   && result->tm_wday == 4
+   && result->tm_yday == 0
+   && result->tm_isdst == 0))
+  {
+fputs ("Skipping test: TZ='Europe/Paris' is not Paris time\n",
+   stderr);
+return 77;
+  }
+  }
+
   /* Create the threads.  */
   gl_thread_create (thread1_func, NULL);
   gl_thread_create (thread2_func, NULL);
diff --git a/tests/test-localtime_r.c b/tests/test-localtime_r.c
index 70ec3b5d4f..6b3da1af19 100644
--- a/tests/test-localtime_r.c
+++ b/tests/test-localtime_r.c
@@ -43,6 +43,27 @@ main (void)
 {
   setenv ("TZ", FRENCH_TZ, 1);
 
+  /* Check that this TZ works.  */
+  {
+time_t t = 0; /* 1970-01-01 01:00:00 */
+struct tm *result = localtime ();
+if (! (result
+   && result->tm_sec == 0
+   && result->tm_min == 0
+   && result->tm_hour == 1
+   && result->tm_mday == 1
+   && result->tm_mon == 1 - 1
+   && result->tm_year == 1970 - 1900
+   && result->tm_wday == 4
+   && result->tm_yday == 0
+   && result->tm_isdst == 0))
+  {
+fputs ("Skipping test: TZ='Europe/Paris' is not Paris time\n",
+   stderr);
+return 77;
+  }
+  }
+
   /* Note: The result->tm_gmtoff values and the result->tm_zone values are the
  same (3600, "CET" or 7200, "CEST") across all tested platforms:
  glibc, musl, macOS, FreeBSD, NetBSD, OpenBSD, Minix, Cygwin, Android.  */
-- 
2.44.0

bug#70070: FAIL: test-localtime_r

[Bruno Haible]

Andreas Schwab wrote:
> > Yes. And make sure that it has a time zone database installed at all.
> 
> Why? That doesn't make any sense.

You can leave the consideration of which test case makes sense or not
on my side.

What we need from you, as a reporter, is a statement on which
platform (distro and version) you saw the test failure.
If we don't have this, there is no good way for us to reproduce the issue,
and we'll have to close the ticket.

Bruno

bug#70070: FAIL: test-localtime_r

[Andreas Schwab]

On Mär 29 2024, Bruno Haible wrote:

> Yes. And make sure that it has a time zone database installed at all.

Why? That doesn't make any sense.

bug#70070: FAIL: test-localtime_r

[Bruno Haible]

Andreas Schwab wrote:
> > FAIL: test-localtime_r
> > ==
> > 
> > test-localtime_r.c:58: assertion 'result->tm_hour == 18' failed
> > FAIL test-localtime_r (exit status: 134)
> > 
> > FAIL: test-localtime_r-mt
> > =
> > 
> > thread2 disturbed by thread1!
> > thread1 disturbed by thread2!
> > FAIL test-localtime_r-mt (exit status: 134)

The second failure is a consequence of the first one. So, there is no MT-safety
problem, "just" that localtime_r produces wrong results.

I've seen this happen on OSes with a time zone database that is older than
5 or 10 years.

Pádraig Brady wrote:
> It may help to detail your platform.

Yes. And make sure that it has a time zone database installed at all.

Bruno

bug#70070: FAIL: test-localtime_r

[Pádraig Brady]

On 29/03/2024 12:40, Andreas Schwab wrote:

FAIL: test-localtime_r
==

test-localtime_r.c:58: assertion 'result->tm_hour == 18' failed
FAIL test-localtime_r (exit status: 134)

FAIL: test-localtime_r-mt
=

thread2 disturbed by thread1!
thread1 disturbed by thread2!
FAIL test-localtime_r-mt (exit status: 134)



CC'ing gnulib as those tests are from gnulib.
It may help to detail your platform.

thanks,
Pádraig.

bug#70070: FAIL: test-localtime_r

[Andreas Schwab]

FAIL: test-localtime_r
==

test-localtime_r.c:58: assertion 'result->tm_hour == 18' failed
FAIL test-localtime_r (exit status: 134)

FAIL: test-localtime_r-mt
=

thread2 disturbed by thread1!
thread1 disturbed by thread2!
FAIL test-localtime_r-mt (exit status: 134)

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

bug#35247: [PATCH] Add alacritty to terminal list supporting colors

[Eike Gebauer]

By now alacritty has been packaged for Fedora for quite a while 
(https://packages.fedoraproject.org/pkgs/rust-alacritty/alacritty/) and 
is used by a relatively large community (>50k Stars on Github).


The corresponding issue on the alacritty side has sadly been closed as 
"wontfix" (https://github.com/alacritty/alacritty/issues/2210), so I 
would like to ask you to add `TERM alacritty` to dircolors.hin instead, 
as originally requested by Doron Behar.


Regards,

Eike

PS
I did not find a way to reply to the old thread from the mailing list 
archive with the correct `In-Reply-To` header, so this might not be 
threaded correctly with the original messages from 2019.


On Sat, 11 May 2019 16:57:40 +0300, Doron Behar wrote:
> Well I'm not a developer of alacritty, just a humble user. I had only
> taken the effort to investigate why I dircolors wouldn't define colors
> for me when in this terminal.
>
> Under it's installation section of the README, listed the supported
> packages for the various distros:
>
> https://github.com/jwilm/alacritty#installation
>
> On Fri, May 10, 2019 at 08:24:09AM -0700, Pádraig Brady wrote:On 
28/03/2024 17:34, bug-coreutils-requ...@gnu.org wrote:

> > On 12/04/19 12:20, address@hidden wrote:
> > > From: Doron Behar 
> > >
> > > https://github.com/jwilm/alacritty
> > > ---
> > >  src/dircolors.hin | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/src/dircolors.hin b/src/dircolors.hin
> > > index 3aa445bfa..424ed5b19 100644
> > > --- a/src/dircolors.hin
> > > +++ b/src/dircolors.hin
> > > @@ -12,6 +12,7 @@
> > >  # against the TERM environment variable to determine if it is 
colorizable.

> > >  TERM Eterm
> > >  TERM ansi
> > > +TERM alacritty
> > >  TERM *color*
> > >  TERM con[0-9]*x[0-9]*
> > >  TERM cons25
> > >
> >
> > I don't see this terminal packaged for Feodra at least.
> > Why does it need its own TERM ?
> > If it does, could one add 'color' to get the existing matching?
> >
> > thanks,
> > Pádraig

bug#69995: Untranslatable string

[Pádraig Brady]

On 25/03/2024 14:02, Göran Uddeborg wrote:

While translating the new version of coreutils to Swedish, I came
across this code from the end of chown-core.h:

   printf (_("\
   --from=CURRENT_OWNER:CURRENT_GROUP\n\
  change the %sgroup of each file only if\n\
  its current owner and/or group match those 
specified\n\
  here.  Either may be omitted, in which case a match\n\
  is not required for the omitted attribute\n\
"), user ? "owner and/or " : "");

As this stands, the part "owner and/or " will not be translated.

Simply marking that part too for translation is not a good solution, even
if it would happen to work for Swedish. See
https://www.gnu.org/savannah-checkouts/gnu/gettext/manual/gettext.html#Entire-sentences


Fixed with:
https://github.com/coreutils/coreutils/commit/26fd96a96

thanks,
Pádraig

bug#69995: Untranslatable string

[Göran Uddeborg]

While translating the new version of coreutils to Swedish, I came
across this code from the end of chown-core.h:

  printf (_("\
  --from=CURRENT_OWNER:CURRENT_GROUP\n\
 change the %sgroup of each file only if\n\
 its current owner and/or group match those specified\n\
 here.  Either may be omitted, in which case a match\n\
 is not required for the omitted attribute\n\
"), user ? "owner and/or " : "");

As this stands, the part "owner and/or " will not be translated.

Simply marking that part too for translation is not a good solution, even
if it would happen to work for Swedish. See
https://www.gnu.org/savannah-checkouts/gnu/gettext/manual/gettext.html#Entire-sentences

bug#69985: Untranslatable message in src/chown-core.h:95

[Pádraig Brady]

On 24/03/2024 16:57, Frédéric Marchal wrote:

Hi,

In src/chown-core.h:95 (coreutils-9.5-pre1), the following message is
impossible to translate as it is created by concatenating string fragments:

static inline void
emit_from_option_description (bool user)
{
   printf (_("\
   --from=CURRENT_OWNER:CURRENT_GROUP\n\
  change the %sgroup of each file only if\n\
  its current owner and/or group match those
specified\n\
  here.  Either may be omitted, in which case a
match\n\
  is not required for the omitted attribute\n\
"), user ? "owner and/or " : "");
}

A translatable message must *never ever* be produced by concatenating
substrings. Even if the substrings are themselves translated, there is no
guaranties that the order of the words is the same in every language.

There is, unfortunately, no easy solution that save keystrokes. It must be
written like this:

if (user)
   printf (_("\
   --from=CURRENT_OWNER:CURRENT_GROUP\n\
  change the owner and/or group of each file only if\n\
  its current owner and/or group match those
specified\n\
  here.  Either may be omitted, in which case a
match\n\
  is not required for the omitted attribute\n\
"));
else
   printf (_("\
   --from=CURRENT_OWNER:CURRENT_GROUP\n\
  change the group of each file only if\n\
  its current owner and/or group match those
specified\n\
  here.  Either may be omitted, in which case a
match\n\
  is not required for the omitted attribute\n\
"));

That make sense as both are distinct messages with different purposes.

I also suspect that the two messages should be even more different than they
currently are. The message, when user is false, suspiciously contains one more
"owner and/or" that is not removed and it says "Either may be omitted" when
only the group is mentioned at the beginning of the message.


The composed strings are accurate, but yes not translatable.
The attached simplifies the description a little,
so there is a single string to cater for both chown and chgrp.

Marking this as done.

thanks,
Pádraig
From baea9881f79690552b0aa3ca6844bbf64fa6e55e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= 
Date: Sun, 24 Mar 2024 20:12:53 +
Subject: [PATCH] doc: fix translation issue in chown/chgrp amalgamation

* src/chown-core.h (emit_from_option_description): The conditional
string composition here caused issues for translators.
Instead move to a more general description ...
(src/chown.c (usage): ... here.
Fixes https://bugs.gnu.org/69985
---
 src/chown-core.h | 12 
 src/chown.c  |  8 +++-
 2 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/src/chown-core.h b/src/chown-core.h
index e1396e3ea..4bd68fed4 100644
--- a/src/chown-core.h
+++ b/src/chown-core.h
@@ -89,16 +89,4 @@ chown_files (char **files, int bit_flags,
  struct Chown_option const *chopt)
   _GL_ATTRIBUTE_NONNULL ();
 
-static inline void
-emit_from_option_description (bool user)
-{
-  printf (_("\
-  --from=CURRENT_OWNER:CURRENT_GROUP\n\
- change the %sgroup of each file only if\n\
- its current owner and/or group match those specified\n\
- here.  Either may be omitted, in which case a match\n\
- is not required for the omitted attribute\n\
-"), user ? "owner and/or " : "");
-}
-
 #endif /* CHOWN_CORE_H */
diff --git a/src/chown.c b/src/chown.c
index 90ce84d67..3194dc75f 100644
--- a/src/chown.c
+++ b/src/chown.c
@@ -109,7 +109,13 @@ With --reference, change the group of each FILE to that of RFILE.\n\
  (useful only on systems that can change the\n\
  ownership of a symlink)\n\
 "), stdout);
-  emit_from_option_description (chown_mode == CHOWN_CHOWN);
+  fputs (_("\
+  --from=CURRENT_OWNER:CURRENT_GROUP\n\
+ change the ownership of each file only if its\n\
+ current owner and/or group match those specified here.\n\
+ Either may be omitted, in which case a match\n\
+ is not required for the omitted attribute\n\
+"), stdout);
   fputs (_("\
   --no-preserve-root  do not treat '/' specially (the default)\n\
   --preserve-rootfail to operate recursively on '/'\n\
-- 
2.44.0

bug#69986: Untranslatable argument in src/chown.c:79

[Frédéric Marchal]

Hi,

In src/chown.c:79 (coreutils-9.5-pre1.fr.po), some part of the sting was moved 
to an untranslated argument:

  printf (_("\
Usage: %s [OPTION]... %s FILE...\n\
  or:  %s [OPTION]... --reference=RFILE FILE...\n\
"),
  program_name,
  chown_mode == CHOWN_CHOWN ? "[OWNER][:[GROUP]]" : "GROUP",
  program_name);

It prevents the translator from translating the "[OWNER][:[GROUP]]" and 
"GROUP" arguments.

Please replace them with _("[OWNER][:[GROUP]]") : _("GROUP").

Frederic

bug#69985: Untranslatable message in src/chown-core.h:95

[Frédéric Marchal]

Hi,

In src/chown-core.h:95 (coreutils-9.5-pre1), the following message is 
impossible to translate as it is created by concatenating string fragments:

static inline void
emit_from_option_description (bool user)
{
  printf (_("\
  --from=CURRENT_OWNER:CURRENT_GROUP\n\
 change the %sgroup of each file only if\n\
 its current owner and/or group match those 
specified\n\
 here.  Either may be omitted, in which case a 
match\n\
 is not required for the omitted attribute\n\
"), user ? "owner and/or " : "");
}

A translatable message must *never ever* be produced by concatenating 
substrings. Even if the substrings are themselves translated, there is no 
guaranties that the order of the words is the same in every language.

There is, unfortunately, no easy solution that save keystrokes. It must be 
written like this:

if (user)
  printf (_("\
  --from=CURRENT_OWNER:CURRENT_GROUP\n\
 change the owner and/or group of each file only if\n\
 its current owner and/or group match those 
specified\n\
 here.  Either may be omitted, in which case a 
match\n\
 is not required for the omitted attribute\n\
"));
else
  printf (_("\
  --from=CURRENT_OWNER:CURRENT_GROUP\n\
 change the group of each file only if\n\
 its current owner and/or group match those 
specified\n\
 here.  Either may be omitted, in which case a 
match\n\
 is not required for the omitted attribute\n\
"));

That make sense as both are distinct messages with different purposes.

I also suspect that the two messages should be even more different than they 
currently are. The message, when user is false, suspiciously contains one more 
"owner and/or" that is not removed and it says "Either may be omitted" when 
only the group is mentioned at the beginning of the message.

Frederic

bug#69979: [PATCH-v2][doc] ls -l doc and size -> maj, min for device files

[Pádraig Brady]

On 24/03/2024 12:40, Stephane Chazelas wrote:

Tags: patch

My bad, the patch was incorrect, it should have said
"replaced by the corresponding device major and minor numbers as two decimal
numbers separated by a comma and at least one space.", as
there's not always only once space between the major and minor.

See also
https://unix.stackexchange.com/questions/773014/where-do-i-find-documentation-for-the-output-of-ls-l
where the issue was reported and where the variations of output
formats between ls implementations is being mentioned.

New patch attached.


Pushed.
Marking this as done.

thanks,
Pádraig

bug#69979: [PATCH-v2][doc] ls -l doc and size -> maj, min for device files

[Stephane Chazelas]

Tags: patch

My bad, the patch was incorrect, it should have said
"replaced by the corresponding device major and minor numbers as two decimal
numbers separated by a comma and at least one space.", as
there's not always only once space between the major and minor.

See also
https://unix.stackexchange.com/questions/773014/where-do-i-find-documentation-for-the-output-of-ls-l
where the issue was reported and where the variations of output
formats between ls implementations is being mentioned.

New patch attached.
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 8a2104831..8f7f43ea0 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7968,6 +7968,10 @@ that it is a number rather than a textual name.
 Print question marks for other information that
 cannot be determined.
 
+For block special files and character special files, the size field is
+replaced by the corresponding device major and minor numbers as two decimal
+numbers separated by a comma and at least one space.
+
 Normally the size is printed as a byte count without punctuation, but
 this can be overridden (@pxref{Block size}).
 For example, @option{--human-readable} (@option{-h})
diff --git a/gnulib b/gnulib
index af32ee824..32c16c45d 16
--- a/gnulib
+++ b/gnulib
@@ -1 +1 @@
-Subproject commit af32ee824ee18255839f9812b8ed61aa5257a82b
+Subproject commit 32c16c45d7378b014d9aac6130104c4d02a9acdb

bug#69979: [PATCH][doc] ls -l doc and size -> maj, min for device files

[Stephane Chazelas]

Package: coreutils
Version: 9.4
Tags: patch

The ls documentation currently doesn't state that for device
files, the size field in the long listing format is replaced by
major, minor.

Patch attached.

-- 
Stephane
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 8a2104831..c8e3bd110 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7968,6 +7968,10 @@ that it is a number rather than a textual name.
 Print question marks for other information that
 cannot be determined.
 
+For block special files and character special files, the size field is
+replaced by the corresponding device major and minor numbers as two decimal
+numbers separated by a comma and a space.
+
 Normally the size is printed as a byte count without punctuation, but
 this can be overridden (@pxref{Block size}).
 For example, @option{--human-readable} (@option{-h})

bug#69951: coreutils: printf formatting bug for nb_NO and nn_NO locales

[Thomas Dreibholz]

Hi,

indeed, the issue seems to be in libc. I can reproduce the problem with 
a simple C program:


#include 
#include 
#include 

int main(int argc, char** argv)
{
   setlocale (LC_ALL, "");

   struct lconv* loc = localeconv();
   printf("Thousands Separator: <%s>\n", loc->thousands_sep);

   for(int i = 1; i \tint <%'10d>\n", f, n);
   }
   return 0;
}

Output with LC_NUMERIC=nb_NO.UTF-8:

Thousands Separator: < >
double < 1> int < 1>
double <    10> int <    10>
double <   100> int <   100>
double < 1 000> int <   1 000>
double <    10 000> int <  10 000>
double <   100 000> int < 100 000>
double < 1 000 000> int <1 000 000>
double <10 000 000> int <10 000 000>

So, for a float (%f), the output is as expected, while it is wrong for 
an integer (%d).


--
Best regards / Mit freundlichen Grüßen / Med vennlig hilsen

===
 Thomas Dreibholz

 Simula Metropolitan Centre for Digital Engineering
 Centre for Resilient Networks and Applications
 Pilestredet 52
 0167 Oslo, Norway
---
 E-Mail:dre...@simula.no
 Homepage:http://simula.no/people/dreibh
===



OpenPGP_signature.asc
Description: OpenPGP digital signature

bug#69951: coreutils: printf formatting bug for nb_NO and nn_NO locales

[Thomas Dreibholz]

Hi,

some further debugging of a hexdump output of printf, i.e.:

#!/bin/bash
for l in de_DE en_US nb_NO nn_NO ; do
   echo "LC_NUMERIC=$l.UTF-8"
   for n in 1 100 1000 1 10 100 1000 ; do
  LC_NUMERIC=$l.UTF-8 /usr/bin/printf "<%'10d>" $n | hexdump -C
   done
done

The output is:

...
LC_NUMERIC=nb_NO.UTF-8
  3c 20 20 20 20 20 20 20  20 20 31 3e  |< 1>|
000c
  3c 20 20 20 20 20 20 20  31 30 30 3e  |<   100>|
000c
  3c 20 20 20 31 e2 80 af  30 30 30 3e  |<   1...000>|
000c
  3c 20 20 31 30 e2 80 af  30 30 30 3e  |<  10...000>|
000c
  3c 20 31 30 30 e2 80 af  30 30 30 3e  |< 100...000>|
000c
  3c 31 e2 80 af 30 30 30  e2 80 af 30 30 30 3e 
|<1...000...000>|

000f
  3c 31 30 e2 80 af 30 30  30 e2 80 af 30 30 30 3e 
 |<10...000...000>|

0010
LC_NUMERIC=nn_NO.UTF-8
  3c 20 20 20 20 20 20 20  20 20 31 3e  |< 1>|
000c
  3c 20 20 20 20 20 20 20  31 30 30 3e  |<   100>|
000c
  3c 20 20 20 31 e2 80 af  30 30 30 3e  |<   1...000>|
000c
  3c 20 20 31 30 e2 80 af  30 30 30 3e  |<  10...000>|
000c
  3c 20 31 30 30 e2 80 af  30 30 30 3e  |< 100...000>|
000c
  3c 31 e2 80 af 30 30 30  e2 80 af 30 30 30 3e 
|<1...000...000>|

000f
  3c 31 30 e2 80 af 30 30  30 e2 80 af 30 30 30 3e 
 |<10...000...000>|

0010

printf seems to insert a 3-byte UTF-8 character 0xe2 0x80 0xaf as 
thousands separator. "0xe2 0x80 0xaf" is UTF-8 NARROW NO-BREAK SPACE -> 
https://www.fileformat.info/info/unicode/char/202f/index.htm 
 . But 
terminal output (tested with Konsole and XTerm) has fixed spacing, so 
"narrow space" should probably be a regular space or regular 
non-breakable space (0xc2 0xa0, HTML "")? Note that also 
LibreOffice cannot produce a correct screen output with UTF-8 NARROW 
NO-BREAK SPACE, even with proportional fonts, when loading the output of 
the test script as a text file.


Screenshots for illustration:

 * Terminal output:
   
https://bugs.launchpad.net/ubuntu/+source/coreutils/+bug/2058775/+attachment/5758462/+files/Screenshot_20240322_213947.png
 * LibreOffice output:
   
https://bugs.launchpad.net/ubuntu/+source/coreutils/+bug/2058775/+attachment/5758464/+files/Screenshot_20240322_222052.png

--
Best regards / Mit freundlichen Grüßen / Med vennlig hilsen

===
 Thomas Dreibholz

 Simula Metropolitan Centre for Digital Engineering
 Centre for Resilient Networks and Applications
 Pilestredet 52
 0167 Oslo, Norway
---
 E-Mail:dre...@simula.no
 Homepage:http://simula.no/people/dreibh
===



OpenPGP_signature.asc
Description: OpenPGP digital signature

bug#69951: coreutils: printf formatting bug for nb_NO and nn_NO locales

[Pádraig Brady]

tag 69951 notabug
close 69951
stop

On 22/03/2024 20:22, Thomas Dreibholz wrote:

Hi,

I just discovered a printf bug for at least the nb_NO and nn_NO locales
when printing numbers with thousands separator. To reproduce:

#!/bin/bash
for l in de_DE nb_NO ; do
     echo "LC_NUMERIC=$l.UTF-8"
     for n in 1 100 1000 1 10 100 1000 ; do
    LC_NUMERIC=$l.UTF-8 /usr/bin/printf "<%'10d>\n" $n
     done
done

The expected output of "%'10d" is a right-formatted number string with
10 characters.

The output of the test script is fine for e.g. LC_NUMERIC=de_DE.UTF-8
and LC_NUMERIC=en_US.UTF-8:

LC_NUMERIC=de_DE.UTF-8
< 1>
<   100>
< 1.000>
<    10.000>
<   100.000>
< 1.000.000>
<10.000.000>



However, for LC_NUMERIC=nb_NO.UTF-8 and LC_NUMERIC=nn_NO.UTF-8, the
formatting is wrong:

LC_NUMERIC=nb_NO.UTF-8
< 1>
<   100>
<   1 000>
<  10 000>
< 100 000>
<1 000 000>
<10 000 000>



I reproduced the issue with coreutils-8.32-4.1ubuntu1.1 (Ubuntu 22.04)
as well as coreutils-9.3-5.fc39.x86_64 (Fedora 39).

Under FreeBSD 14.0-RELEASE (coreutils-9.4_1), the output looks slightly
better but is still wrong:

LC_NUMERIC=nb_NO.UTF-8
< 1>
<   100>
<    1 000>
<   10 000>
<  100 000>
<1 000 000>
<10 000 000>
LC_NUMERIC=nn_NO.UTF-8
< 1>
<   100>
<    1 000>
<   10 000>
<  100 000>
<1 000 000>
<10 000 000>

May be the issue is that the thousands separator for the Norwegian
locales is a space " ", while it is "."/"," for German/US English locales.


The issue looks to be that the thousands separator for Norwegian locales
is “NARROW NO-BREAK SPACE", or more problematically the _three_ byte
UTF8 sequence E2 80 AF. So it looks like an issue with libc routines
counting bytes rather than characters in this case.

One suggestion is to do the alignment after. For example:

$ export LC_NUMERIC=nb_NO.UTF-8
$ printf "%'.f\n" $(seq -f '1E%.f' 7) | column --table-right=1 -t
10
   100
 1 000
10 000
   100 000
 1 000 000
10 000 000

Actually I've just noticed that specifying the %'10.f format
does count characters and not bytes! So another solution is:

$ export LC_NUMERIC=nb_NO.UTF-8
$ printf "%'10.f\n" $(seq -f '1E%.f' 7)
10
   100
 1 000
10 000
   100 000
 1 000 000
10 000 000

The issue if there is one is in libc at least.
It would be worth checking existing glibc reports about this
and reporting if not mentioned.

cheers,
Pádraig.

bug#69532: mv's new -x option should be made orthogonal to -t/-T/default

[Bernhard Voelker]

On 3/22/24 11:22, Karel Zak wrote:
> On Wed, Mar 20, 2024 at 11:53:05PM +0100, Bernhard Voelker wrote:>> On 
userland OTOH, we have broader choice.
>> Karel did his choice in util-linux for exch(1), and coreutils could expose
>> the same functionality.
>>
>> For other feature requests, we were much more reluctant in coreutils ... for
>> good reasons: feature bloat, maintainability, etc.
>>
>> So I'm asking myself what is different this time?
>> - The feature already exists -> util-linux.
>
> Note that we can move exch(1) from util-linux to coreutils if, at the
> end of the brainstorming session, the conclusion will be that mv(1) is
> a bad choice :-)

I'd be for that as well, because ...

>> I'm currently only 20:80 for adding it to mv(1).
>
> I think the functionality will be lost in the mv(1) for many users.

... that's a fair point.

The code for the functionality is in copy.c, so - as with mv.c/cp.c/install.c -
we could have a exch.c using just that part, and thus expose a clearer interface
to the users.

Have a nice day,
Berny

bug#69532: mv's new -x option should be made orthogonal to -t/-T/default

[Bernhard Voelker]

On 3/23/24 02:44, Paul Eggert wrote:

I installed the attached patches to do the above. (Basically, the
problem was that my earlier patches were too ambitious; these patches
scale things back to avoid some optimizations so that mv --exchange is
more like ordinary mv.)

The first patch simplifies the code (and fixes a diagnostic to be more
useful) without otherwise changing behavior; it's more of a refactoring.
The second patch does the real work.


Thanks.

We should put adding more tests on our TODO list still.

Have a nice day,
Berny