On 2026-09-29 Tu 4:20 AM, Nazir Bilal Yavuz wrote:
Hi,

On Tue, 29 Sept 2026 at 02:24, Michael Paquier <[email protected]> wrote:
On Mon, Sep 28, 2026 at 12:54:18PM -0400, Andrew Dunstan wrote:
I think the right way is to have a wrapper function as in the attached, so
instead of calling IPC::Run::run you would call ipc_run ...

Note that this patch only touches the sites that will actually be affected
by this. If we wanted to be consistent we would replace calls in a further
23 files.
That seems much better in the long-run, thanks.  The
ipc_run_text_mode() being published in Utils.pm does not seem
necessary, though.  This is a part that I'm sure could become
confusing if we allow code outside of Utils.pm to use it.
I tested Andrew Dunstan's patch in CI after removing the IPC::Run
version pin from the Windows tasks. The 035_standby_logical_decoding
test failed on both the Windows VS and MinGW tasks [1]:

```
stderr:
# die: IPC::Run: timeout on timer #17 at
C:/Strawberry/perl/site/lib/IPC/Run.pm line 3361.
```

Using ipc_start() in the 035_standby_logical_decoding test fixes the
failure, and CI now passes [2].

Here are three patches:
- 0001 is Andrew Dunstan's patch, unchanged.
- 0002 is a fixup for the 035_standby_logical_decoding test.
- 0003 removes the IPC::Run version pin from the Windows tasks.

[1] https://github.com/nbyavuz/postgres/actions/runs/36537214540
[2] https://github.com/nbyavuz/postgres/actions/runs/36537155885


Great, thanks for testing. I agree with Michael. In fact I think we could just get rid of ipc_run_text_mode altogether and just put the binary call inline in the only place it's used.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com



Reply via email to