make check in contrib
I noticed again that make check in contrib doesn't work, so here is a
patch to fix it. Perhaps someone wants to fill in the Windows support
for it. Naturally, this works only for contrib itself, not for external
packages that use pgxs.
A secondary issue that actually led to this: I was preparing a Debian
package for some module^Wextension^W^Hthing that uses pgxs. The Debian
packaging tools ("dh", to be exact, for the insiders) have this
convenience that by default they examine your makefile and execute the
standard targets, if found, in order. So it runs make all, make check,
which then fails because of
check:
@echo "'make check' is not supported."
@echo "Do 'make install', then 'make installcheck' instead."
@exit 1
You can override this, but it still means that everyone who packages an
extension will have to re-figure this out.
So while this message might be moderately useful (although I'm not sure
whether it's guaranteed that the suggestion will always work), I'd
rather get rid of it and not have a check target in the pgxs case.
Comments?
Attachments:
contrib-make-check.patchtext/x-patch; charset=UTF-8; name=contrib-make-check.patchDownload
diff --git i/GNUmakefile.in w/GNUmakefile.in
index f3c5fe5..79b0da4 100644
--- i/GNUmakefile.in
+++ w/GNUmakefile.in
@@ -60,8 +60,7 @@ check: all
check installcheck installcheck-parallel:
$(MAKE) -C src/test $@
-# TODO: add contrib
-$(call recurse,check-world,src/test src/pl src/interfaces/ecpg,check)
+$(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)
diff --git i/contrib/earthdistance/Makefile w/contrib/earthdistance/Makefile
index 49f6e66..48a7cf8 100644
--- i/contrib/earthdistance/Makefile
+++ w/contrib/earthdistance/Makefile
@@ -6,6 +6,7 @@ EXTENSION = earthdistance
DATA = earthdistance--1.0.sql earthdistance--unpackaged--1.0.sql
REGRESS = earthdistance
+REGRESS_OPTS = --extra-install=contrib/cube
LDFLAGS_SL += $(filter -lm, $(LIBS))
diff --git i/src/makefiles/pgxs.mk w/src/makefiles/pgxs.mk
index 87ade0a..44b0603 100644
--- i/src/makefiles/pgxs.mk
+++ w/src/makefiles/pgxs.mk
@@ -281,13 +281,10 @@ endif
installcheck: submake
$(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)
-# in-tree test doesn't work yet (no way to install my shared library)
-#check: all submake
-# $(pg_regress_check) $(REGRESS_OPTS) $(REGRESS)
-check:
- @echo "'make check' is not supported."
- @echo "Do 'make install', then 'make installcheck' instead."
- @exit 1
+ifndef PGXS
+check: all submake
+ $(pg_regress_check) --extra-install=$(subdir) $(REGRESS_OPTS) $(REGRESS)
+endif
endif # REGRESS
diff --git i/src/test/regress/pg_regress.c w/src/test/regress/pg_regress.c
index 5fe3724..9d894f8 100644
--- i/src/test/regress/pg_regress.c
+++ w/src/test/regress/pg_regress.c
@@ -102,6 +102,7 @@ static bool port_specified_by_user = false;
static char *dlpath = PKGLIBDIR;
static char *user = NULL;
static _stringlist *extraroles = NULL;
+static _stringlist *extra_install = NULL;
/* internal variables */
static const char *progname;
@@ -1894,6 +1895,7 @@ help(void)
printf(_(" --top-builddir=DIR (relative) path to top level build directory\n"));
printf(_(" --port=PORT start postmaster on PORT\n"));
printf(_(" --temp-config=PATH append contents of PATH to temporary config\n"));
+ printf(_(" --extra-install=DIR additional directory to install (e.g., contrib\n"));
printf(_("\n"));
printf(_("Options for using an existing installation:\n"));
printf(_(" --host=HOST use postmaster running on HOST\n"));
@@ -1941,6 +1943,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
{"use-existing", no_argument, NULL, 20},
{"launcher", required_argument, NULL, 21},
{"load-extension", required_argument, NULL, 22},
+ {"extra-install", required_argument, NULL, 23},
{NULL, 0, NULL, 0}
};
@@ -2040,6 +2043,9 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
case 22:
add_stringlist_item(&loadextension, optarg);
break;
+ case 23:
+ add_stringlist_item(&extra_install, optarg);
+ break;
default:
/* getopt_long already emitted a complaint */
fprintf(stderr, _("\nTry \"%s -h\" for more information.\n"),
@@ -2084,6 +2090,7 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
if (temp_install)
{
FILE *pg_conf;
+ _stringlist *sl;
/*
* Prepare the temp installation
@@ -2126,6 +2133,24 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc
exit_nicely(2);
}
+ for (sl = extra_install; sl != NULL; sl = sl->next)
+ {
+#ifndef WIN32_ONLY_COMPILER
+ snprintf(buf, sizeof(buf),
+ SYSTEMQUOTE "\"%s\" -C \"%s/%s\" DESTDIR=\"%s/install\" install >> \"%s/log/install.log\" 2>&1" SYSTEMQUOTE,
+ makeprog, top_builddir, sl->str, temp_install, outputdir);
+#else
+ fprintf(stderr, _("\n%s: --extra-install option not supported on this platform\n", progname));
+ exit_nicely(2);
+#endif
+
+ if (system(buf))
+ {
+ fprintf(stderr, _("\n%s: installation failed\nExamine %s/log/install.log for the reason.\nCommand was: %s\n"), progname, outputdir, buf);
+ exit_nicely(2);
+ }
+ }
+
/* initdb */
header(_("initializing database system"));
snprintf(buf, sizeof(buf),
On Sun, Apr 24, 2011 at 7:18 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
I noticed again that make check in contrib doesn't work, so here is a
patch to fix it. Perhaps someone wants to fill in the Windows support
for it. Naturally, this works only for contrib itself, not for external
packages that use pgxs.A secondary issue that actually led to this: I was preparing a Debian
package for some module^Wextension^W^Hthing that uses pgxs. The Debian
packaging tools ("dh", to be exact, for the insiders) have this
convenience that by default they examine your makefile and execute the
standard targets, if found, in order. So it runs make all, make check,
which then fails because ofcheck:
@echo "'make check' is not supported."
@echo "Do 'make install', then 'make installcheck' instead."
@exit 1You can override this, but it still means that everyone who packages an
extension will have to re-figure this out.So while this message might be moderately useful (although I'm not sure
whether it's guaranteed that the suggestion will always work), I'd
rather get rid of it and not have a check target in the pgxs case.
I think it might be more useful to have a check target that actually
succeeds, even if it does nothing useful. The argument that no check
target at all is more useful than a check target that fails with a
reasonably informative error message seems week to me. It's only
going to be true if - as in the case you mention - external software
is directly inspecting the makefile to figure out what to do. And
that's a pretty weird case to optimize for.
Maybe just change @exit 1 to @exit 0 and call it good?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 04/25/2011 08:53 AM, Robert Haas wrote:
The argument that no check
target at all is more useful than a check target that fails with a
reasonably informative error message seems week to me.
+1 (weak too)
cheers
andrew
Robert Haas <robertmhaas@gmail.com> writes:
On Sun, Apr 24, 2011 at 7:18 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
I noticed again that make check in contrib doesn't work, so here is a
patch to fix it.
I think it might be more useful to have a check target that actually
succeeds, even if it does nothing useful.
That argument seems a bit irrelevant to the proposed patch.
regards, tom lane
On Mon, Apr 25, 2011 at 10:21 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Sun, Apr 24, 2011 at 7:18 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
I noticed again that make check in contrib doesn't work, so here is a
patch to fix it.I think it might be more useful to have a check target that actually
succeeds, even if it does nothing useful.That argument seems a bit irrelevant to the proposed patch.
How so?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
On Mon, Apr 25, 2011 at 10:21 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Sun, Apr 24, 2011 at 7:18 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
I noticed again that make check in contrib doesn't work, so here is a
patch to fix it.
I think it might be more useful to have a check target that actually
succeeds, even if it does nothing useful.
That argument seems a bit irrelevant to the proposed patch.
How so?
The proposed patch is to fix it, not remove it. Surely that's more
useful than a no-op target.
regards, tom lane
On Mon, Apr 25, 2011 at 11:22 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Mon, Apr 25, 2011 at 10:21 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Sun, Apr 24, 2011 at 7:18 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
I noticed again that make check in contrib doesn't work, so here is a
patch to fix it.I think it might be more useful to have a check target that actually
succeeds, even if it does nothing useful.That argument seems a bit irrelevant to the proposed patch.
How so?
The proposed patch is to fix it, not remove it. Surely that's more
useful than a no-op target.
Oh, that's different... never mind.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On mån, 2011-04-25 at 11:22 -0400, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Mon, Apr 25, 2011 at 10:21 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Sun, Apr 24, 2011 at 7:18 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
I noticed again that make check in contrib doesn't work, so here is a
patch to fix it.I think it might be more useful to have a check target that actually
succeeds, even if it does nothing useful.That argument seems a bit irrelevant to the proposed patch.
How so?
The proposed patch is to fix it, not remove it. Surely that's more
useful than a no-op target.
The proposed patch will support make check for contrib modules, but not
for external users of pgxs.
Peter Eisentraut <peter_e@gmx.net> writes:
On mån, 2011-04-25 at 11:22 -0400, Tom Lane wrote:
The proposed patch is to fix it, not remove it. Surely that's more
useful than a no-op target.
The proposed patch will support make check for contrib modules, but not
for external users of pgxs.
So what will happen if an external user tries it? What happens now?
regards, tom lane
On mån, 2011-04-25 at 13:12 -0400, Tom Lane wrote:
Peter Eisentraut <peter_e@gmx.net> writes:
On mån, 2011-04-25 at 11:22 -0400, Tom Lane wrote:
The proposed patch is to fix it, not remove it. Surely that's more
useful than a no-op target.The proposed patch will support make check for contrib modules, but not
for external users of pgxs.So what will happen if an external user tries it? What happens now?
Now:
$ make check
'make check' is not supported.
Do 'make install', then 'make installcheck' instead.
make: *** [check] Error 1
If we removed that, then it would be:
make: Nothing to be done for `check'.
[exit 0]
Hmm, I'm slightly surprised by the latter behavior, but it's the case
that since "check" is a global phony target, if you don't provide
commands for it, it will just do nothing and succeed.
Since some people didn't like removing the hint about "installcheck",
I'd suggest just removing the "exit 1", which should then be pretty
consistent overall.