Re: error out when building pg_xlogdump with pgxs

Hi,

pg_xlogdump cannot properly be built with pgxs since it needs a
sourcetree around. That already has confused some users...

How about the attached patch which will tell it's not supported instead
of an ominous build error about files that have no rules?

Greetings,

Andres Freund

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

Andres Freund wrote:

> pg_xlogdump cannot properly be built with pgxs since it needs a
> sourcetree around. That already has confused some users...
>
> How about the attached patch which will tell it's not supported instead
> of an ominous build error about files that have no rules?

Hmm, the other option is to ignore USE_PGXS completely and build
assuming the source tree is always present. That way, if you build the
whole contrib subdir with USE_PGXS=1, you will end up with all modules
being built, not stop in the middle with an error. This seems more
useful to me. We could just add a comment that USE_PGXS is ignored.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Hi Alvaro,

On 2013-08-27 14:47:49 -0400, Alvaro Herrera wrote:
> Andres Freund wrote:
>
> > pg_xlogdump cannot properly be built with pgxs since it needs a
> > sourcetree around. That already has confused some users...
> >
> > How about the attached patch which will tell it's not supported instead
> > of an ominous build error about files that have no rules?
>
> Hmm, the other option is to ignore USE_PGXS completely and build
> assuming the source tree is always present. That way, if you build the
> whole contrib subdir with USE_PGXS=1, you will end up with all modules
> being built, not stop in the middle with an error. This seems more
> useful to me. We could just add a comment that USE_PGXS is ignored.

What's the point in doing USE_PGXS builds with a full and configured
source present? The only thing I can think of is testing that pgxs
builds are working. In that case it doesn't seem helpful to fake
something into working which is then going to fail for real USE_PGXS
builds (where the original sourcetree won't be at that location
anymore).

Maybe I am missing something?

