Re: pgindent exit status if a file encounters an error - Mailing list pgsql-hackers

From Andrew Dunstan
Subject Re: pgindent exit status if a file encounters an error
Date
Msg-id 07e50709-86af-4d14-8255-ca0e9cb80abe@dunslane.net
Whole thread Raw
In response to pgindent exit status if a file encounters an error  (Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>)
Responses Re: pgindent exit status if a file encounters an error
List pgsql-hackers


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

pgsql-hackers by date:

Previous
From: Dean Rasheed
Date:
Subject: Re: Adding OLD/NEW support to RETURNING
Next
From: Tomas Vondra
Date:
Subject: Re: long-standing data loss bug in initial sync of logical replication