Re: [COMMITTERS] pgsql: Allow copydir() to be interrupted.

Log Message:
-----------
Allow copydir() to be interrupted.

This makes ALTER DATABASE .. SET TABLESPACE and CREATE DATABASE more
sensitive to interrupts. Backpatch to 8.4, where ALTER DATABASE .. SET
TABLESPACE was introduced. We could go back further, but in the absence
of complaints about the CREATE DATABASE case it doesn't seem worth it.

Guillaume Lelarge, with a small correction by me.

Modified Files:
--------------
pgsql/src/port:
copydir.c (r1.36 -> r1.37)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/port/copydir.c?r1=1.36&r2=1.37)

Robert Haas wrote:
> Log Message:
> -----------
> Allow copydir() to be interrupted.
>
> This makes ALTER DATABASE .. SET TABLESPACE and CREATE DATABASE more
> sensitive to interrupts. Backpatch to 8.4, where ALTER DATABASE .. SET
> TABLESPACE was introduced. We could go back further, but in the absence
> of complaints about the CREATE DATABASE case it doesn't seem worth it.
>
> Guillaume Lelarge, with a small correction by me.
>
> Modified Files:
> --------------
> pgsql/src/port:
> copydir.c (r1.36 -> r1.37)
> (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/port/copydir.c?r1=1.36&r2=1.37)
>
>

This appears to have broken MinGW and Cygwin builds on the buildfarm.

cheers

andrew

On Fri, Jul 2, 2010 at 8:10 AM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
> Robert Haas wrote:
>> Log Message:
>> -----------
>> Allow copydir() to be interrupted.
>>
>
> This appears to have broken MinGW and Cygwin builds on the buildfarm.

Well, that's not awesome. IM-ing with Magnus now. I'm wondering if
this is a link-ordering problem of some kind.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

On Fri, Jul 2, 2010 at 2:13 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Fri, Jul 2, 2010 at 8:10 AM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>> Robert Haas wrote:
>>> Log Message:
>>> -----------
>>> Allow copydir() to be interrupted.
>>>
>>
>> This appears to have broken MinGW and Cygwin builds on the buildfarm.
>
> Well, that's not awesome. IM-ing with Magnus now.  I'm wondering if
> this is a link-ordering problem of some kind.

We've seen something like this before, but I don't recall what it was.
It's probably something getting resolved too early when it's built
into libpgport_srv.a. That would explain why it's working fine on MSVC
- we don't actually bother building a server-side .lib there, we just
link the object files directly into postgres.exe. (We do build the
library for client side, of course, since it's used in many different
binaries)

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Fri, Jul 2, 2010 at 9:19 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> On Fri, Jul 2, 2010 at 2:13 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Fri, Jul 2, 2010 at 8:10 AM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>>> Robert Haas wrote:
>>>> Log Message:
>>>> -----------
>>>> Allow copydir() to be interrupted.
>>>>
>>>
>>> This appears to have broken MinGW and Cygwin builds on the buildfarm.
>>
>> Well, that's not awesome. IM-ing with Magnus now.  I'm wondering if
>> this is a link-ordering problem of some kind.
>
> We've seen something like this before, but I don't recall what it was.
> It's probably something getting resolved too early when it's built
> into libpgport_srv.a. That would explain why it's working fine on MSVC
> - we don't actually bother building a server-side .lib there, we just
> link the object files directly into postgres.exe. (We do build the
> library for client side, of course, since it's used in many different
> binaries)

I wonder if we should just move copydir.c to someplace within the
backend, perhaps backend/storage/file or backend/utils/misc. It's
already backend-specific code anyway, so I'm not sure there's much
point in having it in src/port.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>> This appears to have broken MinGW and Cygwin builds on the buildfarm.
>
> Well, that's not awesome. IM-ing with Magnus now. I'm wondering if
> this is a link-ordering problem of some kind.

Possibly an #ifndef FRONTEND will fix it.

regards, tom lane

On Fri, Jul 2, 2010 at 10:18 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>>> This appears to have broken MinGW and Cygwin builds on the buildfarm.
>>
>> Well, that's not awesome. IM-ing with Magnus now.  I'm wondering if
>> this is a link-ordering problem of some kind.
>
> Possibly an #ifndef FRONTEND will fix it.

What's failing to link is postgres.exe

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

