Re: APR 1.0 released - Mailing list pgsql-hackers

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 WIN32while (!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/