Integrate pg_bsd_indent into our build/test infrastructure.

tglsfdc · pull[bot] · commit edb0bacd3d67 · 2023-12-28T16:39:53.000Z
Update the Makefile and build directions for in-tree build, and add Meson build infrastructure. Also convert the ad-hoc test target into a TAP test. Currently, the Make build system will not build pg_bsd_indent by default, while the Meson system will. Both will test it during "make check-world" or "ninja test". Neither will install it automatically. (We might change some of these decisions later.) Also fix a few portability nits noted during early testing. Also, exclude pg_bsd_indent from pgindent's purview; at least for now, we'll leave it formatted similarly to the FreeBSD original. Tom Lane and Andres Freund Discussion: https://postgr.es/m/3935719.1675967430@sss.pgh.pa.us Discussion: https://postgr.es/m/20200812223409.6di3y2qsnvynao7a@alap3.anarazel.de
diff --git a/GNUmakefile.in b/GNUmakefile.in
@@ -68,7 +68,7 @@ check check-tests installcheck installcheck-parallel installcheck-tests: CHECKPR
 check check-tests installcheck installcheck-parallel installcheck-tests: submake-generated-headers
 	$(MAKE) -C src/test/regress $@
 
-$(call recurse,check-world,src/test src/pl src/interfaces contrib src/bin,check)
+$(call recurse,check-world,src/test src/pl src/interfaces contrib src/bin src/tools/pg_bsd_indent,check)
 $(call recurse,checkprep,  src/test src/pl src/interfaces contrib src/bin)
 
 $(call recurse,installcheck-world,src/test src/pl src/interfaces contrib src/bin,installcheck)
diff --git a/src/Makefile b/src/Makefile
@@ -66,11 +66,13 @@ clean:
 	$(MAKE) -C test $@
 	$(MAKE) -C tutorial NO_PGXS=1 $@
 	$(MAKE) -C test/isolation $@
+	$(MAKE) -C tools/pg_bsd_indent $@
 
 distclean maintainer-clean:
 	$(MAKE) -C test $@
 	$(MAKE) -C tutorial NO_PGXS=1 $@
 	$(MAKE) -C test/isolation $@
+	$(MAKE) -C tools/pg_bsd_indent $@
 	rm -f Makefile.port Makefile.global
 
 
diff --git a/src/meson.build b/src/meson.build
@@ -13,6 +13,8 @@ subdir('pl')
 
 subdir('interfaces')
 
+subdir('tools/pg_bsd_indent')
+
 
 ### Generate a Makefile.global that's complete enough for PGXS to work.
 #
diff --git a/src/tools/pg_bsd_indent/.gitignore b/src/tools/pg_bsd_indent/.gitignore
@@ -1,10 +1,4 @@
-# Global excludes across all subdirectories
-*.o
-*.obj
-*.exe
-
-# Local excludes in root directory
 /pg_bsd_indent
