Re: Add SHELL_EXIT_CODE to psql

On Tue, 10 Jan 2023 at 00:06, Corey Huinker <corey(dot)huinker(at)gmail(dot)com> wrote:
>
> On Mon, Jan 9, 2023 at 10:01 AM Maxim Orlov <orlovmg(at)gmail(dot)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], 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