On Fri, Jul 2, 2010 at 10:21 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Fri, Jul 2, 2010 at 10:18 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>>>> This appears to have broken MinGW and Cygwin builds on the buildfarm.
>>>
>>> Well, that's not awesome. IM-ing with Magnus now.  I'm wondering if
>>> this is a link-ordering problem of some kind.
>>
>> Possibly an #ifndef FRONTEND will fix it.
>
> What's failing to link is postgres.exe

I suspect that moving copydir.c into the backend will fix this, but I
don't have an appropriate build environment to test. Can someone
please test the attached patch?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

On Fri, Jul 2, 2010 at 11:07 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Fri, Jul 2, 2010 at 10:21 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Fri, Jul 2, 2010 at 10:18 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>>>>> This appears to have broken MinGW and Cygwin builds on the buildfarm.
>>>>
>>>> Well, that's not awesome. IM-ing with Magnus now.  I'm wondering if
>>>> this is a link-ordering problem of some kind.
>>>
>>> Possibly an #ifndef FRONTEND will fix it.
>>
>> What's failing to link is postgres.exe
>
> I suspect that moving copydir.c into the backend will fix this, but I
> don't have an appropriate build environment to test.  Can someone
> please test the attached patch?

Andrew confirms that this works on mingw, so I'm going to go ahead and
check it in and see what happens.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

On Fri, Jul 2, 2010 at 12:38 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Fri, Jul 2, 2010 at 11:07 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Fri, Jul 2, 2010 at 10:21 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>> On Fri, Jul 2, 2010 at 10:18 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>>>>>> This appears to have broken MinGW and Cygwin builds on the buildfarm.
>>>>>
>>>>> Well, that's not awesome. IM-ing with Magnus now.  I'm wondering if
>>>>> this is a link-ordering problem of some kind.
>>>>
>>>> Possibly an #ifndef FRONTEND will fix it.
>>>
>>> What's failing to link is postgres.exe
>>
>> I suspect that moving copydir.c into the backend will fix this, but I
>> don't have an appropriate build environment to test.  Can someone
>> please test the attached patch?
>
> Andrew confirms that this works on mingw, so I'm going to go ahead and
> check it in and see what happens.

hamerkop [Windows Server 2008 R2 (64bit) Visual C++ 2005 AMD64] is not
happy with this.

http://pgbuildfarm.org/cgi-bin/show_log.pl?nm=hamerkop&dt=2010-07-02%2018:45:44

I don't really understand most of the log messages, but the problem
seems to be here (some garbage removed for clarity):

LINK : fatal error LNK1104: '.\Release\postgres\src_port_copydir.obj'
LIB : fatal error LNK1181: '.\Release\libpgport\copydir.obj'

What doesn't make sense about that is that that file should not have
existed in the snapshot hamerkop ran against.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

Robert Haas wrote:
>>> I suspect that moving copydir.c into the backend will fix this, but I
>>> don't have an appropriate build environment to test. Can someone
>>> please test the attached patch?
>>>
>> Andrew confirms that this works on mingw, so I'm going to go ahead and
>> check it in and see what happens.
>>
>
> hamerkop [Windows Server 2008 R2 (64bit) Visual C++ 2005 AMD64] is not
> happy with this.
>
> http://pgbuildfarm.org/cgi-bin/show_log.pl?nm=hamerkop&dt=2010-07-02%2018:45:44
>
> I don't really understand most of the log messages, but the problem
> seems to be here (some garbage removed for clarity):
>
> LINK : fatal error LNK1104: '.\Release\postgres\src_port_copydir.obj'
> LIB : fatal error LNK1181: '.\Release\libpgport\copydir.obj'
>
> What doesn't make sense about that is that that file should not have
> existed in the snapshot hamerkop ran against.
>
>

MSVC was looking for it and not finding it. I have committed a fix that
I hope will fix the MSVC builds, by removing it from the list of files
in libpgport.

cheers

andrew

On Fri, Jul 2, 2010 at 7:30 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
> MSVC was looking for it and not finding it. I have committed a fix that I
> hope will fix the MSVC builds, by removing it from the list of files in
> libpgport.

You'll need to do the same thing in 8.4...

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

Robert Haas wrote:
> On Fri, Jul 2, 2010 at 7:30 PM, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
>> MSVC was looking for it and not finding it. I have committed a fix that I
>> hope will fix the MSVC builds, by removing it from the list of files in
>> libpgport.
>>
>
> You'll need to do the same thing in 8.4...
>
>

Darn. Ok, done.

cheers

andrew