getting rid of maintainer-check

Started by Peter Eisentrautover 12 years ago7 messages
#1Peter Eisentraut
peter_e@gmx.net

The maintainer-check target never really caught on, I think. Most
people don't run it, and that in turn annoys those who do. Also, it
doesn't provide much functionality.

I propose that we get rid of it and roll the functionality into the
regular build.

Specifically:

- Running duplicate_oids during the regular build was already discussed
elsewhere recently. There are some details to be resolved there, but
it's doable.

- Checking for tabs in SGML files can be run during the regular
documentation build without problems.

- The NLS checks can also be run during the regular NLS-enabled build.

That's it. Any concerns?

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#2Robert Haas
robertmhaas@gmail.com
In reply to: Peter Eisentraut (#1)
Re: getting rid of maintainer-check

On Tue, Sep 3, 2013 at 10:41 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

The maintainer-check target never really caught on, I think. Most
people don't run it, and that in turn annoys those who do. Also, it
doesn't provide much functionality.

I propose that we get rid of it and roll the functionality into the
regular build.

Specifically:

- Running duplicate_oids during the regular build was already discussed
elsewhere recently. There are some details to be resolved there, but
it's doable.

- Checking for tabs in SGML files can be run during the regular
documentation build without problems.

- The NLS checks can also be run during the regular NLS-enabled build.

That's it. Any concerns?

I can't speak for anyone else, but personally I think that sounds like
a significant improvement.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Andres Freund
andres@2ndquadrant.com
In reply to: Peter Eisentraut (#1)
Re: getting rid of maintainer-check

On 2013-09-03 22:41:17 -0400, Peter Eisentraut wrote:

The maintainer-check target never really caught on, I think. Most
people don't run it, and that in turn annoys those who do. Also, it
doesn't provide much functionality.

I propose that we get rid of it and roll the functionality into the
regular build.

Specifically:

- Running duplicate_oids during the regular build was already discussed
elsewhere recently. There are some details to be resolved there, but
it's doable.

Maybe we should also badger cpluspluscheck into a state where it can be
run as part of a normal build if a c++ compiler was detected?

I think it misses vpath support and it might be dependant on some
bashims.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Peter Eisentraut
peter_e@gmx.net
In reply to: Andres Freund (#3)
Re: getting rid of maintainer-check

On 9/4/13 11:12 AM, Andres Freund wrote:

Maybe we should also badger cpluspluscheck into a state where it can be
run as part of a normal build if a c++ compiler was detected?

I think it misses vpath support and it might be dependant on some
bashims.

That might also be doable. If we could at the same time stick a usable
C++ compiler configuration into PGXS, that would also help the growing
number of extensions that need that and are currently using variously
bad workarounds.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Euler Taveira
euler@timbira.com.br
In reply to: Peter Eisentraut (#1)
Re: getting rid of maintainer-check

On 03-09-2013 23:41, Peter Eisentraut wrote:

The maintainer-check target never really caught on, I think. Most
people don't run it, and that in turn annoys those who do. Also, it
doesn't provide much functionality.

It has its use (before each release) but I agree that it isn't used
during minor version updates (because you need to update only one or two
po files).

I propose that we get rid of it and roll the functionality into the
regular build.

By 'regular build' you mean --enable-nls? If so, +1.

- Running duplicate_oids during the regular build was already discussed
elsewhere recently. There are some details to be resolved there, but
it's doable.

This has been bashing sufficient developers along the years. +1.

- Checking for tabs in SGML files can be run during the regular
documentation build without problems.

This one too. +1.

--
Euler Taveira Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Peter Eisentraut
peter_e@gmx.net
In reply to: Peter Eisentraut (#1)
2 attachment(s)
Re: getting rid of maintainer-check

On Tue, 2013-09-03 at 22:41 -0400, Peter Eisentraut wrote:

The maintainer-check target never really caught on, I think. Most
people don't run it, and that in turn annoys those who do. Also, it
doesn't provide much functionality.

I propose that we get rid of it and roll the functionality into the
regular build.

Here is a patch for that. I also integrated Andrew's Perl version of
duplicate_oids.

The MSVC build needs to be updated separately, if they want to have that
same functionality.

Attachments:

0002-Remove-maintainer-check-target-fold-into-normal-buil.patchtext/x-patch; charset=UTF-8; name=0002-Remove-maintainer-check-target-fold-into-normal-buil.patchDownload
>From 7474945c3f2ae3e0f1ff24654f8557106fa6d526 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Wed, 11 Sep 2013 14:34:28 -0400
Subject: [PATCH 2/2] Remove maintainer-check target, fold into normal build

make maintainer-check was obscure and rarely called in practice, and
many breakages were missed.  Fold everything that make maintainer-check
used to do into the normal build.  Specifically:

- Call duplicate_oids when genbki.pl is called.

- Check for tabs in SGML files when the documentation is built.

- Run msgfmt with the -c option during the regular build.  Add an
  additional configure check to see whether we are using the GNU
  version.  (make maintainer-check probably used to fail with non-GNU
  msgfmt.)

Keep maintainer-check as around as phony target for the time being in
case anyone is calling it.  But it won't do anything anymore.

diff --git a/GNUmakefile.in b/GNUmakefile.in
index 17b1b3b..80116a1 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -70,8 +70,6 @@ $(call recurse,check-world,src/test src/pl src/interfaces/ecpg contrib,check)
 
 $(call recurse,installcheck-world,src/test src/pl src/interfaces/ecpg contrib,installcheck)
 
-$(call recurse,maintainer-check,doc src config contrib)
-
 GNUmakefile: GNUmakefile.in $(top_builddir)/config.status
 	./config.status $@
 
diff --git a/config/programs.m4 b/config/programs.m4
index c70a0c2..fd3a9a4 100644
--- a/config/programs.m4
+++ b/config/programs.m4
@@ -197,6 +197,11 @@ AC_DEFUN([PGAC_CHECK_GETTEXT],
   if test -z "$MSGFMT"; then
     AC_MSG_ERROR([msgfmt is required for NLS])
   fi
+  AC_CACHE_CHECK([for msgfmt flags], pgac_cv_msgfmt_flags,
+[if test x"$MSGFMT" != x"" && "$MSGFMT" --version 2>&1 | grep "GNU" >/dev/null; then
+    pgac_cv_msgfmt_flags=-c
+fi])
+  AC_SUBST(MSGFMT_FLAGS, $pgac_cv_msgfmt_flags)
   AC_CHECK_PROGS(MSGMERGE, msgmerge)
   AC_CHECK_PROGS(XGETTEXT, xgettext)
 ])# PGAC_CHECK_GETTEXT
diff --git a/configure b/configure
index c685ca3..db8e006 100755
--- a/configure
+++ b/configure
@@ -659,6 +659,7 @@ TCL_CONFIG_SH
 TCLSH
 XGETTEXT
 MSGMERGE
+MSGFMT_FLAGS
 MSGFMT
 HAVE_POSIX_SIGNALS
 LDAP_LIBS_BE
@@ -29171,6 +29172,19 @@ done
 $as_echo "$as_me: error: msgfmt is required for NLS" >&2;}
    { (exit 1); exit 1; }; }
   fi
+  { $as_echo "$as_me:$LINENO: checking for msgfmt flags" >&5
+$as_echo_n "checking for msgfmt flags... " >&6; }
+if test "${pgac_cv_msgfmt_flags+set}" = set; then
+  $as_echo_n "(cached) " >&6
+else
+  if test x"$MSGFMT" != x"" && "$MSGFMT" --version 2>&1 | grep "GNU" >/dev/null; then
+    pgac_cv_msgfmt_flags=-c
+fi
+fi
+{ $as_echo "$as_me:$LINENO: result: $pgac_cv_msgfmt_flags" >&5
+$as_echo "$pgac_cv_msgfmt_flags" >&6; }
+  MSGFMT_FLAGS=$pgac_cv_msgfmt_flags
+
   for ac_prog in msgmerge
 do
   # Extract the first word of "$ac_prog", so it can be a program name with args.
diff --git a/doc/Makefile b/doc/Makefile
index 2e5e09e..aee3cc0 100644
--- a/doc/Makefile
+++ b/doc/Makefile
@@ -12,5 +12,5 @@ subdir = doc
 top_builddir = ..
 include $(top_builddir)/src/Makefile.global
 
-all distprep html man install installdirs uninstall clean distclean maintainer-clean maintainer-check:
+all distprep html man install installdirs uninstall clean distclean maintainer-clean:
 	$(MAKE) -C src $@
diff --git a/doc/src/Makefile b/doc/src/Makefile
index b0d4f1f..30d8838 100644
--- a/doc/src/Makefile
+++ b/doc/src/Makefile
@@ -4,5 +4,5 @@ subdir = doc/src
 top_builddir = ../..
 include $(top_builddir)/src/Makefile.global
 
-all distprep html man install installdirs uninstall clean distclean maintainer-clean maintainer-check:
+all distprep html man install installdirs uninstall clean distclean maintainer-clean:
 	$(MAKE) -C sgml $@
diff --git a/doc/src/sgml/Makefile b/doc/src/sgml/Makefile
index aeade13..50d9203 100644
--- a/doc/src/sgml/Makefile
+++ b/doc/src/sgml/Makefile
@@ -22,9 +22,9 @@ top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global
 
 
-all: html man
+all: check-tabs html man
 
-distprep: html distprep-man
+distprep: check-tabs html distprep-man
 
 
 ifndef JADE
@@ -301,7 +301,7 @@ MAKEINFO = makeinfo
 ##
 
 # Quick syntax check without style processing
-check maintainer-check: postgres.sgml $(ALMOSTALLSGML) check-tabs
+check: postgres.sgml $(ALMOSTALLSGML) check-tabs
 	$(NSGMLS) $(SPFLAGS) $(SGMLINCLUDE) -s $<
 
 
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index bb732bb..96ef163 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -18,11 +18,11 @@
 #
 # Meta configuration
 
-standard_targets = all install installdirs uninstall distprep clean distclean maintainer-clean coverage check installcheck maintainer-check init-po update-po
+standard_targets = all install installdirs uninstall distprep clean distclean maintainer-clean coverage check installcheck init-po update-po
 # these targets should recurse even into subdirectories not being built:
 standard_always_targets = distprep clean distclean maintainer-clean
 
-.PHONY: $(standard_targets) install-strip html man installcheck-parallel
+.PHONY: $(standard_targets) install-strip html man installcheck-parallel maintainer-check
 
 # make `all' the default target
 all:
@@ -283,6 +283,7 @@ perl_embed_ldflags	= @perl_embed_ldflags@
 AWK	= @AWK@
 LN_S	= @LN_S@
 MSGFMT  = @MSGFMT@
+MSGFMT_FLAGS = @MSGFMT_FLAGS@
 MSGMERGE = @MSGMERGE@
 PYTHON	= @PYTHON@
 TAR	= @TAR@
diff --git a/src/backend/catalog/Makefile b/src/backend/catalog/Makefile
index c4d3f3c..eca15af 100644
--- a/src/backend/catalog/Makefile
+++ b/src/backend/catalog/Makefile
@@ -62,7 +62,8 @@ schemapg.h: postgres.bki ;
 # even in distribution tarballs.  So this is cheating a bit, but it
 # will achieve the goal of updating the version number when it
 # changes.
-postgres.bki: genbki.pl Catalog.pm $(POSTGRES_BKI_SRCS) $(top_srcdir)/configure
+postgres.bki: genbki.pl Catalog.pm $(POSTGRES_BKI_SRCS) $(top_srcdir)/configure $(top_srcdir)/src/include/catalog/duplicate_oids
+	cd $(top_srcdir)/src/include/catalog && ./duplicate_oids
 	$(PERL) -I $(catalogdir) $< $(pg_includes) --set-version=$(MAJORVERSION) $(POSTGRES_BKI_SRCS)
 
 .PHONY: install-data
diff --git a/src/backend/common.mk b/src/backend/common.mk
index 2e56151..5d599db 100644
--- a/src/backend/common.mk
+++ b/src/backend/common.mk
@@ -45,4 +45,4 @@ clean: clean-local
 clean-local:
 	rm -f $(subsysfilename) $(OBJS)
 
-$(call recurse,coverage maintainer-check)
+$(call recurse,coverage)
diff --git a/src/include/Makefile b/src/include/Makefile
index c553e74..578a778 100644
--- a/src/include/Makefile
+++ b/src/include/Makefile
@@ -75,6 +75,3 @@ clean:
 
 distclean maintainer-clean: clean
 	rm -f pg_config.h pg_config_ext.h pg_config_os.h dynloader.h stamp-h stamp-ext-h
-
-maintainer-check:
-	cd catalog && ./duplicate_oids
diff --git a/src/nls-global.mk b/src/nls-global.mk
index 8f06e2d..da91c90 100644
--- a/src/nls-global.mk
+++ b/src/nls-global.mk
@@ -68,7 +68,7 @@ BACKEND_COMMON_GETTEXT_FLAGS = \
 all-po: $(MO_FILES)
 
 %.mo: %.po
-	$(MSGFMT) -o $@ $<
+	$(MSGFMT) $(MSGFMT_FLAGS) -o $@ $<
 
 ifeq ($(word 1,$(GETTEXT_FILES)),+)
 po/$(CATALOG_NAME).pot: $(word 2, $(GETTEXT_FILES)) $(MAKEFILE_LIST)
@@ -113,12 +113,6 @@ clean-po:
 	rm -f po/$(CATALOG_NAME).pot
 
 
-maintainer-check-po: $(ALL_PO_FILES)
-	for file in $^; do \
-	  $(MSGFMT) -c -v -o /dev/null $$file || exit 1; \
-	done
-
-
 init-po: po/$(CATALOG_NAME).pot
 
 
@@ -155,7 +149,6 @@ install: install-po
 installdirs: installdirs-po
 uninstall: uninstall-po
 clean distclean maintainer-clean: clean-po
-maintainer-check: maintainer-check-po
 
 .PHONY: all-po install-po installdirs-po uninstall-po clean-po \
-        maintainer-check-po init-po update-po
+        init-po update-po
-- 
1.8.4.rc3

0001-Replace-duplicate_oids-with-Perl-implementation.patchtext/x-patch; charset=UTF-8; name=0001-Replace-duplicate_oids-with-Perl-implementation.patchDownload
>From 1beea944acf339bc13a8759262a0c8c41fc1760c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Wed, 11 Sep 2013 14:47:44 -0400
Subject: [PATCH 1/2] Replace duplicate_oids with Perl implementation

It is more portable, more robust, and more readable.

From: Andrew Dunstan <andrew@dunslane.net>

diff --git a/src/include/catalog/duplicate_oids b/src/include/catalog/duplicate_oids
index 82c12f3..f3d1136 100755
--- a/src/include/catalog/duplicate_oids
+++ b/src/include/catalog/duplicate_oids
@@ -1,30 +1,36 @@
-#!/bin/sh
-#
-# duplicate_oids
-#
-# src/include/catalog/duplicate_oids
-#
-# finds manually-assigned oids that are duplicated in the system tables.
-#
-# run this script in src/include/catalog.
-#
+#!/usr/bin/perl
 
-# note: we exclude BKI_BOOTSTRAP relations since they are expected to have
-# matching DATA lines in pg_class.h and pg_type.h
+use strict;
+use warnings;
 
-cat pg_*.h toasting.h indexing.h | \
-egrep -v -e '^CATALOG\(.*BKI_BOOTSTRAP' | \
-sed -n	-e 's/^DATA(insert *OID *= *\([0-9][0-9]*\).*$/\1/p' \
-	-e 's/^CATALOG([^,]*, *\([0-9][0-9]*\).*BKI_ROWTYPE_OID(\([0-9][0-9]*\)).*$/\1,\2/p' \
-	-e 's/^CATALOG([^,]*, *\([0-9][0-9]*\).*$/\1/p' \
-	-e 's/^DECLARE_INDEX([^,]*, *\([0-9][0-9]*\).*$/\1/p' \
-	-e 's/^DECLARE_UNIQUE_INDEX([^,]*, *\([0-9][0-9]*\).*$/\1/p' \
-	-e 's/^DECLARE_TOAST([^,]*, *\([0-9][0-9]*\), *\([0-9][0-9]*\).*$/\1,\2/p' | \
-tr ',' '\n' | \
-sort -n | \
-uniq -d | \
-grep '.'
+BEGIN
+{
+	@ARGV = (glob("pg_*.h"), qw(indexing.h toasting.h));
+}
 
-# nonzero exit code if lines were produced
-[ $? -eq 1 ]
-exit
+my %oidcounts;
+
+while(<>)
+{
+	next if /^CATALOG\(.*BKI_BOOTSTRAP/;
+	next unless
+		/^DATA\(insert *OID *= *(\d+)/ ||
+		/^CATALOG\([^,]*, *(\d+).*BKI_ROWTYPE_OID\((\d+)\)/ ||
+		/^CATALOG\([^,]*, *(\d+)/ ||
+		/^DECLARE_INDEX\([^,]*, *(\d+)/ ||
+		/^DECLARE_UNIQUE_INDEX\([^,]*, *(\d+)/ ||
+		/^DECLARE_TOAST\([^,]*, *(\d+), *(\d+)/;
+	$oidcounts{$1}++;
+	$oidcounts{$2}++ if $2;
+}
+
+my $found = 0;
+
+foreach my $oid (sort {$a <=> $b} keys %oidcounts)
+{
+	next unless $oidcounts{$oid} > 1;
+	$found = 1;
+	print "$oid\n";
+}
+
+exit $found;
-- 
1.8.4.rc3

#7Robert Haas
robertmhaas@gmail.com
In reply to: Peter Eisentraut (#6)
Re: getting rid of maintainer-check

On Thu, Sep 12, 2013 at 11:21 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

On Tue, 2013-09-03 at 22:41 -0400, Peter Eisentraut wrote:

The maintainer-check target never really caught on, I think. Most
people don't run it, and that in turn annoys those who do. Also, it
doesn't provide much functionality.

I propose that we get rid of it and roll the functionality into the
regular build.

Here is a patch for that. I also integrated Andrew's Perl version of
duplicate_oids.

The MSVC build needs to be updated separately, if they want to have that
same functionality.

These patches look OK to me, and everyone who has commented has been
in favor of this proposal. I'll mark this Ready for Committer.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers