public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Jeff King <peff@peff•net>
To: "Rubén Justo" <rjusto@gmail•com>
Cc: phillip.wood@dunelm•org.uk, Git List <git@vger•kernel.org>,
	Junio C Hamano <gitster@pobox•com>,
	Dragan Simic <dsimic@manjaro•org>
Subject: Re: [PATCH v4 5/6] test-terminal: introduce --no-stdin-pty
Date: Thu, 6 Jun 2024 04:27:48 -0400	[thread overview]
Message-ID: <20240606082748.GD658959@coredump.intra.peff.net> (raw)
In-Reply-To: <9c76f1f3-f858-400e-8fc7-8e3bc9764e87@gmail.com>

On Thu, Jun 06, 2024 at 12:50:39AM +0200, Rubén Justo wrote:

> On Tue, Jun 04, 2024 at 11:05:15AM +0100, Phillip Wood wrote:
> 
> > Rather than adding a new flag to work around a bug in our script it might be
> > better to try and fix the bug by using a loop that reads blocks of data from
> > the source and writes them to the destination instead of calling copy.
> 
> To be honest, I've tried.  I haven't found a way to fix it properly.

I think File::Copy() is not the culprit. The problem is more at the
syscall level. Once test_terminal finishes writing all of the output to
the tty, it closes its end of the descriptor. And now the tty is "gone",
isatty() returns false, and further reads get EIO (even if we didn't
read all of the bytes! They're lost).

So I think the only thing we could do is _not_ actually close the
descriptor. But now we don't have a way to signal EOF to the other side.
Unless perhaps there is some clever way to do so. I'd still favor
ripping it out.

> I also thought about removing it.  I agree with Peff, removing the stdin
> redirection makes more sense.  IMHO simplifies.
> 
> But, perhaps we can happily live another almost-decade with this new
> --no-stdin-pty, before finally removing the stdin redirection in this
> helper. :-)

Hopefully not a decade. ;) I just sent a series, and I think you could
either build on top, or the merge resolution could just drop your new
option.

-Peff

  reply	other threads:[~2024-06-06  8:27 UTC|newest]

