Re: [HACKERS] Proposed patch to getaddrinfo.c to support

It, or some related patch appears to have broken the build on buildfarm member snake.

I haven't had time to investigate.

/D

-----Original Message-----
From: "Bruce Momjian"<pgman(at)candle(dot)pha(dot)pa(dot)us>
Sent: 25/08/05 01:14:54
To: "Tom Lane"<tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "Andrew Dunstan"<andrew(at)dunslane(dot)net>, "Chuck McDevitt"<cmcdevitt(at)greenplum(dot)com>, "pgsql-patches(at)postgresql(dot)org"<pgsql-patches(at)postgresql(dot)org>, "PostgreSQL-development"<pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] [PATCHES] Proposed patch to getaddrinfo.c to support

Does this fix IPv6 on Win32?

---------------------------------------------------------------------------

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> >> Context diff, please, diff -c.
>
> > It needed dos2unix and pgindent as well. Here's a cleaned patch.
> > Thanks to Chuck for doing this work.
>
> Applied, thanks.
>
> regards, tom lane
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
message can get through to the mailing list cleanly

-----Unmodified Original Message-----

Does this fix IPv6 on Win32?

---------------------------------------------------------------------------

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> >> Context diff, please, diff -c.
>
> > It needed dos2unix and pgindent as well. Here's a cleaned patch.
> > Thanks to Chuck for doing this work.
>
> Applied, thanks.
>
> regards, tom lane
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
message can get through to the mailing list cleanly

Dave Page wrote:
> It, or some related patch appears to have broken the build on buildfarm member snake.
>
> I haven't had time to investigate.
>
> /D

Atached patch fixes it and also adds proper gai_strerror for windows.
(It's patch against CVS *after* Chucks patch was aplied)

--
Regards
Petr Jelinek (PJMODOS)

Petr Jelinek wrote:

> Dave Page wrote:
>
>> It, or some related patch appears to have broken the build on
>> buildfarm member snake.
>>
>> I haven't had time to investigate.
>>
>> /D
>
>
> Atached patch fixes it and also adds proper gai_strerror for windows.
> (It's patch against CVS *after* Chucks patch was aplied)
>

I thought this had been tested. I should have tested it myself. Apologies.

Anyway, with Petr's extra patch I get a clean build, but "make check"
fails with a postmaster bind failure and a pgsql failure, both with and
without IPv6 installed, on my Xp-PRO SP1 box. When IPv6 is installed it
complains about an unknown family 23 (which is Windows-speak for AF_INET6).

So, not quite there yet.

I'm out of action for pretty much the rest of today and tomorrow, so
won't be doing more testing for a while.

cheers

andrew

Petr Jelinek <pjmodos(at)seznam(dot)cz> writes:
> Dave Page wrote:
>> It, or some related patch appears to have broken the build on buildfarm member snake.

> Atached patch fixes it and also adds proper gai_strerror for windows.

Applied. I had to #ifdef the gai_strerror additions to avoid breakage
on my own machine.

regards, tom lane

Andrew Dunstan wrote:
>
> I thought this had been tested. I should have tested it myself. Apologies.
>
Right, I thought I tested it, well maybe it was my version dunno but
surely it was my mistake.

> Anyway, with Petr's extra patch I get a clean build, but "make check"
> fails with a postmaster bind failure and a pgsql failure, both with and
> without IPv6 installed, on my Xp-PRO SP1 box. When IPv6 is installed it
> complains about an unknown family 23 (which is Windows-speak for AF_INET6).
>
> So, not quite there yet.

[I did make check only in W2K because I don't have direct access to XP
machine now]
No thats not windows error thats postgres error (look at pqcomm.c),
which means HAVE_IPV6 is not defined. I think it should be made that
HAVE_IPV6 and HAVE_STRUCT_ADDRINFO is always defined under windows (and
also #include <ws2tcpip.h> in getaddrinfo.h otherwise it won't build)
but I am not familiar with build system so somebody else will have to do
it (I am not familiar with whole configure thingy at all).
With those changes it should finally work.

--
Regards
Petr Jelinek (PJMODOS)

Petr Jelinek <pjmodos(at)seznam(dot)cz> writes:
> Andrew Dunstan wrote:
>> So, not quite there yet.

> [I did make check only in W2K because I don't have direct access to XP
> machine now]
> No thats not windows error thats postgres error (look at pqcomm.c),
> which means HAVE_IPV6 is not defined.

Possibly, but that's apparently not the only problem. I'm looking at
the first buildfarm result with this patch,
http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=loris&dt=2005-08-25%2018:56:02

The interesting part is the postmaster log at the bottom:

LOG: could not bind IPv4 socket: No error
HINT: Is another postmaster already running on port 55678? If not, wait a few seconds and retry.
WARNING: could not create listen socket for "localhost"
FATAL: could not create any TCP/IP sockets

Apparently, access to IPv4 sockets isn't working either (and the "No
error" isn't very helpful; would seem we're not reading the right
status value).

regards, tom lane

Tom Lane wrote:
> Possibly, but that's apparently not the only problem. I'm looking at
> the first buildfarm result with this patch,
> http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=loris&dt=2005-08-25%2018:56:02
>
> The interesting part is the postmaster log at the bottom:
>
> LOG: could not bind IPv4 socket: No error
> HINT: Is another postmaster already running on port 55678? If not, wait a few seconds and retry.
> WARNING: could not create listen socket for "localhost"
> FATAL: could not create any TCP/IP sockets
>
> Apparently, access to IPv4 sockets isn't working either (and the "No
> error" isn't very helpful; would seem we're not reading the right
> status value).

/me kicks brother out of winXP machine to see whats going on

Yep those changes proposed in my previous email fixes IPv4 too.

LOG: database system was shut down at 2005-08-26 00:05:51 [removed
unreadable chars :)]
LOG: checkpoint record is at 0/390CE0
LOG: redo record is at 0/390CE0; undo record is at 0/0; shutdown TRUE
LOG: next transaction ID: 562; next OID: 10791
LOG: next MultiXactId: 1; next MultiXactOffset: 0
LOG: database system is ready
LOG: transaction ID wrap limit is 2147484144, limited by database
"postgres"