-/*.out
-/*.list
-/tests.diff
+# Generated by test suite
+/log/
+/tmp_check/
diff --git a/src/tools/pg_bsd_indent/Makefile b/src/tools/pg_bsd_indent/Makefile
@@ -1,35 +1,55 @@
 #-------------------------------------------------------------------------
 #
-# Makefile for pg_bsd_indent
+# src/tools/pg_bsd_indent/Makefile
 #
-# Copyright (c) 2017, PostgreSQL Global Development Group
+# Copyright (c) 2017-2023, PostgreSQL Global Development Group
 #
 #-------------------------------------------------------------------------
 
 PGFILEDESC = "pg_bsd_indent - indent C code nicely"
 PGAPPICON = win32
 
-PROGRAM = pg_bsd_indent
-OBJS	= args.o err.o indent.o io.o lexi.o parse.o pr_comment.o $(WIN32RES)
+subdir = src/tools/pg_bsd_indent
+top_builddir = ../../..
+include $(top_builddir)/src/Makefile.global
 
-# clean junk left behind by "make test"
-EXTRA_CLEAN = *.out *.list tests.diff
+OBJS = \
+	$(WIN32RES) \
+	args.o \
+	err.o \
+	indent.o \
+	io.o \
+	lexi.o \
+	parse.o \
+	pr_comment.o
 
-PG_CONFIG = pg_config
-PGXS := $(shell $(PG_CONFIG) --pgxs)
-include $(PGXS)
+all: pg_bsd_indent
 
-# pgxs.mk assumes too much about what "make check" means, so call it "test"
+pg_bsd_indent: $(OBJS) | submake-libpgport
+	$(CC) $(CFLAGS) $^ $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
+
+install: all installdirs
+	$(INSTALL_PROGRAM) pg_bsd_indent$(X) '$(DESTDIR)$(bindir)/pg_bsd_indent$(X)'
+
+installdirs:
+	$(MKDIR_P) '$(DESTDIR)$(bindir)'
+
+uninstall:
+	rm -f '$(DESTDIR)$(bindir)/pg_bsd_indent$(X)'
+
+clean distclean maintainer-clean:
+	rm -f pg_bsd_indent$(X) $(OBJS)
+	rm -rf log/ tmp_check/
+
+check: pg_bsd_indent
+	$(prove_check)
+
+installcheck:
+	$(prove_installcheck)
+
+# Provide this alternate test name to allow testing pg_bsd_indent
+# without building all of the surrounding Postgres installation.
 .PHONY: test
 
-test: $(PROGRAM)
-	@rm -f tests.diff
-	@cp $(srcdir)/tests/*.list .
-	@for testsrc in $(srcdir)/tests/*.0; do \
-		test=`basename "$$testsrc" .0`; \
-		./$(PROGRAM) $$testsrc $$test.out -P$(srcdir)/tests/$$test.pro || echo FAILED >>$$test.out; \
-		diff -u $$testsrc.stdout $$test.out >>tests.diff 2>&1 || true; \
-	done
-	@cat tests.diff
-	@test '!' -s tests.diff
-	@echo Tests complete.
+test: pg_bsd_indent
+	$(prove_installcheck)
diff --git a/src/tools/pg_bsd_indent/README b/src/tools/pg_bsd_indent/README
@@ -1,3 +1,40 @@
+src/tools/pg_bsd_indent/README
+
+This is a lightly modified version of the "indent" program maintained
+by the FreeBSD project.  The modifications are mostly to make it portable
+to non-BSD-ish platforms, though we do have one formatting switch we
+couldn't convince upstream to take.
+
+To build it, configure the surrounding Postgres source tree,
+then run "make" in this directory.
+Optionally, run "make test" for some simple tests.
+
+You'll need to install pg_bsd_indent somewhere in your PATH before
+using it.  Most likely, if you're a developer, you don't want to
+put it in the same place as where the surrounding Postgres build
+gets installed.  Therefore, do this part with something like
+
+	make install prefix=/usr/local
+
+If you are using Meson to build, the standard build targets will
+build pg_bsd_indent and also test it, but there is not currently
+provision for installing it anywhere.  Manually copy the built
+executable from build/src/tools/pg_bsd_indent/pg_bsd_indent to
+wherever you want to put it.
+
+
+If you happen to be hacking upon the indent source code, the closest
+approximation to the existing indentation style seems to be
+
+	./pg_bsd_indent -i4 -l79 -di12 -nfc1 -nlp -sac somefile.c
+
+although this has by no means been rigorously adhered to.
+(What was that saw about the shoemaker's children?)
+We're not planning to re-indent to Postgres style, because that
+would make it difficult to compare to the FreeBSD sources.
+
+----------
+
 The FreeBSD originals of the files in this directory bear the
 "4-clause" version of the BSD license.  We have removed the
 "advertising" clauses, as per UC Berkeley's directive here:
diff --git a/src/tools/pg_bsd_indent/README.pg_bsd_indent b/src/tools/pg_bsd_indent/README.pg_bsd_indent
diff --git a/src/tools/pg_bsd_indent/args.c b/src/tools/pg_bsd_indent/args.c
@@ -176,7 +176,7 @@ void
 set_profile(const char *profile_name)
 {
     FILE *f;
-    char fname[PATH_MAX];
+    char fname[MAXPGPATH];
     static char prof[] = ".indent.pro";
 
     if (profile_name == NULL)
diff --git a/src/tools/pg_bsd_indent/indent.c b/src/tools/pg_bsd_indent/indent.c
@@ -60,7 +60,7 @@ const char *in_name = "Standard Input";	/* will always point to name of input
 					 * file */
 const char *out_name = "Standard Output";	/* will always point to name
 						 * of output file */
-char        bakfile[MAXPATHLEN] = "";
+char        bakfile[MAXPGPATH] = "";
 
 int
 main(int argc, char **argv)
@@ -207,7 +207,7 @@ main(int argc, char **argv)
 							 * the file */
 		    errx(1, "input and output files must be different");
 		}
-		output = fopen(out_name, "w");
+		output = fopen(out_name, "wb");
 		if (output == NULL)	/* check for create error */
 			err(1, "%s", out_name);
 		continue;
@@ -1232,7 +1232,7 @@ bakcopy(void)
     if (input == NULL)
 	err(1, "%s", bakfile);
     /* now the original input file will be the output */
