Re: [Postgres-xc-developers] Minor Fixes

[Matt W. <MW...@XI...>]

Just to follow up, the change to configure.in is trivial to include "__SOLARIS__", but it looks to me like in the master branch that someone else is already hacking the configure file and not updating configure.in.

Examples:
Someone has set CFLAGS="-DPGXC $CFLAGS" in configure but not in configure.in
PACKAGE_XC_VERSION, PGXC_MAJORVERSION, and PACKAGE_XC_VERSION are not defined in configure.in
PACKAGE_BUGREPORT is set to pgs...@po... in configure.in

Not trying to harp, just wondering how best to get this change incorporated.

?

Matt

-----Original Message-----
From: Matt Warner 
Sent: Wednesday, October 02, 2013 4:43 PM
To: 'Michael Paquier'
Cc: 鈴木 幸市; Postgres-XC Developers; Koichi Suzuki
Subject: RE: [Postgres-xc-developers] Minor Fixes

Hi,

First off, thank you for taking the time to review the patches!

Responses to the items you noted:

1. Solaris 10 is still a very active OS: Oracle shows it's end of support is not until January 2021. I think that means we should not necessarily expect everyone's build environment to be S11. If we can update maksgml to avoid strndup, then problem solved either way. Solaris has excellent backward binary compatibility, meaning what we compile on S9 or S10 will run on S11, but the reverse might or might not be true.

2. I actually did look at updating configure.in at one point in the past but when I ran autoconf, it produced a configure file that looked nothing like what was supplied with the version in the repository. I don't recall at this point which version I was looking at. I see from more recent builds that everything looks OK now, so I accept the -1 as my bad for not doing this the right way. I'm more than happy to update configure.in instead of configure. I can modify it to set something like "__SOLARIS_10__", although in your note 4 you indicated you didn't like the use of the CFLAG.

3. Glad to help!

4. I used the CFLAG because I was unsuccessful in writing a macro to do this. If you'd prefer a macro to the configure and CFLAG route, please let me know and I'll pursue it further. Otherwise, I'll plan to update configure.in

5. Thanks!

Matt

-----Original Message-----
From: Michael Paquier [mailto:mic...@gm...] 
Sent: Wednesday, October 02, 2013 6:45 AM
To: Matt Warner
Cc: 鈴木 幸市; Postgres-XC Developers; Koichi Suzuki
Subject: Re: [Postgres-xc-developers] Minor Fixes

Hi,

I have set up a Solaris 11 instance in my lab... So here is some
feedback about those patches:
1) makesgml patch: strndup is available in Solaris 11, not in Solaris
<= 10. IMO this patch is not necessary for 2 reasons:
- makesgml should not use strndup.
- Solaris 10 was out in 2005... That's getting old.
So we should patch makesgml and remove the calls to strndup inside it
instead of creating a fake version of it as you do.
2) configure patch. Not necessary, and -1 for adding a flag like that.
Isn't it possible to use something like __sparc__ or __SOLARIS__
instead? You need also to keep in mind that configure is generated
with autoconfigure, so you would actually need to patch configure.in.
3) Makefile patch, actually this is not only a bug of Solaris, so I
went ahead and committed it. That's a good catch.
4) pgxcnode patch. This is indeed necessary for the compilation on
Solaris, FIONREAD is declared in filio.h, but I don't like the use of
a CFLAGS as you did in configure.
5) GTM proxy patch. It looks like you spotted a couple of other bugs!
There is a strange mixture of things returned from functions where
they shouldn't, and this is not only a problem with Solaris. I'll
rework it a bit and commit it later after checking that it works in my
lab.

Thanks for the report and really sorry for the delay in responding!
Regards,
-- 
Michael