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@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@2424: Stuff 
> missing from end of file
>
> Failure in ./src/backend/optimizer/util/appendinfo.c: Error@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