Обсуждение: 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?
Вложения
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 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.
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.
Peter Eisentraut <peter_e@gmx.net> writes:
> 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.
Works for me.
regards, tom lane