-    output = fopen(in_name, "w");
+    output = fopen(in_name, "wb");
     if (output == NULL) {
 	unlink(bakfile);
 	err(1, "%s", in_name);
diff --git a/src/tools/pg_bsd_indent/indent.h b/src/tools/pg_bsd_indent/indent.h
@@ -28,7 +28,7 @@
 __FBSDID("$FreeBSD: head/usr.bin/indent/indent.h 303746 2016-08-04 15:27:09Z pfg $");
 #endif
 
-#define nitems(array) (sizeof (array) / sizeof (array[0]))
+#define	nitems(x)	(sizeof((x)) / sizeof((x)[0]))
 
 void	add_typename(const char *);
 void	alloc_typenames(void);
diff --git a/src/tools/pg_bsd_indent/meson.build b/src/tools/pg_bsd_indent/meson.build
@@ -0,0 +1,40 @@
+# Copyright (c) 2022-2023, PostgreSQL Global Development Group
+
+pg_bsd_indent_sources = files(
+  'args.c',
+  'err.c',
+  'indent.c',
+  'io.c',
+  'lexi.c',
+  'parse.c',
+  'pr_comment.c',
+)
+
+if host_system == 'windows'
+  pg_bsd_indent_sources += rc_bin_gen.process(win32ver_rc, extra_args: [
+    '--NAME', 'pg_bsd_indent',
+    '--FILEDESC', 'pg_bsd_indent - indent C code nicely'])
+endif
+
+pg_bsd_indent = executable('pg_bsd_indent',
+  pg_bsd_indent_sources,
+  dependencies: [frontend_code],
+  include_directories: include_directories('.'),
+  kwargs: default_bin_args + {
+    'install': false,
+# possibly at some point do this:
+#   'install_dir': dir_pgxs / 'src/tools/pg_bsd_indent',
+  },
+)
+bin_targets += pg_bsd_indent
+
+tests += {
+  'name': 'pg_bsd_indent',
+  'sd': meson.current_source_dir(),
+  'bd': meson.current_build_dir(),
+  'tap': {
+    'tests': [
+      't/001_pg_bsd_indent.pl',
+    ],
+  },
+}
diff --git a/src/tools/pg_bsd_indent/t/001_pg_bsd_indent.pl b/src/tools/pg_bsd_indent/t/001_pg_bsd_indent.pl
@@ -0,0 +1,53 @@
+# pg_bsd_indent: some simple tests
+
+# The test cases come from FreeBSD upstream, but this test scaffolding is ours.
+# Copyright (c) 2017-2023, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+
+use Cwd qw(getcwd);
+use File::Copy "cp";
+use File::Spec;
+
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# We expect to be started in the source directory (even in a VPATH build);
+# we want to run pg_bsd_indent in the tmp_check directory to reduce clutter.
+# (Also, it's caller's responsibility that pg_bsd_indent be in the PATH.)
+my $src_dir = getcwd;
+chdir ${PostgreSQL::Test::Utils::tmp_check};
+
+# Basic tests: pg_bsd_indent knows --version but not much else.
+program_version_ok('pg_bsd_indent');
+
+# Run pg_bsd_indent on pre-fab test cases.
+# Any diffs in the generated files will be accumulated here.
+my $diff_file = "tests.diff";
+
+# Copy support files to current dir, so *.pro files don't need to know path.
+while (my $file = glob("$src_dir/tests/*.list"))
+{
+	cp($file, ".") || die "cp $file failed: $!";
+}
+
+while (my $test_src = glob("$src_dir/tests/*.0"))
+{
+	# extract test basename
+	my ($volume, $directories, $test) = File::Spec->splitpath($test_src);
+	$test =~ s/\.0$//;
+	# run pg_bsd_indent
+	command_ok(
+		[
+			'pg_bsd_indent', $test_src,
+			"$test.out",     "-P$src_dir/tests/$test.pro"
+		],
+		"pg_bsd_indent succeeds on $test");
+	# check result matches, adding any diff to $diff_file
+	my $result = run_log([ 'diff', '-upd', "$test_src.stdout", "$test.out" ],
+		'>>', $diff_file);
+	ok($result, "pg_bsd_indent output matches for $test");
+}
+
+done_testing();
diff --git a/src/tools/pgindent/exclude_file_patterns b/src/tools/pgindent/exclude_file_patterns
@@ -47,6 +47,10 @@ src/pl/plperl/ppport\.h$
 src/pl/plperl/SPI\.c$
 src/pl/plperl/Util\.c$
 #
+# pg_bsd_indent has its own, idiosyncratic indentation style.
+# We'll stick to that to permit comparison with the FreeBSD upstream.
+src/tools/pg_bsd_indent/.*
+#
 # Exclude any temporary installations that may be in the tree.
 /tmp_check/
 /tmp_install/

Original file line number	Diff line number	Diff line change
`@@ -13,6 +13,8 @@ subdir('pl')`
`13`	`13`
`14`	`14`	`subdir('interfaces')`
`15`	`15`
	`16`	`+subdir('tools/pg_bsd_indent')`
	`17`	`+`
`16`	`18`
`17`	`19`	`### Generate a Makefile.global that's complete enough for PGXS to work.`
`18`	`20`	`#`
Original file line number	Diff line number	Diff line change
`@@ -176,7 +176,7 @@ void`
`176`	`176`	`set_profile(const char *profile_name)`
`177`	`177`	`{`
`178`	`178`	`FILE *f;`
`179`		`- char fname[PATH_MAX];`
	`179`	`+ char fname[MAXPGPATH];`
`180`	`180`	`static char prof[] = ".indent.pro";`
`181`	`181`
`182`	`182`	`if (profile_name == NULL)`