Re: APR 1.0 released

Andrew Dunstan schrieb:
> Reini Urban said:
>>Bruce Momjian schrieb:
>>
>>>I looked at the APR code to get some ideas for the Win32 port. Some
>>>of the ideas were good, but in other places like rename they didn't do
>>>very well we were better off doing it ourselves and getting it right.
>>>
>>>I remember looking at their code to fix the rename/unlink while the
>>>file is open problem, and they didn't seem to have a fix for that so
>>>we developed our own method that works like Unix.
>>
>>sorry, but your rename doesn't work on cygwin. maybe it works with
>>mingw.
>>
>>cygwin has it's own and working way of doing rename's.
>>maybe you should have looked at the cygwin sources instead.
>>(src/winsup/cygwin/syscalls.cc)
>>
>>first doing a WinAPI MoveFileEx and then after a failure trying the
>>cygwin version, which will also try its own MoveFile loop, will not
>>work. they are conflicting.
>>
>>same with unlink, but at least the mingw and cygwin unlink versions
>>don't conflict here. here you don't stack two conflicting loops
>>together. nevertheless cygwin's unlink is much better than pgunlink in
>>case of locking problems. it does its own sort of delayed removal
>>then.
>>
>>IMHO port/dirmod.c is a dirty and half-baked hack, which works for
>>mingw only.
>
>
>
> Are you sure you are reading this code correctly? Your reading would only be
> correct if WIN32 is defined on Cygwin - it isn't IIRC (don't have a
> convenient way to test ATM). The relevant code is this:
>
> #ifdef WIN32
> while (!MoveFileEx(from, to, MOVEFILE_REPLACE_EXISTING))
> #endif
> #ifdef __CYGWIN__
> while (rename(from, to) < 0)
> #endif
>
> If the code doesn't work, please submit empirical proof, rather than make
> assertions of "half-baked hack". If it's broken we'll fix it. Bruce's point
> about the usefulness of APR remains, nonetheless.

I already posted my needed patches to make beta2 work on cygwin.
But on the pqsql-cygwin mailinglist:
http://xarch.tu-graz.ac.at/home/rurban/software/cygwin/postgresql/postgresql-8.0.0beta2-1
Only a plperl problem is pending. BTW: plperl never worked on cygwin before.

FYI: WIN32 is also defined because <windows.h> is included.
(/usr/incluse/w32api/windef.h)
If you want this or that, do proper nesting, and use #else.

#ifdef __CYGWIN__
while (rename(from, to) < 0)
#else
#ifdef WIN32
while (!MoveFileEx(from, to, MOVEFILE_REPLACE_EXISTING))
#endif
#endif

You cannot safely assume WIN32 is only defined on mingw, but not on
__CYGWIN__. And you need <windows.h> because of some winapi calls below.
The same false assumption was also in src/include/utils/palloc.h

But the whole pgrename #ifdef is fragile and a mess.
cygwin rename works good enough, and I just #ifdef'ed it away.
The two #undef need to be inserted before #include <unistd.h>, otherwise
pgrename will be declared, but rename not.

gcc -E -I../include dirmod-orig.c:
int
pgrename(const char *from, const char *to)
{
int loops = 0;
while (!MoveFileExA(from, to, 1))
while (rename(from, to) < 0)
{
if (GetLastError() != 5L)
if ((*__errno()) != 13)
return -1;
pg_usleep(100000);
if (loops == 30)
errstart(0, "dirmod-orig.c", 87,
__func__), elog_finish(15, "could not rename \"%s\" to \"%s\",
continuing to try",
from, to);
loops++;
}
if (loops > 30)
errstart(0, "dirmod-orig.c", 98, __func__),
elog_finish(15, "completed rename of \"%s\" to \"%s\"", from, to);
return 0;
}

--
Reini Urban
http://xarch.tu-graz.ac.at/home/rurban/