What we could do is something like:
ifdef USE_PGXS
$(warning "pg_xlogdump cannot be built using pgxs)
else

rest of the build rules

endif

i.e. only warn and do nothing if pgxs is specified so you can still do
make USE_PGXS in the contrib directory.

Greetings,

Andres Freund

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

Le mercredi 28 août 2013 00:12:22 Andres Freund a écrit :
> Hi Alvaro,
>
> On 2013-08-27 14:47:49 -0400, Alvaro Herrera wrote:
> > Andres Freund wrote:
> > > pg_xlogdump cannot properly be built with pgxs since it needs a
> > > sourcetree around. That already has confused some users...
> > >
> > > How about the attached patch which will tell it's not supported instead
> > > of an ominous build error about files that have no rules?
> >
> > Hmm, the other option is to ignore USE_PGXS completely and build
> > assuming the source tree is always present. That way, if you build the
> > whole contrib subdir with USE_PGXS=1, you will end up with all modules
> > being built, not stop in the middle with an error. This seems more
> > useful to me. We could just add a comment that USE_PGXS is ignored.
>
> What's the point in doing USE_PGXS builds with a full and configured
> source present? The only thing I can think of is testing that pgxs
> builds are working. In that case it doesn't seem helpful to fake
> something into working which is then going to fail for real USE_PGXS
> builds (where the original sourcetree won't be at that location
> anymore).

I had the same idea when Peter wished to remove PGXS from the contrib shiped
with postgreSQL.

I've been convinced that if we want to apply testing on pgxs makefile then we
need something dedicated. Not abusing the current options.

I'm in favor of removing PGXS from all contrib makefile, not only this one.

--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation

On 2013-08-29 11:49:00 +0200, Cédric Villemain wrote:
> Le mercredi 28 août 2013 00:12:22 Andres Freund a écrit :
> > Hi Alvaro,
> >
> > On 2013-08-27 14:47:49 -0400, Alvaro Herrera wrote:
> > > Andres Freund wrote:
> > > > pg_xlogdump cannot properly be built with pgxs since it needs a
> > > > sourcetree around. That already has confused some users...
> > > >
> > > > How about the attached patch which will tell it's not supported instead
> > > > of an ominous build error about files that have no rules?
> > >
> > > Hmm, the other option is to ignore USE_PGXS completely and build
> > > assuming the source tree is always present. That way, if you build the
> > > whole contrib subdir with USE_PGXS=1, you will end up with all modules
> > > being built, not stop in the middle with an error. This seems more
> > > useful to me. We could just add a comment that USE_PGXS is ignored.
> >
> > What's the point in doing USE_PGXS builds with a full and configured
> > source present? The only thing I can think of is testing that pgxs
> > builds are working. In that case it doesn't seem helpful to fake
> > something into working which is then going to fail for real USE_PGXS
> > builds (where the original sourcetree won't be at that location
> > anymore).
>
> I had the same idea when Peter wished to remove PGXS from the contrib shiped
> with postgreSQL.
>
> I've been convinced that if we want to apply testing on pgxs makefile then we
> need something dedicated. Not abusing the current options.
>
> I'm in favor of removing PGXS from all contrib makefile, not only this one.

Can we please discuss that in another thread? Anything like removing
PGXS wholesale is for sure not going to be something in 9.3 and this
specific issue should be fixed there as it already confused experienced
users.

Greetings,

Andres Freund

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

Le jeudi 29 août 2013 11:52:36 Andres Freund a écrit :
> On 2013-08-29 11:49:00 +0200, Cédric Villemain wrote:
> > Le mercredi 28 août 2013 00:12:22 Andres Freund a écrit :
> > > Hi Alvaro,
> > >
> > > On 2013-08-27 14:47:49 -0400, Alvaro Herrera wrote:
> > > > Andres Freund wrote:
> > > > > pg_xlogdump cannot properly be built with pgxs since it needs a
> > > > > sourcetree around. That already has confused some users...
> > > > >
> > > > > How about the attached patch which will tell it's not supported
> > > > > instead
> > > > > of an ominous build error about files that have no rules?
> > > >
> > > > Hmm, the other option is to ignore USE_PGXS completely and build
> > > > assuming the source tree is always present. That way, if you build
> > > > the
> > > > whole contrib subdir with USE_PGXS=1, you will end up with all modules
> > > > being built, not stop in the middle with an error. This seems more
> > > > useful to me. We could just add a comment that USE_PGXS is ignored.
> > >
> > > What's the point in doing USE_PGXS builds with a full and configured
> > > source present? The only thing I can think of is testing that pgxs
> > > builds are working. In that case it doesn't seem helpful to fake
> > > something into working which is then going to fail for real USE_PGXS
> > > builds (where the original sourcetree won't be at that location
> > > anymore).
> >
> > I had the same idea when Peter wished to remove PGXS from the contrib
> > shiped with postgreSQL.
> >
> > I've been convinced that if we want to apply testing on pgxs makefile then
> > we need something dedicated. Not abusing the current options.
> >
> > I'm in favor of removing PGXS from all contrib makefile, not only this
> > one.
>
> Can we please discuss that in another thread? Anything like removing
> PGXS wholesale is for sure not going to be something in 9.3 and this
> specific issue should be fixed there as it already confused experienced
> users.

Andres, I was answering your question.
Short and re-phrased:
* we should not abuse make USE_PGXS to test the contrib build
* I believe your patch is correct to issue an error when trying to build
pg_xlogdump with PGXS, it is not possible, dot.

--
Cédric Villemain +33 (0)6 20 30 22 52
http://2ndQuadrant.fr/
PostgreSQL: Support 24x7 - Développement, Expertise et Formation

Cédric Villemain wrote:

> Andres, I was answering your question.
> Short and re-phrased:
> * we should not abuse make USE_PGXS to test the contrib build
> * I believe your patch is correct to issue an error when trying to build
> pg_xlogdump with PGXS, it is not possible, dot.

There being no disagreement, I have pushed the original patch.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services