Re: [Tarantool-patches] [PATCH 0/7] popen: various fixes and a test

[Alexander Turenko <alexander.turenko@tarantool.org>]

I don't have objections anymore. LGTM.

The actual version is on the branch gorcunov/gh-4031-popen-fixup-5.

I'm a bit afraid that we don't test POPEN_FLAG_SETSID. However the code
itself looks right and we maybe will return to this later.

NB: We'll need to test two things to verify setsid():

* The situation when a controlling terminal died. Maybe use screen /
  tmux here?
* The ability to kill a spawned process group at whole (but w/o
  tarantool itself). Say, "sh -c 'foo | bar'".

Pushed also gorcunov/gh-4031-popen-fixup-5-full-ci to verify on all
targets (just in case). See results here:
https://gitlab.com/tarantool/tarantool/pipelines/125648893 (they are now
in fly).

WBR, Alexander Turenko.

On Mon, Mar 02, 2020 at 11:12:20PM +0300, Cyrill Gorcunov wrote:
> Here are a few fixes for popen engine I discovered during
> testing on macos. Now the unit test passes.
> 
> branch gorcunov/gh-4031-popen-fixup
> 
> Cyrill Gorcunov (7):
>   core/say: Export logger fd
>   popen: allow accessing environ variable
>   popen: close_inherited_fds - add support for macos/freebsd
>   popen: log errors if popen creation failed
>   popen: add logging in child process
>   popen: handle setsid os specifics
>   test/unit: add popen test
> 
>  src/lib/core/popen.c     |  92 ++++++++------
>  src/lib/core/say.c       |   6 +
>  src/lib/core/say.h       |   7 ++
>  test/unit/CMakeLists.txt |   3 +
>  test/unit/popen.c        | 250 +++++++++++++++++++++++++++++++++++++++
>  test/unit/popen.result   |  25 ++++
>  6 files changed, 348 insertions(+), 35 deletions(-)
>  create mode 100644 test/unit/popen.c
>  create mode 100644 test/unit/popen.result
> 
> 
> base-commit: 5e5d5a4a7a567891de2082e7f6c173497bb4e84b
> -- 
> 2.20.1
>