Thread: [HACKERS] make check false success
I noticed that the `check` Makefile rule imported by PGXS is giving a success exit code even when it is unsupported. The attached patch fixes that. --strk; () Free GIS & Flash consultant/developer /\ https://strk.kbt.io/services.html -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
On Thu, Jun 1, 2017 at 10:18 AM, Sandro Santilli <strk@kbt.io> wrote: > I noticed that the `check` Makefile rule imported by PGXS is giving > a success exit code even when it is unsupported. > > The attached patch fixes that. Hmm. I'm not 100% sure that the existing behavior is wrong. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Fri, Jun 02, 2017 at 08:20:25AM -0400, Robert Haas wrote: > On Thu, Jun 1, 2017 at 10:18 AM, Sandro Santilli <strk@kbt.io> wrote: > > I noticed that the `check` Makefile rule imported by PGXS is giving > > a success exit code even when it is unsupported. > > > > The attached patch fixes that. > > Hmm. I'm not 100% sure that the existing behavior is wrong. Why not ? The caller is attempting to make an unsupported target, how's that different from calling `make unexistent` ? --strk;
Sandro Santilli <strk@kbt.io> writes:
> On Fri, Jun 02, 2017 at 08:20:25AM -0400, Robert Haas wrote:
>> On Thu, Jun 1, 2017 at 10:18 AM, Sandro Santilli <strk@kbt.io> wrote:
>>> I noticed that the `check` Makefile rule imported by PGXS is giving
>>> a success exit code even when it is unsupported.
>> Hmm. I'm not 100% sure that the existing behavior is wrong.
> Why not ? The caller is attempting to make an unsupported target,
> how's that different from calling `make unexistent` ?
The key question here is whether you break recursive behavior.
We wouldn't be happy if, for instance, contrib/Makefile had to know
exactly which subdirectories it was safe to recurse into when doing
"make check". Now, that doesn't apply directly because we're not
using the PGXS logic when we do that --- but it seems entirely
possible to me that a PGXS-using module might be a subdirectory
of some larger project, so that it would not want to fail just
because it had no tests it could run.
A related point is that if you try "make check" in a subdirectory
that doesn't have REGRESS defined at all, you don't get an error
(and this patch wouldn't change that). It would be pretty inconsistent
to throw an error if REGRESS is defined but not if it isn't, IMO.
regards, tom lane
On Mon, Jun 5, 2017 at 5:23 AM, Sandro Santilli <strk@kbt.io> wrote: > Why not ? The caller is attempting to make an unsupported target, > how's that different from calling `make unexistent` ? That's a good point, but what Tom wrote is along the lines of my concerns also, especially his last paragraph about REGRESS not being defined at all. I think we have a convention that 'make check' succeeds if it runs all of the tests, even if the set of all tests happens to be the empty set. What was your motivation for wanting this changed in the first place? It seems like either behavior could be more convenient for someone, depending on the context. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company