pgsql: Make hstore_plperl's build even more like plperl's

On 05/01/2015 10:20 PM, Peter Eisentraut wrote:

Make hstore_plperl's build even more like plperl's

Combine the two places that set CPPFLAGS into one. Also, some settings
should be restricted to Windows only. More precisely, -Wno-comment is
a GCC-only option, but Windows in a makefile implies GCC at the moment.

Also, since -Wno-comment is more properly a preprocessor option, move it
to CPPFLAGS to simplify things a bit.

Did you actually test this? On my system you have just managed to break
what I had unbroken. I'm not pleased.

cheers

andrew

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

On 5/2/15 4:32 AM, Andrew Dunstan wrote:

On 05/01/2015 10:20 PM, Peter Eisentraut wrote:

Make hstore_plperl's build even more like plperl's

Combine the two places that set CPPFLAGS into one. Also, some settings
should be restricted to Windows only. More precisely, -Wno-comment is
a GCC-only option, but Windows in a makefile implies GCC at the moment.

Also, since -Wno-comment is more properly a preprocessor option, move it
to CPPFLAGS to simplify things a bit.

Did you actually test this? On my system you have just managed to break
what I had unbroken.

I have re-fixed my fix.

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

On 05/02/2015 08:05 AM, Peter Eisentraut wrote:

On 5/2/15 4:32 AM, Andrew Dunstan wrote:

On 05/01/2015 10:20 PM, Peter Eisentraut wrote:

Make hstore_plperl's build even more like plperl's

Combine the two places that set CPPFLAGS into one. Also, some settings
should be restricted to Windows only. More precisely, -Wno-comment is
a GCC-only option, but Windows in a makefile implies GCC at the moment.

Also, since -Wno-comment is more properly a preprocessor option, move it
to CPPFLAGS to simplify things a bit.

Did you actually test this? On my system you have just managed to break
what I had unbroken.

I have re-fixed my fix.

No you haven't. Putting the CORE directory at the end of PG_CPPFLAGS
doesn't work. Using the override to put it at the end of CPPFLAGS does
work. Please either completely fix this mess you've created for mingw
builds or stop meddling with my fixes WHICH I AM ACTUALLY TESTING.

cheers

andrew

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

On 5/2/15 10:20 AM, Andrew Dunstan wrote:

Putting the CORE directory at the end of PG_CPPFLAGS doesn't work. Using
the override to put it at the end of CPPFLAGS does work.

It's strange that the include directory order should matter. What is
the explanation for that? I don't see this documented anywhere. I
don't see any evidence in the buildfarm that this makes a difference.

The intent of my changes was to unbreak other platforms that had been
broken by these Windows-related changes (which did succeed).

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

On 05/02/2015 10:40 AM, Peter Eisentraut wrote:

On 5/2/15 10:20 AM, Andrew Dunstan wrote:

Putting the CORE directory at the end of PG_CPPFLAGS doesn't work. Using
the override to put it at the end of CPPFLAGS does work.

It's strange that the include directory order should matter. What is
the explanation for that? I don't see this documented anywhere. I
don't see any evidence in the buildfarm that this makes a difference.

The intent of my changes was to unbreak other platforms that had been
broken by these Windows-related changes (which did succeed).

What failures on other platforms? There's not a single buildfarm failure
I can see attributable to this. All the recent failures are listed
here: <http://www.pgbuildfarm.org/cgi-bin/show_failures.pl&gt;

The evidence in favor of my changes is in my testing. When I do it your
way I get errors, when I do it my way I don't.

Note that the override is EXACTLY what is done unconditionally in the
plperl GNUmakefile:

override CPPFLAGS := -I. -I$(srcdir) $(CPPFLAGS)
-I$(perl_archlibexp)/CORE

So, contrary to your claims, you have not made this makefile more like
plperl's - you've made it less like plperl's, and detrimentally so.

cheers

andrew

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

On 05/02/2015 11:05 AM, Andrew Dunstan wrote:

On 05/02/2015 10:40 AM, Peter Eisentraut wrote:

On 5/2/15 10:20 AM, Andrew Dunstan wrote:

Putting the CORE directory at the end of PG_CPPFLAGS doesn't work.
Using
the override to put it at the end of CPPFLAGS does work.

It's strange that the include directory order should matter. What is
the explanation for that? I don't see this documented anywhere. I
don't see any evidence in the buildfarm that this makes a difference.

The intent of my changes was to unbreak other platforms that had been
broken by these Windows-related changes (which did succeed).

What failures on other platforms? There's not a single buildfarm
failure I can see attributable to this. All the recent failures are
listed here: <http://www.pgbuildfarm.org/cgi-bin/show_failures.pl&gt;

The evidence in favor of my changes is in my testing. When I do it
your way I get errors, when I do it my way I don't.

