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.

[1] https://en.wikipedia.org/wiki/Exit_status
-- 
Best Wishes,
Ashutosh Bapat
From f43a90ead02637896e375144b015b971b5e86fce Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Wed, 26 Jun 2024 15:46:53 +0530
Subject: [PATCH 1/2] pgindent exit status on error

When pg_bsd_indent exits with a non-zero status or reports an error, make
pgindent exit with non-zero status 3. The program does not exit on
the first instance of error. Instead it continues to process remaining
files as long as some other exit condition is encountered. In that case
exit status corresponding to that condition is reported.

This helps when pgindent is run in shell which interpret non-zero exit
status as failure and allow configuring actions on failure.

Author: Ashutosh Bapat
---
 src/tools/pgindent/pgindent | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index 48d83bc434..7dbd3a67c2 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -409,6 +409,7 @@ foreach my $source_filename (@files)
 	if ($source eq "")
 	{
 		print STDERR "Failure in $source_filename: " . $error_message . "\n";
+		$status = 3;
 		next;
 	}
 
-- 
2.34.1

Reply via email to