Re: [HACKERS] Patch to make pgindent work cleanly
On Tue, Feb 19, 2013 at 04:50:45PM -0500, Gurjeet Singh wrote:
Please find attached the patch for some cleanup and fix bit rot in pgindent
script.
There were a few problems with the script.
.) It failed to use the $ENV{PGENTAB} even if it was set.
.) The file it tries to download from Postgres' ftp site is no longer present.
ftp://ftp.postgresql.org/pub/dev/indent.netbsd.patched.tgz
.) The tar command extracted the above-mentioned file to a child directory,
but
did not descend into it before running make on it.
I think it expected a tarbomb, but clearly the current .tgz file on ftp
site is not a tarbomb.
I don't like the fact that it dies with a message fetching xyz rather than
saying Could not fetch xyz, but this patch does not address that since it
doesn't really affect the output when script does work.
With this patch in place I could very easily run it on any arbitrary file,
which seemed like a black-magic before the patch.
src/tools/pgindent/pgindent --build your file path here
I have applied this patch. However, I modified the tarball name to
reference $INDENT_VERSION, rather than hard-coding 1.2.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch to make pgindent work cleanly
On Fri, Apr 12, 2013 at 11:44 AM, Bruce Momjian br...@momjian.us wrote:
On Tue, Feb 19, 2013 at 04:50:45PM -0500, Gurjeet Singh wrote:
Please find attached the patch for some cleanup and fix bit rot in
pgindent
script.
There were a few problems with the script.
.) It failed to use the $ENV{PGENTAB} even if it was set.
.) The file it tries to download from Postgres' ftp site is no longer
present.
ftp://ftp.postgresql.org/pub/dev/indent.netbsd.patched.tgz
.) The tar command extracted the above-mentioned file to a child
directory, but
did not descend into it before running make on it.
I think it expected a tarbomb, but clearly the current .tgz file on
ftp
site is not a tarbomb.
I don't like the fact that it dies with a message fetching xyz rather
than
saying Could not fetch xyz, but this patch does not address that since
it
doesn't really affect the output when script does work.
With this patch in place I could very easily run it on any arbitrary
file,
which seemed like a black-magic before the patch.
src/tools/pgindent/pgindent --build your file path here
I have applied this patch. However, I modified the tarball name to
reference $INDENT_VERSION, rather than hard-coding 1.2.
Thanks!
Can you also improve the output when it dies upon failure to fetch
something? Currently the only error message it emits is fetching xyz, and
leaves the user confused as to what really the problem was. The only
indication of a problem might be the exit code, but I'm not sure of that,
and that doesn't help the interactive user running it on terminal.
--
Gurjeet Singh
http://gurjeet.singh.im/
EnterpriseDB Inc.
Re: [HACKERS] Patch to make pgindent work cleanly
On Fri, Apr 12, 2013 at 01:34:49PM -0400, Gurjeet Singh wrote:
Can you also improve the output when it dies upon failure to fetch something?
Currently the only error message it emits is fetching xyz, and leaves the
user confused as to what really the problem was. The only indication of a
problem might be the exit code, but I'm not sure of that, and that doesn't
help the interactive user running it on terminal.
Good point. I have reviewed all the error messages and improved them
with the attached, applied patch, e.g.:
cannot locate typedefs file xyz at /usr/local/bin/pgindent line 121.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index 2e9d443..73237ca 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -30,7 +30,7 @@ my %options = (
excludes=s = \$excludes,
indent=s= \$indent,
build = \$build,);
-GetOptions(%options) || die bad command line;
+GetOptions(%options) || die bad command line argument;
run_build($code_base) if ($build);
@@ -118,10 +118,11 @@ sub load_typedefs
if (-f $tdtry/src/tools/pgindent/typedefs.list);
$tdtry = $tdtry/..;
}
- die no typedefs file unless $typedefs_file -f $typedefs_file;
+ die cannot locate typedefs file \$typedefs_file\
+ unless $typedefs_file -f $typedefs_file;
open(my $typedefs_fh, '', $typedefs_file)
- || die opening $typedefs_file: $!;
+ || die cannot open typedefs file \$typedefs_file\: $!;
my @typedefs = $typedefs_fh;
close($typedefs_fh);
@@ -143,7 +144,7 @@ sub process_exclude
{
if ($excludes @files)
{
- open(my $eh, '', $excludes) || die opening $excludes;
+ open(my $eh, '', $excludes) || die cannot open exclude file \$excludes\;
while (my $line = $eh)
{
chomp $line;
@@ -162,7 +163,7 @@ sub read_source
my $source;
open(my $src_fd, '', $source_filename)
- || die opening $source_filename: $!;
+ || die cannot open file \$source_filename\: $!;
local ($/) = undef;
$source = $src_fd;
close($src_fd);
@@ -177,7 +178,7 @@ sub write_source
my $source_filename = shift;
open(my $src_fh, '', $source_filename)
- || die opening $source_filename: $!;
+ || die cannot open file \$source_filename\: $!;
print $src_fh $source;
close($src_fh);
}
@@ -436,25 +437,25 @@ sub run_build
$code_base = $code_base/..;
}
- die no src/tools/pgindent directory in $code_base
+ die cannot locate src/tools/pgindent directory in \$code_base\
unless -d $code_base/src/tools/pgindent;
chdir $code_base/src/tools/pgindent;
- my $rv = getstore(http://buildfarm.postgresql.org/cgi-bin/typedefs.pl;,
- tmp_typedefs.list);
+ my $typedefs_list_url = http://buildfarm.postgresql.org/cgi-bin/typedefs.pl;;
- die fetching typedefs.list unless is_success($rv);
+ my $rv = getstore($typedefs_list_url, tmp_typedefs.list);
+
+ die cannot fetch typedefs list from $typedefs_list_url unless is_success($rv);
$ENV{PGTYPEDEFS} = abs_path('tmp_typedefs.list');
- my $pg_bsd_indent_name = pg_bsd_indent- . $INDENT_VERSION . .tar.gz;
+ my $pg_bsd_indent_url = ftp://ftp.postgresql.org/pub/dev/pg_bsd_indent-; .
+ $INDENT_VERSION . .tar.gz;
- $rv =
- getstore(ftp://ftp.postgresql.org/pub/dev/$pg_bsd_indent_name;,
- pg_bsd_indent.tgz);
+ $rv = getstore($pg_bsd_indent_url, pg_bsd_indent.tgz);
- die fetching $pg_bsd_indent_name unless is_success($rv);
+ die cannot fetch BSD indent tarfile from $pg_bsd_indent_url unless is_success($rv);
# XXX add error checking here
@@ -484,7 +485,7 @@ sub build_clean
$code_base = $code_base/..;
}
- die no src/tools/pgindent directory in $code_base
+ die cannot locate src/tools/pgindent directory in \$code_base\
unless -d $code_base/src/tools/pgindent;
chdir $code_base;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch to make pgindent work cleanly
On Fri, Apr 12, 2013 at 3:26 PM, Bruce Momjian br...@momjian.us wrote: On Fri, Apr 12, 2013 at 01:34:49PM -0400, Gurjeet Singh wrote: Can you also improve the output when it dies upon failure to fetch something? Currently the only error message it emits is fetching xyz, and leaves the user confused as to what really the problem was. The only indication of a problem might be the exit code, but I'm not sure of that, and that doesn't help the interactive user running it on terminal. Good point. I have reviewed all the error messages and improved them with the attached, applied patch, e.g.: cannot locate typedefs file xyz at /usr/local/bin/pgindent line 121. Thanks! -- Gurjeet Singh http://gurjeet.singh.im/ EnterpriseDB Inc.
[HACKERS] Patch to make pgindent work cleanly
Please find attached the patch for some cleanup and fix bit rot in pgindent
script.
There were a few problems with the script.
.) It failed to use the $ENV{PGENTAB} even if it was set.
.) The file it tries to download from Postgres' ftp site is no longer
present.
ftp://ftp.postgresql.org/pub/dev/indent.netbsd.patched.tgz
.) The tar command extracted the above-mentioned file to a child directory,
but did not descend into it before running make on it.
I think it expected a tarbomb, but clearly the current .tgz file on ftp
site is not a tarbomb.
I don't like the fact that it dies with a message fetching xyz rather
than saying Could not fetch xyz, but this patch does not address that
since it doesn't really affect the output when script does work.
With this patch in place I could very easily run it on any arbitrary file,
which seemed like a black-magic before the patch.
src/tools/pgindent/pgindent --build your file path here
--
Gurjeet Singh
http://gurjeet.singh.im/
EnterprsieDB Inc.
pgindent.patch
Description: Binary data
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
