Why does pg_bsd_indent need to be installed?
Why does pgindent require that pg_bsd_indent be installed in the path?
Couldn't pgindent call the pg_bsd_indent built inside the tree? That
would make the whole process much smoother.
Peter Eisentraut <peter@eisentraut.org> writes:
Why does pgindent require that pg_bsd_indent be installed in the path?
Couldn't pgindent call the pg_bsd_indent built inside the tree? That
would make the whole process much smoother.
Well, the current expectation is that you run it in a distclean'd
tree, in which case it won't be there. VPATH builds would have
a problem finding it as well.
I'm not sure if there's any problem in applying it in a built-out
tree, but the VPATH scenario seems like a problem in any case,
especially since (IIUC) meson builds have to be done that way.
I wouldn't object to adding more logic to the calling script
to support multiple usage scenarios, of course.
regards, tom lane
Hi,
On 2023-05-25 09:09:45 -0400, Tom Lane wrote:
Peter Eisentraut <peter@eisentraut.org> writes:
Why does pgindent require that pg_bsd_indent be installed in the path?
Couldn't pgindent call the pg_bsd_indent built inside the tree? That
would make the whole process much smoother.Well, the current expectation is that you run it in a distclean'd
tree, in which case it won't be there. VPATH builds would have
a problem finding it as well.I'm not sure if there's any problem in applying it in a built-out
tree, but the VPATH scenario seems like a problem in any case,
especially since (IIUC) meson builds have to be done that way.
Isn't the situation actually *easier* in VPATH builds? There's no build
artifacts in the source tree, so you can just invoke the pg_bsd_indent built
in the build directory against the source tree, without any problems?
I'd like to add a build system target for reindenting with the in-tree
pg_bsd_indent, obviously with the right dependencies to build pg_bsd_indent
first.
And yes, meson only supports building in a separate directory (which obviously
can be inside the source directory, although I don't do that, because the
autoconf vpath build copies such directories, which isn't fun).
Greetings,
Andres Freund
Andres Freund <andres@anarazel.de> writes:
On 2023-05-25 09:09:45 -0400, Tom Lane wrote:
Peter Eisentraut <peter@eisentraut.org> writes:
Why does pgindent require that pg_bsd_indent be installed in the path?
Isn't the situation actually *easier* in VPATH builds? There's no build
artifacts in the source tree, so you can just invoke the pg_bsd_indent built
in the build directory against the source tree, without any problems?
Well, if you know where the build directory is, sure. But any way you
slice it there is an extra bit of knowledge required. Since pg_bsd_indent
changes so seldom, keeping it in your PATH is at least as easy as any
other solution, IMO.
Another reason why I like to do it that way is that it supports running
pgindent on files that aren't in the source tree at all, which suits
some old habits of mine.
But, as I said before, I'm open to adding support for other scenarios
as long as we don't remove that one.
regards, tom lane
Hi,
On 2023-05-25 13:05:57 -0400, Tom Lane wrote:
Andres Freund <andres@anarazel.de> writes:
On 2023-05-25 09:09:45 -0400, Tom Lane wrote:
Peter Eisentraut <peter@eisentraut.org> writes:
Why does pgindent require that pg_bsd_indent be installed in the path?
Isn't the situation actually *easier* in VPATH builds? There's no build
artifacts in the source tree, so you can just invoke the pg_bsd_indent built
in the build directory against the source tree, without any problems?Well, if you know where the build directory is, sure.
I'm imaginging adding make / meson targets 'indent-tree' and 'indent-head' or
such. Obviously the buildsystem knows where the source dir is, and you'd
invoke it in the build dir, so it'd know all that'd need to be known.
Attached is a prototype adding such meson targets. It's easier with a
parameter telling pgindent where the source tree is, so I addeded that too
(likely would need to be cleaned up some).
Since pg_bsd_indent
changes so seldom, keeping it in your PATH is at least as easy as any
other solution, IMO.Another reason why I like to do it that way is that it supports running
pgindent on files that aren't in the source tree at all, which suits
some old habits of mine.
But, as I said before, I'm open to adding support for other scenarios
as long as we don't remove that one.
I can't imagine that we'd remove support for doing so...
Greetings,
Andres Freund
Attachments:
v1-0001-add-meson-target-to-run-pgindent.patchtext/x-diff; charset=us-asciiDownload
From 2ae58700a37a0a9f1c5cad527e89bd00fc586858 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Sat, 27 May 2023 11:38:27 -0700
Subject: [PATCH v1] add meson target to run pgindent
Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
meson.build | 12 ++++++++++++
src/tools/pgindent/pgindent | 12 ++++++++++--
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/meson.build b/meson.build
index 16b2e866462..1c7ed9c87f9 100644
--- a/meson.build
+++ b/meson.build
@@ -3005,7 +3005,19 @@ run_target('install-test-files',
depends: testprep_targets,
)
+indent_base = [perl, files('src/tools/pgindent/pgindent'),
+ '--indent', pg_bsd_indent.full_path(),
+ '--sourcetree=@SOURCE_ROOT@']
+run_target('indent-head',
+ command: indent_base + ['--commit=HEAD'],
+ depends: pg_bsd_indent
+)
+
+run_target('indent-tree',
+ command: indent_base + ['.'],
+ depends: pg_bsd_indent
+)
###############################################################
# Test prep
diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index bce63d95daf..08b0c95814f 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -23,7 +23,7 @@ my $devnull = File::Spec->devnull;
my ($typedefs_file, $typedef_str, @excludes,
$indent, $build, $show_diff,
- $silent_diff, $help, @commits,);
+ $silent_diff, $sourcetree, $help, @commits,);
$help = 0;
@@ -35,7 +35,8 @@ my %options = (
"excludes=s" => \@excludes,
"indent=s" => \$indent,
"show-diff" => \$show_diff,
- "silent-diff" => \$silent_diff,);
+ "silent-diff" => \$silent_diff,
+ "sourcetree=s" => \$sourcetree);
GetOptions(%options) || usage("bad command line argument");
usage() if $help;
@@ -53,6 +54,13 @@ $typedefs_file ||= $ENV{PGTYPEDEFS};
# get indent location for environment or default
$indent ||= $ENV{PGINDENT} || $ENV{INDENT} || "pg_bsd_indent";
+# When invoked from the build directory, change into source tree, otherwise
+# the heuristics in locate_sourcedir() don't work.
+if (defined $sourcetree)
+{
+ chdir $sourcetree;
+}
+
my $sourcedir = locate_sourcedir();
# if it's the base of a postgres tree, we will exclude the files
--
2.37.1.188.g71a8fab31b
On 25.05.23 13:05, Tom Lane wrote:
Well, if you know where the build directory is, sure. But any way you
slice it there is an extra bit of knowledge required. Since pg_bsd_indent
changes so seldom, keeping it in your PATH is at least as easy as any
other solution, IMO.
The reason I bumped into this is that 15 and 16 use different versions
of pg_bsd_indent, so you can't just keep one copy in, like, ~/bin/.
Peter Eisentraut <peter@eisentraut.org> writes:
On 25.05.23 13:05, Tom Lane wrote:
Well, if you know where the build directory is, sure. But any way you
slice it there is an extra bit of knowledge required. Since pg_bsd_indent
changes so seldom, keeping it in your PATH is at least as easy as any
other solution, IMO.
The reason I bumped into this is that 15 and 16 use different versions
of pg_bsd_indent, so you can't just keep one copy in, like, ~/bin/.
Well, personally, I never bother to adjust patches to the indentation
rules of old versions, so using the latest pg_bsd_indent suffices.
regards, tom lane
On Wed, May 31, 2023 at 01:21:05PM -0400, Tom Lane wrote:
Peter Eisentraut <peter@eisentraut.org> writes:
On 25.05.23 13:05, Tom Lane wrote:
Well, if you know where the build directory is, sure. But any way you
slice it there is an extra bit of knowledge required. Since pg_bsd_indent
changes so seldom, keeping it in your PATH is at least as easy as any
other solution, IMO.The reason I bumped into this is that 15 and 16 use different versions
of pg_bsd_indent, so you can't just keep one copy in, like, ~/bin/.Well, personally, I never bother to adjust patches to the indentation
rules of old versions, so using the latest pg_bsd_indent suffices.
I guess we could try looking for pg_bsd_indent-$MAJOR_VERSION first,
then pg_bsd_indent.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.
On 2023-May-31, Bruce Momjian wrote:
I guess we could try looking for pg_bsd_indent-$MAJOR_VERSION first,
then pg_bsd_indent.
Do you mean with $MAJOR_VERSION being Postgres' version? That means we
need to install a new symlink every year, even when pg_bsd_indent
doesn't change. I'd rather have it call pg_bsd_indent-$INDENT_VERSION
first, and pg_bsd_indent if that doesn't exist. I already have it that
way.
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
Do you mean with $MAJOR_VERSION being Postgres' version? That means we
need to install a new symlink every year, even when pg_bsd_indent
doesn't change. I'd rather have it call pg_bsd_indent-$INDENT_VERSION
first, and pg_bsd_indent if that doesn't exist. I already have it that
way.
Sounds reasonable.
regards, tom lane
On Tue, Jun 20, 2023 at 06:54:56PM +0200, Álvaro Herrera wrote:
On 2023-May-31, Bruce Momjian wrote:
I guess we could try looking for pg_bsd_indent-$MAJOR_VERSION first,
then pg_bsd_indent.Do you mean with $MAJOR_VERSION being Postgres' version? That means we
need to install a new symlink every year, even when pg_bsd_indent
doesn't change. I'd rather have it call pg_bsd_indent-$INDENT_VERSION
first, and pg_bsd_indent if that doesn't exist. I already have it that
way.
Yes, your idea makes more sense.
--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EDB https://enterprisedb.com
Only you can decide what is important to you.