Add SHELL_EXIT_CODE to psql

Oops, that sample output was from a previous run, should have been:

-- SHELL_EXIT_CODE is undefined
\echo :SHELL_EXIT_CODE
:SHELL_EXIT_CODE
-- bad \!
\! borp
sh: line 1: borp: command not found
\echo :SHELL_EXIT_CODE
127
-- bad backtick
\set var `borp`
sh: line 1: borp: command not found
\echo :SHELL_EXIT_CODE
127
-- good \!
\! true
\echo :SHELL_EXIT_CODE
0
-- play with exit codes
\! exit 4
\echo :SHELL_EXIT_CODE
4
\set var `exit 3`
\echo :SHELL_EXIT_CODE
3

On Fri, Nov 4, 2022 at 5:08 AM Corey Huinker <corey.huinker@gmail.com>
wrote:

Rebased. Still waiting on feedback before working on documentation.

On Fri, Nov 4, 2022 at 5:23 AM Corey Huinker <corey.huinker@gmail.com>
wrote:

I've rebased and updated the patch to include documentation.

Regression tests have been moved to a separate patchfile because error
messages will vary by OS and configuration, so we probably can't do a
stable regression test, but having them handy at least demonstrates the
feature.

On Sun, Dec 4, 2022 at 12:35 AM Corey Huinker <corey.huinker@gmail.com>
wrote:

Hi!

The patch is implementing what is declared to do. Shell return code is now
accessible is psql var.
Overall code is in a good condition. Applies with no errors on master.
Unfortunately, regression tests are failing on the macOS due to the
different shell output.

@@ -1308,13 +1308,13 @@
 deallocate q;
 -- test SHELL_EXIT_CODE
 \! nosuchcommand
-sh: line 1: nosuchcommand: command not found
+sh: nosuchcommand: command not found
 \echo :SHELL_EXIT_CODE
 127
 \set nosuchvar `nosuchcommand`
-sh: line 1: nosuchcommand: command not found
+sh: nosuchcommand: command not found
 \! nosuchcommand
-sh: line 1: nosuchcommand: command not found
+sh: nosuchcommand: command not found
 \echo :SHELL_EXIT_CODE
 127

Since we do not want to test shell output in these cases, but only return
code,
what about using this kind of commands?
postgres=# \! true > /dev/null 2>&1
postgres=# \echo :SHELL_EXIT_CODE
0
postgres=# \! false > /dev/null 2>&1
postgres=# \echo :SHELL_EXIT_CODE
1
postgres=# \! nosuchcommand > /dev/null 2>&1
postgres=# \echo :SHELL_EXIT_CODE
127

It is better to use spaces around "=".
+               if (WIFEXITED(exit_code))
+                       exit_code=WEXITSTATUS(exit_code);
+               else if(WIFSIGNALED(exit_code))
+                       exit_code=WTERMSIG(exit_code);
+               else if(WIFSTOPPED(exit_code))
+                       exit_code=WSTOPSIG(exit_code);

--
Best regards,
Maxim Orlov.

On Wed, Dec 28, 2022 at 5:59 AM Maxim Orlov <orlovmg@gmail.com> wrote:

Hi!

The patch is implementing what is declared to do. Shell return code is now
accessible is psql var.
Overall code is in a good condition. Applies with no errors on master.
Unfortunately, regression tests are failing on the macOS due to the
different shell output.

That was to be expected.