Thread overview: 113+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-19  7:06 [PATCH 0/5] use the pager in 'add -p' Rubén Justo
2024-05-19  7:10 ` [PATCH 1/5] add-patch: test for 'p' command Rubén Justo
2024-05-19  7:12 ` [PATCH 2/5] pager: do not close fd 2 unnecessarily Rubén Justo
2024-05-20 19:14   ` Junio C Hamano
2024-05-20 22:33     ` Rubén Justo
2024-05-21 20:57       ` Junio C Hamano
2024-05-21 21:35         ` Rubén Justo
2024-05-21 22:00           ` Junio C Hamano
2024-05-22 17:19             ` Rubén Justo
2024-05-22 17:40               ` Junio C Hamano
2024-05-26  6:48                 ` Rubén Justo
2024-05-26 21:26                   ` Junio C Hamano
2024-05-19  7:13 ` [PATCH 3/5] pager: introduce wait_for_pager Rubén Justo
2024-05-19  7:14 ` [PATCH 4/5] test-terminal: introduce --no-stdin-pty Rubén Justo
2024-05-19  7:14 ` [PATCH 5/5] add-patch: render hunks through the pager Rubén Justo
2024-05-20 19:30   ` Junio C Hamano
2024-05-20 19:45     ` Dragan Simic
2024-05-20 22:35       ` Rubén Justo
2024-05-20 23:54         ` Dragan Simic
2024-05-21 19:56           ` Rubén Justo
2024-05-21  7:07       ` Jeff King
2024-05-21 19:59         ` Rubén Justo
2024-05-23  9:06           ` Jeff King
2024-05-23 14:00             ` Junio C Hamano
2024-05-23 14:18               ` Dragan Simic
2024-05-23 23:04                 ` Junio C Hamano
2024-05-23 23:28                   ` Dragan Simic
2024-05-23 23:43                     ` Dragan Simic
2024-05-23 23:54                     ` Junio C Hamano
2024-05-23 23:57                       ` Dragan Simic
2024-05-25  4:54               ` Jeff King
2024-05-23 22:25             ` Rubén Justo
2024-05-23 23:03               ` Dragan Simic
2024-05-20 22:47     ` Rubén Justo
2024-05-20 23:18       ` Junio C Hamano
2024-05-20 23:27         ` Rubén Justo
2024-05-21 20:49 ` [PATCH v2 0/5] use the pager in 'add -p' Rubén Justo
2024-05-21 20:51   ` [PATCH v2 1/5] add-patch: test for 'p' command Rubén Justo
2024-05-21 20:52   ` [PATCH v2 2/5] pager: do not close fd 2 unnecessarily Rubén Justo
2024-05-21 20:52   ` [PATCH v2 3/5] pager: introduce wait_for_pager Rubén Justo
2024-05-21 20:52   ` [PATCH v2 4/5] test-terminal: introduce --no-stdin-pty Rubén Justo
2024-05-21 20:52   ` [PATCH v2 5/5] add-patch: render hunks through the pager Rubén Justo
2024-05-22  8:09     ` Dragan Simic
2024-05-22 18:47       ` Junio C Hamano
2024-05-22 21:23       ` Rubén Justo
2024-05-22 21:27         ` Dragan Simic
2024-06-02 15:38   ` [PATCH v3 0/6] use the pager in 'add -p' Rubén Justo
2024-06-02 15:42     ` [PATCH v3 1/6] add-patch: test for 'p' command Rubén Justo
2024-06-02 15:42     ` [PATCH v3 2/6] pager: do not close fd 2 unnecessarily Rubén Justo
2024-06-02 15:43     ` [PATCH v3 3/6] pager: introduce wait_for_pager Rubén Justo
2024-06-02 15:43     ` [PATCH v3 4/6] pager: introduce setup_custom_pager Rubén Justo
2024-06-02 15:43     ` [PATCH v3 5/6] test-terminal: introduce --no-stdin-pty Rubén Justo
2024-06-02 15:44     ` [PATCH v3 6/6] add-patch: introduce the command '|' Rubén Justo
2024-06-02 16:36     ` [PATCH v3 0/6] use the pager in 'add -p' Junio C Hamano
2024-06-02 17:11       ` Junio C Hamano
2024-06-02 17:33         ` Rubén Justo
2024-06-02 17:13       ` Rubén Justo
2024-06-02 17:46       ` Dragan Simic
2024-06-03  9:03         ` Junio C Hamano
2024-06-03 10:21           ` Dragan Simic
2024-06-03 15:28             ` Junio C Hamano
2024-06-04 17:34               ` Dragan Simic
2024-06-02 17:36     ` Dragan Simic
2024-06-03 16:01       ` Junio C Hamano
2024-06-04 17:41         ` Dragan Simic
2024-06-04 17:42           ` Dragan Simic
2024-06-03 20:19       ` Rubén Justo
2024-06-04 18:13         ` Dragan Simic
2024-06-03 20:35     ` [PATCH v4 " Rubén Justo
2024-06-03 20:38       ` [PATCH v4 1/6] add-patch: test for 'p' command Rubén Justo
2024-06-03 20:38       ` [PATCH v4 2/6] pager: do not close fd 2 unnecessarily Rubén Justo
2024-06-04 15:50         ` Junio C Hamano
2024-06-03 20:38       ` [PATCH v4 3/6] pager: introduce wait_for_pager Rubén Justo
2024-06-04 10:00         ` Phillip Wood
2024-06-04 16:29           ` Junio C Hamano
2024-06-05 22:03           ` Rubén Justo
2024-06-04 16:25         ` Junio C Hamano
2024-06-03 20:38       ` [PATCH v4 4/6] pager: introduce setup_custom_pager Rubén Justo
2024-06-04 16:43         ` Junio C Hamano
2024-06-03 20:38       ` [PATCH v4 5/6] test-terminal: introduce --no-stdin-pty Rubén Justo
2024-06-04 10:05         ` Phillip Wood
2024-06-04 10:33           ` Jeff King
2024-06-05 15:39             ` Junio C Hamano
2024-06-06  8:24               ` Jeff King
2024-06-05 22:50           ` Rubén Justo
2024-06-06  8:27             ` Jeff King [this message]
2024-06-09  7:26               ` Rubén Justo
2024-06-03 20:38       ` [PATCH v4 6/6] add-patch: introduce the command '|' Rubén Justo
2024-06-04 17:12         ` Junio C Hamano
2024-06-04 20:05           ` Dragan Simic
2024-06-05  5:16           ` Junio C Hamano
2024-06-04 10:17     ` [PATCH v3 0/6] use the pager in 'add -p' Jeff King
2024-06-04 15:32       ` Junio C Hamano
2024-06-05  9:09         ` Jeff King
2024-06-05 13:21           ` Phillip Wood
2024-06-08  5:54             ` Dragan Simic
2024-06-09  7:44               ` Rubén Justo
2024-06-09  7:57                 ` Dragan Simic
2024-06-10 19:09                   ` Rubén Justo
2024-06-10 21:02                     ` Dragan Simic
2024-06-10 14:09                 ` Phillip Wood
2024-06-10 16:13                   ` Junio C Hamano
2024-06-10 19:14                   ` Rubén Justo
2024-06-10 19:56                     ` Junio C Hamano
2024-06-10 21:08                     ` Dragan Simic
2024-06-10 19:28                   ` Dragan Simic
2024-06-10 20:08                     ` Junio C Hamano
2024-06-10 21:16                       ` Dragan Simic
2024-06-09 14:29               ` phillip.wood123
2024-06-09 17:20                 ` Dragan Simic
2024-06-10  8:27                   ` Phillip Wood
2024-06-10  9:09                     ` Dragan Simic
2024-06-05 17:24           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240606082748.GD658959@coredump.intra.peff.net \
    --to=peff@peff$(echo .)net \
    --cc=dsimic@manjaro$(echo .)org \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=phillip.wood@dunelm$(echo .)org.uk \
    --cc=rjusto@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox