Re: pgindent exit status if a file encounters an error

On 2024-06-26 We 6:37 AM, Ashutosh Bapat wrote:
> Hi All,
>
> If pgindent encounters an error while applying pg_bsd_indent on a
> file, it reports an error to stderr but does not exit with non-zero
> status.
>
> $ src/tools/pgindent/pgindent .
> Failure in ./src/backend/optimizer/util/relnode.c: Error(at)2412: Stuff
> missing from end of file
>
> $ echo $?
> 0
>
> A zero status usually indicates success [1]. In that sense pgindent
> shouldn't be returning 0 in this case. It has not been able to process
> file/s successfully. Not returning non-zero exit status in such cases
> means we can not rely on `set -e` or `git rebase` s automatic
> detection of command failures. I propose to add non-zero exit status
> in the above case.
>
> In the attached patch I have used exit code 3 for file processing
> errors. The program exits only after reporting all such errors instead
> of exiting on the first instance. That way we get to know all the
> errors upfront. But I am ok if we want to exit on the first instance.
> That might simplify its interaction with other exit codes.
>
> With this change, if I run pgident in `git rebase` it stops after
> those errors automatically like below
> ```
> Executing: src/tools/pgindent/pgindent .
> Failure in ./src/backend/optimizer/util/relnode.c: Error(at)2424: Stuff
> missing from end of file
>
> Failure in ./src/backend/optimizer/util/appendinfo.c: Error(at)1028:
> Stuff missing from end of file
>
> warning: execution failed: src/tools/pgindent/pgindent .
> You can fix the problem, and then run
>
>   git rebase --continue
> ```
>
> I looked at pgindent README but it doesn't mention anything about exit
> codes. So I believe this change is acceptable as per documentation.
>
> With --check option pgindent reports a non-zero exit code instead of
> making changes. So I feel the above change should happen at least if
> --check is provided. But certainly better if we do it even without
> --check.
>
> In case --check is specified and both the following happen a.
> pg_bsd_indent exits with non-zero status while processing some file
> and b. changes are produced while processing some other file, the
> program will exit with status 2. It may be argued that instead it
> should exit with code 3. I am open to both.

Yeah, I think this is reasonable but we should adjust the status setting
a few lines lower to

   $status ||= 2;

cheers

andrew

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