I wonder if there is value in setting up a psql on/off var
SHELL_ERROR_OUTPUT construct that when set to "off/false"
suppresses standard error via appending "2> /dev/null" (or "2> nul" if
#ifdef WIN32). At the very least, it would allow for tests like this to be
done with standard regression scripts.

On Fri, Dec 30, 2022 at 2:17 PM Corey Huinker <corey.huinker@gmail.com>
wrote:

On Wed, Dec 28, 2022 at 5:59 AM Maxim Orlov <orlovmg@gmail.com> wrote:

Hi!

The patch is implementing what is declared to do. Shell return code is
now accessible is psql var.
Overall code is in a good condition. Applies with no errors on master.
Unfortunately, regression tests are failing on the macOS due to the
different shell output.

That was to be expected.

I wonder if there is value in setting up a psql on/off var
SHELL_ERROR_OUTPUT construct that when set to "off/false"
suppresses standard error via appending "2> /dev/null" (or "2> nul" if
#ifdef WIN32). At the very least, it would allow for tests like this to be
done with standard regression scripts.

Thinking on this some more a few ideas came up:

1. The SHELL_ERROR_OUTPUT var above. This is good for simple situations,
but it would fail if the user took it upon themselves to redirect output,
and suddenly "myprog 2> /dev/null" works fine on its own but will fail when
we append our own "2> /dev/null" to it.

2. Exposing a PSQL var that identifies the shell snippet for "how to
suppress standard error", which would be "2> /dev/null" for -nix systems
and "2> NUL" for windows systems. This again, presumes that users will
actually need this feature, and I'm not convinced that they will.

3. Exposing a PSQL var that is basically an "is this environment running on
windows" and let them construct it from there. That gets complicated with
WSL and the like, so I'm not wild about this, even though it would be
comparatively simple to implement.

4. We could inject an environment variable into our own regression tests
directly based on the "#ifdef WIN32" in our own code, and we can use a
couple of \gset commands to construct the error-suppressed shell commands
as needed. This seems like the best starting point, and we can always turn
that env var into a real variable if it proves useful elsewhere.

On Sat, 31 Dec 2022 at 16:47, Corey Huinker <corey.huinker@gmail.com> wrote:

I wonder if there is value in setting up a psql on/off var
SHELL_ERROR_OUTPUT construct that when set to "off/false"
suppresses standard error via appending "2> /dev/null" (or "2> nul" if
#ifdef WIN32). At the very least, it would allow for tests like this to be
done with standard regression scripts.

Thinking on this some more a few ideas came up:

1. The SHELL_ERROR_OUTPUT var above. This is good for simple situations,
but it would fail if the user took it upon themselves to redirect output,
and suddenly "myprog 2> /dev/null" works fine on its own but will fail when
we append our own "2> /dev/null" to it.

Rather than attempting to append redirection directives to the command,
would it work to redirect stderr before invoking the shell? This seems to
me to be more reliable and it should allow an explicit redirection in the
shell command to still work. The difference between Windows and Unix then
becomes the details of what system calls we use to accomplish the
redirection (or maybe none, if an existing abstraction layer takes care of
that - I'm not really up on Postgres internals enough to know), rather than
what we append to the provided command.

On Sat, Dec 31, 2022 at 5:28 PM Isaac Morland <isaac.morland@gmail.com>
wrote:

On Sat, 31 Dec 2022 at 16:47, Corey Huinker <corey.huinker@gmail.com>
wrote:

I wonder if there is value in setting up a psql on/off var
SHELL_ERROR_OUTPUT construct that when set to "off/false"
suppresses standard error via appending "2> /dev/null" (or "2> nul" if
#ifdef WIN32). At the very least, it would allow for tests like this to be
done with standard regression scripts.

Thinking on this some more a few ideas came up:

1. The SHELL_ERROR_OUTPUT var above. This is good for simple situations,
but it would fail if the user took it upon themselves to redirect output,
and suddenly "myprog 2> /dev/null" works fine on its own but will fail when
we append our own "2> /dev/null" to it.

Rather than attempting to append redirection directives to the command,
would it work to redirect stderr before invoking the shell? This seems to
me to be more reliable and it should allow an explicit redirection in the
shell command to still work. The difference between Windows and Unix then
becomes the details of what system calls we use to accomplish the
redirection (or maybe none, if an existing abstraction layer takes care of
that - I'm not really up on Postgres internals enough to know), rather than
what we append to the provided command.

Inside psql, it's a call to the system() function which takes a single
string. All output, stdout or stderr, is printed. So for the regression
test we have to compose a command that is OS appropriate AND suppresses
stderr.

On Wed, 21 Dec 2022 at 11:04, Corey Huinker <corey.huinker@gmail.com> wrote:

I've rebased and updated the patch to include documentation.

Regression tests have been moved to a separate patchfile because error messages will vary by OS and configuration, so we probably can't do a stable regression test, but having them handy at least demonstrates the feature.

On Sun, Dec 4, 2022 at 12:35 AM Corey Huinker <corey.huinker@gmail.com> wrote:

Rebased. Still waiting on feedback before working on documentation.

On Fri, Nov 4, 2022 at 5:23 AM Corey Huinker <corey.huinker@gmail.com> wrote:

Oops, that sample output was from a previous run, should have been:

-- SHELL_EXIT_CODE is undefined
\echo :SHELL_EXIT_CODE
:SHELL_EXIT_CODE
-- bad \!
\! borp
sh: line 1: borp: command not found
\echo :SHELL_EXIT_CODE
127
-- bad backtick
\set var `borp`
sh: line 1: borp: command not found
\echo :SHELL_EXIT_CODE
127
-- good \!
\! true
\echo :SHELL_EXIT_CODE
0
-- play with exit codes
\! exit 4
\echo :SHELL_EXIT_CODE
4
\set var `exit 3`
\echo :SHELL_EXIT_CODE
3

On Fri, Nov 4, 2022 at 5:08 AM Corey Huinker <corey.huinker@gmail.com> wrote:

Over in /messages/by-id/eaf326ad693e74eba068f33a7f518039@oss.nttdata.com Justin Pryzby suggested that psql might need the ability to capture the shell exit code.

This is a POC patch that does that, but doesn't touch on the ON_ERROR_STOP stuff.

I've added some very rudimentary tests, but haven't touched the documentation, because I strongly suspect that someone will suggest a better name for the variable.

But basically, it works like this

-- SHELL_EXIT_CODE is undefined
\echo :SHELL_EXIT_CODE
:SHELL_EXIT_CODE
-- bad \!
\! borp
sh: line 1: borp: command not found
\echo :SHELL_EXIT_CODE
32512
-- bad backtick
\set var `borp`
sh: line 1: borp: command not found
\echo :SHELL_EXIT_CODE
127
-- good \!
\! true
\echo :SHELL_EXIT_CODE
0
-- play with exit codes
\! exit 4
\echo :SHELL_EXIT_CODE
1024
\set var `exit 3`
\echo :SHELL_EXIT_CODE
3

Feedback welcome.

CFBot shows some compilation errors as in [1]https://cirrus-ci.com/task/5424476720988160, please post an updated
version for the same:
[02:35:49.924] psqlscanslash.l: In function ‘evaluate_backtick’:
[02:35:49.924] psqlscanslash.l:822:11: error: implicit declaration of
function ‘WIFSTOPPED’ [-Werror=implicit-function-declaration]
[02:35:49.924] 822 | exit_code=WSTOPSIG(exit_code);
[02:35:49.924] | ^~~~~~~~~~
[02:35:49.924] psqlscanslash.l:823:14: error: implicit declaration of
function ‘WSTOPSIG’ [-Werror=implicit-function-declaration]
[02:35:49.924] 823 | }
[02:35:49.924] | ^
[02:35:49.924] cc1: all warnings being treated as errors
[02:35:49.925] make[3]: *** [<builtin>: psqlscanslash.o] Error 1

[1]: https://cirrus-ci.com/task/5424476720988160

Regards,
Vignesh

On Tue, Jan 3, 2023 at 5:36 AM vignesh C <vignesh21@gmail.com> wrote:

On Wed, 21 Dec 2022 at 11:04, Corey Huinker <corey.huinker@gmail.com>
wrote:

I've rebased and updated the patch to include documentation.

Regression tests have been moved to a separate patchfile because error

messages will vary by OS and configuration, so we probably can't do a
stable regression test, but having them handy at least demonstrates the
feature.

On Sun, Dec 4, 2022 at 12:35 AM Corey Huinker <corey.huinker@gmail.com>

wrote:

Rebased. Still waiting on feedback before working on documentation.

On Fri, Nov 4, 2022 at 5:23 AM Corey Huinker <corey.huinker@gmail.com>

wrote:

Oops, that sample output was from a previous run, should have been:

-- SHELL_EXIT_CODE is undefined
\echo :SHELL_EXIT_CODE
:SHELL_EXIT_CODE
-- bad \!
\! borp
sh: line 1: borp: command not found
\echo :SHELL_EXIT_CODE
127
-- bad backtick
\set var `borp`
sh: line 1: borp: command not found
\echo :SHELL_EXIT_CODE
127
-- good \!
\! true
\echo :SHELL_EXIT_CODE
0
-- play with exit codes
\! exit 4
\echo :SHELL_EXIT_CODE
4
\set var `exit 3`
\echo :SHELL_EXIT_CODE
3

On Fri, Nov 4, 2022 at 5:08 AM Corey Huinker <corey.huinker@gmail.com>

wrote:

Over in

/messages/by-id/eaf326ad693e74eba068f33a7f518039@oss.nttdata.com
Justin Pryzby suggested that psql might need the ability to capture the
shell exit code.

This is a POC patch that does that, but doesn't touch on the

ON_ERROR_STOP stuff.

I've added some very rudimentary tests, but haven't touched the

documentation, because I strongly suspect that someone will suggest a
better name for the variable.

But basically, it works like this

-- SHELL_EXIT_CODE is undefined
\echo :SHELL_EXIT_CODE
:SHELL_EXIT_CODE
-- bad \!
\! borp
sh: line 1: borp: command not found
\echo :SHELL_EXIT_CODE
32512
-- bad backtick
\set var `borp`
sh: line 1: borp: command not found
\echo :SHELL_EXIT_CODE
127
-- good \!
\! true
\echo :SHELL_EXIT_CODE
0
-- play with exit codes
\! exit 4
\echo :SHELL_EXIT_CODE
1024
\set var `exit 3`
\echo :SHELL_EXIT_CODE
3

Feedback welcome.

CFBot shows some compilation errors as in [1], please post an updated
version for the same:
[02:35:49.924] psqlscanslash.l: In function ‘evaluate_backtick’:
[02:35:49.924] psqlscanslash.l:822:11: error: implicit declaration of
function ‘WIFSTOPPED’ [-Werror=implicit-function-declaration]
[02:35:49.924] 822 | exit_code=WSTOPSIG(exit_code);
[02:35:49.924] | ^~~~~~~~~~
[02:35:49.924] psqlscanslash.l:823:14: error: implicit declaration of
function ‘WSTOPSIG’ [-Werror=implicit-function-declaration]
[02:35:49.924] 823 | }
[02:35:49.924] | ^
[02:35:49.924] cc1: all warnings being treated as errors
[02:35:49.925] make[3]: *** [<builtin>: psqlscanslash.o] Error 1

[1] - https://cirrus-ci.com/task/5424476720988160

Regards,
Vignesh

Thanks. I had left sys/wait.h out of psqlscanslash.

Attached is v3 of this patch, I've made the following changes:

1. pg_regress now creates an environment variable called PG_OS_TARGET,
which regression tests can use to manufacture os-specific commands. For our
purposes, this allows the regression test to manufacture a shell command
that has either "2> /dev/null" or "2> NUL". This seemed the least invasive
way to make this possible. If for some reason it becomes useful in general
psql scripting, then we can consider promoting it to a regular psql var.

2. There are now two psql variables, SHELL_EXIT_CODE, which has the return
code, and SHELL_ERROR, which is a true/false flag based on whether the exit
code was nonzero. These variables are the shell result analogues of
SQLSTATE and ERROR.

Hi!

In overall, I think we move in the right direction. But we could make code
better, should we?

+               /* Capture exit code for SHELL_EXIT_CODE */
+               close_exit_code = pclose(fd);
+               if (close_exit_code == -1)
+               {
+                       pg_log_error("%s: %m", cmd);
+                       error = true;
+               }
+               if (WIFEXITED(close_exit_code))
+                       exit_code=WEXITSTATUS(close_exit_code);
+               else if(WIFSIGNALED(close_exit_code))
+                       exit_code=WTERMSIG(close_exit_code);
+               else if(WIFSTOPPED(close_exit_code))
+                       exit_code=WSTOPSIG(close_exit_code);
+               if (exit_code)
+                       error = true;
I think, it's better to add spaces around middle if block. It will be easy
to read.
Also, consider, adding spaces around assignment in this block.

+       /*
+       snprintf(exit_code_buf, sizeof(exit_code_buf), "%d",
WEXITSTATUS(exit_code));
+       */
Probably, this is not needed.

1. pg_regress now creates an environment variable called PG_OS_TARGET

Maybe, we can use env "OS"? I do not know much about Windows, but I think
this is kind of standard environment variable there.

--
Best regards,
Maxim Orlov.

On Mon, Jan 9, 2023 at 10:01 AM Maxim Orlov <orlovmg@gmail.com> wrote:

Hi!

In overall, I think we move in the right direction. But we could make code
better, should we?

+               /* Capture exit code for SHELL_EXIT_CODE */
+               close_exit_code = pclose(fd);
+               if (close_exit_code == -1)
+               {
+                       pg_log_error("%s: %m", cmd);
+                       error = true;
+               }
+               if (WIFEXITED(close_exit_code))
+                       exit_code=WEXITSTATUS(close_exit_code);
+               else if(WIFSIGNALED(close_exit_code))
+                       exit_code=WTERMSIG(close_exit_code);
+               else if(WIFSTOPPED(close_exit_code))
+                       exit_code=WSTOPSIG(close_exit_code);
+               if (exit_code)
+                       error = true;
I think, it's better to add spaces around middle if block. It will be easy
to read.
Also, consider, adding spaces around assignment in this block.

Noted and will implement.

+       /*
+       snprintf(exit_code_buf, sizeof(exit_code_buf), "%d",
WEXITSTATUS(exit_code));
+       */
Probably, this is not needed.

Heh. Oops.

1. pg_regress now creates an environment variable called PG_OS_TARGET

Maybe, we can use env "OS"? I do not know much about Windows, but I think
this is kind of standard environment variable there.

I chose a name that would avoid collisions with anything a user might
potentially throw into their environment, so if the var "OS" is fairly
standard is a reason to avoid using it. Also, going with our own env var
allows us to stay in perfect synchronization with the build's #ifdef WIN32
... and whatever #ifdefs may come in the future for new OSes. If there is
already an environment variable that does that for us, I would rather use
that, but I haven't found it.

The 0001 patch is unchanged from last time (aside from anything rebasing
might have done).

On Mon, 9 Jan 2023 at 21:36, Corey Huinker <corey.huinker@gmail.com> wrote:

I chose a name that would avoid collisions with anything a user might
potentially throw into their environment, so if the var "OS" is fairly
standard is a reason to avoid using it. Also, going with our own env var
allows us to stay in perfect synchronization with the build's #ifdef WIN32
... and whatever #ifdefs may come in the future for new OSes. If there is
already an environment variable that does that for us, I would rather use
that, but I haven't found it.

Perhaps, I didn't make myself clear. Your solution is perfectly adapted to

our needs.
But all Windows since 2000 already have an environment variable
OS=Windows_NT. So, if env OS is defined and equal Windows_NT, this had to
be Windows.
May we use it in our case? I don't insist, just asking.

--
Best regards,
Maxim Orlov.

On Tue, Jan 10, 2023 at 3:54 AM Maxim Orlov <orlovmg@gmail.com> wrote:

On Mon, 9 Jan 2023 at 21:36, Corey Huinker <corey.huinker@gmail.com>
wrote:

I chose a name that would avoid collisions with anything a user might
potentially throw into their environment, so if the var "OS" is fairly
standard is a reason to avoid using it. Also, going with our own env var
allows us to stay in perfect synchronization with the build's #ifdef WIN32
... and whatever #ifdefs may come in the future for new OSes. If there is
already an environment variable that does that for us, I would rather use
that, but I haven't found it.

Perhaps, I didn't make myself clear. Your solution is perfectly adapted

to our needs.
But all Windows since 2000 already have an environment variable
OS=Windows_NT. So, if env OS is defined and equal Windows_NT, this had to
be Windows.
May we use it in our case? I don't insist, just asking.

Ah, that makes more sense. I don't have a strong opinion on which we should
use, and I would probably defer to someone who does windows (and possibly
cygwin) builds more often than I do.

On Tue, 10 Jan 2023 at 00:06, Corey Huinker <corey.huinker@gmail.com> wrote:

On Mon, Jan 9, 2023 at 10:01 AM Maxim Orlov <orlovmg@gmail.com> wrote:

Hi!

In overall, I think we move in the right direction. But we could make code better, should we?

+               /* Capture exit code for SHELL_EXIT_CODE */
+               close_exit_code = pclose(fd);
+               if (close_exit_code == -1)
+               {
+                       pg_log_error("%s: %m", cmd);
+                       error = true;
+               }
+               if (WIFEXITED(close_exit_code))
+                       exit_code=WEXITSTATUS(close_exit_code);
+               else if(WIFSIGNALED(close_exit_code))
+                       exit_code=WTERMSIG(close_exit_code);
+               else if(WIFSTOPPED(close_exit_code))
+                       exit_code=WSTOPSIG(close_exit_code);
+               if (exit_code)
+                       error = true;
I think, it's better to add spaces around middle if block. It will be easy to read.
Also, consider, adding spaces around assignment in this block.

Noted and will implement.

+       /*
+       snprintf(exit_code_buf, sizeof(exit_code_buf), "%d", WEXITSTATUS(exit_code));
+       */
Probably, this is not needed.

Heh. Oops.

1. pg_regress now creates an environment variable called PG_OS_TARGET

Maybe, we can use env "OS"? I do not know much about Windows, but I think this is kind of standard environment variable there.

I chose a name that would avoid collisions with anything a user might potentially throw into their environment, so if the var "OS" is fairly standard is a reason to avoid using it. Also, going with our own env var allows us to stay in perfect synchronization with the build's #ifdef WIN32 ... and whatever #ifdefs may come in the future for new OSes. If there is already an environment variable that does that for us, I would rather use that, but I haven't found it.

The 0001 patch is unchanged from last time (aside from anything rebasing might have done).

The patch does not apply on top of HEAD as in [1]http://cfbot.cputube.org/patch_41_4073.log, please post a rebased patch:

=== Applying patches on top of PostgreSQL commit ID
3c6fc58209f24b959ee18f5d19ef96403d08f15c ===
=== applying patch
./v4-0002-Add-psql-variables-SHELL_ERROR-and-SHELL_EXIT_COD.patch
patching file doc/src/sgml/ref/psql-ref.sgml
Hunk #1 FAILED at 4264.
1 out of 1 hunk FAILED -- saving rejects to file
doc/src/sgml/ref/psql-ref.sgml.rej

[1]: http://cfbot.cputube.org/patch_41_4073.log

Regards,
Vignesh

The patch does not apply on top of HEAD as in [1], please post a rebased
patch:

Conflict was due to the doc patch applying id tags to psql variable names.
I've rebased and added my own id tags to the two new variables.

Unfortunately, cirrus-ci still not happy
https://cirrus-ci.com/task/6502730475241472

[23:14:34.206] time make -s -j${BUILD_JOBS} world-bin
[23:14:43.174] psqlscanslash.l: In function ‘evaluate_backtick’:
[23:14:43.174] psqlscanslash.l:827:11: error: implicit declaration of
function ‘WIFSTOPPED’ [-Werror=implicit-function-declaration]
[23:14:43.174] 827 | exit_code = WSTOPSIG(close_exit_code);
[23:14:43.174] | ^~~~~~~~~~
[23:14:43.174] psqlscanslash.l:828:16: error: implicit declaration of
function ‘WSTOPSIG’ [-Werror=implicit-function-declaration]
[23:14:43.174] 828 |
[23:14:43.174] | ^
[23:14:43.174] cc1: all warnings being treated as errors

and on FreeBSD

[23:13:03.914] cc -o ...
[23:13:03.914] ld: error: undefined symbol: WEXITSTATUS
[23:13:03.914] >>> referenced by command.c:5036
(../src/bin/psql/command.c:5036)
[23:13:03.914] >>>
src/bin/psql/psql.p/command.c.o:(exec_command_shell_escape)
[23:13:03.914] cc: error: linker command failed with exit code 1 (use -v to
see invocation)

and on Windows

[23:19:51.870] meson-generated_.._psqlscanslash.c.obj : error LNK2019:
unresolved external symbol WIFSTOPPED referenced in function
evaluate_backtick
[23:19:52.197] meson-generated_.._psqlscanslash.c.obj : error LNK2019:
unresolved external symbol WSTOPSIG referenced in function evaluate_backtick
[23:19:52.197] src\bin\psql\psql.exe : fatal error LNK1120: 2 unresolved
externals

I belive, we need proper includes.

--
Best regards,
Maxim Orlov.

I belive, we need proper includes.

Given that wait_error.c already seems to have the right includes worked out
for WEXITSTATUS/WIFSTOPPED/etc, I decided to just add a function there.

I named it wait_result_to_exit_code(), but I welcome suggestions of a
better name.

On Fri, 13 Jan 2023 at 07:50, Corey Huinker <corey.huinker@gmail.com> wrote:

I named it wait_result_to_exit_code(), but I welcome suggestions of a
better name.

Thanks! But CF bot still not happy. I think, we should address issues from
here https://cirrus-ci.com/task/5391002618298368

--
Best regards,
Maxim Orlov.