On Tue, 10 Jan 2023 at 00:06, Corey Huinker <corey.huin...@gmail.com> wrote: > > On Mon, Jan 9, 2023 at 10:01 AM Maxim Orlov <orlo...@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], 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