Note that the override is EXACTLY what is done unconditionally in the
plperl GNUmakefile:

override CPPFLAGS := -I. -I$(srcdir) $(CPPFLAGS)
-I$(perl_archlibexp)/CORE

So, contrary to your claims, you have not made this makefile more like
plperl's - you've made it less like plperl's, and detrimentally so.

FYI, the attached patch allows jacana to build and test hstore_plperl
without error.

cheers

andrew

On 5/2/15 11:05 AM, Andrew Dunstan wrote:

Note that the override is EXACTLY what is done unconditionally in the
plperl GNUmakefile:

override CPPFLAGS := -I. -I$(srcdir) $(CPPFLAGS)
-I$(perl_archlibexp)/CORE

The override is a red herring. It's done this way because all CPPFLAGS
settings have to be done with override, for unrelated reasons. In a
pgxs build, however, you can set PG_CPPFLAGS, which is later put into
CPPFLAGS with override.

Also, requiring that -I$(perl_archlibexp)/CORE is last seems bizarre.
That would mean that an earlier include directory contains a file that
is also in .../CORE, and you don't want to the one in .../CORE. In that
case, why add .../CORE at all? We get Perl headers from .../CORE, and
that would then mean that other include directories contain some Perl
headers that you want to use in preference to the ones in .../CORE, but
at the same time you want the ones in .../CORE as a fallback.

That might possibly be the actual case, but it would be quite odd and
should be properly explained for posterity.

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

On 5/2/15 11:05 AM, Andrew Dunstan wrote:

What failures on other platforms? There's not a single buildfarm failure
I can see attributable to this.

http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=binturong&amp;dt=2015-05-01%2020%3A13%3A12&amp;stg=make-contrib

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

On 05/02/2015 01:03 PM, Peter Eisentraut wrote:

On 5/2/15 11:05 AM, Andrew Dunstan wrote:

What failures on other platforms? There's not a single buildfarm failure
I can see attributable to this.

http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=binturong&amp;dt=2015-05-01%2020%3A13%3A12&amp;stg=make-contrib

OK, that has nothing to do with the include order. I'm fine with making
the no-comment flag windows only, that was an error on my part. But you
changed more than that.

I don't have an explanation of why the include order matters. But it
does. If you don't think it does, try it, like I have.

cheers

andrew

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

On 05/02/2015 01:02 PM, Peter Eisentraut wrote:

On 5/2/15 11:05 AM, Andrew Dunstan wrote:

Note that the override is EXACTLY what is done unconditionally in the
plperl GNUmakefile:

override CPPFLAGS := -I. -I$(srcdir) $(CPPFLAGS)
-I$(perl_archlibexp)/CORE

The override is a red herring. It's done this way because all CPPFLAGS
settings have to be done with override, for unrelated reasons. In a
pgxs build, however, you can set PG_CPPFLAGS, which is later put into
CPPFLAGS with override.

Also, requiring that -I$(perl_archlibexp)/CORE is last seems bizarre.
That would mean that an earlier include directory contains a file that
is also in .../CORE, and you don't want to the one in .../CORE. In that
case, why add .../CORE at all? We get Perl headers from .../CORE, and
that would then mean that other include directories contain some Perl
headers that you want to use in preference to the ones in .../CORE, but
at the same time you want the ones in .../CORE as a fallback.

That might possibly be the actual case, but it would be quite odd and
should be properly explained for posterity.

here's a list of the .h files in perl's CORE library on jacana.

I can see plenty of opportunities for conflict there.

cheers

andrew

arps/inet.h
av.h
bitcount.h
BuildInfo.h
charclass_invlists.h
config.h
cop.h
cv.h
dirent.h
dosish.h
embed.h
embedvar.h
EXTERN.h
fakesdio.h
fakethr.h
feature.h
form.h
git_version.h
gv.h
handy.h
hv.h
INTERN.h
intrpvar.h
iperlsys.h
keywords.h
l1_char_class_tab.h
malloc_ctl.h
metaconfig.h
mg.h
mg_data.h
mg_raw.h
mg_vtable.h
mydtrace.h
netdb.h
nostdio.h
op.h
opcode.h
opnames.h
op_reg_common.h
overload.h
pad.h
parser.h
patchlevel.h
perl.h
perlapi.h
perlhost.h
perlio.h
perliol.h
perlsdio.h
perlsfio.h
perlvars.h
perly.h
pp.h
pp_proto.h
proto.h
reentr.h
regcharclass.h
regcomp.h
regexp.h
regnodes.h
scope.h
sv.h
sys/socket.h
thread.h
time64.h
time64_config.h
uconfig.h
unixish.h
utf8.h
utfebcdic.h
util.h
uudmap.h
vdir.h
vmem.h
warnings.h
win32.h
win32iop-o.h
win32iop.h
win32thread.h
wince.h
XSUB.h

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