Re: Modify buffering of standard streams via environment variables (not LD_PRELOAD)?
On Sat, 20 Apr 2024, Zachary Santer wrote: I don't know how buffering works when stdout and stderr get redirected to the same pipe. You'd think, whatever it is, it would have to be smart enough to keep them interleaved in the same order they were printed to in. That in mind, I would assume they both get placed into the same block buffer by default. On Sun, 21 Apr 2024, wrotycz wrote: Sat, Apr 20, 2024 at 16:45 Carl Edquist wrote: However, stdout and stderr are still separate streams even if they refer to the same output file/pipe/device, so partial lines are not interleaved in the order that they were printed. will output "abc\n123\n" instead of "a1b2c3\n\n", even if you run it as $ ./abc123 2>&1 | cat It seems that it's 'interleaved' when buffer is written to a file or pipe, and because stdout is buffered it waits until buffer is full or flushed, while stderr is not and it doesn't wait and write immediately. Right; my point was just that stdout and stderr are still separate streams (with distinct buffers & buffering modes), even if fd 1 & 2 refer to the same pipe. Carl
Re: Modify buffering of standard streams via environment variables (not LD_PRELOAD)?
On Sat, 20 Apr 2024, Zachary Santer wrote:
This was actually in RHEL 7.
Oh. In that case it might be worth looking into ...
I don't know how buffering works when stdout and stderr get redirected
to the same pipe. You'd think, whatever it is, it would have to be smart
enough to keep them interleaved in the same order they were printed to
in. That in mind, I would assume they both get placed into the same
block buffer by default.
My take is always to try it and find out. Though in this case I think the
default (without using stdbuf) is that the program's stderr is output to
the pipe immediately (ie, unbuffered) on each library call (fprintf(3),
fputs(3), putc(3), fwrite(3)), while stdout is written to the pipe at
block boundaries - even though fd 1 and 2 refer to the same pipe.
If you force line buffering for stdout and stderr, that is likely what you
want, and it will interleave _lines_ in the order that they were printed.
However, stdout and stderr are still separate streams even if they refer
to the same output file/pipe/device, so partial lines are not interleaved
in the order that they were printed.
For example:
#include
int main()
{
putc('a', stderr);
putc('1', stdout);
putc('b', stderr);
putc('2', stdout);
putc('c', stderr);
putc('3', stdout);
putc('\n', stderr);
putc('\n', stdout);
return 0;
}
will output "abc\n123\n" instead of "a1b2c3\n\n", even if you run it as
$ ./abc123 2>&1 | cat
or
$ stdbuf -oL -eL ./abc123 2>&1 | cat
...
Not that that's relevant for what you're doing :)
Carl
Re: Modify buffering of standard streams via environment variables (not LD_PRELOAD)?
On Thu, 18 Apr 2024, Zachary Santer wrote: On Wed, Mar 20, 2024 at 4:54 AM Carl Edquist wrote: However, if stdbuf's magic env vars are exported in your shell (either by doing a trick like 'export $(env -i stdbuf -oL env)', or else more simply by first starting a new shell with 'stdbuf -oL bash'), then every command in your pipelines will start with the new default line-buffered stdout. That way your line-items from build.sh should get passed all the way through the pipeline as they are produced. Finally had a chance to try to build with 'stdbuf --output=L --error=L --' in front of the build script, and it caused some crazy problems. For what it's worth, when I was trying that out msys2 (since that's what you said you were using), I also ran into some very weird errors when just trying to export LD_PRELOAD and _STDBUF_O to what stdbuf -oL sets. It was weird because I didn't see issues when just running a command (including bash) directly under stdbuf. I didn't get to the bottom of it though and I don't have access to a windows laptop any more to experiment. Also I might ask, why are you setting "--error=L" ? Not that this is the problem you're seeing, but in any case stderr is unbuffered by default, and you might mess up the output a bit by line buffering it, if it's expecting to output partial lines for progress or whatever. Carl
Re: ls flag to show number of hard links
On Tue, 9 Apr 2024, Tony Freeman wrote: I seem to recall that some years ago ls -l would show the number of hard links to a file. Yes with "ls -l" it's the first number you see (after the permissions string). Or you can use "stat -c %h filename" to just grab that number specifically. Carl
Re: RFE: enable buffering on null-terminated data
On Tue, 19 Mar 2024, Zachary Santer wrote: On Tue, Mar 19, 2024 at 1:24 AM Kaz Kylheku wrote: But what tee does is set up _IONBF on its output streams, including stdout. So it doesn't buffer at all. Awesome. Nevermind. Yay! :D And since tee uses fwrite to copy whatever input is available, that will mean 'records' are output on the same boundaries as the input (whether that be newlines, nuls, or just block boundaries). So putting tee in the middle of a pipeline shouldn't itself interfere with whatever else you're up to. (AND it's still relatively efficient, compared to some tools like cut that putchar a byte at a time.) My note about pipelines like this though: $ ./build.sh | sed s/what/ever/ | tee build.log is that with the default stdio buffering, while all the commands in build.sh will be implicitly self-flushing, the sed in the middle will end up batching its output into blocks, so tee will also repeat them in blocks. However, if stdbuf's magic env vars are exported in your shell (either by doing a trick like 'export $(env -i stdbuf -oL env)', or else more simply by first starting a new shell with 'stdbuf -oL bash'), then every command in your pipelines will start with the new default line-buffered stdout. That way your line-items from build.sh should get passed all the way through the pipeline as they are produced. (But, proof's in the pudding, so whatever works for you :D ) Happy putting all the way! Carl
Re: RFE: enable buffering on null-terminated data
On Mon, 11 Mar 2024, Zachary Santer wrote:
On Mon, Mar 11, 2024 at 7:54 AM Carl Edquist
wrote:
(In my coprocess management library, I effectively run every coproc
with --output=L by default, by eval'ing the output of 'env -i stdbuf
-oL env', because most of the time for a coprocess, that's whats
wanted/necessary.)
Surrounded by 'set -a' and 'set +a', I guess? Now that's interesting.
Ah, no - I use the 'VAR=VAL command line' syntax so that it's specific to
the command (it's not left exported to the shell).
Effectively the coprocess commands are run with
LD_PRELOAD=... _STDBUF_O=L command line
This allow running shell functions for the command line, which will all
get the desired stdbuf behavior. Because you can't pass a shell function
(within the context of the current shell) as the command to stdbuf.
As far as I can tell, the stdbuf tool sets LD_PRELOAD (to point to
libstdbuf.so) and your custom buffering options in _STDBUF_{I,O,E}, in the
environment for the program it runs. The double-env thing there is just a
way to cleanly get exactly the env vars that stdbuf sets. The values
don't change, but since they are an implementation detail of stdbuf, it's
a bit more portable to grab the values this way rather than hard code
them. This is done only once per shell session to extract the values, and
save them to a private variable, and then they are used for the command
line as show above.
Of course, if "command line" starts with "stdbuf --output=0" or whatever,
that will override the new line-buffered default.
You can definitely export it to your shell though, either with 'set -a'
like you said, or with the export command. After that everything you run
should get line-buffered stdio by default.
I just added that to a script I have that prints lines output by another
command that it runs, generally a build script, to the command line, but
updating the same line over and over again. I want to see if it updates
more continuously like that.
So, a lot of times build scripts run a bunch of individual commands.
Each of those commands has an implied flush when it terminates, so you
will get the output from each of them promptly (as each command
completes), even without using stdbuf.
Where things get sloppy is if you add some stuff in a pipeline after your
build script, which results in things getting block-buffered along the
way:
$ ./build.sh | sed s/what/ever/ | tee build.log
And there you will definitely see a difference.
sloppy () {
for x in {1..10}; do sleep .2; echo $x; done |
sed s/^/:::/ | cat
}
{
echo before:
sloppy
echo
export $(env -i stdbuf -oL env)
echo after:
sloppy
}
Yeah, there's really no way to break what I'm doing into a standard
pipeline.
I admit I'm curious what you're up to :)
Of course, using line-buffered or unbuffered output in this situation
makes no sense. Where it might be useful in a pipeline is when an
earlier command in a pipeline might only print things occasionally, and
you want those things transformed and printed to the command line
immediately.
Right ... And in that case, losing the performance benefit of a larger
block buffer is a smaller price to pay.
My assumption is that line-buffering through setbuf(3) was implemented
for printing to the command line, so its availability to stdbuf(1) is
just a useful side effect.
Right, stdbuf(1) leverages setbuf(3).
setbuf(3) tweaks the buffering behavior of stdio streams (stdin, stdout,
stderr, and anything else you open with, eg, fopen(3)). It's not really
limited to terminal applications, but yeah it makes it easier to ensure
that your calls to printf(3) actually get output after each line (whether
that's to a file or a pipe or a tty), without having to call an explicit
fflush(3) of stdout every time.
stdbuf(1) sets LD_PRELOAD to libstdbuf.so for your program, causing it to
call setbuf(3) at program startup based on the values of _STDBUF_* in the
environment (which stdbuf(1) also sets).
(That's my read of it anyway.)
In the BUGS section in the man page for stdbuf(1), we see: On GLIBC
platforms, specifying a buffer size, i.e., using fully buffered mode
will result in undefined operation.
Eheh xD
Oh, I imagine "undefined operation" means something more like
"unspecified" here. stdbuf(1) uses setbuf(3), so the behavior you'll get
should be whatever the setbuf(3) from the libc on your system does.
I think all this means is that the C/POSIX standards are a bit loose about
what is required of setbuf(3) when a buffer size is specified, and there
is room in the standard for it to be interpreted as only a hint.
If I'm not mistaken, then buffer modes other than 0 and L don't actually
work. Maybe I should count my b
Re: RFE: enable buffering on null-terminated data
On Sun, 10 Mar 2024, Zachary Santer wrote: On Sun, Mar 10, 2024 at 4:36 PM Carl Edquist wrote: Out of curiosity, do you have an example command line for your use case? My use for 'stdbuf --output=L' is to be able to run a command within a bash coprocess. Oh, cool, now you're talking! ;) (Really, a background process communicating with the parent process through FIFOs, since Bash prints a warning message if you try to run more than one coprocess at a time. Shouldn't make a difference here.) (Kind of a side-note ... bash's limited coprocess handling was a long standing annoyance for me in the past, to the point that I wrote a bash coprocess management library to handle multiple active coprocess and give convenient methods for interaction. Perhaps the trickiest bit about multiple coprocesses open at once (which I suspect is the reason support was never added to bash) is that you don't want the second and subsequent coprocesses to inherit the pipe fds of prior open coprocesses. This can result in deadlock if, for instance, you close your write end to coproc1, but coproc1 continues to wait for input because coproc2 also has a copy of a write end of the pipe to coproc1's input. So you need to be smart about subsequent coprocesses first closing all fds associated with other coprocesses. Word to the wise: you might encounter this issue (coproc2 prevents coproc1 from seeing its end-of-input) even though you are rigging this up yourself with FIFOs rather than bash's coproc builtin.) See coproc-buffering, attached. Thanks! Without making the command's output either line-buffered or unbuffered, what I'm doing there would deadlock. I feed one line in and then expect to be able to read a transformed line immediately. If that transformed line is stuck in a buffer that's still waiting to be filled, then nothing happens. I swear doing this actually makes sense in my application. Yeah makes sense! I am familiar with the problem you're describing. (In my coprocess management library, I effectively run every coproc with --output=L by default, by eval'ing the output of 'env -i stdbuf -oL env', because most of the time for a coprocess, that's whats wanted/necessary.) ... Although, for your example coprocess use, where the shell both produces the input for the coproc and consumes its output, you might be able to simplify things by making the producer and consumer separate processes. Then you could do a simpler 'producer | filter | consumer' without having to worry about buffering at all. But if the producer and consumer need to be in the same process (eg they share state and are logically interdependent), then yeah that's where you need a coprocess for the filter. ... On the other hand, if the issue is that someone is producing one line at a time _interactively_ (that is, inputting text or commands from a terminal), then you might argue that the performance hit for unbuffered output will be insignificant compared to time spent waiting for terminal input. $ ./coproc-buffering 10 Line-buffered: real0m17.795s user0m6.234s sys 0m11.469s Unbuffered: real0m21.656s user0m6.609s sys 0m14.906s Yeah, this makes sense in your particular example. It looks like expand(1) uses putchar(3), so in unbuffered mode this translates to one write(2) call for every byte. sed(1) is not quite as bad - in unbuffered it appears to output the line and the newline terminator separately, so two write(2) calls for every line. So in both cases (but especially for expand), line buffering reduces the number of write(2) calls. (Although given your time output, you might say the performance hit for unbuffered is not that huge.) When I initially implemented this thing, I felt lucky that the data I was passing in were lines ending in newlines, and not null-terminated, since my script gets to benefit from 'stdbuf --output=L'. :thumbsup: Truth be told, I don't currently have a need for --output=N. Mmm-hmm :) Of course, sed and all sorts of other Linux command-line tools can produce or handle null-terminated data. Definitely. So in the general case, theoretically it seems as useful to buffer output on nul bytes. Note that for gnu sed in particular, there is a -u/--unbuffered option, which will effectively give you line buffered output, including buffering on nul bytes with -z/--null-data . ... I'll be honest though, I am having trouble imagining a realistic pipeline that filters filenames with embedded newlines using expand(1) ;) ... But, I want to be a good sport here and contrive an actual use case. So for fun, say I want to use cut(1) (which performs poorly when unbuffered) in a coprocess that takes null-terminated file paths on input and outputs the first directory component (which possibly contains embedded newlines). The basic command in the coprocess would be: cut -d/ -f1 -z but with the default
Re: RFE: enable buffering on null-terminated data
Hi Zack, This sounds like a potentially useful feature (it'd probably belong with a corresponding new buffer mode in setbuf(3)) ... Filenames should be passed between utilities in a null-terminated fashion, because the null byte is the only byte that can't appear within one. Out of curiosity, do you have an example command line for your use case? If I want to buffer output data on null bytes, the closest I can get is 'stdbuf --output=0', which doesn't buffer at all. This is pretty inefficient. I'm just thinking that find(1), for instance, will end up calling write(2) exactly once per filename (-print or -print0) if run under stdbuf unbuffered, which is the same as you'd get with a corresponding stdbuf line-buffered mode (newline or null-terminated). It seems that where line buffering improves performance over unbuffered is when there are several calls to (for example) printf(3) in constructing a single line. find(1), and some filters like grep(1), will write a line at a time in unbuffered mode, and thus don't seem to benefit at all from line buffering. On the other hand, cut(1) appears to putchar(3) a byte at a time, which in unbuffered mode will (like you say) be pretty inefficient. So, depending on your use case, a new null-terminated line buffered option may or may not actually improve efficiency over unbuffered mode. You can run your commands under strace like stdbuf --output=X strace -c -ewrite command ... | ... to count the number of actual writes for each buffering mode. Carl PS, "find -printf" recognizes a '\c' escape to flush the output, in case that helps. So "find -printf '%p\0\c'" would, for instance, already behave the same as "stdbuf --output=N find -print0" with the new stdbuf output mode you're suggesting. (Though again, this doesn't actually seem to be any more efficient than running "stdbuf --output=0 find -print0") On Sun, 10 Mar 2024, Zachary Santer wrote: Was "stdbuf feature request - line buffering but for null-terminated data" See below. On Sun, Mar 10, 2024 at 5:38 AM Pádraig Brady wrote: On 09/03/2024 16:30, Zachary Santer wrote: 'stdbuf --output=L' will line-buffer the command's output stream. Pretty useful, but that's looking for newlines. Filenames should be passed between utilities in a null-terminated fashion, because the null byte is the only byte that can't appear within one. If I want to buffer output data on null bytes, the closest I can get is 'stdbuf --output=0', which doesn't buffer at all. This is pretty inefficient. 0 means unbuffered, and Z is already taken for, I guess, zebibytes. --output=N, then? Would this require a change to libc implementations, or is it possible now? This does seem like useful functionality, but it would require support for libc implementations first. cheers, Pádraig
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
Hi Pádraig, O2 At your convenience, please see my attached patch set. ... A couple very small implementation details I guess I'll mention here. 1. In iopoll.c, I use isapipe() from gnulib, as is done in tail.c. I am not wild about the gnulib implementation of isapipe() - I would rather do a simple S_ISFIFO check, as the FIFO vs pipe distinction is irrelevant for iopoll(), and I'm not sure whether the "pipes are sockets" for Darwin 7.7 still needs to be supported. But I was happy to see (at least for me on Linux) it looks like the config.h macros end up compiling away the complicated parts of this function, so I guess it's not that bad :) 2. I believe the "if (errno == EINTR) continue;" logic in iopoll() is unnecessary, as an interrupted poll() or select() call will result in repeating the loop anyway. I've left it for clarity, but as far as I can tell it could be removed just the same. On Tue, 13 Dec 2022, Arsen Arsenović wrote: It might also be good to give a quick test on FreeBSD, since it has some popularity too. I don't have easy access to play with this, but yeah I think I've seen the BSD man page for poll(2), and I imagine poll would work there. If so we can tweak the IOPOLL_USES_POLL definition in iopoll.c to include an appropriate preprocessor check for BSD. If you prefer, I'll have some time in the latter part of this week too. Let's not forget to include the testcase posted previously (with -p instead of -P, since it was suggested to enable polling for -p): Hi Arsen, Would you like to add your test case and comments in a separate commit? Thanks! Carl ( sleep 5 | (timeout 3 tee -p 2>/dev/null && echo TEST_PASSED >&8) | : ) 8>&1 | grep -qx TEST_PASSED To annotate it, and let's include this info in a comment: - sleep emulates misbehaving input. - The timeout is our failure safety-net. - We ignore stderr from tee, and should have no stdout anyway. - If that succeeds, we print TEST_PASSED into FD 8 to grep for later. (FD 8 was selected by a D20 roll, or rather, a software emulation) - The colon is the immediately closed output process. - We redirect 8 back into stdout to grep it. If tee fails, for instance because it times out, or it fails to recognize -P for some reason, the echo simply won't run. The grep options are in POSIX (or, at least, in POSIX.1-2017). Thank you both, have a great night. -- Arsen Arsenović From d2a62057cacd25730243c20506edc66a27e3bf0d Mon Sep 17 00:00:00 2001 From: Carl Edquist Date: Thu, 15 Dec 2022 06:10:33 -0600 Subject: [PATCH 1/2] iopoll: broken pipe detection while waiting for input When a program's output becomes a broken pipe, future attempts to write to that ouput will fail (SIGPIPE/EPIPE). Once it is known that all future write attepts will fail (due to broken pipes), in many cases it becomes pointless to wait for further input for slow devices like ttys. Ideally, a program could use this information to exit early once it is known that future writes will fail. Introduce iopoll() to wait on a pair of fds (input & output) for input to become ready or output to become a broken pipe. This is relevant when input is intermittent (a tty, pipe, or socket); but if input is always ready (a regular file or block device), then a read() will not block, and write failures for a broken pipe will happen normally. Introduce iopoll_input_ok() to check whether an input fd is relevant for iopoll(). Experimentally, broken pipes are only detectable immediately for pipes, but not sockets. Errors for other file types will be detected in the usual way, on write failure. Introduce iopoll_output_ok() to check whether an output fd is suitable for iopoll() -- namely, whether it is a pipe. iopoll() is best implemented with a native poll(2) where possible, but fall back to a select(2)-based implementation platforms where there are portability issues. See also discussion in tail.c. In general, adding a call to iopoll() before a read() in filter programs also allows broken pipes to "propagate" backwards in a shell pipeline. * src/iopoll.c, src/iopoll.h (iopoll): New function implementing broken pipe detection on output while waiting for input. (IOPOLL_BROKEN_OUTPUT, IOPOLL_ERROR): Return codes for iopoll(). (IOPOLL_USES_POLL): Macro for poll() vs select() implementation. (iopoll_input_ok): New function to check whether an input fd is relevant for iopoll(). (iopoll_output_ok): New function to check whether an input fd is suitable for iopoll(). * src/local.mk (noinst_HEADERS): add src/iopoll.h. --- src/iopoll.c | 128 +++ src/iopoll.h | 6 +++ src/local.mk | 1 + 3 files changed, 135 insertions(+) create mode 100644 src/iopoll.c create mode 100644 src/iopoll.h diff --git a/src/iopoll.c b/src/iopoll.c new file mode 100644 index 000..32462d8 --- /dev/null +++ b/src/iopoll.c @@ -0,0 +1,128 @@ +/* iopoll.c -- broke
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
On Tue, 13 Dec 2022, Pádraig Brady wrote:
Thanks to both of you for working through the details on this. This does
seem like a useful feature, and would be appropriate to add.
A summarized patch set would be appreciated at this stage, thanks.
Re HAVE_INOTIFY, that's really a proxy for a linux kernel, and so would
be most appropriately changed to:
defined __linux__ || defined __ANDROID__
I'm thinking these hardcoded defines would be best for now at least as
it covers the vast majority of systems, and avoids complicated (cross)
compile time checks.
A modularised iopoll.c would be better, given the potential uses by
other tools, though we'd probably release for just tee initially.
As for interface to this functionality I'm wondering if we could just
have the existing tee {-p,--output-error} imply the use of poll() on
output.
I.e. from a high level -p just means to deal more appropriately with non
file outputs, and as part of that, dealing immediately with closed
outputs would be an improvement.
Note also tail(1) enables this functionality by default. I'm not sure
about other utilities, but we can deal with that later if needed.
Thanks Pádraig for the feedback - that all sounds good.
Will try to follow-up sometime this week...
Carl
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
On Mon, 12 Dec 2022, Arsen Arsenović wrote: Hi Carl, Carl Edquist writes: [2. text/x-diff; 0001-tee-only-fstat-outputs-if-pipe_check-is-active.patch]... [3. text/x-diff; 0002-tee-skip-pipe-checks-if-input-is-always-ready-for-re.patch]... Thanks for writing these, and the other patches. I've once again been stripped of time, but I think we've nailed the concept down for the most part. I think we should wait for Pádraig to voice his opinion at this point. Details can be ironed out later, and pretty easily too. Sure... Sorry for throwing so many patches at the list :) I did have in mind also to send a summarized patch series in a single email to make reviewing everything a bit easier. Pádraig is the gatekeeper here, in any case. The main question on my mind currently is still what's the best way to do the platform preprocessor logic for poll vs select ... (assuming HAVE_INOTIFY is only in tail.c for "tail -f" mode) And, i don't know if this would help "sell" it or not, but locally i have separated out the iopoll implementation to a separate iopoll.c/.h, and as an example successfully added a --pipe-check option to cat and tr. (Not to pressure anyone to take it, but, sometimes it's easier to tell if you like an idea or not when you have a patch to look at...) Personally, i see this broken-pipe detection being useful in tee, cat, sed, and grep for starters. (Though i know sed and grep live outside coreutils.) In some ways i feel like tee is actually the most immediate use case though. cat and sed mostly came into play for me as a way to type commands into a socket; though as it seems to me now, "broken-pipe" detection for a socket may not even be possible. All for now. Have a good one, Carl
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
On Mon, 12 Dec 2022, Arsen Arsenović wrote: Hi Rob, Rob Landley writes: This sort of thing is why I added -i to toybox's "timeout" command: -i Only kill for inactivity (restart timeout when command produces output) It runs the command's stdout through a pipe and does a poll() with the -i seconds value, and signals the program if the poll() expires. The android guys found it useful, but I was waiting to hear back about "cut -DF" before bringing it up here... That's interesting, might be worth adding to the GNU timeout, however, it's not appropriate for what I'm using tee for, since compiler processes could appear idle for a long time, if doing LTO for instance. Thanks Rob for the idea. Sounds like it could be useful if you know what an appropriate timeout ought to be ... though in the general data-driven case i feel like filters should only quit for relevant events like end-of-input or write failure. (The broken-pipe detection idea being an extension that detects the point when future writes will fail.) Sounds like a neat idea to add to a timeout command (where you know the output timeout you want) if it can be made to work with arbitrary programs. Carl
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
On Fri, 9 Dec 2022, Carl Edquist wrote:
On Fri, 9 Dec 2022, Arsen Arsenović wrote:
... Also, i suspect that the pipe_check option can be disabled if the
_input_ is a regular file (or block device), since (i think) these
always show up as ready for reading. (This check would only need to
be done once for fd 0 at program start.)
Yes, there's no point poll-driving those, since it'll be always
readable, up to EOF, and never hesitate to bring more data. It might
just end up being a no-op if used in current form (but I haven't
tried).
Well currently if the input is a regular file, poll() immediately
returns POLLIN, along with any potential errors for the output. But yes
the net effective behavior is the same as if the poll() call had been
skipped.
In a way i don't think it's so necessary (as it's just an optimization,
and it's only relevant if the user calls tee with -P/--pipe-check despite
stdin being a regular file or block device), but anyway my attached patch
(0002-tee-skip-pipe-checks-if-input-is-always-ready-for-re.patch) adds
logic to disable pipe_check mode in this case. (Though in order not to
change the SIGPIPE semantics, -P still implies -p.)
CarlFrom fb3227b0e252b897f60053a6ffae48eb6ec19e62 Mon Sep 17 00:00:00 2001
From: Carl Edquist
Date: Sat, 10 Dec 2022 10:00:12 -0600
Subject: [PATCH 1/2] tee: only fstat outputs if pipe_check is active
* src/tee.c (tee_files): avoid populating out_is_pipe array if
pipe_check is not enabled.
---
src/tee.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/tee.c b/src/tee.c
index 482967a..1f5c171 100644
--- a/src/tee.c
+++ b/src/tee.c
@@ -313,10 +313,12 @@ tee_files (int nfiles, char **files)
In both arrays, entry 0 corresponds to standard output. */
descriptors = xnmalloc (nfiles + 1, sizeof *descriptors);
- out_is_pipe = xnmalloc (nfiles + 1, sizeof *out_is_pipe);
+ if (pipe_check)
+out_is_pipe = xnmalloc (nfiles + 1, sizeof *out_is_pipe);
files--;
descriptors[0] = stdout;
- out_is_pipe[0] = is_pipe (fileno (descriptors[0]));
+ if (pipe_check)
+out_is_pipe[0] = is_pipe (fileno (descriptors[0]));
files[0] = bad_cast (_("standard output"));
setvbuf (stdout, NULL, _IONBF, 0);
n_outputs++;
@@ -325,7 +327,8 @@ tee_files (int nfiles, char **files)
{
/* Do not treat "-" specially - as mandated by POSIX. */
descriptors[i] = fopen (files[i], mode_string);
- out_is_pipe[i] = is_pipe (fileno (descriptors[i]));
+ if (pipe_check)
+out_is_pipe[i] = is_pipe (fileno (descriptors[i]));
if (descriptors[i] == NULL)
{
error (output_error == output_error_exit
@@ -397,7 +400,8 @@ tee_files (int nfiles, char **files)
}
free (descriptors);
- free (out_is_pipe);
+ if (pipe_check)
+free (out_is_pipe);
return ok;
}
--
2.9.0
From 07853a6624ca805497ad5438dfc3092c3fdf41bc Mon Sep 17 00:00:00 2001
From: Carl Edquist
Date: Sat, 10 Dec 2022 10:21:48 -0600
Subject: [PATCH 2/2] tee: skip pipe checks if input is always ready for
reading
Regular files and block devices are always ready for reading, even
at EOF where they just return 0 bytes. Thus if the input is one of
these, it's never necessary to poll outputs, since we will never be
in a situation stuck waiting for input while all remaining outputs
are broken pipes.
Note that -P/--pipe-check still implies -p in this case, otherwise
a SIGPIPE would kill the process early.
* src/tee.c (always_ready): helper function to test an fd file type.
(main): disable pipe_check if STDIN is always ready for reading.
---
src/tee.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/tee.c b/src/tee.c
index 1f5c171..f09324e 100644
--- a/src/tee.c
+++ b/src/tee.c
@@ -47,6 +47,7 @@
#define IOPOLL_ERROR -3
static bool tee_files (int nfiles, char **files);
+static bool always_ready (int fd);
/* If true, append to output files rather than truncating them. */
static bool append;
@@ -184,6 +185,11 @@ main (int argc, char **argv)
if (output_error != output_error_sigpipe)
signal (SIGPIPE, SIG_IGN);
+ /* No need to poll outputs if input is always ready for reading. */
+
+ if (pipe_check && always_ready (STDIN_FILENO))
+ pipe_check = false;
+
/* Do *not* warn if tee is given no file arguments.
POSIX requires that it work when given no arguments. */
@@ -276,13 +282,25 @@ fail_output(FILE **descriptors, char **files, int i)
return fail;
}
-/* Return true if fd refers to a pipe */
+/* Return true if fd refers to a pipe. */
static bool
is_pipe(int fd)
{
struct stat st;
- return fstat (fd, ) == 0 && S_ISFIFO(st.st_mode);
+ return fstat (fd, ) == 0 && S_ISFIFO (st.st_mode);
+}
+
+
+/* Return true if fd is always ready for reading,
+ ie, it is a regular file or block device. */
+
+static bool
+always_ready(int fd)
+{
+ struct sta
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
On Fri, 9 Dec 2022, Carl Edquist wrote:
A quick note, this check only needs to be done a total of once per
output, it shouldn't be done inside iopoll(), which would result in
an additional redundant fstat() per read().
Could this be handled by get_next_out?
Sure, either in that function or immediately after it gets called. But
also once for stdout before the while (n_outputs) loop. Alternatively,
allocate a file type array and populate it in the for loop that does all
the fopen() calls.
Patch attached for this. I ended up going with a bool array to keep track
of whether each output is a pipe or not. Potentially a very small amount
of extra work up front, but it seemed to make implementing the rest of it
simpler.
CarlFrom 488e1398da6a15b70399d52346b05d9770db4732 Mon Sep 17 00:00:00 2001
From: Carl Edquist
Date: Sat, 10 Dec 2022 09:31:03 -0600
Subject: [PATCH] tee: only do iopoll for pipe outputs
We can skip the broken-pipe detection for file types other than pipes.
Sockets behave similarly to pipes in some ways, but the semantics
appear to be different in important ways, such that poll() doesn't
detect when the remote read end is closed.
* src/tee.c (is_pipe): add helper function detecting if fd is a pipe.
* (tee_files): populate out_is_pipe bool array corresponding to whether
items in descriptors array are pipes, only do pipe_check/iopoll() logic
for a given output if it is a pipe.
---
src/tee.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/src/tee.c b/src/tee.c
index b254466..482967a 100644
--- a/src/tee.c
+++ b/src/tee.c
@@ -276,6 +276,15 @@ fail_output(FILE **descriptors, char **files, int i)
return fail;
}
+/* Return true if fd refers to a pipe */
+
+static bool
+is_pipe(int fd)
+{
+ struct stat st;
+ return fstat (fd, ) == 0 && S_ISFIFO(st.st_mode);
+}
+
/* Copy the standard input into each of the NFILES files in FILES
and into the standard output. As a side effect, modify FILES[-1].
Return true if successful. */
@@ -285,6 +294,7 @@ tee_files (int nfiles, char **files)
{
size_t n_outputs = 0;
FILE **descriptors;
+ bool *out_is_pipe;
char buffer[BUFSIZ];
ssize_t bytes_read = 0;
int i;
@@ -303,8 +313,10 @@ tee_files (int nfiles, char **files)
In both arrays, entry 0 corresponds to standard output. */
descriptors = xnmalloc (nfiles + 1, sizeof *descriptors);
+ out_is_pipe = xnmalloc (nfiles + 1, sizeof *out_is_pipe);
files--;
descriptors[0] = stdout;
+ out_is_pipe[0] = is_pipe (fileno (descriptors[0]));
files[0] = bad_cast (_("standard output"));
setvbuf (stdout, NULL, _IONBF, 0);
n_outputs++;
@@ -313,6 +325,7 @@ tee_files (int nfiles, char **files)
{
/* Do not treat "-" specially - as mandated by POSIX. */
descriptors[i] = fopen (files[i], mode_string);
+ out_is_pipe[i] = is_pipe (fileno (descriptors[i]));
if (descriptors[i] == NULL)
{
error (output_error == output_error_exit
@@ -329,7 +342,7 @@ tee_files (int nfiles, char **files)
while (n_outputs)
{
- if (pipe_check)
+ if (pipe_check && out_is_pipe[first_out])
{
int err = iopoll (STDIN_FILENO, fileno (descriptors[first_out]));
@@ -384,6 +397,7 @@ tee_files (int nfiles, char **files)
}
free (descriptors);
+ free (out_is_pipe);
return ok;
}
--
2.9.0
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
On Fri, 9 Dec 2022, Carl Edquist wrote: On Fri, 9 Dec 2022, Arsen Arsenović wrote: Originally i had imagined (or hoped) that this broken-pipe detection could also be used for sockets (that was how the issue came up for me), but it seems the semantics for sockets are different than for pipes. This might require POLLPRI or POLLRDHUP or such. Can you try with those to the set of events in pollfd? Oh interesting! I had assumed these wouldn't help - POLLPRI is for OOB data, and, I had assumed POLLRDHUP was only for polling the read side of a socket (thus POLL*RD*HUP), to determine if the remote write end was shutdown. But you are right, poll() returns with POLLRDHUP in revents as soon as the remote end is closed. Thanks for the tip! I assume the reason this works is, even though i have in mind that i am monitoring the socket as an output, the socket serves as an input also (it's open for RW). So, if my interpretation is correct, POLLRDHUP is actually monitoring the local read side of the socket. And so, poll() is actually detecting the remote write side getting shutdown. This is not technically the same thing as monitoring the local output side, but if the remote end of the socket closes entirely (shutting down the remote read and write ends together), then that tells us about the local output side as well. This definitely seems to work for the case i was playing with, though i'm not sure if it would behave as intended if the remote side only shut down its read or write end (but not both). Alright, i got a chance to play around with this more locally with socketpairs. Unfortunately, i've confirmed that POLLRDHUP is clearly intended for monitoring the local read end of a socket, not the local write end. (Thus it is not what we want for monitoring an output.) In each case below, i started with a socket open for RW on both ends, and poll()ed the local socket fd with POLLRDHUP. I observed the following: - shutdown local read -> POLLRDHUP - shutdown local write -> nothing detected - shutdown remote read -> nothing detected - shutdown remote write -> POLLRDHUP This is exactly the reverse of what we want when monitoring a socket as an output. Shutting down the local read end should be fine (since we are using it for output). Shutting down the local write end should be considered like a broken pipe, since nothing further can be sent. Shutting down the remote read end should also be considered like a broken pipe, as with a normal pipe. Shutting down the remote write end should be fine, since supposedly the local end of the socket is used for output. Again, the reason POLLRDHUP worked in my case is that the remote end closed completely (shutting down remote read and write). But for cases where either end (local or remote) shuts down only read or write, using POLLRDHUP checks for exactly the wrong thing. It seems that there ought to be a "POLLWRHUP", but as far as i can tell that does not exist. ... My conclusion (as far as i can tell) is that we can't do this iopoll jazz correctly for sockets. The up side though is that that makes it ok to explicitly check that the output is a pipe, and then only worry about pipe semantics. Which makes the select-based implementation safe, since pipe fds are not open for RW. Carl
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
Hi Pádraig,
Getting back to this portability question:
On Fri, 2 Dec 2022, Pádraig Brady wrote:
Anyway if it's possible just to use poll(2) (the system one, not the
gnulib replacement), that might simplify the portability logic.
Yes it would be better to use poll() if possible,
and that's what I attempted with tail(1) recently.
From the gnulib docs on poll():
"
Portability problems fixed by Gnulib:
This function is missing on some platforms:
mingw, MSVC 14, HP NonStop.
This function doesn't work on special files like @file{/dev/null} and ttys
like
@file{/dev/tty} on some platforms:
AIX 5.3, Mac OS X 10.4.0
Portability problems not fixed by Gnulib:
Under Windows, when passing a pipe, Gnulib's @code{poll} replacement might
return 0 even before the timeout has passed. Programs using it with pipes
can
thus busy wait.
On some platforms, file descriptors other than sockets do not support
POLLHUP; they will return a "readable" or "writable" status instead:
AIX 7.2, HP NonStop.
"
So to use poll() everywhere we'd need the gnulib module.
But empirically the replacement didn't work on macos at least
for this use case, and we know the select emulation wouldn't work on AIX.
So portable use of poll() will be awkward.
Don't remember if i said so already or not, but thanks for the summary!
In my last patch i made a first attempt to define a
HAVE_POLL_PIPE_CLOSE_DETECTION in configure.ac using AC_RUN_IFELSE to test
poll() functionality. Arsen pointed out that this does not work for
cross-compiling. (That's too bad, but, i think i understand now why it's
a problem.)
We could still do just an AC_COMPILE_IFELSE to test whether native poll()
is available, though if i understand your summary right there are a number
of platforms where native poll() is available but won't work correctly
here.
So, if we need to separate out the poll() vs select() implementations
using a list of platforms, do we have a good idea what list of platforms
are "ok for native poll pipe detection", or alternatively a list of
platforms that are not ok?
So tail.c uses:
#if defined _AIX || defined __sun || defined __APPLE__ || HAVE_INOTIFY
but as far as i can tell HAVE_INOTIFY relates to the use of inotify in
tail, which is specific to "tail -f" mode, and perhaps not relevant to the
logic in tee (or other filters).
The section you quoted "From the gnulib docs on poll()" makes it sound
like there might be problems with: Windows, mingw, MSVC 14, HP NonStop,
AIX 5.3 & 7.2, and Mac OS X 10.4.0. (Already that makes me wonder about
_AIX and __APPLE__ being in the list of platforms to use poll().)
Anyway, just trying to get a handle on what might be a good way to do the
preprocessor macro logic to decide between the poll() and select()
implementations.
I guess in worst case, it sounds like select() could be used everywhere.
And there is some appeal to having a single implementation. But select()
does seem less efficient if poll() is available; for instance with
select(), two fd_sets need to be zeroed out (128 bytes each) before every
call to read(), compared to the poll() version that only needs to
initialize two struct pollfds (8 bytes each). So i still see some value
in doing poll() where possible.
Any thoughts on how to put together preprocessor logic that gets at the
heart of the issue?
Thanks,
Carl
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
On Fri, 9 Dec 2022, Arsen Arsenović wrote:
Similar to the situation here, i was seeing things annoyingly look like
they are still 'alive' longer than they ought to be when providing
input from the terminal.
Huh, I never tried that, honestly.
Here is a simple example:
exec 3<> /dev/tcp/example.com/80
echo_crlf () { printf "%s\r\n" "$@"; }
{ echo_crlf "GET / HTTP/1.1"
echo_crlf "Host: example.com"
echo_crlf "Connection: close"
echo_crlf ""
} >&3
cat <&3
In this example, i'm sending the input to the socket (on fd 3) directly
using the 'printf' builtin and shell redirection, and i request the server
close its side of the connection after this request.
But if i want to type the request by hand, i might do:
sed -u 's/$/\r/' >&3
to read from the terminal, convert line terminators to CRLF, and write to
the socket. Only, similar to the situation with tee and pipes, sed here
will not detect when the remote read end of the socket has closed, so it
hangs waiting for more input.
Did polling help your use-case?
Mostly no, as i kind of got into later last time. It seems the semantics
are different for sockets than pipes; for some reason it seems the socket
doesn't behave like a broken pipe until after the remote read end shuts
down and then another write is made to the local end. Only after these
does poll() seem to detect the broken-pipe -like state.
Oh interesting. That wasn't on my radar at all. I guess this means
that when cross-compiling, the configure script is run on the
cross-compiling host, rather than on the target platform; so any test
programs in configure.ac with AC_RUN_IFELSE don't necessarily check the
target platform functionality (?)
Or worse, is unable to run at all (and always fails), if the binary is
for a different kernel or architecture.
Ok, so i guess for the sake of cross-compiling we are limited to
compile/link checks.
Originally i had imagined (or hoped) that this broken-pipe detection
could also be used for sockets (that was how the issue came up for me),
but it seems the semantics for sockets are different than for pipes.
This might require POLLPRI or POLLRDHUP or such. Can you try with those
to the set of events in pollfd?
Oh interesting! I had assumed these wouldn't help - POLLPRI is for OOB
data, and, I had assumed POLLRDHUP was only for polling the read side of a
socket (thus POLL*RD*HUP), to determine if the remote write end was
shutdown. But you are right, poll() returns with POLLRDHUP in revents as
soon as the remote end is closed. Thanks for the tip!
I assume the reason this works is, even though i have in mind that i am
monitoring the socket as an output, the socket serves as an input also
(it's open for RW). So, if my interpretation is correct, POLLRDHUP is
actually monitoring the local read side of the socket. And so, poll() is
actually detecting the remote write side getting shutdown.
This is not technically the same thing as monitoring the local output
side, but if the remote end of the socket closes entirely (shutting down
the remote read and write ends together), then that tells us about the
local output side as well.
This definitely seems to work for the case i was playing with, though i'm
not sure if it would behave as intended if the remote side only shut down
its read or write end (but not both).
Also, my bits/poll.h seems to suggest this is a Linux extension:
#ifdef __USE_GNU
/* These are extensions for Linux. */
# define POLLMSG0x400
# define POLLREMOVE 0x1000
# define POLLRDHUP 0x2000
#endif
Anyway, this is all Good To Know. I don't know what the semantics with
poll() for sockets are supposed to be in the general case (beyond Linux),
so i don't feel i'm in a good position to advocate for it in coreutils.
But maybe someone who knows better can comment on this topic.
A quick note, this check only needs to be done a total of once per
output, it shouldn't be done inside iopoll(), which would result in an
additional redundant fstat() per read().
Could this be handled by get_next_out?
Sure, either in that function or immediately after it gets called. But
also once for stdout before the while (n_outputs) loop. Alternatively,
allocate a file type array and populate it in the for loop that does all
the fopen() calls.
Either way, the idea would be to then replace
if (pipe_check)
with something like
if (pipe_check && first_out_is_pipe)
or (with a file type array)
if (pipe_check && S_ISFIFO (output_types[first_out]))
... Also, i suspect that the pipe_check option can be disabled if the
_input_ is a regular file (or block device), since (i think) these
always show up as ready for reading. (This check would only need to be
done once for fd 0 at program start.)
Yes, there's no point poll-driving those, since it'll be always
readable, up to EOF, and never hesitate to bring more data. It might
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
On Thu, 8 Dec 2022, Arsen Arsenović wrote:
Apologies for my absence, Tuesdays and Wednesdays are long workdays for
me.
No need for apologies - I feel like i am the one who should apologize for
my high volume of email to the list. People have lives after all! :)
The timing of this thread caught my attention because I had recently been
wrestling with a similar issue, trying to use shell utils to talk over a
socket with the help of bash's /dev/tpc/host/port interface.
Similar to the situation here, i was seeing things annoyingly look like
they are still 'alive' longer than they ought to be when providing input
from the terminal.
Biggest item is making a new configure macro based on whether poll() is
present and and works as intended for pipes. With 0 timeout, polling
the write-end of a pipe that is open on both ends for errors does not
indicate a broken pipe; but polling the write-end of a pipe with the
read-end closed does indicate a broken pipe.
This might be a bit problematic when cross compiling (which is why I
imagine systems were hard-coded before).
Oh interesting. That wasn't on my radar at all. I guess this means that
when cross-compiling, the configure script is run on the cross-compiling
host, rather than on the target platform; so any test programs in
configure.ac with AC_RUN_IFELSE don't necessarily check the target
platform functionality (?)
That's too bad. I had hoped to come up with a better way to indicate a
working poll() for this feature than maintaining a list of platforms.
So I guess (on Linux at least) that means a "readable event on STDOUT" is
equivalent to (POLLRDNORM | POLLRDBAND | POLLIN | POLLHUP | POLLERR).
So, it'll catch the relevant poll() errors (POLLHUP | POLLERR), but
the inclusion of POLLIN results in the gotcha that it will be a false
positive if stdout is already open for RW (eg a socket) and there is
actually data ready.
Ah - yes. tail.c guards against this by checking the type of the file
descriptor before selecting it, and makes sure it's among the "one-way"
file descriptors:
if (forever && ignore_fifo_and_pipe (F, n_files))
{
/* If stdout is a fifo or pipe, then monitor it
so that we exit if the reader goes away. */
struct stat out_stat;
if (fstat (STDOUT_FILENO, _stat) < 0)
die (EXIT_FAILURE, errno, _("standard output"));
monitor_output = (S_ISFIFO (out_stat.st_mode)
|| (HAVE_FIFO_PIPES != 1 && isapipe (STDOUT_FILENO)));
Good catch! It completely slipped by my mind.
Ah, yeah if we know it's a pipe we shouldn't have to worry about an output
being open for RW.
Originally i had imagined (or hoped) that this broken-pipe detection could
also be used for sockets (that was how the issue came up for me), but it
seems the semantics for sockets are different than for pipes.
Experimentally, it seems that once the remote read end of the socket is
shutdown, poll() does not detect a broken pipe - it will wait
indefinitely. But at this point if a write() is done on the local end of
the socket, the first write() will succeed, and then _after_ this it will
behave like a broken pipe - poll() returns POLLERR|POLLHUP, and write()
results in SIGPIPE/EPIPE.
It's fairly confusing. But it seems due to the difference in semantics
with sockets, likely this broken-pipe detection will only really work
properly for pipes.
So yeah, back to your point, there is a little room for improvement here
by fstat()ing the output and only doing the iopoll() waiting if the output
is a pipe.
A quick note, this check only needs to be done a total of once per output,
it shouldn't be done inside iopoll(), which would result in an additional
redundant fstat() per read().
... Also, i suspect that the pipe_check option can be disabled if the
_input_ is a regular file (or block device), since (i think) these always
show up as ready for reading. (This check would only need to be done once
for fd 0 at program start.)
But ... one step at a time! :)
Carl
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
Alright, lest I be guilty of idle nay-saying, I've attached another patch
to address all of my complaints.
(Apply it after Arsen's last one, which comes after my previous one.
Otherwise if desired I can send a single summary patch.)
Biggest item is making a new configure macro based on whether poll() is
present and and works as intended for pipes. With 0 timeout, polling the
write-end of a pipe that is open on both ends for errors does not indicate
a broken pipe; but polling the write-end of a pipe with the read-end
closed does indicate a broken pipe.
Hopefully this test covers all platforms. The only part that I'm iffy
about was this bit that Pádraig mentioned:
Portability problems fixed by Gnulib:
This function doesn't work on special files like @file{/dev/null} and
ttys like @file{/dev/tty} on some platforms: AIX 5.3, Mac OS X 10.4.0
If indeed there are issues on these platforms using poll() against these
special files (character devices), I suspect we can just test the relevant
behavior in the test program for the new configure macro. (eg, poll() for
/dev/null should always show POLLIN when open for reading, and should not
returning any errors when open for writing.)
Beyond that, I revised the select()-based implementation of iopoll() to
address my previous comments. Sorry I got my grubby hands all over it.
I do hope you'll like it though! :)
Cheers,
Carl
On Tue, 6 Dec 2022, Carl Edquist wrote:
(4.)
+ /* readable event on STDOUT is equivalent to POLLERR,
+ and implies an error condition on output like broken pipe. */
I know this is what the comment from tail.c says, but is it actually
documented to be true somewhere? And on what platforms? I don't see it
being documented in my select(2) on Linux, anyway. (Though it does seem
to work.) Wondering if this behavior is "standard".
Ah!
Well, it's not documented in my (oldish) select(2), but I do find the
following in a newer version of that manpage:
Correspondence between select() and poll() notifications
Within the Linux kernel source, we find the following definitions which
show the correspondence between the readable, writable, and exceptional
condition notifications of select() and the event notifications pro-
vided by poll(2) (and epoll(7)):
#define POLLIN_SET (POLLRDNORM | POLLRDBAND | POLLIN | POLLHUP |
POLLERR)
/* Ready for reading */
#define POLLOUT_SET (POLLWRBAND | POLLWRNORM | POLLOUT | POLLERR)
/* Ready for writing */
#define POLLEX_SET (POLLPRI)
/* Exceptional condition */
So I guess (on Linux at least) that means a "readable event on STDOUT" is
equivalent to (POLLRDNORM | POLLRDBAND | POLLIN | POLLHUP | POLLERR).
So, it'll catch the relevant poll() errors (POLLHUP | POLLERR), but the
inclusion of POLLIN results in the gotcha that it will be a false positive if
stdout is already open for RW (eg a socket) and there is actually data ready.
Also, the POLLEX_SET definition of (POLLPRI) doesn't seem relevant; so I
might suggest removing the 'xfd' arg for the select()-based implementation:
POLLPRI
There is some exceptional condition on the file descriptor.
Possibilities include:
* There is out-of-band data on a TCP socket (see tcp(7)).
* A pseudoterminal master in packet mode has seen a state
change on the slave (see ioctl_tty(2)).
* A cgroup.events file has been modified (see cgroups(7)).
Of course, those definitions live in the Linux kernel source, and on Linux
we'd get to use poll() instead of select().
Don't know if the select() <-> poll() correspondence differs at all on other
platforms .. ?
(But, this does give me a bit more confidence about select() being a (mostly)
viable alternative.)
Carl
From 26f991a2a3d8c3d4ea6300b5e00e47276d7544be Mon Sep 17 00:00:00 2001
From: Carl Edquist
Date: Wed, 7 Dec 2022 09:01:35 -0600
Subject: [PATCH] tee: define and use HAVE_POLL_PIPE_CLOSE_DETECTION
It's preferable to use native poll(2) wherever possible for broken pipe
detection, otherwise fall back to using select(2). Rather than doing
the preprocessor logic using a collection of macros representing
different platforms, simply define a macro at configure time based on
whether or not poll can be used for this purpose.
While we're at it, revise the select()-based implementation of iopoll().
('xfd' shouldn't be used; ret isn't needed; some comments needed
clarification; and a few items were arranged to match the poll()-based
version of iopoll().)
* configure.ac (HAVE_POLL_PIPE_CLOSE_DETECTION): New macro based on
program testing whether poll() can be used to detect broken pipes.
* src/tee.c (headers): include sys/select.h when using select()-based
implementation of iopoll().
(iopoll): use HAVE_POLL_PIPE_CLOSE_DETECTION macro to determine poll()
or select()-ba
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
On Tue, 6 Dec 2022, Carl Edquist wrote: (4.) + /* readable event on STDOUT is equivalent to POLLERR, + and implies an error condition on output like broken pipe. */ I know this is what the comment from tail.c says, but is it actually documented to be true somewhere? And on what platforms? I don't see it being documented in my select(2) on Linux, anyway. (Though it does seem to work.) Wondering if this behavior is "standard". Ah! Well, it's not documented in my (oldish) select(2), but I do find the following in a newer version of that manpage: Correspondence between select() and poll() notifications Within the Linux kernel source, we find the following definitions which show the correspondence between the readable, writable, and exceptional condition notifications of select() and the event notifications pro- vided by poll(2) (and epoll(7)): #define POLLIN_SET (POLLRDNORM | POLLRDBAND | POLLIN | POLLHUP | POLLERR) /* Ready for reading */ #define POLLOUT_SET (POLLWRBAND | POLLWRNORM | POLLOUT | POLLERR) /* Ready for writing */ #define POLLEX_SET (POLLPRI) /* Exceptional condition */ So I guess (on Linux at least) that means a "readable event on STDOUT" is equivalent to (POLLRDNORM | POLLRDBAND | POLLIN | POLLHUP | POLLERR). So, it'll catch the relevant poll() errors (POLLHUP | POLLERR), but the inclusion of POLLIN results in the gotcha that it will be a false positive if stdout is already open for RW (eg a socket) and there is actually data ready. Also, the POLLEX_SET definition of (POLLPRI) doesn't seem relevant; so I might suggest removing the 'xfd' arg for the select()-based implementation: POLLPRI There is some exceptional condition on the file descriptor. Possibilities include: * There is out-of-band data on a TCP socket (see tcp(7)). * A pseudoterminal master in packet mode has seen a state change on the slave (see ioctl_tty(2)). * A cgroup.events file has been modified (see cgroups(7)). Of course, those definitions live in the Linux kernel source, and on Linux we'd get to use poll() instead of select(). Don't know if the select() <-> poll() correspondence differs at all on other platforms .. ? (But, this does give me a bit more confidence about select() being a (mostly) viable alternative.) Carl
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
Hi Arsen, thanks for your feedback! On Tue, 6 Dec 2022, Arsen Arsenović wrote: The stubborn part of me might say, for platforms that do not natively support poll(2), we could simply leave out support for this feature. If that's not acceptable, we could add a select(2)-based fallback for platforms that do not have a native poll(2). There's no need to omit it. iopoll() seems sufficiently easy to implement via select(): So, you're right... I think the stubborn part of me was reacting (badly) to what looked to be a messy situation in tail.c's mix of poll() and select(). I got further disenchanted when i saw gnulib's select()-based emulation of poll() :( But hey, if it works... Anyway a few comments: (1.) +#if defined _AIX || defined __sun || defined __APPLE__ || HAVE_INOTIFY I know this is what tail.c has, but I would like to seriously rethink this. What do these macros have to do with poll()? (And just as importantly, how can you tell by looking at it?) First of all, I think poll(2) (the native system one, not the gnulib emulation) should be used wherever possible. Which is to say, select() should only be used where native poll() is unavailable (or if it's "available" but broken in some important way). I believe the HAVE_INOTIFY bit is specific to tail.c, which relates to its inotify mode. I don't think it should be part of the equation here in tee.c. The remaining macros _AIX, __sun, __APPLE__ -- do these cover all systems that have a working native poll() -- eg, Linux and the BSDs? (I suspect not?) And they do not clearly communicate anything to the reader about why they were picked / what they represent in relation to the availability of poll(). *Ideally,* there might be some configure macro like HAVE_POLL, except one that refers specifically to the system one and not the gnulib emulation. Something like "HAVE_NATIVE_poll". Then it would be clear what the preprocessor logic is trying to accomplish. (2.) if we want to use #if preprocessor logic (and preferably "#if HAVE_NATIVE_poll") for this poll/select fallback mechanism, I'd suggest to include each entire version of the function (including the function header and comments) in separate preprocessor blocks. (One complete function in #if, and the other in #else.) The iopoll() implementation I provided is straightforward by itself, but its readability gets trampled badly when trying to mix the body of the select()-based implementation in with it. Just my 2 cents. (3.) + FD_SET(fdout, ); + FD_SET(fdin, ); Is it documented somewhere that the 'exceptfds' argument (xfd) to select(2) can be used in a useful way here? The only thing I've seen (from select_tut(2)) is that these can be used to detect out-of-band data for reading on a socket, or that control status information is available for a pty in packet mode (tty_ioctl(4)). I believe both of these would be false positives if encountered on input, and it's not clear what they would mean for output. (4.) + /* readable event on STDOUT is equivalent to POLLERR, + and implies an error condition on output like broken pipe. */ I know this is what the comment from tail.c says, but is it actually documented to be true somewhere? And on what platforms? I don't see it being documented in my select(2) on Linux, anyway. (Though it does seem to work.) Wondering if this behavior is "standard". I guess there is also a little gotcha. In tee, all the outputs *except* stdout are explicitly open()ed, but stdout itself is not, and therefore it inherits the open mode that fd 1 had when tee was exec()ed. In particular, if fd 1 / stdout is a socket, open for RW, then a "readable event on STDOUT" could be a legitimate indication that data is ready for reading. So that would be a false positive causing stdout to be removed even though successful writes are still possible. (Maybe nobody will care about this, but it's an interesting corner-case.) ... "But other than that", sure! I guess I'm all for it :) Carl
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
On Fri, 2 Dec 2022, Carl Edquist wrote:
Although tee has multiple outputs, you only need to monitor a single
output fd at a time. Because the only case you actually need to catch
is when the final valid output becomes a broken pipe. (So I don't
think it's necessary to poll(2) all the output fds together.)
That is technically true, but I think coupling this to two FDs might
prove a bit inelegant in implementation (since you'd have to decide
which entry from an unorganized list with holes do you pick? any of
them could spontaneously go away), so I'm not sure the implementation
would be cleaner that way.
It'd be best to explain with a patch - I'll plan to send one later for a
concrete example.
Ok it's patch time. Attached! Feedback welcome.
A few notes:
Originally I had in mind to put the read() call inside the poll() loop.
But if we keep this feature as an option, it felt it was a bit easier just
to add the "if (pipe_check) {...}" block before the read().
The new iopoll() function is the core concept here for waiting on an
[input, output] pair of fds - waiting for the input to become ready to
read, or the output to have an error or become a broken pipe.
For Pádraig, I think the same function & approach here could be used in
other filters (cat for example). The stubborn part of me might say, for
platforms that do not natively support poll(2), we could simply leave out
support for this feature. If that's not acceptable, we could add a
select(2)-based fallback for platforms that do not have a native poll(2).
Unique to tee is its multiple outputs. The new get_next_out() helper
simply advances to select the next remaining (ie, not-yet-removed) output.
As described last time, it's sufficient to track a single output at a
time, and perhaps it even simplifies the implementation. It also avoids
the need for a malloc() for the pollfd array before every read().
I moved the somewhat complicated write-failure logic from tee_files() out
to a new helper function, fail_output(), which now also gets called for
broken pipes that we want to remove.
Note also that I make -P imply -p. I think this is necessary, otherwise
an output pipe becoming broken always produces an error. But normally, an
output pipe breaking does not necessarily produce an error, since EOF can
arrive before any further input, and in that case no write is then
attempted into the broken pipe.
Happy hacking / feedback welcome.
Thanks,
Carlcommit cabf34aa748afa676f10a7943de0e25a962d23a4
Author: Carl Edquist
Date: Mon Dec 5 15:43:24 2022 -0600
tee: add -P/--pipe-check option to remove broken pipe outputs
In case input is intermittent (a tty, pipe, or socket), and all
remaining outputs are pipes (eg, >(cmd) process substitutions), provide
a way to exit early when they have all become broken pipes (and thus
future writes will fail), without having to wait for more input to tee,
to actually encounter the signal or write failure (SIGPIPE/EPIPE).
Broken pipe detection is done by calling poll(2) with an unlimited
timeout, waiting for input to be available, or errors on input or the
first remaining output. The iopoll() function that implements this
could be of general use for other filter utils, even cat for instance.
This would allow broken pipes to "propagate" backwards in a shell
pipeline.
Note also that -P implies -p. This is necessary, otherwise an output
pipe becoming broken always produces an error. But normally, an output
pipe breaking does not produce an error if EOF is hit on input before
any further data, as no write is then attempted into a broken pipe.
* src/tee.c (iopoll): New function implementing broken pipe detection.
(pipe_check, long_options, main): Add -P/--pipe-check option.
(get_next_out): Helper function for finding next valid output.
(tee_files): Add broken pipe detection before calling read().
(tee_files, fail_output): Break out write failure/output removal logic
to helper function.
diff --git a/src/tee.c b/src/tee.c
index 971b768..c17c5c7 100644
--- a/src/tee.c
+++ b/src/tee.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include "system.h"
#include "argmatch.h"
@@ -37,6 +38,9 @@
proper_name ("Richard M. Stallman"), \
proper_name ("David MacKenzie")
+#define IOPOLL_BROKEN_OUTPUT -2
+#define IOPOLL_ERROR -3
+
static bool tee_files (int nfiles, char **files);
/* If true, append to output files rather than truncating them. */
@@ -45,6 +49,9 @@ static bool append;
/* If true, ignore interrupts. */
static bool ignore_interrupts;
+/* If true, detect if next output becomes broken while waiting for input. */
+static bool pipe_check;
+
enum output_error
{
output_error_sigpipe, /* traditional behavior, sigpipe enabled. */
@@ -61,6 +68,7 @@ st
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
On Fri, 2 Dec 2022, Arsen Arsenović wrote: I'm concerned with adding such a behavior change by default still. I can imagine this "lifetime extension" properly having been relied on in the last many decades it has been around for ;) That's fair! :) I'd be curious to hear about a use-case for that; but anyway yeah if it's an option at least there won't be any surprises. Although tee has multiple outputs, you only need to monitor a single output fd at a time. Because the only case you actually need to catch is when the final valid output becomes a broken pipe. (So I don't think it's necessary to poll(2) all the output fds together.) That is technically true, but I think coupling this to two FDs might prove a bit inelegant in implementation (since you'd have to decide which entry from an unorganized list with holes do you pick? any of them could spontaneously go away), so I'm not sure the implementation would be cleaner that way. It'd be best to explain with a patch - I'll plan to send one later for a concrete example. But to try to answer your question: you'd have to decide which entry from an unorganized list with holes do you pick? So as you've seen there is a "descriptors" array corresponding to all the outputs. What I had in mind is to maintain an int that keeps track of the index of the first item in the descriptors array that is still valid. (They are actually FILE *'s, which are set to NULL when they become invalid.) So, you'd start with the first one (which happens always to be stdout). If the output at the current index (starting with stdout) ever becomes invalid (due to broken pipe detection, or due to other non-fatal write failure), then increase the index until the next valid output is found (skipping NULL entries). If the last output is closed, it's not really important what happens to the index - it can be left as-is or set to -1; it won't be used again in any case. any of them could spontaneously go away I think it should be OK just to check the current output index in the list and advance if that one closes. If a pipe becomes broken in the middle of the list, I think it's fine to let it be. Why is this OK? First of all, the current index still refers to a valid output. That means you are _not_ in a situation where all outputs are broken pipes, so the right thing to do is to continue waiting for input. Then, if input arrives, it will get written to all the valid (ie, non-NULL) outputs, including the one that has now become a broken pipe (without being detected). But this is OK, because (as we've discussed before) we should already be ignoring SIGPIPE (and handling EPIPE), to prevent races that can happen where a fatal SIGPIPE comes in after a read() and before the corresponding write(). So, this broken pipe gets removed due to the write failure (EPIPE), rather than the broken-pipe detection. But it does not affect the lifetime of the tee process, as any time poll() is waiting, the index will point to an output that is still valid and is not (yet) a broken pipe. ... But the proof is in the pudding; so I'll try to draft something up and you can see what you think technically & aesthetically... Cheers! Carl
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
PS: On Fri, 2 Dec 2022, Carl Edquist wrote: On the topic of implementation - I was thinking more about a general solution for filter utils, and I am thinking the key thing is to provide a replacement (wrapper) for read(2), that polls two fds together (one input and one ouput), with no timeout. It would check for POLLIN on input (in which case do the read()). Otherwise if there is an error (POLLERR or POLLHUP) on input, treat it as EOF. Otherwise if there's an error on output, remove this output, or handle it similar to SIGPIPE/EPIPE. (Nothing is written to the output fd here, it's just used for polling.) ... I think this general approach of using poll(2) for a single input along with a single ouput could be used for both tee and other filters that only write to stdout. A question for Pádraig (or anyone else who might know) - Are there portability concerns with the system poll(2), or just with the gnulib replacement for poll(), which is based on select(2) ? The commit message (3a1c328cd) seems to suggest the latter. In other words, are there any platforms that do not support using the system poll(2) directly? tail.c seems to use poll(2) for _AIX || __sun || __APPLE__ || HAVE_INOTIFY and *otherwise* it uses select(2). But I am thinking, if it's available, poll() is the more appropriate interface for this check in the first place. The 'trick' in tail.c of using select() to test when STDOUT becomes available for reading, and taking that as an indication of an error condition, doesn't seem to be documented (in my linux select(2)), and also seems like it might not work as intended if fd 1 is open for read-write. Anyway if it's possible just to use poll(2) (the system one, not the gnulib replacement), that might simplify the portability logic. What do you think? Thanks, Carl
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
On Wed, 30 Nov 2022, Arsen Arsenović wrote: Carl Edquist writes: It sounds like one way or another you want to copy your endless but intermittent input to multiple output pipes, but you want to quit as soon as all the output pipes become broken. Precisely. The most important requirement there is that the tee-based substitute imitates the lifetime of it's longest lived output. Now I'm thinking, maybe --pipe-check should also block SIGPIPE, to prevent the race between poll, process death and write (which would result in the process getting killed, as it'd happen right now, to see what I mean, try ``tee >(sleep 100) >(:)'' and press enter after a bit; a race could make --pipe-check behave like that). Right, you need to ignore SIGPIPE (like "tee -p" does) for multiple outputs if any of them can exit early... Sometimes I forget that '-p' is not on by default for tee. I don't think I've encountered a use-case for specifically wanting this option to be off. I'll keep this in mind, for v2, which is currently waiting on me having some time to research the portability of this whole thing, and for a decision on whether to even include this feature is made. On the topic of implementation - I was thinking more about a general solution for filter utils, and I am thinking the key thing is to provide a replacement (wrapper) for read(2), that polls two fds together (one input and one ouput), with no timeout. It would check for POLLIN on input (in which case do the read()). Otherwise if there is an error (POLLERR or POLLHUP) on input, treat it as EOF. Otherwise if there's an error on output, remove this output, or handle it similar to SIGPIPE/EPIPE. (Nothing is written to the output fd here, it's just used for polling.) ... Although tee has multiple outputs, you only need to monitor a single output fd at a time. Because the only case you actually need to catch is when the final valid output becomes a broken pipe. (So I don't think it's necessary to poll(2) all the output fds together.) I think this general approach of using poll(2) for a single input along with a single ouput could be used for both tee and other filters that only write to stdout. (But again, "tail -f" is different, because it checks for regular files to grow as the source of intermittent input - which I don't think is something you can use poll() to wait for.) I'll try to put together a patch do demo what I have in mind... But if you don't have control over that, the fundamental problem is detecting broken pipes *without writing to them*, and I don't think that can be solved with any amount of extra pipes and fd redirection... I imagine that, technically, this is attainable by editing the process substitutions involved to also signal the original process back; however, this feels less elegant and generally useful than tee handling this, given that tee's use-case is redirecting data to many places. Ah, yeah, I bet you could rig something up waiting on processes and/or using signals. In the general case there is a slight difference between "waiting for a process to terminate" and "waiting for the pipe to become broken" (the process attached to the read-end of the pipe could close its stdin early, or it could exit after forking a child that keeps the pipe open), but yeah in the most common case the two events happen together. Have a nice day :) Carl
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
On Tue, 29 Nov 2022, Arsen Arsenović wrote: The issue here isn't the compilers hanging, it's tee living longer than all the compilers do because it's stdin doesn't EOF (it'd be preferable for it to only live as long as the last of the compilers). I can imagine attempting to implement this with enough pipe and fd redirection magic, but I'm not sure how (in)elegant that becomes. It sounds like one way or another you want to copy your endless but intermittent input to multiple output pipes, but you want to quit as soon as all the output pipes become broken. To me, tee sounds like exactly the place to do that. Otherwise, you'd have to add the broken-pipe detection (as in your patch) to your own program, along with the rest of tee's basic functionality :) It would be one thing if you controlled the programs that consume the input (you could have them handle 'heartbeats' in the input stream, and once these programs terminate, the heartbeats would trip on the broken pipe). (However (in)elegant that becomes to implement...) But if you don't have control over that, the fundamental problem is detecting broken pipes *without writing to them*, and I don't think that can be solved with any amount of extra pipes and fd redirection... Carl
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
On Tue, 29 Nov 2022, Pádraig Brady wrote:
On 29/11/2022 17:32, Carl Edquist wrote:
...
If this kind of detect-broken-output-pipe logic were added to filter
utils generally, the above example (with 4 cats) would return to the
shell immediately.
Right. Thanks for discussing the more general pattern.
I.e. that SIGPIPE doesn't cascade back up the pipeline,
only upon attempted write to the pipe.
So it's not really a tee issue, more of a general pattern.
So it wouldn't be wrong to add this to tee (by default),
but I'm not sure how useful it is given this is a general issue for all
filters.
That makes sense; though I suppose it would continue to become more useful
for these types of cases as it gets added to more filters :)
Also I'm a bit wary of inducing SIGPIPE as traditionally it hasn't been
handled well:
But wait now, are we talking about inducing SIGPIPE?
In the current patch for tee, I think the idea was just to remove an
output from the list when it's detected to be a broken pipe, allowing tee
to exit sooner if all of its outputs are detected to be broken.
Similarly for the general filter case (usually with only a single output),
the idea would be to allow the program to exit right away if it's detected
that the output is a broken pipe.
https://www.pixelbeat.org/programming/sigpipe_handling.html
Actually, to me, if anything this page seems to serve as motivation to add
broken-pipe checking to coreutils filters in general.
The article you linked ("Don't fear SIGPIPE!") discusses three topics:
1. First it discusses default SIGPIPE behavior - programs that do not
specifically handle SIGPIPE do the right thing by default (they get
killed) when they continue writing to their output pipe after it
breaks.
We wouldn't be changing this for any filters in coreutils. Writing to a
broken pipe should still produce a SIGPIPE to kill the program. (Even
with broken-pipe detection, this can still happen if input becomes ready,
then the output pipe's read end is closed, then the write is attempted.)
tee is actually a special case (if -p is given), because then SIGPIPE does
not kill the process, but the write will still fail with EPIPE and tee
will remove that output.
We also wouldn't really be inducing anything new for programs earlier in
the pipeline. If they don't handle SIGPIPE, they will just get killed
with it more promptly - they will end up writing to a broken pipe one
write(2) call sooner.
2. The "Incorrect handling of SIGPIPE" section discusses programs that
attempt to handle SIGPIPE but do so poorly. This doesn't apply to us
either. Filters that add broken-pipe detection do not need to add SIGPIPE
handling. And programs that handle it poorly, earlier in the pipeline,
will have their problems regardless. (Again, just one write(2) call
sooner.)
3. Finally, the "Cases where SIGPIPE is not useful" section actually
highlights why we *should* add this broken-pipe checking to filters in
general.
The "Intermittent sources" subsection discusses exactly what we are
talking about fixing:
For example 'cat | grep -m1 exit' will only exit, when you type a line
after you type "exit".
If we added broken-pipe checking to cat, then this example would behave
like the user would have wanted - typing "exit" would cause grep to exit,
and cat will detect it's output pipe breaking, and exit immediately.
The other example about 'tail' was fixed already, as this kind of checking
was added to tail, as we've discussed. It's a good start! The more utils
we add it to, the more will be able to benefit.
The "Multiple outputs" subsection is specific to tee, and if anything
perhaps suggests that the '-p' option should be on by default. That is,
it makes an argument for why it makes sense for tee to avoid letting a
SIGPIPE kill it, but rather only to exit when all the input is consumed or
all the outputs have been removed due to write errors.
The "Long lived services" subsection is a generalization of what was just
said about tee - namely that it makes sense that some programs want to
continue after a failed write attempt into a broken pipe, and such
programs need to handle or ignore SIGPIPE. This is true for such programs
already, and adding broken-pipe checking to a filter in the same pipeline
doesn't change that at all. (Again, it will just cause them to get a
SIGPIPE/EPIPE *promptly* - one write call sooner - when the final consumer
of the output completes.)
...
Or perhaps when you mention "inducing SIGPIPE", you are referring to how
tail(1) does things currently (when it detects a broken output), by
attempting raise(SIGPIPE) followed by exit(EXIT_FAILURE). It seems this
is just an attempt to make it look to the waiting parent process that tail
died trying to write to a broken pipe (somewhat of a white lie). Most
likely it could ju
Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
Hi all, On Mon, 28 Nov 2022, Arsen Arsenović wrote: Pádraig Brady writes: Trying to understand your use case better, ... The bug we observed is that on occasion, for instance when running with a tty, or with a script that (for some reason) has a pipe on stdin, the tee-based "compiler" would hang. To replicate this, try: tee >(gcc test.c -o a.out.1) >(gcc test.c -o a.out.2) in a tty (here, the stdin is meant to be irrelevant). If I may try to provide a simple example of the problem, consider the following command line: tee >(sleep 3) | sleep 5 Let tee's stdin be a terminal to supply the "intermittent input". You'll see that after 5 seconds, this will hang indefinitely until you hit Enter. For the first 3 seconds, when hitting the Enter key, tee will successfully write the line to each pipe. Between 3 and 5 seconds, the pipe to "sleep 3" will be broken, which tee will notice, and then tee will continue writing the lines to the "sleep 5" pipe. But after 5 seconds, when "sleep 5" has terminated and that pipe becomes broken, tee will continue to "hang" waiting for input (in this case the intermittent input from the terminal) indefinitely, despite the fact that all of tee's outputs are now broken pipes. tee will only "notice" that the "sleep 5" pipe is broken when it receives input after that point, because then the write to that pipe fails with EPIPE (and/or a SIGPIPE is delivered). ... It seems the ideal thing to happen here is for tee to terminate once it determines that all of its outputs are broken pipes. It comes close to this already, but it only learns about this when write attempts fail, and it only attempts a write when it has input to tee. As I suppose was suggested in the patch, perhaps poll(2) could be used to wait for POLLIN from fd 0, and POLLHUP for outputs (perhaps limited to pipes / sockets). The patch subject suggests adding --pipe-check as an option, but on first blush it seems like this would actually be a good thing to do by default... Cheers, Carl
Re: date command is great but could use improvements
On Sun, 3 Jul 2022, David Chmelik wrote:
On 5/26/22 12:16 PM, Carl Edquist wrote:
In the states i am used to Sunday being the first "day of the week",
but "weekdays" refer specifically to Monday-Friday, thus even in the
states, Monday is the first "weekday" :)
'Weekday' abbreviates 'day of week' (including /workdays///schooldays/,
weekends) as in date command manpage ('%A locale's full weekday name
(e.g., Sunday: )')
I was mostly poking fun there, but I'll paste the "weekday" definition
from dict.org, which (despite the way the date(1) manpage uses it)
explains the sense i'm used to in the states:
1 definitions retrieved
"weekday" wn "WordNet (r) 3.0 (2006)"
weekday
n 1: any day except Sunday (and sometimes except Saturday)
I think the confusion comes from conflating ordinal and cardinal
numbers.
No confusion; not at all. How mathematicians/programmers think about
numbers/variables and most think about time/date differ.
Well, if someone thinks 0 is wrong because it implies "zeroth" (an
improperly used ordinal term, which, as you said, nobody uses for the day
of the week), i think that points to the confusion conflating these two
types of units.
Ordinal numbers (first, second, third) describe the order in the
sequence, and cardinal numbers (zero, one, two, three) count how many.
There's no consensus among computer scientists (CS) & mathematicians/CS,
and mathematicians; many say natural (N) & etc. (counting, whole,
whatever one calls various) numbers start from one, but
mathematicians/CS often say from zero: different definitions, but I
almost never focus on 'cardinals versus ordinals:' terms not mentioned
in my original post (OP.)
The difference between cardinal vs ordinal is not about whether you start
the natural numbers with 0 or 1. It's the conceptual difference between
"one" and "first".
I know you didn't mention the terms "cardinal" and "ordinal" by name, but
you did express your shock over days of the week being numbered starting
at 0 (for a variable that represents a cardinal number) instead of 1, and
your reasoning was because people say the week starts with the "first" day
(implying you were thinking of it as an ordinal number).
I really do think this is the key distinction to make to understand these
units. I suspect the confusion tends to come from not making the proper
conceptual distinction between cardinal and ordinal numbers.
Most the world, including most the English-speaking world, uses 24-hour
civilian time but even when says "current time is 0:00" that's ordinal!
To use this as an example: "0:00" has cardinal units in the sense that it
means "0 hours and 0 minutes past midnight" (counting the hours and
minutes). But, to describe that time (midnight) in ordinal terms, it is
"the first minute of the first hour of the day" (describing the place of
the hour and minute within their respective sequences).
Again this emphasizes how cardinal and ordinal units are conceptually two
different perspectives about the same point in time.
The made-up word "zeroth" is a fiction, i suppose, imagined by people
who blur the distinction between ordinal and cardinal numbers. When
something is _first_, nothing comes before it! :)
En.wikipedia.org/wiki/Zeroth cites 'zeroth' in mathematics/CS since
1960s (I heard term in those high school and/or college/university
subjects) and though I couldn't check recent OED updates, Oxford's newer
(not de facto, but documents newer British usage) English dictionary
mentions 'zeroth' since 1800s.
No doubt "zeroth" is a word that gets used in the English language, as is
"unicorn". But i think there is an argument to be made that, when this
word is used, it represents a fiction.
The "Zeroth" wikipedia landing page you mentioned links to the
"Zero-based_numbering" article [1], which i think contains some helpful
points.
[1] https://en.wikipedia.org/wiki/Zero-based_numbering
First of all, i think there is no disagreement that (as it says)
"Numbering sequences starting at 0 is quite common in mathematics
notation".
But the question is what it means when people try to use "zeroth" ("0th")
as an ordinal term. The article actually describes two different uses:
(1) simply an ordinal referring to the initial element in a 0-indexed
sequence, and (2) an ordinal referring to an element that could naturally
be placed _before_ the initial element of a sequence, but that does not
properly belong to the sequence itself.
About (1) it says, "Under zero-based numbering, the initial element is
sometimes termed the zeroth element, rather than the first element", and
it calls this "zeroth" a "co
Re: date command is great but could use improvements
Hi David! Looks like Bob gave an excellent, thorough reply. (Thanks Bob!) For fun, a couple details about "counting" caught my eye from your email: On Fri, 20 May 2022, David Chmelik wrote: I'm European-American, programming since 1993, and may recall Monday was first weekday growing up in The United Kingdom (UK,) but moved to USA at age seven and they said Sunday is /first/ weekday, so that's what I use until revisiting Europe/Britain. In the states i am used to Sunday being the first "day of the week", but "weekdays" refer specifically to Monday-Friday, thus even in the states, Monday is the first "weekday" :) I was shocked to see one can only display Sunday starting week at 0. Everyone thinks of days 1 to 7, even programmers/sysadmins having to talk so users understand. ... where Sunday is first weekday (The United States of America, USA, and I've never heard a programmer/sysadmin/scientist/professor who uses UNIX/GNU//Linux say 'zeroth weekday') I think the confusion comes from conflating ordinal and cardinal numbers. Ordinal numbers (first, second, third) describe the order in the sequence, and cardinal numbers (zero, one, two, three) count how many. If you take a close look at the definitions of the members of the tm structure (as provided in Bob's email), you will see "number of seconds", "number of minutes", "number of hours", "number of days", "number of months" -- these are all cardinal numbers (they count how many) and thus start with zero. On the other hand, the "day of the month" is an ordinal term, so it starts with the _first_ (the range is actually *1st* - *31st*). We're familiar with the "day of the month" being ordinal, but in plain English, the "day of the week" is usually given with the day's _name_ rather than its ordering. (On the other hand, in Greek, the name for Monday means "Second", and the name for "Tuesday" means "Third", etc.) ... The made-up word "zeroth" is a fiction, i suppose, imagined by people who blur the distinction between ordinal and cardinal numbers. When something is _first_, nothing comes before it! :) This of course is the explanation for why 2022 is part of the _21st_ century, as the century is an ordinal description. And for those who care, even the year number here is ordinal (it actually refers to the 2022th year), which explains why there is no such thing as "year 0". (There's No Such Thing as the zeroth century or the zeroth year!) On the other hand, the tm structure stores "The number of years since 1900", which is a cardinal number (how many), thus tm_year=0 -> 1900. (And, no, 1900 was not the zeroth year of the 20th century. It was the 100th year of the 19th century!) Happy counting! :) Carl
Re: new snapshot available: coreutils-8.32.251-7b0db.tar.xz
Hi Pádraig, Just wondering, but, do you think you will include this one? https://lists.gnu.org/archive/html/coreutils/2021-04/msg00049.html Thanks, Carl On Mon, 20 Sep 2021, Pádraig Brady wrote: We plan to release coreutils-9.0 in the coming week so any testing you can do on various different systems between now and then would be most welcome. -- You can download the coreutils snapshot in xz format (5.3 MB) from: https://pixelbeat.org/cu/coreutils-ss.tar.xz And verify with gpg or md5sum with: https://pixelbeat.org/cu/coreutils-ss.tar.xz.sig MD5 (coreutils-ss.tar.xz) = 435896896eb3f9d4e550fc7810961822 -- To test follow this standard procedure: tar -xf coreutils-ss.tar.xz cd coreutils-8.32.251-7b0db/ ./configure && make check VERBOSE=yes Failures are reported, and details are in tests/test-suite.log Please report/attach any issues to coreutils@gnu.org -- This is a new major release of coreutils, the previous being about 12 years ago. Major changes are: - cp has changed how it handles data - enables CoW by default (through FICLONE ioctl), - uses copy offload where available (through copy_file_range), - detects holes differently (though SEEK_HOLE) - This also applies to mv and install. - utilities are more tuned to the hardware available - wc uses avx2 instructions to count lines - cksum uses pclmul instructions for --algorithm=crc - Other digest tunings remain delegated to libcrypto - More amalgamation of utilities - cksum now supports the -a (--algorithm) option to select any digest. - This is the preferred interface, rather than existing sha*sum etc. - This is similar to the amalgamation of encoding utilities introduced in the basenc command in v8.31. Changes in coreutils since v8.32 are summarized at https://git.sv.gnu.org/cgit/coreutils.git/tree/NEWS and all commits grouped by author are as follows: Andreas Schwab (1): tests: avoid spurious testsuite failure Arman Absalan (1): chroot,comm,join: fix usage options style Assaf Gordon (1): basenc: fix bug49741: using wrong decoding buffer length Ben Pfaff (1): doc: clarify in texinfo that `test == ...` is non portable Benno Schulenberg (1): doc: fix punctuation in stat --help Bernhard Voelker (17): maint: add texi2dvi build directory to doc/.gitignore build: update gnulib to latest - to avoid du(1) crash on XFS tests: fix removed-directory test maint: fix syntax-check failure from recent adjustment maint: copy FDL from gnulib instead of using it as module doc: timeout: improve documentation of the exit status stat,tail: add support for the VBOXSF file system doc: add timeout examples doc: clarify 'timeout -k' behavior doc: show version in title of HTML manual tests: skip some parts of 'tests/rmdir/ignore.sh' if run as root maint: minor cleanup maint: exempt 'doc/fdl.texi' from 'make update-copyright' doc: make formatting of SEE ALSO in cat.1 and tac.1 consistent maint: update bootstrap from gnulib tests: fix FP in ls/stat-free-color.sh maint: fix sc_space_before_open_paren failure Carl Edquist (2): ls: add --sort=width option to sort by file name width build: fix __get_cpuid_count check to catch link failure Emanuele Giacomelli (1): csplit: fix regex suppression with specific match count Erik Auerswald (1): pr: fix alignment of input tabs to multiple columns Grigorii Sokolik (2): maint: update docs for build prerequisites maint: remove already handled FIXME in tail.c Jason Kim (1): ls: allow --classify to be ignored for non tty output Jim Meyering (7): maint: avoid new sort.c warning from upcoming GCC11 maint: bootstrap: remove reference to unused hash-pjw module maint: avoid new syntax-check failure build: avoid new chmod.c warnings from upcoming GCC12 tests: env-s.pl: avoid spurious failure on OS X cksum: list Pádraig as coauthor doc: drop extraneous single quotes in help Justin Tracey (1): tests: narrow scope of faulty join args KOBAYASHI Takashi (2): nl: support a negative --line-increment nl: fix --section-delimiter handling of single characters Kamil Dudka (4): dd: drop old workaround for lseek() bug in Linux kernel stat,tail: add support for the exfat file system ln: fix memory leaks in do_link df: fix duplicated remote entries due to bind mounts Kristoffer Brånemyr (3): cksum: use more efficient slice by 8 algorithm cksum: use pclmul hardware instruction for CRC32 calculation wc: use avx2 optimization when counting only lines Nikolay Nechaev (1): maint: remove redundant checks on buffer sizes in tail Nishant Nayan (1): rm: do not skip files upon
Re: Enhancement to SORT command.
I think 'sort -h' does just that, though for your input you'll need to remove the spaces between the numeric part and the units. Eg: $ ( tr -d ' ' | sort -h ) << EOF 100.0 Kb 303 Mb 4.01 Gb 20 Mb EOF 100.0Kb 20Mb 303Mb 4.01Gb Works great with, eg, 'du -sh * | sort -h' Carl On Mon, 20 Sep 2021, mm1mi...@gmail.com wrote: Hello, The SORT command in GNU coreutils does not have a sort by DATA SIZE parameter. for example, to sort:- --- 100.0 Kb 303 Mb 4.01 Gb 20 Mb --- sorts output will be:- --- 303 Mb 100.0 Kb 20 Mb 4.01 Gb --- It would be nice if thier was a parameter that sorted by size so the output would be 100.0 Kb, 20 Mb, 303 Mb, 4.01 Gb Would be great if such a parameter was added. Regards.
Re: Enhancement to SORT command.
Works great with, eg, 'du -sh * | sort -h' Or, well, if you want to be more careful about it, shopt -s dotglob du -sh0 -- * | sort -hz | tr '\0\n' '\n?' On Mon, 20 Sep 2021, Carl Edquist wrote: I think 'sort -h' does just that, though for your input you'll need to remove the spaces between the numeric part and the units. Eg: $ ( tr -d ' ' | sort -h ) << EOF 100.0 Kb 303 Mb 4.01 Gb 20 Mb EOF 100.0Kb 20Mb 303Mb 4.01Gb Works great with, eg, 'du -sh * | sort -h' Carl On Mon, 20 Sep 2021, mm1mi...@gmail.com wrote: Hello, The SORT command in GNU coreutils does not have a sort by DATA SIZE parameter. for example, to sort:- --- 100.0 Kb 303 Mb 4.01 Gb 20 Mb --- sorts output will be:- --- 303 Mb 100.0 Kb 20 Mb 4.01 Gb --- It would be nice if thier was a parameter that sorted by size so the output would be 100.0 Kb, 20 Mb, 303 Mb, 4.01 Gb Would be great if such a parameter was added. Regards.
Re: How to count the last line when it does not end with a newline character?
Hi Peng, On Sun, 5 Sep 2021, Peng Yu wrote: I got 1 instead of 2 in the following example. How to count the last even when it does not end with a newline character? Thanks. $ printf 'a\nb'|wc -l 1 Here is a little trick. You can append a newline if/when missing to the end of input with: sed '$a\' So for your example: $ printf 'a\nb' | sed '$a\' | wc -l 2 $ printf 'a\nb\n' | sed '$a\' | wc -l 2 You can apply this to a bunch of files at once also, with potentially missing newlines in their final lines: $ sed '$a\' *.txt Note this is not the same as $ cat *.txt | sed '$a\' which will only add a missing newline once to the end of the entire input. Hope that helps, Carl
Re: how to speed up sort for partially sorted input?
On Wed, 25 Aug 2021, arthur200...@gmail.com wrote:
The current sort algorithm used by coreutils sort(1) is a mergesort,
which unfortunately does not pick up cues on partially-sorted input. A
relatively well-known algorithm that picks up partial sort order is
timsort [...]
[1]: https://en.wikipedia.org/wiki/Timsort
If i read correctly, timsort takes advantage of input sections that are
already sorted ("runs"), by merge-sorting consecutive runs.
This is a bit different than the partially sorted input originally
described in this thread, where you are sorting on (in this case) column
1, then column 2, but it is known that the input is already sorted
according to column 1.
In the timsort case, if you have two consecutive runs, A and B, then you
have
i < j --> A[i] < A[j]
and
i < j --> B[i] < B[j]
but there is no guarantee that
A[i] < B[j]
On the other hand, the scenario for this thread is the opposite. If you
have two consecutive chuncks, A (column 1 = A), and B (column 1 = B), then
you always have
A[i] < B[j]
but within chunk A the lines are unsorted, as within chunk B.
...
So, while a timsort algorithm might be an interesting alternative, it
doesn't seem it will improve things for this particular case.
Carl
Re: how to speed up sort for partially sorted input?
On Thu, 12 Aug 2021, Kaz Kylheku (Coreutils) wrote:
The solution doesn't exist today, whereas that Gawk program should run
even in ten year old installations.
For the solution to be useful, it only has to beat the actual sort which
you have available today
Hi Kaz,
The thing is, it seems regular ("unoptimized") sort is still a lot faster
than an "optimized" awk-based solution.
For instance, i'll generate 2M partially-sorted lines:
$ head -c $((6 * 1024**2)) /dev/urandom | xxd -p |
fold -w6 | sed -r 's/(..)()/\1 \2/' > unsorted
$ sort -s -k1,1 unsorted > partially-sorted
$ wc -l partially-sorted
2097152 partially-sorted
$ head partially-sorted
00 fcbc
00 52fa
00 8a5a
00 b630
00 a572
00 535d
00 e7d9
00 c987
00 cb53
00 574e
So here we have 2M lines, and 256 distinct values, so an average chunk
size of 8K.
Now time a few different sort methods:
(1) sort col1, col2 (the original specification)
$ time sort -k1,1 -k2,2 partially-sorted | md5sum
cbba6a6e9c80ade9dcbe49d26a6d94ce -
real0m1.363s
user0m4.817s
sys 0m0.074s
(2) just sort the whole lines
$ time sort partially-sorted | md5sum
cbba6a6e9c80ade9dcbe49d26a6d94ce -
real0m0.499s
user0m1.466s
sys 0m0.071s
(3) your (g)awk program
$ time ./kaz.gawk partially-sorted | md5sum
cbba6a6e9c80ade9dcbe49d26a6d94ce -
real0m7.583s
user0m7.597s
sys 0m0.136s
(I include md5sum just to show the outputs are identical.)
(2) will produce identical output to (1) so long as the column separator
characters sort before the non-column chars, which is a clear improvement
as long as this assumption is valid. Sadly the "optimized" awk program
(3) does not help at all. (It's significantly worse.)
...
For comparison, i've also done the same but giving a higher partially-
sorted ratio, to give more advantage to the partially-sorted optimization.
Still 2M lines, but 64k distinct values for col1 (instead of 256), leaving
an average chunk size of 32 lines (instead of 8k).
[In terms of sort algorithm complexity (number of comparisons), for
optimized partially-sorted input this is 2.6x better than the above case
that only had 256 distinct values. (Left as an exercise to the reader is
why it's exactly 2.6x.) But there is other overhead involved also, so
there is no expectation that the overall run time will improve that much.]
$ head -c $((6 * 1024**2)) /dev/urandom | xxd -p |
fold -w6 | sed -r 's/()(..)/\1 \2/' > unsorted42
$ sort -s -k1,1 unsorted42 > partially-sorted42
$ head partially-sorted42
fd
3a
81
96
6a
94
88
92
89
e9
And the times:
$ time sort -s -k1,1 -k2,2 partially-sorted42 | md5sum
efb0303789a4e52c3ab27600fad1dac4 -
real0m0.777s
user0m2.499s
sys 0m0.071s
$ time sort partially-sorted42 | md5sum
efb0303789a4e52c3ab27600fad1dac4 -
real0m0.378s
user0m1.015s
sys 0m0.071s
$ time ./kaz.awk partially-sorted42 | md5sum
71e7c99d2ef6e67e5ad29115e97872b0 -
real0m4.541s
user0m4.663s
sys 0m0.081s
Times improve across the board, but with the same pattern as before.
An interesting observation is that performance of the regular sort command
shows improvement with partially sorted input. (That is, without being
told explicitly that it's already partially sorted on col1.)
That makes me even more skeptical about how much room there is left for an
explicit optimization to improve things any further. (Though i'd still be
curious to try it and find out, perhaps at the cost of some sleep.)
Anyway, happy friday!
Carl
PS: for the careful reader who notices the checksum does not match in the
final instance - Kaz it appears there is a bug in your awk script.
(Probably you can reproduce setting up inputs like i have above.)
In particular, comparisons like '$1 != prev_1' will treat things as
numeric where possible, meaning '001' == '1', '00e0' == '0' (scientific
notation), and (surprise!) '00e0' == '00e6' (also scientific notation).
As a result, your awk program treats everything from 00e0 through 00e9 as
the same key, and sorts that entire range according to col2 alone.
You can avoid/fix this by putting '$1 ""' wherever you reference '$1' (to
force interpretation as a string rather than numeric).
Re: how to speed up sort for partially sorted input?
On Wed, 11 Aug 2021, pengyu...@gmail.com wrote: Yes, this involves many calls of the coreuils' sort, which is not efficient. Would it make sense to add an option in sort so that sort can sort a partially sorted input in one shot. Might be nice! Not sure how convenient that would be to work into the existing sort code. Or how much of an overall speedup would actually be realized. Just curious, what size data sets are you working with? (Number of lines in the input file (N), and number of distinct values (M) for column 1? [for fun i'll pretend to do some "math" here] Just in terms of complexity, if the sorting work (without the partial sort of column 1) is ~ N * log(N), then for the optimized partial sort it would be ~ N * log(N/M). That means the "speedup" (in terms of total sorting work) is ~ log(N)/log(N/M). More generally, if N = M**P, the speedup is P/(P-1). Or (equivalently) if you have N = 2**N' and M = 2**M', the speedup is N'/(N'-M'). So if you have N = M*M (eg, 64k lines with 256 distinct values for col 1) the theoretical speedup is ~2x. And N = M*M*M (eg, 16M lines and 256 distinct values) gives only a 1.5x speedup. Though if N and M are relatively closer, eg 1M lines and 64k distinct values (so, chunks of 16 lines each), the speedup is 5x. Nevertheless, there is probably an overhead cost that's perhaps linear with M. (Though its weight depends on implementation...) I'll add that, if your "sort by column 1, then column 2" requirement is also satisfied just by sorting whole lines(*), then asking sort to parse the lines into columns is extra work and you may actually make the whole thing slower even if there is less to compare. (I mean here "sort" vs "sort [-s] -k1,1 -k2,2") (*) not necessarily the case, depending on how the column separators sort relative to non-separator characters Anyway i'd be curious how big of an actual speedup (even theoretically, which might be optimistic) you might have in mind here. How many seconds might actually be saved, for how long of a run? And is that projected savings enticing enough to inspire implementing it? (My guess is if someone wanted to add it though, the interface might be a new option, effectively "assume sorted", which could be applied to the OPTS for the KEYDEF specified for -k/--key ...) Carl
Re: how to speed up sort for partially sorted input?
On Tue, 10 Aug 2021, Kaz Kylheku (Coreutils) wrote:
On 2021-08-07 17:46, Peng Yu wrote:
Hi,
Suppose that I want to sort an input by column 1 and column 2 (column
1 is of a higher priority than column 2). The input is already sorted
by column1.
Is there a way to speed up the sort (compared with not knowing column
1 is already sorted)? Thanks.
Since you know that colum 1 is sorted, it means that a sequential scan
of the data will reveal chunks that have the same colum1 value.
You just have to read and separate these chunks, and sort each one
individually by column 2.
Neat observation.
You could do that tersely in awk by piping each chunk to a separate sort
process, like:
awk '
c1 != $1 { close(sort); c1 = $1 }
{ print | sort }
' sort="sort -k2,2" partially-sorted-input.txt
In theory, that would bring the sorting work down from ~ O(n * log(n)) to
~ O(n * log(n/m)) (for a partially-sorted file with n lines and m
column-1 chunks of equal size).
But the overhead of starting a new sort process for each chunk is likely
going to outweigh that advantage. In the end, just sorting the whole file
at once (despite column 1 already being sorted) is still likely to be
faster.
(With just a bit more work, you can do all your sorting in a single awk
process too (without piping out to sort), but i think you'll still be
disappointed with the performance compared to a single sort command.)
Carl
gnulib update and bits/long-double.h
Hi, (Attn: Paul, involved in the relevant coreutils & gnulib updates) The recent gnulib update in coreutils [1] broke the build for me, since the bits/long-double.h header does not exist on my system. Looks like this has been discussed and fixed in gnulib [2]. At some point maybe consider bumping the gnulib submodule in coreutils again to include the fix. In the mean time (for anyone else hitting this issue), I found it suffices just to create an empty /usr/include/bits/long-double.h Thanks, Carl [1] https://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=5b5622f6 [2] https://lists.gnu.org/r/bug-gnulib/2021-04/msg00198.html
[PATCH] configure.ac: fix __get_cpuid_count check to catch link failure
(Was: Re: [PATCH] wc: Add AVX2 optimization when counting only lines) So, it looks like the check for avx2 support is broken - the build broke for me after the update. Seems it is doing a compile check when a link check is necessary. I was seeing this in my build: src/wc.c: In function 'avx2_supported': src/wc.c:153:13: warning: implicit declaration of function '__get_cpuid_count' [-Wimplicit-function-declaration] if (! __get_cpuid_count (7, 0, , , , )) ^ ... src/wc.o: In function `avx2_supported': /.../coreutils/src/wc.c:153: undefined reference to `__get_cpuid_count' collect2: error: ld returned 1 exit status Makefile:9415: recipe for target 'src/wc' failed My config.h has: #define USE_AVX2_WC_LINECOUNT 1 My config.log has: configure:77113: checking if __get_cpuid_count exists configure:77129: gcc -c -g -O2 conftest.c >&5 conftest.c: In function 'main': conftest.c:842:7: warning: implicit declaration of function '__get_cpuid_count' [-Wimplicit-function-declaration] __get_cpuid_count(7, 0, , , , ); ^ configure:77129: $? = 0 configure:77131: result: yes ... So it looks like '__get_cpuid_count' not being declared is just treated as a warning, and so the compile test succeeds. The actual failure happens at link time. Would it be more appropriate to use AC_LINK_IFELSE instead of AC_COMPILE_IFELSE ? I tried swapping this out (as in the attached patch), and now it correctly omits the #define for USE_AVX2_WC_LINECOUNT, and wc compiles & links properly. Thanks, CarlFrom 4f7e27ae1745331772a835564f868e25ecae070c Mon Sep 17 00:00:00 2001 From: Carl Edquist Date: Mon, 10 May 2021 05:22:11 -0500 Subject: [PATCH] configure.ac: fix __get_cpuid_count check to catch link failure The test program will compile successfully even if __get_cpuid_count is not declared. The error for the missing symbol will only show up at link time. Thus, use AC_LINK_IFELSE instead of AC_COMPILE_IFELSE. * configure.ac: (__get_cpuid_count check) use C_LINK_IFELSE instead of AC_COMPILE_IFELSE. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index f0fbbd9..f826a4b 100644 --- a/configure.ac +++ b/configure.ac @@ -576,7 +576,7 @@ AM_CONDITIONAL([USE_PCLMUL_CRC32], CFLAGS=$ac_save_CFLAGS AC_MSG_CHECKING([if __get_cpuid_count exists]) -AC_COMPILE_IFELSE( +AC_LINK_IFELSE( [AC_LANG_SOURCE([[ #include -- 2.9.0
Re: [PATCH] ls: add --files0-from=FILE option
On Wed, 21 Apr 2021, Carl Edquist wrote: Thanks Pádraig for the thoughtful reply! You bring up some good points, which for the sake of interesting discussion i'd like to follow up on also. (Maybe later this week...) So to follow up - i don't have any action items here, just some details that i thought might make for interesting discussion. On Tue, 20 Apr 2021, p...@draigbrady.com wrote: In this case, xargs batches *13* separate invocations of ls; so the overall sorting is completely lost. But with the new option: [linux]$ find -name '*.[ch]' -print0 | ls -lrSh --files0-from=- The sizes all scroll in order One can also implement this functionality with the DSU pattern like: nlargest=10 find . -printf '%s\t%p\0' | sort -z -k1,1n | tail -z -n"$nlargest" | cut -z -f2 | xargs -r0 ls -lUd --color=auto -- I appreciate you taking the time to write out a full DSU example. I was going to save this topic for a separate thread, but yeah one of my observations over the years is i would see people make (i would say oversimplified) comments that "you can do that with find", but they rarely go through the effort to show it (and thus demonstrate just how much more complicated the equivalent find-based solution is to type out). (More in this vein a bit later.) Arguably that's more scalable as the sort operation will not be restricted by available RAM, and will use temp storage. Agreed! This is a good point to keep in mind in general for ls. On the one hand, ls already has file metadata information available to it in raw format (eg, size, timestamps), so there isn't the overhead to convert relevant keys to sortable strings and write it all across pipes for IPC. But on the other hand, as you are saying, for the things that find(1) can give sort(1) to sort (which is _most_ of the ls --sort modes), sort(1) can sort a bit more efficiently and can handle situations with very many file names and very limited system memory. But by the same argument, find . -mindepth 1 -maxdepth 1 ! -name .\* -printf '%f\0' | sort -z | xargs -r0 ls $LS_OPTIONS -dU -- is more scalable than a bare 'ls' invocation, since there can be literally millions of entries in a single directory. But 'ls' is certainly easier to type. And if you want to sort on anything more interesting than the name itself, the DSU pipeline just gets more complex. So at some point you can weigh scalability vs usability based on the situation. (In the linux tree example above, the "ls -lrSh --files0-from=-" run with 45648 input source files has a maxrss of ~16M, so for instance this use case is small enough for me not to worry about mem use.) [By the same token, the sorting of shell globs in general (which in bash can be expanded to an arbitrary number of arguments), can be more scalably done outside of the shell (with find|sort) than in a shell glob itself. And while that is not a coreutils issue, the point is that despite this, (as with ls) shell globs can still be more convenient to use for sorting a set of files, perhaps in most cases, than an equivalent multi-tool pipeline.] But doing the sorting in ls (rather than find|sort|cut|xargs) is not just easier to type -- it's also easier to _get it right_ without a lot of trial and error. And for casual interactive use, that's kind of a feature, too. For instance, in your DSU example, i spy a subtle bug in the undecorate step: looks like it should be "cut -zf2-" instead of "cut -zf2" ... because after all, tabs are legal in filenames, too. The DSU in pixelbeat's "newest" script (mentioned later) is also not free from filename handling issues [1]. (Point being - even for experienced users, writing a replacement for ls's internal sorting with an equivalent DSU pipeline can be tricky & time consuming to get right, and easy to get subtly wrong.) Also there are some other subtle differences that might be worth keeping in mind: - To get the same sort order as ls, you actually need "sort -rzk1,1n" instead of just "sort -zk1,1n", since for tie-breaking ls sorts the keys and the names in opposite directions for ls -t and -S (whether or not you pass -r to ls). (Likewise if you want the same order as ls -S without the -r, you need a slightly different "sort -zk1,1nr".) - when you put 'ls' after a pipe (as in "... | ls -lrSh --files0-from=-") you typically get the alias (eg, slackware has ls='/bin/ls $LS_OPTIONS'). Meanwhile 'xargs ls' is whichever 'ls' is first in PATH, and does not include LS_OPTIONS. Not a big deal, but, buyer beware. - lastly (and perhaps the only thing you can't do anything about), when you use 'xargs ls' rather than a single ls invocation, you lose the aggregate width alignments for any ls formats with column output (in this case -l, but it's also true for -C and -x). Also
Re: [PATCH] ls: add --files0-from=FILE option
On Tue, 20 Apr 2021, p...@draigbrady.com wrote: > One can also implement this functionality with the DSU pattern like: > > nlargest=10 > find . -printf '%s\t%p\0' | > sort -z -k1,1n | tail -z -n"$nlargest" | cut -z -f2 | > xargs -r0 ls -lUd --color=auto -- > > Arguably that's more scalable as the sort operation will not > be restricted by available RAM, and will use temp storage. > > Also it's more robust as ls is the last step in the pipeline, > presenting the files to the user. If you wanted the largest 10 > with ls --files0-from then file names containing a newline > would mess up further processing by tail etc. > >> Similarly, say you would like to view / scroll through your extensive mp3 >> collection in chronological order (based on when you added the files to >> your collection). You can do it now with the new option: >> >> [music]$ find -name \*.mp3 -print0 | ls -lrth --files0-from=- > > I've used a https://www.pixelbeat.org/scripts/newest script > for a long time with similar find|xargs technique as I described above. > > In saying all of the above, I do agree though that for consistency > commands that need to process all arguments with global context, > should have a --files0-from option. > Currently that's du and wc for total counts, and sort(1) for sorting. > Since ls has sorting functionality, it should have this option too. Thanks Pádraig for the thoughtful reply! You bring up some good points, which for the sake of interesting discussion i'd like to follow up on also. (Maybe later this week...) Have a good day! Carl
Re: [PATCH] ls: add --files0-from=FILE option
Hi Berny, On Wed, 21 Apr 2021, Bernhard Voelker wrote: shouldn't it use the 'argv-iter' gnulib module (like du.c and wc.c) instead of directly using the underlying ... +#include "readtokens0.h" I considered this, too! :) I think the short answer is that du and wc don't actually need to keep any information around from all the files they process except for the running totals, so "at scale" the argv-iter version for files0 processing allows for a constant (?) memory usage in du and wc regardless of the number of input items. But for ls, even if using argv-iter would result in one less copy of the filenames, the scaled memory requirements are still the same for sorting (fileinfo for each entry has to be kept till the end). [If anything, i would look more carefully into whether gobble_file in ls.c really needs to make a copy of each filename, rather than just using the command line arguments or names from files0 input directly.] So i ended up following the pattern closer to what's in sort.c. Note that argv-iter is not used in there either, perhaps for a similar reason (it's not going to bring the memory requirements down to O(1), the way it could for wc and du). The one case ls might possibly find an improvement with argv-iter is for unsorted, non-column output (so, specifically -U1), where the names are only needed once. But in that case there's no need to use the '--files0-from' option for global sort or summary info -- you could use 'xargs -0 ls -U1 ...' instead for identical output. It just didn't seem like there was a strong argument for it (same memory scaling regardless). And the other frank truth is (if you look at wc.c and du.c for examples), it seemed like adding argv_iterator processing would significantly complicate the code. Those were my thoughts anyway :) Have a good one, Carl
[PATCH] ls: add --files0-from=FILE option
Greetings Coreutils,
I'm submitting for your consideration here a patch to add the standard
'--files0-from=FILE' option to ls.
(To read NUL-terminated names from FILE rather than using command line
arguments.)
Motivation for adding this to ls is mainly to let ls sort arbitrarily many
items, though it is also necessary for getting the correct aggregate
column widths to align long format (-l) output across all file names.
As a real example, if you want to use ls to list (say, in long format with
human sizes) all the sources in the linux kernel tree according to size,
you might naively try one of
[linux]$ find -name '*.[ch]' -exec ls -lrSh {} +
[linux]$ find -name '*.[ch]' -print0 | xargs -0 ls -lrSh
but you'll see the sizes spiral over and over, finally ending somewhere in
the middle:
...
-rw-r--r-- 1 kx users 81K Apr 15 04:30 ./arch/arm/boot/dts/imx6sll-pinfunc.h
-rw-r--r-- 1 kx users 83K Apr 15 04:30 ./arch/arm/boot/dts/imx6dl-pinfunc.h
-rw-r--r-- 1 kx users 87K Apr 15 04:30 ./arch/arm/mach-imx/iomux-mx35.h
-rw-r--r-- 1 kx users 107K Apr 15 04:30 ./arch/arm/boot/dts/imx7d-pinfunc.h
-rw-r--r-- 1 kx users 143K Apr 15 04:30 ./arch/arm/boot/dts/imx6sx-pinfunc.h
In this case, xargs batches *13* separate invocations of ls; so the
overall sorting is completely lost.
But with the new option:
[linux]$ find -name '*.[ch]' -print0 | ls -lrSh --files0-from=-
The sizes all scroll in order, finally ending in
...
-rw-r--r-- 1 kx users 5.0M Apr 15 04:31
./drivers/gpu/drm/amd/include/asic_reg/nbio/nbio_7_4_sh_mask.h
-rw-r--r-- 1 kx users 5.5M Apr 15 04:31
./drivers/gpu/drm/amd/include/asic_reg/dcn/dcn_1_0_sh_mask.h
-rw-r--r-- 1 kx users 6.6M Apr 15 04:31
./drivers/gpu/drm/amd/include/asic_reg/dce/dce_12_0_sh_mask.h
-rw-r--r-- 1 kx users 13M Apr 15 04:31
./drivers/gpu/drm/amd/include/asic_reg/nbio/nbio_7_0_sh_mask.h
-rw-r--r-- 1 kx users 14M Apr 15 04:31
./drivers/gpu/drm/amd/include/asic_reg/nbio/nbio_6_1_sh_mask.h
(The biggest files are where they belong, at the end of the listing.)
...
Similarly, say you would like to view / scroll through your extensive mp3
collection in chronological order (based on when you added the files to
your collection). You can do it now with the new option:
[music]$ find -name \*.mp3 -print0 | ls -lrth --files0-from=-
(Sidenote: Rob, the record store owner in High Fidelity (2000) [who was
also known for his habit of making "Top 5" ordered lists], called this
sorting of his records "autobiographical" [1].)
...
Additionally, note that ls can already list and sort an individual
directory with arbitrarily many entries, but you run into trouble if you
want to limit the output to a subset of those entries (eg, a particular
glob pattern).
For instance if you have a system with many status files in a directory
representing tasks, and want to list in chronological order the
'completed' tasks, with something like:
[tasks]$ ls -lrt *.completed
This will eventually fail, once the argument list limit is hit.
Again, a robust solution is possible with the new option:
[tasks]$ find -mindepth 1 -maxdepth 1 -name \*.completed -printf '%f\0' |
ls -lrt --files0-from=-
(The more complicated find expression is used here just to demonstrate how
to match the behavior of the single-directory ls invocation with a glob
pattern.)
That's about it. The feature should already be well understood from other
programs, but hopefully the examples demonstrate its utility within ls.
Any feedback / requests for improvement are of course welcome.
Carl
-=-=-+-=-=-
[1] https://youtu.be/AQvOnDlql5gFrom f47996c749d7f155a10ca0fa8c1976821059ad50 Mon Sep 17 00:00:00 2001
From: Carl Edquist
Date: Fri, 10 May 2019 17:05:47 -0500
Subject: [PATCH] ls: add --files0-from=FILE option
Useful for things like
find [...] -type f -print0 | ls -lrSh --files0-from=-
where an attempt to do the same with 'find -print0 | xargs -0' or
'find -exec' would fail to fit all the filenames onto a single command
line, and thus ls would not sort all the input items together, nor
align the columns from -l across all items.
* src/ls.c (files_from): New var for input filename.
(long_options): Add new long option.
(read_files0): Add helper function to consume files0 input.
(main): Add logic for files_from handling.
(decode_switches): Handle FILES0_FROM_OPTION.
* tests/ls/files0-from-option.sh: Excercise new option.
* tests/local.mk: Include new test.
* doc/coreutils.texi: Document --files0-from=FILE option.
* NEWS: Mention the new feature.
---
NEWS | 3 +++
doc/coreutils.texi | 2 ++
src/ls.c | 61 +++---
tests/local.mk | 1 +
tests/ls/files0-from-option.sh | 40 +++
5 files changed, 104 insertions(+), 3 deletions(-)
create mode 100755 tests/ls/files0-from-option.sh
diff --git a/NEWS
Re: [PATCH] ls: add --files0-from=FILE option
On Mon, 19 Apr 2021, Carl Edquist wrote:
I'm submitting for your consideration here a patch to add the standard
'--files0-from=FILE' option to ls.
Oops! Somehow I ended up attaching an old version of the patch - this one
includes some cleanups and help output.
Thanks,
CarlFrom 30fb711962bbd432aea4268baf93d7ba17aae4bc Mon Sep 17 00:00:00 2001
From: Carl Edquist
Date: Fri, 10 May 2019 17:05:47 -0500
Subject: [PATCH] ls: add --files0-from=FILE option
Useful for things like
find [...] -type f -print0 | ls -lrSh --files0-from=-
where an attempt to do the same with 'find -print0 | xargs -0' or
'find -exec' would fail to fit all the file names onto a single command
line, and thus ls would not sort all the input items together, nor
align the columns from -l across all items.
* src/ls.c (files_from): New var for input filename.
(long_options): Add new long option.
(read_files0): Add helper function to consume files0 input.
(main): Add logic for files_from handling.
(decode_switches): Handle FILES0_FROM_OPTION.
* tests/ls/files0-from-option.sh: Excercise new option.
* tests/local.mk: Include new test.
* doc/coreutils.texi: Document --files0-from=FILE option.
* NEWS: Mention the new feature.
---
NEWS | 3 ++
doc/coreutils.texi | 2 ++
src/ls.c | 69 +++---
tests/local.mk | 1 +
tests/ls/files0-from-option.sh | 40
5 files changed, 111 insertions(+), 4 deletions(-)
create mode 100755 tests/ls/files0-from-option.sh
diff --git a/NEWS b/NEWS
index 090fbc7..7f7f86b 100644
--- a/NEWS
+++ b/NEWS
@@ -73,6 +73,9 @@ GNU coreutils NEWS-*- outline -*-
ls now accepts the --sort=width option, to sort by file name width.
This is useful to more compactly organize the default vertical column output.
+ ls now accepts the --files0-from=FILE option, where FILE contains a
+ list of NUL-terminated file names.
+
nl --line-increment can now take a negative number to decrement the count.
** Improvements
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 3e3aedb..e008746 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7467,6 +7467,8 @@ option has been specified (@option{--classify} (@option{-F}),
@option{--dereference} (@option{-L}), or
@option{--dereference-command-line} (@option{-H})).
+@filesZeroFromOption{ls,,sorted output (and aligned columns in -l mode)}
+
@item --group-directories-first
@opindex --group-directories-first
Group all the directories before the files and then sort the
diff --git a/src/ls.c b/src/ls.c
index 4586b5e..0d0678b 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -102,6 +102,7 @@
#include "mpsort.h"
#include "obstack.h"
#include "quote.h"
+#include "readtokens0.h"
#include "smack.h"
#include "stat-size.h"
#include "stat-time.h"
@@ -356,6 +357,9 @@ static bool align_variable_outer_quotes;
static void **sorted_file;
static size_t sorted_file_alloc;
+/* input filename for --files0-from */
+static char *files_from;
+
/* When true, in a color listing, color each symlink name according to the
type of file it points to. Otherwise, color them according to the 'ln'
directive in LS_COLORS. Dangling (orphan) symlinks are treated specially,
@@ -833,6 +837,7 @@ enum
COLOR_OPTION,
DEREFERENCE_COMMAND_LINE_SYMLINK_TO_DIR_OPTION,
FILE_TYPE_INDICATOR_OPTION,
+ FILES0_FROM_OPTION,
FORMAT_OPTION,
FULL_TIME_OPTION,
GROUP_DIRECTORIES_FIRST_OPTION,
@@ -892,6 +897,7 @@ static struct option const long_options[] =
{"block-size", required_argument, NULL, BLOCK_SIZE_OPTION},
{"context", no_argument, 0, 'Z'},
{"author", no_argument, NULL, AUTHOR_OPTION},
+ {"files0-from", required_argument, NULL, FILES0_FROM_OPTION},
{GETOPT_HELP_OPTION_DECL},
{GETOPT_VERSION_OPTION_DECL},
{NULL, 0, NULL, 0}
@@ -1626,11 +1632,38 @@ signal_restore (void)
signal_setup (false);
}
+static void
+read_files0(struct Tokens *tokp)
+{
+ FILE *stream;
+ if (STREQ (files_from, "-"))
+stream = stdin;
+ else
+{
+ stream = fopen (files_from, "r");
+ if (stream == NULL)
+die (EXIT_FAILURE, errno, _("cannot open %s for reading"),
+ quoteaf (files_from));
+}
+
+ readtokens0_init (tokp);
+
+ if (! readtokens0 (stream, tokp) || fclose (stream) != 0)
+die (LS_FAILURE, 0, _("cannot read file names from %s"),
+ quoteaf (files_from));
+
+ if (! tokp->n_tok)
+die (LS_FAILURE, 0, _("no input from %s"),
+ quoteaf (files_from));
+}
+
int
main (int argc, char **argv)
{
int i;
struct pending *thispend;
+ struct Tokens tok;
+ char **files;
int n_files;
initialize_main (, );
@@ -1727,6 +1760,23 @@ main (int argc, char **argv)
clear_files ();
Re: [PATCH] ls: add --sort=width (-W) option to sort by filename width
Hi Kaz, On Fri, 16 Apr 2021, Kaz Kylheku (Coreutils) wrote: > The multi column output can be done by ... I don't know if you read the original post in this thread, but it contains a brief (but complete) example for doing all this in the shell, and handles arbitrary characters in filenames. But one problem mentioned with measuring the "length" of filenames (number of characters), say in (g)awk, is that it does not consider the printed widths (in terminal columns) of unicode characters (sometimes multiple columns wide), which ls obtains via wcwidth(3). Carl
Re: [PATCH] ls: add --sort=width (-W) option to sort by filename width
On Thu, 15 Apr 2021, p...@draigbrady.com wrote: > On 15/04/2021 14:33, Carl Edquist wrote: >> PS if you are willing to cheat a bit and treat the f->width member as >> "mutable" (modifying it even when f is pointer to const), you could >> simplify things a bit if you just store the value from >> quote_name_width() in fileinfo_name_width(). Something like [1]. > > Yes that was my first iteration, > but it felt wrong to cast the const away in the presentation functions. > I think this way is more maintainable in future. Makes sense. Too bad there isn't a cleaner way to specify a mutable member of a const struct in C. Thanks! Carl
Re: [PATCH] ls: add --sort=width (-W) option to sort by filename width
On Thu, 15 Apr 2021, Michael Stone wrote: I think it's a heck of a lot easier to add short options than it is to remove them, and that there are more ideas for things to do than there are letters. I'm on the side of make it a long option. In a few years if it's a commonly used thing with a groundswell of demand for a short option, it can always be added. There are a lot of existing short options that aren't used very often, are redundant, or are simply confusing, and it would be really nice (but impossible) to have those letters back. Continuing the tradition of burning short options by default just makes the hole deeper. That makes sense! Thanks for the feedback, I appreciate the discussion. Carl
Re: [PATCH] ls: add --sort=width (-W) option to sort by filename width
Hi all,
On Sun, 11 Apr 2021, Jim Meyering wrote:
> On Sat, Apr 10, 2021 at 8:22 PM Pádraig Brady wrote:
>> I really like how much more concise it makes output in general.
>
> Nice. I like this new option, too! Who would have thought :-)
Glad to hear the positive reception! :)
Thank you for accepting the feature.
Regarding the short option, I have some thoughts for your consideration
... but at the end of the day, of course do whatever makes sense for GNU.
> I would avoid the -W short option, as that would clash with ls from
> FreeBSD for example.
That is a bit unfortunate about the option clash. I admit was only paying
attention to GNU ls, where -W was, um, wide open...
If the concern is clashing with a FreeBSD ls option, maybe use -Y instead?
(Mnemonic: 'Y' for "wide" ?)
There are only a handful of mutually-free short-option letters ...
$ echo {a..z} {A..Z} |
tr -d aAbBcCdDfFgGhHiIklLmnNopqQrRsStTuUvwxXZ | # GNU ls options
tr -d ABCDFGHILPRSTUWZabcdfghiklmnopqrstuwxy | # FreeBSD ls options[2]
tr -d AaBbCcdFfghikLlMmnOoPpqRrSsTtuWwXx | # NetBSD ls options[3]
tr -d ACFHLRSTacdfghiklmnopqrstux | # OpenBSD ls options[4]
tr -d ' '
ejzEJKVY
None comes quite as naturally as -W though...
...
>From what I can tell[1][2], FreeBSD ls's -W option has to do with the
(BSD-specific) directory whiteout entries. It was added in the 90s,
so I don't get the feeling there has been any desire for GNU ls to adopt
that feature.
It just feels like a bit of a waste to retire an unused option character
just because a BSD ls is using it for a feature that GNU doesn't seem to
care about anyway.
And are the GNU and BSD flavors of ls really known for having compatible
sets of option flags..? (admittedly a rhetorical question)
For one, the other GNU -w option (--width) already clashes with FreeBSD's
-w (raw-print non-printable chars to terminal ... AKA -N in GNU ls). So
maybe using -W in GNU ls for a different feature too would not be that
out-of-character.
(In case anybody's counting, the existing -B -D -G -I -T -U -w & -Z
options from GNU ls all clash with FreeBSD ls. So it's already a bit of a
jumble beyond reconciling.)
I'll also note here that for GNU ls's other sort options, the
_super-useful_ -v (--sort=version), and even -X (--sort=extension)
apparently never made it into the BSDs.
If FreeBSD hasn't added -X by now (which has been around in GNU ls since
at least 1992), I imagine maybe they never will.
Anyway, maybe the BSDs just don't care to adopt cool GNU extensions, which
is fine - but in that case maybe there's no need to hold out for them.
...
But if there is any hope to leave the door open for the possibility that
FreeBSD ls adopts a width-sort option, with the same short-option letter
as GNU ls, then as mentioned I might suggest '-Y'.
...
In the last case (not providing a short option at all), it might be
entertaining/ironic if FreeBSD adopts the feature (with a short option, of
course, as they don't have a --sort). They either pick a letter the GNU
ls already uses for something else (putting -W back on the table for
GNU?), or they pick a letter that GNU ls is also not yet using, and then
GNU can consider whether to follow suit with the letter FreeBSD picks :)
> It's probably best to not provide a short option for this at all.
Personally, I don't mind so much typing out long option names for scripts
(they are write-once, run-many; and the long names are self-documenting
for an unfamiliar reader). But for interactive command line use, short
options (as with short program names!) become a feature in themselves.
They flow all the more effortlessly off the fingertips without penalizing
the user for wanting to use them the fly.
If I had to type 'ls --format=long' every time I wanted to see 'ls -l', I
suspect I would be less inclined to use it.
(Sidenote: does FreeBSD ls even _have_ long options? :)
...
Anyway, after some time if you find yourself wanting to use the feature a
lot but being bored to type --sort=width each time, maybe that will
provide more inclination to add a short option...
("Yes," you might say, "but that's what shell aliases are for."
But...don't you like munging short options? :)
"Search your feelings, Luke."
...
Lastly, in the help / manpage output, "width" will looks a bit lonely
among the sort options, as the only one without an accompanying short
option:
--sort=WORD sort by WORD instead of name: none (-U), size (-S),
time (-t), version (-v), extension (-X), width
And, it might be worth noting that in the help / manpage output, the
_only_ documentation (although pithy) for what the different sort types
actually do is in the description for their corresponding short options.
So, you lose the chance to document it in the --help output if you omit
the short opiton.
...
What do you think? :)
In any case thanks again for accepting the
Re: [PATCH] ls: add --sort=width (-W) option to sort by filename width
On Sat, 10 Apr 2021, p...@draigbrady.com wrote:
> I've attached two patches ...
>
> The first adjusts your patch to:
> ...
> Expand in texinfo that --sort=width is most useful with -C (the default)
FWIW i run 'ls -lW' often enough also, to refresh myself of the details of
the files with pesky long names that i might want to do something about.
> The second is a performance improvement,
> as we're now calling quote_name_width a lot more.
Cool!
Yeah in theory i felt bad about the performance overhead, but in practice
it took a directory with on the order of 10k entries before the runtime of
'ls -W >/dev/null' would even be visible to the human eye, after hitting
the enter key. Which seemed reasonable for most interactive purposes.
And if you are actually writing to the terminal in such cases, the runtime
is completely dominated by the terminal rendering.
Still, the version with caching is definitely more efficient, so that is
great.
PS if you are willing to cheat a bit and treat the f->width member as
"mutable" (modifying it even when f is pointer to const), you could
simplify things a bit if you just store the value from quote_name_width()
in fileinfo_name_width(). Something like [1].
Then you can completely drop the new update_current_files_info function,
and get the caching automatically whenever width needs to be calculated.
(And i see your point about avoiding saving the value in the case of
'--format=commas', to maximize hardware caching - but as far as i could
tell, even with 100k directory entries, there was no measurable cpu
performance hit with --format=commas between the current version with
update_current_files_info(), and the version that always saves f->width
when it is calculated.)
Anyway, fun times! :)
Carl
[1]
--- a/src/ls.c
+++ b/src/ls.c
@@ -3892,9 +3892,12 @@ cmp_extension (struct fileinfo const *a, struct fileinfo
const *b,
static inline size_t
fileinfo_name_width (struct fileinfo const *f)
{
- return f->width
- ? f->width
- : quote_name_width (f->name, filename_quoting_options, f->quoted);
+ if (!f->width)
+{
+ ((struct fileinfo *) f)->width =
+quote_name_width (f->name, filename_quoting_options, f->quoted);
+}
+ return f->width;
}
static inline int
Re: [PATCH] ls: add --sort=width (-W) option to sort by filename width
On Fri, 9 Apr 2021, lsatenst...@yahoo.com wrote: > Perhaps FreeBSD would be interested in this enhancement. Sure, why not. The implementation will be different though, as the ls internals are fairly different for the BSDs. (And they'd have to use a short option flag, since they don't have a separate --sort option.) But the concept is simple enough if someone from BSD-land would like it. Sometimes the hard part is just figuring out that something simple might be useful. Carl
[PATCH] ls: add --sort=width (-W) option to sort by filename width
st.c
tty.c mknod.c remove.h cu-progs.mk make-prime-list.o
who.c nohup.c runcon.c dircolors.c find-mount-point.c
yes.c nproc.c stdbuf.c dircolors.h find-mount-point.h
blake2 paste.c system.h getlimits.c a-some-obnoxiously-longish-filename
comm.c pinky.c unlink.c ioblksize.h z-some-obnoxiously-longish-filenameFrom dc7cd08682a7618e1bb2ef9764960e39de14237f Mon Sep 17 00:00:00 2001
From: Carl Edquist
Date: Fri, 26 Mar 2021 04:27:54 -0500
Subject: [PATCH] ls: add --sort=width (-W) option to sort by filename width
This helps identify the outliers for long filenames, and also produces
a more compact display of columns when listing a directory with many
entries of various widths.
* src/ls.c (sort_type, sort_types, sort_width): New sort_width sort
type.
(sort_args): Add "width" sort arg.
(decode_switches): Parse '-W' option.
(cmp_width, fileinfo_width): New sort function and helper for filename
width.
(quote_name_width): Add function prototype declaration.
(usage): Document -W/--sort=width option.
* doc/coreutils.texi: Document -W/--sort=width option.
* tests/local.mk: Add new test.
* tests/ls/sort-width_W-option.sh: Exercise --sort=width and -W options.
* NEWS: Mention the new feature.
---
NEWS| 2 ++
doc/coreutils.texi | 7 ++
src/ls.c| 36 ---
tests/local.mk | 1 +
tests/ls/sort-width_W-option.sh | 43 +
5 files changed, 85 insertions(+), 4 deletions(-)
create mode 100755 tests/ls/sort-width_W-option.sh
diff --git a/NEWS b/NEWS
index 802f4b427..4ba164e85 100644
--- a/NEWS
+++ b/NEWS
@@ -70,6 +70,8 @@ GNU coreutils NEWS-*- outline -*-
ls --classify now supports the "always", "auto", or "never" flags,
to support only outputting classifier characters if connected to a tty.
+ ls now accepts the --sort=width (-W) option, to sort by filename width.
+
nl --line-increment can now take a negative number to decrement the count.
** Improvements
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 06ecdd74c..0c7bb8d44 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7939,6 +7939,13 @@ Sort by version name and number, lowest first. It behaves like a default
sort, except that each sequence of decimal digits is treated numerically
as an index/version number. (@xref{Version sort ordering}.)
+@item -W
+@itemx --sort=width
+@opindex -W
+@opindex --sort
+@opindex width@r{, sorting option for @command{ls}}
+Sort by printed width of filenames.
+
@item -X
@itemx --sort=extension
@opindex -X
diff --git a/src/ls.c b/src/ls.c
index 2d0450e54..12ea550e3 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -307,6 +307,10 @@ static void parse_ls_color (void);
static void getenv_quoting_style (void);
+static size_t quote_name_width (const char *name,
+struct quoting_options const *options,
+int needs_general_quoting);
+
/* Initial size of hash table.
Most hierarchies are likely to be shallower than this. */
#define INITIAL_TABLE_SIZE 30
@@ -475,6 +479,7 @@ enum sort_type
sort_none = -1, /* -U */
sort_name, /* default */
sort_extension, /* -X */
+sort_width, /* -W */
sort_size, /* -S */
sort_version, /* -v */
sort_time, /* -t */
@@ -903,11 +908,11 @@ ARGMATCH_VERIFY (format_args, format_types);
static char const *const sort_args[] =
{
- "none", "time", "size", "extension", "version", NULL
+ "none", "time", "size", "extension", "version", "width", NULL
};
static enum sort_type const sort_types[] =
{
- sort_none, sort_time, sort_size, sort_extension, sort_version
+ sort_none, sort_time, sort_size, sort_extension, sort_version, sort_width
};
ARGMATCH_VERIFY (sort_args, sort_types);
@@ -1958,7 +1963,7 @@ decode_switches (int argc, char **argv)
{
int oi = -1;
int c = getopt_long (argc, argv,
- "abcdfghiklmnopqrstuvw:xABCDFGHI:LNQRST:UXZ1",
+ "abcdfghiklmnopqrstuvw:xABCDFGHI:LNQRST:UWXZ1",
long_options, );
if (c == -1)
break;
@@ -2155,6 +2160,11 @@ decode_switches (int argc, char **argv)
sort_type_specified = true;
break;
+case 'W':
+ sort_type = sort_width;
+ sort_type_specified = true;
+ break;
+
case 'X':
sort_type = sort_extension;
sort_type_specified = true;
@@ -3877,6 +3887,20 @@ cmp_extension (struct fileinfo const *a, struct fileinfo const *b,
return diff ? diff : cmp (a->name, b->name);
}
+static inline size_t
+fileinfo_width (struct fileinfo const *f)
+{
+ return quote_name_width (f->na
Re: submitting contributions
On Thu, 8 Apr 2021, p...@draigbrady.com wrote: A `git format-patch` attached to an email is easiest to apply and test locally. Generally emails would be more verbose when discussing patches. As for the commits themselves my rule of thumb for the commit message is: reason for change limited to 70 chars or so (the why) Optional short paragraph giving an overview of the change. Usually this is only included for new features / substantial changes, that require some context. * file_name_1 (where): what has changed (the what) * file_name_n (where): Likewise. Thanks Pádraig, that sounds good! Carl
submitting contributions
Greetings Coreutils maintainers! I have a few small contributions I am interested to submit to Coreutils. Since I'm new to the list, I'd like to ask if there are any etiquette things I should have in mind going about this. I have read over the HACKING document, but I am still a little fuzzy about what the process looks like to submit a patch to the list. It discusses running 'git format-patch', but it's not clear if the patch output should be included in-line in the email, or as an attachment. Basically, what does the normal process look like once I have a commit ready to submit? And, is it OK to include a longer discussion in the email about the changes, besides what's in the commit message itself? Thanks for any tips! Carl
Re: make ls -l dir1 dir2 in the same order as dir1,2 are specified
On Sat, 27 Mar 2021, L A Walsh wrote: On 2021/03/27 12:34, Glenn Golden wrote: $ ls -fl dir1 dir2 will produce what the OP asked for. -- Interesting. How can ls return the files in the order they are on disk? What you want is the '-U' option (same as --sort=none). This will display items in the order they appear on the command line, and will list directory entries in the order they appear on disk. (The '-f' option that Glenn mentioned implies '-U', but also has a couple other affects, like listing dot-files (-a) and disabling color.)
Re: make ls -l dir1 dir2 in the same order as dir1,2 are specified
Try adding -U On Fri, 26 Mar 2021, pengyu...@gmail.com wrote: Hi, When I try `ls -l dir1 dir2`, the order of dir1 and dir2 in the output is not necessarily the same as the input. How to make it the same as the input order? Thanks. -- Regards, Peng