Looks ok, i don't know what IPv4 has to do with all of this though.

--
Regards
Petr Jelinek (PJMODOS)

Petr Jelinek <pjmodos(at)seznam(dot)cz> writes:
> Yep those changes proposed in my previous email fixes IPv4 too.

Apparently not on loris (unless there was another patch that I missed).
Maybe something to do with a different version of Windows?

regards, tom lane

Tom Lane wrote:

>Petr Jelinek <pjmodos(at)seznam(dot)cz> writes:
>
>
>>Yep those changes proposed in my previous email fixes IPv4 too.
>>
>>
>
>Apparently not on loris (unless there was another patch that I missed).
>Maybe something to do with a different version of Windows?
>
>
>
>

I suspected we'd forgotten something.

The attached small patch appears to be what's required (at least on
loris). "make check" failed but not for any apparent ipv6 reason. More
importantly, we correctly set HAVE_IPV6 and HAVE_STRUCT_ADDRINFO.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> I suspected we'd forgotten something.

> The attached small patch appears to be what's required (at least on
> loris). "make check" failed but not for any apparent ipv6 reason. More
> importantly, we correctly set HAVE_IPV6 and HAVE_STRUCT_ADDRINFO.

Applied. One step at a time ;-)

regards, tom lane

Andrew Dunstan wrote:
> I suspected we'd forgotten something.
>
> The attached small patch appears to be what's required (at least on
> loris). "make check" failed but not for any apparent ipv6 reason. More
> importantly, we correctly set HAVE_IPV6 and HAVE_STRUCT_ADDRINFO.

Well this is what I ment with those proposed changes - I haven't sent
patch, just said whats needed - always define HAVE_IPV6 and
HAVE_STRUCT_ADDRINFO and include ws2tcpip.h, if you do just that include
like your patch did, you'll break building on W2k (and like I said I
don't know how to make HAVE_IPV6 and HAVE_STRUCT_ADDRINFO always defined
under windows because I am not familiar with configure and thats why I
haven't sent patch).

--
Regards
Petr Jelinek (PJMODOS)

Petr Jelinek wrote:

> Andrew Dunstan wrote:
>
>> I suspected we'd forgotten something.
>>
>> The attached small patch appears to be what's required (at least on
>> loris). "make check" failed but not for any apparent ipv6 reason.
>> More importantly, we correctly set HAVE_IPV6 and HAVE_STRUCT_ADDRINFO.
>
>
> Well this is what I ment with those proposed changes - I haven't sent
> patch, just said whats needed - always define HAVE_IPV6 and
> HAVE_STRUCT_ADDRINFO and include ws2tcpip.h, if you do just that
> include like your patch did, you'll break building on W2k (and like I
> said I don't know how to make HAVE_IPV6 and HAVE_STRUCT_ADDRINFO
> always defined under windows because I am not familiar with configure
> and thats why I haven't sent patch).
>

Really? Please don't assert it, test it and tell us what the error is. I
find it highly unlikely that it will break building on w2k, and only
slightly less unlikely that it will break running on w2k. But I want to
see the evidence (make error or error from runtime log).

The patch I sent should be exactly what is required to have HAVE_IPV6
and HAVE_STRUCT_ADDRINFO defined on windows. That should be true
regardless of which windows you are building on - the headers should be
the same.

cheers

andrew

Andrew Dunstan wrote:
>
> The patch I sent should be exactly what is required to have HAVE_IPV6
> and HAVE_STRUCT_ADDRINFO defined on windows. That should be true
> regardless of which windows you are building on - the headers should be
> the same.

Oh, if that include makes HAVE_IPV6 defined than it should be ok, I
guess I just misunderstood your mail about your changes to HAVE_IPV6
check in configure under windows. Sorry for misinformation.
[making...]
Yes it actually builds, make check has nine failures for me but that has
nothing to do with IPv6 (looks like postgres doesn't like my locale
because with initdb --no-locale it passes without prob).

So I hope IPv6 episode is finally over :)

--
Regards
Petr Jelinek (PJMODOS)

Petr Jelinek wrote:

>
> So I hope IPv6 episode is finally over :)
>

Almost :-)

The initdb code is failing because we didn't call WSAStartup() - this
just took me ages to track down. All the rest works.

The attached patch seems to do the trick.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> The initdb code is failing because we didn't call WSAStartup() - this
> just took me ages to track down. All the rest works.

> The attached patch seems to do the trick.

Applied.

regards, tom lane