Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2023-03-01 Thread Pádraig Brady
On 01/03/2023 09:25, Arsen Arsenović wrote: Morning Padraig, Pádraig Brady writes: I'm squashing in the following to handle illumos and macOS. Also it decouples the code from the pollfd structure layout, by using C99 designated initializers. Thanks. I read over the patches you attached

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2023-03-01 Thread Arsen Arsenović
Morning Padraig, Pádraig Brady writes: > I'm squashing in the following to handle illumos and macOS. > Also it decouples the code from the pollfd structure layout, > by using C99 designated initializers. Thanks. I read over the patches you attached and they seem reasonable. Thanks for

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2023-02-28 Thread Pádraig Brady
On 27/02/2023 18:26, Pádraig Brady wrote: Attached are the current patches I have for this functionality. I'll do a bit more testing and hope to push them early tomorrow. I'm squashing in the following to handle illumos and macOS. Also it decouples the code from the pollfd structure layout, by

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2023-02-27 Thread Pádraig Brady
Attached are the current patches I have for this functionality. I'll do a bit more testing and hope to push them early tomorrow. thanks, PádraigFrom 229a87ed0e45eb5f6875147c48719820291e5238 Mon Sep 17 00:00:00 2001 From: Carl Edquist Date: Thu, 15 Dec 2022 06:10:33 -0600 Subject: [PATCH 1/4]

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2023-02-27 Thread Pádraig Brady
On 03/01/2023 17:49, Arsen Arsenović wrote: Hi Padraig, Pádraig Brady writes: Really nice work on this. Only very small syntax tweaks (attached) at this point. I'm going to do testing with this and will add an appropriate test case. I spotted some a slightly less minor error, and notified

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2023-02-02 Thread Arsen Arsenović
Pádraig Brady writes: > Yes definitely. > This is the top of my list to merge. Lovely, thanks! -- Arsen Arsenović signature.asc Description: PGP signature

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2023-02-02 Thread Pádraig Brady
On 02/02/2023 09:40, Arsen Arsenović wrote: Hi Padraig, I saw that you are planning on making a coreutils release soon. Can these patches be included in it? Thanks in advance, have a lovely day. Yes definitely. This is the top of my list to merge. thanks, Pádraig

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2023-02-02 Thread Arsen Arsenović
Hi Padraig, I saw that you are planning on making a coreutils release soon. Can these patches be included in it? Thanks in advance, have a lovely day. -- Arsen Arsenović signature.asc Description: PGP signature

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2023-01-03 Thread Arsen Arsenović
Hi Padraig, Pádraig Brady writes: > Really nice work on this. > Only very small syntax tweaks (attached) at this point. > I'm going to do testing with this and will add an appropriate test case. I spotted some a slightly less minor error, and notified Carl off-list, but you beat us to

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2023-01-03 Thread Pádraig Brady
On 15/12/2022 18:38, Carl Edquist wrote: Aaak! Sorry, please disregard the patchset in my previous email. I messed up a rebase I was doing. I am attaching a corrected patchset now. Really nice work on this. Only very small syntax tweaks (attached) at this point. I'm going to do testing with

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-15 Thread Carl Edquist
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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-13 Thread Arsen Arsenović
Hi Carl, Padraig, Thanks for the ACK. I've sent the signed copyright assignment form; I'll keep you posted on that. On Tue, 13 Dec 2022, Pádraig Brady wrote: >> Re HAVE_INOTIFY, that's really a proxy for a linux kernel, and so would be >> most appropriately changed to: >> >> defined

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-13 Thread Carl Edquist via GNU coreutils General Discussion
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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-13 Thread Pádraig Brady
On 12/12/2022 22:29, Carl Edquist wrote: 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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-12 Thread Carl Edquist via GNU coreutils General Discussion
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.

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-12 Thread Carl Edquist via GNU coreutils General Discussion
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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-12 Thread Arsen Arsenović
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,

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-12 Thread Arsen Arsenović
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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-11 Thread Rob Landley
On 12/9/22 08:26, Carl Edquist via GNU coreutils General Discussion 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. > >

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-10 Thread Carl Edquist
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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-10 Thread Carl Edquist
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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-10 Thread Carl Edquist via GNU coreutils General Discussion
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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-09 Thread Carl Edquist via GNU coreutils General Discussion
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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-09 Thread Carl Edquist via GNU coreutils General Discussion
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<>

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-09 Thread Arsen Arsenović
Carl Edquist writes: > 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!

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-08 Thread Carl Edquist via GNU coreutils General Discussion
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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-08 Thread Arsen Arsenović
Hi Carl, Apologies for my absence, Tuesdays and Wednesdays are long workdays for me. Carl Edquist writes: > 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.

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-07 Thread Carl Edquist
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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-06 Thread Carl Edquist
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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-06 Thread Carl Edquist via GNU coreutils General Discussion
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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-06 Thread Arsen Arsenović
Arsen Arsenović writes: > From 582e0a27b7995aac90cc360463ec8bde1a44cfe4 Mon Sep 17 00:00:00 2001 > From: Paul Eggert ^ Whoops, I forgot to fix this after committing with the wrong hash in --reuse-commit. I don't want to confuse anyone, I authored the patch. Apologies. -- Arsen Arsenović

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-06 Thread Arsen Arsenović
Hi Carl, Carl Edquist writes: > 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(). Yes, I do agree that this is likely cleaner. > For

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-05 Thread Carl Edquist
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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-02 Thread Carl Edquist via GNU coreutils General Discussion
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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-02 Thread Pádraig Brady
On 02/12/2022 11:47, Carl Edquist wrote: 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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-02 Thread Arsen Arsenović
Carl Edquist writes: > 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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-02 Thread Carl Edquist via GNU coreutils General Discussion
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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-12-02 Thread Carl Edquist via GNU coreutils General Discussion
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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-11-30 Thread Arsen Arsenović
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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-11-29 Thread Carl Edquist via GNU coreutils General Discussion
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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-11-29 Thread Pádraig Brady
On 29/11/2022 21:48, Carl Edquist wrote: 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). Yes this is what I was alluding to. It seems this is

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-11-29 Thread Carl Edquist via GNU coreutils General Discussion
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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-11-29 Thread Arsen Arsenović
William Bader writes: > For the case of testing two compile runs, could you use something like the > bash > command below (replacing 'sleep ...' with 'gcc ...')? 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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-11-29 Thread Arsen Arsenović
Pádraig Brady writes: > 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

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-11-29 Thread William Bader
iambader=hotmail@gnu.org on behalf of Carl Edquist via GNU coreutils General Discussion Sent: Tuesday, November 29, 2022 12:32 PM To: Arsen Arsenović Cc: Pádraig Brady ; coreutils@gnu.org Subject: Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs Oh and, ... On

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-11-29 Thread Pádraig Brady
On 29/11/2022 17:32, Carl Edquist wrote: Oh and, ... On Mon, 28 Nov 2022, Pádraig Brady wrote: I'm presuming the input generator is depending on the compiler runs (written to by tee) to exit cleanly, before exiting / generating more? Hence the hangs? If that was the case then there still

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-11-29 Thread Carl Edquist via GNU coreutils General Discussion
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"

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-11-28 Thread Arsen Arsenović
Pádraig Brady writes: > To get a better handle on the generality of this > I went back about 16 years on the list, > and even though there have been many tee functionality requests over the > years, > this is the first time this has been requested. I'm glad not to have wasted time on a

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-11-28 Thread Pádraig Brady
On 19/11/2022 19:27, Arsen Arsenović wrote: Pádraig Brady writes: Thanks a lot for the patch. There is similar functionality in tail.c (which should probably be reused if we do decide to implement this in tee as it's tricky to do portably). Anyway the tail(1) case makes sense considering:

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-11-27 Thread Arsen Arsenović
Hi, Pádraig Brady writes: > I vaguely remember on macos that POLLRDBAND needed to be set on read fds for > select, > though didn't check all combinations since we didn't actually need > the poll() replacement. Also select() (emulation of poll) doesn't work on > Solaris or AIX, so we needed to

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-11-27 Thread Pádraig Brady
On 27/11/2022 11:32, Arsen Arsenović wrote: Arsen Arsenović writes: Looking at ``check_output_alive'', this logic should be fairly easy to reuse. Worst case, I'd need to refactor some of the code to handle the non-pipe-check case more elegantly so that it doesn't become too unreadable when

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-11-27 Thread Arsen Arsenović
Arsen Arsenović writes: > Looking at ``check_output_alive'', this logic should be fairly easy to > reuse. Worst case, I'd need to refactor some of the code to handle the > non-pipe-check case more elegantly so that it doesn't become too > unreadable when also adding the logic to pick between

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-11-19 Thread Arsen Arsenović
Pádraig Brady writes: > Thanks a lot for the patch. > > There is similar functionality in tail.c (which should probably be reused > if we do decide to implement this in tee as it's tricky to do portably). > Anyway the tail(1) case makes sense considering: Looking at ``check_output_alive'',

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-11-19 Thread Pádraig Brady
On 19/11/2022 08:23, Arsen Arsenović wrote: This flag comes in useful in scripts in which tee is used to duplicate output over process substitutions, so that dying processes and no stdin won't lead to a hang. Without this option, a command like ``tee >(compiler1) >(compiler2) | compiler3''

[PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs

2022-11-19 Thread Arsen Arsenović
This flag comes in useful in scripts in which tee is used to duplicate output over process substitutions, so that dying processes and no stdin won't lead to a hang. Without this option, a command like ``tee >(compiler1) >(compiler2) | compiler3'' would hang forever if stdin was a terminal and the