Thread: Cygwin PostgreSQL postmaster abort problem

Cygwin PostgreSQL postmaster abort problem

From
Jason Tishler
Date:
While controlling the PostgreSQL JDBC driver with the following JBDC connection
pool manager:

    http://sourceforge.net/projects/protomatter/

I discovered that the Cygwin PostgreSQL postmaster could abort with the
following error message:

    /usr/local/pgsql/bin/postmaster: ServerLoop: select failed: No children

After much tracing of the Cygwin code, I determined that the problem was
not in the Cygwin DLL so I started to examine the relevant area of the
PostgreSQL source.

Fairly quickly, I determined and verified that the problem is in
postmaster's reaper().  When postmaster is blocked on a select() and the
JDBC client drops its connections, reaper() is fired (as a signal handler)
to clean up.  Since it always calls waitpid() one more time than is
actually needed, errno is inadvertently set to ECHILD which overwrites
the EINTR value set by Cygwin's select().  Hence, when select() returns,
postmaster finds errno set to ECHILD (instead of EINTR) and exits.

The attached minimal patch solves this problem for Cygwin and has been
verified not to break PostgreSQL on Red Hat 6.2 Linux.  Note that I did
not surround the added code with Cygwin specific conditional
compilation because I feel that the change is appropriate for all
platforms.

The procedure to apply this patch is as follows:

    $ cd postgresql-7.0.3/src/backend/postmaster
    $ # save attached patch to current directory
    $ patch <Cygwin-1.1.6-PostgreSQL-7.0.3-postmaster.patch

Would someone more intimate with this area of the PostgreSQL source
critically evaluate this patch and let the list know if there is a better
way of fixing this problem?

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corporation         Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Attachment

Re: Cygwin PostgreSQL postmaster abort problem

From
Jason Tishler
Date:
On Thu, Dec 21, 2000 at 02:27:22PM -0500, Jason Tishler wrote:
> Would someone more intimate with this area of the PostgreSQL source
> critically evaluate this patch and let the list know if there is a better
> way of fixing this problem?

I was able to answer the above by reading the CVS log entry for
postmaster.c, version 1.199:

    Ensure that 'errno' is saved and restored by all signal handlers that
    might change it.  Experimentation shows that the signal handler call
    mechanism does not save/restore errno for you, at least not on Linux
    or HPUX, so this is definitely a real risk.

Sorry for the noise.

Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: Cygwin PostgreSQL postmaster abort problem

From
Jason Tishler
Date:
Bruce,

On Fri, Dec 29, 2000 at 02:06:52PM -0500, Bruce Momjian wrote:
> Can you download the current snapshot and give it a try?

I just verified that the current snapshot:

    ftp://ftp.postgresql.org/pub/dev/postgresql-snapshot.tar.gz

with a time stamp of:

    Fri Dec 29 09:03:00 2000

has this problem corrected.

Unfortunately, I found some more Cygwin build issues.  I guess that it
is time to make another patch...

Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corporation         Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: Cygwin PostgreSQL postmaster abort problem

From
Tom Lane
Date:
Jason Tishler <Jason.Tishler@dothill.com> writes:
> Unfortunately, I found some more Cygwin build issues.  I guess that it
> is time to make another patch...

No time like the present ;-)

We are pointing for a 7.1beta2 release towards the end of next week.
If you can send patches in the next few days it'd be very timely...

            regards, tom lane

Re: Cygwin PostgreSQL postmaster abort problem

From
Jason Tishler
Date:
Tom,

On Fri, Dec 29, 2000 at 11:58:16PM -0500, Tom Lane wrote:
> Jason Tishler <Jason.Tishler@dothill.com> writes:
> > Unfortunately, I found some more Cygwin build issues.  I guess that it
> > is time to make another patch...
>
> No time like the present ;-)
>
> We are pointing for a 7.1beta2 release towards the end of next week.
> If you can send patches in the next few days it'd be very timely...

See attached for ChangeLog and patch.  The instructions to apply the
patch is as follows:

    $ tar -xzvf postgresql-snapshot.tar.gz
    $ cd postgresql-snapshot
    $ patch -p1 <Cygwin-1.1.7-PostgreSQL-20001229.patch

Note that I ran the regression tests and all 76 test passed.

Unfortunately, I was bitten by the pq.dll not in the PATH problem again --
shame on me.  I should have sent in a patch for this before and I will
do so shortly.  Due to the impending release, I don't want to hold up
the Cygwin (compilation) patch.

BTW, I was doing the builds and regression testing on my machine at
work via ssh so I didn't see Windows pop up its standard dialog until
after I VNC-ed in.  Hence, when the regression test initially "hung,"
it became a real head banger there for a moment...

Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corporation         Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Attachment

Re: Cygwin PostgreSQL postmaster abort problem

From
Tom Lane
Date:
Jason Tishler <Jason.Tishler@dothill.com> writes:
> --- postgresql-20001229.orig/src/backend/utils/mmgr/mcxt.c    Tue Aug 22 03:00:13 2000
> +++ postgresql-20001229/src/backend/utils/mmgr/mcxt.c    Fri Dec 29 23:46:31 2000
> @@ -45,7 +45,7 @@ MemoryContext PostmasterContext = NULL;
>  MemoryContext CacheMemoryContext = NULL;
>  MemoryContext QueryContext = NULL;
>  MemoryContext TopTransactionContext = NULL;
> -MemoryContext TransactionCommandContext = NULL;
> +DLLIMPORT MemoryContext TransactionCommandContext = NULL;

These changes look fine as far as they go, but if you think
TransactionCommandContext needs to be DLLIMPORT, why not the other
global context variables, too?  What led you to mark it DLLIMPORT
anyway?

            regards, tom lane

Re: Cygwin PostgreSQL postmaster abort problem

From
Jason Tishler
Date:
Tom,

On Sat, Dec 30, 2000 at 10:56:43PM -0500, Tom Lane wrote:
> Jason Tishler <Jason.Tishler@dothill.com> writes:
> > --- postgresql-20001229.orig/src/backend/utils/mmgr/mcxt.c    Tue Aug 22 03:00:13 2000
> > +++ postgresql-20001229/src/backend/utils/mmgr/mcxt.c    Fri Dec 29 23:46:31 2000
> > @@ -45,7 +45,7 @@ MemoryContext PostmasterContext = NULL;
> >  MemoryContext CacheMemoryContext = NULL;
> >  MemoryContext QueryContext = NULL;
> >  MemoryContext TopTransactionContext = NULL;
> > -MemoryContext TransactionCommandContext = NULL;
> > +DLLIMPORT MemoryContext TransactionCommandContext = NULL;
>
> These changes look fine as far as they go, but if you think
> TransactionCommandContext needs to be DLLIMPORT, why not the other
> global context variables, too?

I took the minimalist approach -- meaning that I only made the minimum
number of changes necessary to get PostgreSQL to compile cleanly under
Cygwin.  You are correct, the other global context variables should
also be marked DLLIMPORT.  There are probably other global variables
that should be marked too.

The completeness approach would DLLIMPORT "everything" (similar to
the way that Python uses DL_EXPORT), but that would be a lot of source
code changes...

> What led you to mark it DLLIMPORT anyway?

I marked TransactionCommandContext as DLLIMPORT to get plpgsql.dll to
link without unresolved symbol errors.

Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corporation         Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: Re: Cygwin PostgreSQL postmaster abort problem

From
Peter Eisentraut
Date:
Jason Tishler writes:

> Sat Dec 30 21:13:30 2000  Jason Tishler <jt@dothill.com>
>
>         * src/backend/utils/error/elog.c: Add conditional compilation to deal
>         with sys_nerr/_sys_nerr inconsistencies.
>         * src/backend/utils/error/exc.c: Ditto.

This is the wrong place.  You should look into src/include/port/win.h to
fix this.

>         * src/utils/dllinit.c: Update to be consistent with Cygwin Net Release.

Will this break old releases of Cygwin?

--
Peter Eisentraut      peter_e@gmx.net       http://yi.org/peter-e/


Re: Cygwin PostgreSQL postmaster abort problem

From
Tom Lane
Date:
Jason Tishler <Jason.Tishler@dothill.com> writes:
>> These changes look fine as far as they go, but if you think
>> TransactionCommandContext needs to be DLLIMPORT, why not the other
>> global context variables, too?

> I took the minimalist approach -- meaning that I only made the minimum
> number of changes necessary to get PostgreSQL to compile cleanly under
> Cygwin.  You are correct, the other global context variables should
> also be marked DLLIMPORT.  There are probably other global variables
> that should be marked too.

> The completeness approach would DLLIMPORT "everything" (similar to
> the way that Python uses DL_EXPORT), but that would be a lot of source
> code changes...

Seems like that's heading in the wrong direction.  Isn't there a
compiler switch or something we could give to make ALL global vars be
automatically marked DLLIMPORT?  That's generally how it works on Unix
platforms (for example, on HPUX the -E linker switch makes these symbols
available to dynamically linked shlibs).  I don't really like the idea
of cluttering the source code for the benefit of one platform...

            regards, tom lane

Re: Re: Cygwin PostgreSQL postmaster abort problem

From
Jason Tishler
Date:
On Sun, Dec 31, 2000 at 12:12:18AM -0500, Jason Tishler wrote:
> > What led you to mark it DLLIMPORT anyway?
>
> I marked TransactionCommandContext as DLLIMPORT to get plpgsql.dll to
> link without unresolved symbol errors.

Please remove the src/backend/utils/mmgr/mcxt.c hunk from my patch -- it
is unnecessary.  I was mislead by an unnecessary DLLIMPORT located a few
lines above my erroneous change to mcxt.c.  Specifically, DLLIMPORT can
be also safely removed from:

    DLLIMPORT MemoryContext CurrentMemoryContext = NULL;

The DLLIMPORT is only necessary in the corresponding header files (i.e.,
src/include/utils/memutils.h and src/include/utils/palloc.h).

Otherwise, you can wait for me to redo the patch once the remaining issues
settle down...

Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corporation         Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: Cygwin PostgreSQL postmaster abort problem

From
Jason Tishler
Date:
Tom,

On Sun, Dec 31, 2000 at 05:53:09PM -0500, Tom Lane wrote:
> Jason Tishler <Jason.Tishler@dothill.com> writes:
> >> These changes look fine as far as they go, but if you think
> >> TransactionCommandContext needs to be DLLIMPORT, why not the other
> >> global context variables, too?
>
> > I took the minimalist approach -- meaning that I only made the minimum
> > number of changes necessary to get PostgreSQL to compile cleanly under
> > Cygwin.  You are correct, the other global context variables should
> > also be marked DLLIMPORT.  There are probably other global variables
> > that should be marked too.
>
> > The completeness approach would DLLIMPORT "everything" (similar to
> > the way that Python uses DL_EXPORT), but that would be a lot of source
> > code changes...
>
> Seems like that's heading in the wrong direction.  Isn't there a
> compiler switch or something we could give to make ALL global vars be
> automatically marked DLLIMPORT?  That's generally how it works on Unix
> platforms (for example, on HPUX the -E linker switch makes these symbols
> available to dynamically linked shlibs).

There is a way and the PostgreSQL build is already taking advantage of
it:

    dlltool --export-all --output-def postgres.def access/SUBSYS.o ...

However, there is a downside to the above.  Now all functions and global
variables are exported from postgres.exe -- not just the ones that make
sense.  This is why the Python approach may be considered "better."

Unfortunately, the above still does not mitigate the need for marking
global variables DLLIMPORT in their corresponding header files to
prevent unresolved linker errors by clients of libpostgres.a.

> I don't really like the idea of cluttering the source code for the
> benefit of one platform...

Agreed.

Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corporation         Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: Re: Cygwin PostgreSQL postmaster abort problem

From
Jason Tishler
Date:
Peter,

On Sun, Dec 31, 2000 at 01:12:59PM +0100, Peter Eisentraut wrote:
> Jason Tishler writes:
>
> > Sat Dec 30 21:13:30 2000  Jason Tishler <jt@dothill.com>
> >
> >         * src/backend/utils/error/elog.c: Add conditional compilation to deal
> >         with sys_nerr/_sys_nerr inconsistencies.
> >         * src/backend/utils/error/exc.c: Ditto.
>
> This is the wrong place.

Sorry, but I'm not that intimate with the PostgreSQL code so when I saw
the following in elog.c and exc.c:

    #ifdef __CYGWIN__
    # define sys_nerr _sys_nerr
    #endif
    extern int  sys_nerr;

I thought that it was reasonable to change it to:

    #ifdef __CYGWIN__
    # define sys_nerr _sys_nerr
    #else
    extern int  sys_nerr;
    #endif

Note that if the following line:

    extern int  sys_nerr;

is not guarded from Cygiwn, then one will always get an unresolved linker
error because of a missing " __declspec(dllimport)".

> You should look into src/include/port/win.h to fix this.

How is this file suppose to be used?  When I do a find, I get the
following:

    $ find . -type f | xargs fgrep win.h
    ./backend/port/dynloader/darwin.h:/* $Header:
/home/projects/pgsql/cvsroot/pgsql/src/backend/port/dynloader/darwin.h,v1.3 2000/12/11 00:49:54 tgl Exp $ */ 
    ./backend/port/dynloader/win.c: * see win.h
    ./backend/port/dynloader/win.h: * win.h
    ./backend/port/dynloader/win.h: * win.h,v 1.2 1995/03/17 06:40:18 andrew Exp

Hence, no source file is currently including it.

> >         * src/utils/dllinit.c: Update to be consistent with Cygwin Net Release.
>
> Will this break old releases of Cygwin?

I don't know since I don't have b20.1 or older lying around anymore --
but my guess is yes.  However, without this change PostgreSQL does *not*
build OOTB with the current Cygwin release.

Note that this file is no longer needed with current Cygwin releases.
Cygwin provides its own "DllMain" now.  May be a compromise would be to
have make check the Cygwin version and eliminate dllinit.c from the
build if it is not necessary.  I am willing to submit such a patch if it
was deemed reasonable.

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corporation         Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: Re: Cygwin PostgreSQL postmaster abort problem

From
Peter Eisentraut
Date:
Jason Tishler writes:

> > >         * src/backend/utils/error/elog.c: Add conditional compilation to deal
> > >         with sys_nerr/_sys_nerr inconsistencies.
> > >         * src/backend/utils/error/exc.c: Ditto.
> >
> > This is the wrong place.
>
> Sorry, but I'm not that intimate with the PostgreSQL code so when I saw
> the following in elog.c and exc.c:
>
>     #ifdef __CYGWIN__
>     # define sys_nerr _sys_nerr
>     #endif
>     extern int  sys_nerr;

I guess there were some people before you that weren't that intimate with
the code either.  ;-)

> I thought that it was reasonable to change it to:
>
>     #ifdef __CYGWIN__
>     # define sys_nerr _sys_nerr
>     #else
>     extern int  sys_nerr;
>     #endif

I think if Cygwin's strerror() copes with out-of-range errno's, then we
can just leave of the whole business completely.

> > You should look into src/include/port/win.h to fix this.
>
> How is this file suppose to be used?  When I do a find, I get the
> following:

> Hence, no source file is currently including it.

src/include/port/anything.h is symlinked to src/include/port.h when you
run configure.  port.h is included by config.h, which is included by c.h,
which is included by postgres.h, which is included by just about
everything.  (Note to self:  Are that many levels really necessary?)

> > >         * src/utils/dllinit.c: Update to be consistent with Cygwin Net Release.
> >
> > Will this break old releases of Cygwin?
>
> I don't know since I don't have b20.1 or older lying around anymore --
> but my guess is yes.  However, without this change PostgreSQL does *not*
> build OOTB with the current Cygwin release.

Well, it's not like I really care, as I obviously don't use Cygwin, but
dropping support for old OS version just because new ones came out is not
that cool, unless you can make a really good argument that no one in their
right mind would use that old version anymore.

> Note that this file is no longer needed with current Cygwin releases.
> Cygwin provides its own "DllMain" now.  May be a compromise would be to
> have make check the Cygwin version and eliminate dllinit.c from the
> build if it is not necessary.  I am willing to submit such a patch if it
> was deemed reasonable.

Sounds great to me.  Maybe the CYGWIN_VERSION_API_MAJOR/MINOR symbols
could be used to #ifdef out the entire dllinit.c if not needed?  (A more
"correct" approach would probably be to check for the existance of
DllMain, but I'm not sure if you/we are up to that at this point.)

--
Peter Eisentraut      peter_e@gmx.net       http://yi.org/peter-e/


Re: Re: Cygwin PostgreSQL postmaster abort problem

From
Jason Tishler
Date:
Peter,

On Tue, Jan 02, 2001 at 06:00:26PM +0100, Peter Eisentraut wrote:
> I think if Cygwin's strerror() copes with out-of-range errno's, then we
> can just leave of the whole business completely.

On Wed, Jan 03, 2001 at 08:46:58AM +0000, Pete Forman wrote:
> Peter Eisentraut writes:
>  > I think if Cygwin's strerror() copes with out-of-range errno's,
>  > then we can just leave of the whole business completely.
>
> It does.

Given the above, I'm willing to rip out the sys_nerr stuff in elog.c and
exc.c but I'm afraid of breaking other platforms.  Please advise.

> src/include/port/anything.h is symlinked to src/include/port.h when you
> run configure.  port.h is included by config.h, which is included by c.h,
> which is included by postgres.h, which is included by just about
> everything.  (Note to self:  Are that many levels really necessary?)

I presume that you really meant "os.h" instead of "port.h" above.

> > > >         * src/utils/dllinit.c: Update to be consistent with Cygwin Net Release.
> > >
> > > Will this break old releases of Cygwin?
> >
> > I don't know since I don't have b20.1 or older lying around anymore --
> > but my guess is yes.  However, without this change PostgreSQL does *not*
> > build OOTB with the current Cygwin release.
>
> Well, it's not like I really care, as I obviously don't use Cygwin, but
> dropping support for old OS version just because new ones came out is not
> that cool, unless you can make a really good argument that no one in their
> right mind would use that old version anymore.

I will reiterate that Cygnus (a.k.a Red Hat) has stated that they no
longer support b20.1 anymore.  They are actually seeking out mirrors
with the old releases and requesting that they be deleted.  It will
become harder and harder to find b20.1 soon...

> > Note that this file is no longer needed with current Cygwin releases.
> > Cygwin provides its own "DllMain" now.  May be a compromise would be to
> > have make check the Cygwin version and eliminate dllinit.c from the
> > build if it is not necessary.  I am willing to submit such a patch if it
> > was deemed reasonable.
>
> Sounds great to me.  Maybe the CYGWIN_VERSION_API_MAJOR/MINOR symbols
> could be used to #ifdef out the entire dllinit.c if not needed?  (A more
> "correct" approach would probably be to check for the existance of
> DllMain, but I'm not sure if you/we are up to that at this point.)

Can I assume that dllinit.c is only used by Cygwin (i.e, not by straight
Win32)?  If so, then I can surround the contents with:

    #include <cygwin/version.h>
    #if CYGWIN_VERSION_DLL_MAJOR < 1001
    ...
    #endif /* CYGWIN_VERSION_DLL_MAJOR */

Otherwise, this part of the patch is going to get ugly.

I will resubmit my patch after the above issues are resolved.

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

RE: Re: Cygwin PostgreSQL postmaster abort problem

From
Horák Daniel
Date:
> > > Note that this file is no longer needed with current
> Cygwin releases.
> > > Cygwin provides its own "DllMain" now.  May be a
> compromise would be to
> > > have make check the Cygwin version and eliminate
> dllinit.c from the
> > > build if it is not necessary.  I am willing to submit
> such a patch if it
> > > was deemed reasonable.
> >
> > Sounds great to me.  Maybe the
> CYGWIN_VERSION_API_MAJOR/MINOR symbols
> > could be used to #ifdef out the entire dllinit.c if not
> needed?  (A more
> > "correct" approach would probably be to check for the existance of
> > DllMain, but I'm not sure if you/we are up to that at this point.)
>
> Can I assume that dllinit.c is only used by Cygwin (i.e, not
> by straight

Yes, it is used only by Cygwin.

> Win32)?  If so, then I can surround the contents with:
>
>     #include <cygwin/version.h>
>     #if CYGWIN_VERSION_DLL_MAJOR < 1001
>     ...
>     #endif /* CYGWIN_VERSION_DLL_MAJOR */

I think that better will be to use CYGWIN_VERSION_API_MAJOR and
..._MINOR. These numbers depend on changes in the library that have
influence for other applications.

>
> Otherwise, this part of the patch is going to get ugly.


            Dan

Re: Re: Cygwin PostgreSQL postmaster abort problem

From
Jason Tishler
Date:
Daniel,

On Thu, Jan 04, 2001 at 02:01:46PM +0100, Horák Daniel wrote:
> > Can I assume that dllinit.c is only used by Cygwin ...
>
> Yes, it is used only by Cygwin.

Thanks for the confirmation.

> > Win32)?  If so, then I can surround the contents with:
> >
> >     #include <cygwin/version.h>
> >     #if CYGWIN_VERSION_DLL_MAJOR < 1001
> >     ...
> >     #endif /* CYGWIN_VERSION_DLL_MAJOR */
>
> I think that better will be to use CYGWIN_VERSION_API_MAJOR and
> ..._MINOR. These numbers depend on changes in the library that have
> influence for other applications.

The purpose of the above guard is to include the file contents if the
Cygwin version is less than 1.1.x (i.e. b20.1) and exclude it otherwise.

From cygwin/version.h, we have the following:

    /* We used to use the DLL major/minor to track
       non-backward-compatible interface changes to the API.  Now we
       use an API major/minor number for this purpose. */

If I understand the semantics of CYGWIN_VERSION_API_MAJOR
and CYGWIN_VERSION_API_MINOR correctly, then it is possibly
(although unlikely) for different Cygwin versions to have the same
CYGWIN_VERSION_API_MAJOR and CYGWIN_VERSION_API_MINOR values.  Hence, this
is why I feel that CYGWIN_VERSION_DLL_MAJOR is better suited in this case.
Additionally, I have no idea what combination of CYGWIN_VERSION_API_MAJOR
and CYGWIN_VERSION_API_MINOR corresponds to b20.1.  Do you?

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

RE: Re: Cygwin PostgreSQL postmaster abort problem

From
Horák Daniel
Date:
> > I think that better will be to use CYGWIN_VERSION_API_MAJOR and
> > ..._MINOR. These numbers depend on changes in the library that have
> > influence for other applications.
>
> The purpose of the above guard is to include the file contents if the
> Cygwin version is less than 1.1.x (i.e. b20.1) and exclude it
> otherwise.
>
> From cygwin/version.h, we have the following:
>
>     /* We used to use the DLL major/minor to track
>        non-backward-compatible interface changes to the API.  Now we
>        use an API major/minor number for this purpose. */
>
> If I understand the semantics of CYGWIN_VERSION_API_MAJOR
> and CYGWIN_VERSION_API_MINOR correctly, then it is possibly
> (although unlikely) for different Cygwin versions to have the same
> CYGWIN_VERSION_API_MAJOR and CYGWIN_VERSION_API_MINOR values.
>  Hence, this
> is why I feel that CYGWIN_VERSION_DLL_MAJOR is better suited

we could also use CYGWIN_VERSION_DLL_COMBINED ;-)

> in this case.
> Additionally, I have no idea what combination of
> CYGWIN_VERSION_API_MAJOR
> and CYGWIN_VERSION_API_MINOR corresponds to b20.1.  Do you?

in cygwin/version.h from Cygwin B20.1 I have
API_MAJOR 0
API_MINOR 3

and
DLL_MAJOR 20
DLL_MINOR 1

With "DLL" we will depend on releases of the cygwin library and with
"API" on smaller incompatible changes. But there can be also changes in
the correspoding C library and its headers. So now I think that binding
to DLL version is a better solution, because the cygwin dll is packaged
with the C library.


        Dan

Re: Re: Cygwin PostgreSQL postmaster abort problem

From
Pete Forman
Date:
Jason Tishler writes:
 > Additionally, I have no idea what combination of
 > CYGWIN_VERSION_API_MAJOR and CYGWIN_VERSION_API_MINOR corresponds
 > to b20.1.  Do you?

I see these settings in B20.1

CYGWIN_VERSION_DLL_MAJOR 20
CYGWIN_VERSION_DLL_MINOR 1
CYGWIN_VERSION_API_MAJOR 0
CYGWIN_VERSION_API_MINOR 3

And these in 1.1.4

CYGWIN_VERSION_DLL_MAJOR 1001
CYGWIN_VERSION_DLL_MINOR 4
CYGWIN_VERSION_API_MAJOR 0
CYGWIN_VERSION_API_MINOR 26


However I am unhappy with this approach in general and would prefer
that configure be used to establish what works.
--
Pete Forman                 -./\.- Disclaimer: This post is originated
WesternGeco                   -./\.-  by myself and does not represent
pete.forman@westerngeco.com     -./\.-  opinion of Schlumberger, Baker
http://www.crosswinds.net/~petef  -./\.-  Hughes or their divisions.

Re: Re: Cygwin PostgreSQL postmaster abort problem

From
Jason Tishler
Date:
Daniel,

On Thu, Jan 04, 2001 at 03:49:33PM +0100, Horák Daniel wrote:
> > If I understand the semantics of CYGWIN_VERSION_API_MAJOR
> > and CYGWIN_VERSION_API_MINOR correctly, then it is possibly
> > (although unlikely) for different Cygwin versions to have the same
> > CYGWIN_VERSION_API_MAJOR and CYGWIN_VERSION_API_MINOR values.
> >  Hence, this
> > is why I feel that CYGWIN_VERSION_DLL_MAJOR is better suited
>
> we could also use CYGWIN_VERSION_DLL_COMBINED ;-)

Unfortunately, we cannot :,( -- see the following for details:

    http://www.cygwin.com/ml/cygwin-developers/2001-01/msg00000.html

Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: Re: Cygwin PostgreSQL postmaster abort problem

From
Jason Tishler
Date:
Pete,

On Thu, Jan 04, 2001 at 02:55:25PM +0000, Pete Forman wrote:
> However I am unhappy with this approach in general and would prefer
> that configure be used to establish what works.

I'm not completely enamored with it either.  Can you be more explicit
with your configure proposal?  Do you mean something like having
configure determine the Cygwin version and add/remove references to
dllinit.c to/from the makefiles as appropriate?  If so, then that would
be a lot of work...

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: Re: Cygwin PostgreSQL postmaster abort problem

From
Pete Forman
Date:
Peter Eisentraut writes:
 > I think if Cygwin's strerror() copes with out-of-range errno's,
 > then we can just leave of the whole business completely.

It does.



 > Well, it's not like I really care, as I obviously don't use Cygwin,
 > but dropping support for old OS version just because new ones came
 > out is not that cool, unless you can make a really good argument
 > that no one in their right mind would use that old version anymore.

Old versions of Cygwin can be made to work but it is just more
effort.  A lot of the items to be done are no longer needed.  For
example, if you use B20.1 then you need to install your own copy of
libcrypt.  The current Cygwin has it as standard.

If PostgreSQL 6 users hit known problems, do we not recommend
upgrading to v7 rather than making patches against v6?  The latter can
be done but it is not the preferred choice.
--
Pete Forman                 -./\.- Disclaimer: This post is originated
WesternGeco                   -./\.-  by myself and does not represent
pete.forman@westerngeco.com     -./\.-  opinion of Schlumberger, Baker
http://www.crosswinds.net/~petef  -./\.-  Hughes or their divisions.

Re: Re: Cygwin PostgreSQL postmaster abort problem

From
Jason Tishler
Date:
Tom,

On Fri, Dec 29, 2000 at 11:58:16PM -0500, Tom Lane wrote:
> Jason Tishler <Jason.Tishler@dothill.com> writes:
> > Unfortunately, I found some more Cygwin build issues.  I guess that
> > it
> > is time to make another patch...
>
> No time like the present ;-)
>
> We are pointing for a 7.1beta2 release towards the end of next week.
> If you can send patches in the next few days it'd be very timely...

On Wed, Jan 03, 2001 at 03:34:50PM -0500, Jason Tishler wrote:
> I will resubmit my patch after the above issues are resolved.

Unfortunately, I have not received any more feedback regarding the
Cygwin patch issues.  Should I submit my latest patch anyway?  Did I
miss the 7.1beta2 release?

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: Re: Cygwin PostgreSQL postmaster abort problem

From
Peter Eisentraut
Date:
Jason Tishler writes:

> > Seems like that's heading in the wrong direction.  Isn't there a
> > compiler switch or something we could give to make ALL global vars be
> > automatically marked DLLIMPORT?  That's generally how it works on Unix
> > platforms (for example, on HPUX the -E linker switch makes these symbols
> > available to dynamically linked shlibs).
>
> There is a way and the PostgreSQL build is already taking advantage of
> it:
>
>     dlltool --export-all --output-def postgres.def access/SUBSYS.o ...
>
> However, there is a downside to the above.  Now all functions and global
> variables are exported from postgres.exe -- not just the ones that make
> sense.

That's the same across all platforms though.  "Sense" is something the
user can find out about.

> Unfortunately, the above still does not mitigate the need for marking
> global variables DLLIMPORT in their corresponding header files to
> prevent unresolved linker errors by clients of libpostgres.a.

What's the point of --export-all then?

--
Peter Eisentraut      peter_e@gmx.net       http://yi.org/peter-e/


Re: Cygwin PostgreSQL postmaster abort problem

From
Tom Lane
Date:
Jason Tishler <Jason.Tishler@dothill.com> writes:
>> I will resubmit my patch after the above issues are resolved.

> Unfortunately, I have not received any more feedback regarding the
> Cygwin patch issues.  Should I submit my latest patch anyway?  Did I
> miss the 7.1beta2 release?

I think you've missed the window for the beta3 (nee beta2) release,
but please do send in the patch anyway as soon as you're happy with it.

            regards, tom lane

Re: Re: Cygwin PostgreSQL postmaster abort problem

From
Jason Tishler
Date:
Peter,

On Tue, Jan 09, 2001 at 06:59:54PM +0100, Peter Eisentraut wrote:
> Jason Tishler writes:
> > Unfortunately, the above still does not mitigate the need for marking
> > global variables DLLIMPORT in their corresponding header files to
> > prevent unresolved linker errors by clients of libpostgres.a.
>
> What's the point of --export-all then?

The --export-all option is very helpful for the DLL provider, but
unfortunately nothing analogous exists for the DLL consumer.  Welcome
to the wonderful world of Windows!

Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: Cygwin PostgreSQL postmaster abort problem

From
Jason Tishler
Date:
Tom,

On Tue, Jan 09, 2001 at 01:05:13PM -0500, Tom Lane wrote:
> Jason Tishler <Jason.Tishler@dothill.com> writes:
> >> I will resubmit my patch after the above issues are resolved.
>
> > Unfortunately, I have not received any more feedback regarding the
> > Cygwin patch issues.  Should I submit my latest patch anyway?  Did I
> > miss the 7.1beta2 release?
>
> I think you've missed the window for the beta3 (nee beta2) release,
> but please do send in the patch anyway as soon as you're happy with it.

Will do.  I was happy with it last week -- the issue is will others be
happy with it too...

BTW, is the release schedule documented anywhere?  I tried to find this
information on the web site but have not had any luck.

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: Cygwin PostgreSQL postmaster abort problem

From
Tom Lane
Date:
Jason Tishler <Jason.Tishler@dothill.com> writes:
> BTW, is the release schedule documented anywhere?

Schedule?  There is no schedule.  We'll release when we think it's ready.

Possibly around the end of the month, but don't hold me to that.

            regards, tom lane

Re: Re: Cygwin PostgreSQL postmaster abort problem

From
Peter Eisentraut
Date:
Jason Tishler writes:

> Unfortunately, I have not received any more feedback regarding the
> Cygwin patch issues.  Should I submit my latest patch anyway?  Did I
> miss the 7.1beta2 release?

I've already fixed the sys_nerr and removed -L$(libdir), as per your
previous patch.  I think that leaves you with adding DLLIMPORT at the
appropriate places and doing something about dllinit.c.  I'm not sure what
the outcome of the discussion about the latter was, but I think using the
API version number as a conditional was thought to be feasible.

--
Peter Eisentraut      peter_e@gmx.net       http://yi.org/peter-e/


Cygwin PostgreSQL CVS Patch Question

From
Jason Tishler
Date:
Peter,

On Tue, Jan 09, 2001 at 09:06:10PM +0100, Peter Eisentraut wrote:
> Jason Tishler writes:
>
> > Unfortunately, I have not received any more feedback regarding the
> > Cygwin patch issues.  Should I submit my latest patch anyway?  Did I
> > miss the 7.1beta2 release?
>
> I've already fixed the sys_nerr and removed -L$(libdir), as per your
> previous patch.  I think that leaves you with adding DLLIMPORT at the
> appropriate places and doing something about dllinit.c.  I'm not sure what
> the outcome of the discussion about the latter was, but I think using the
> API version number as a conditional was thought to be feasible.

I'm working against CVS now to provide the remainder of the Cygwin patch.
Why did you remove the following from src/include/port/win.h?

    #if (CYGWIN_VERSION_API_MAJOR >= 0) && (CYGWIN_VERSION_API_MINOR >= 8)
    #define sys_nerr _sys_nerr
    #endif

This definitely breaks Cygwin.

I guess that the discussion continues... :,)

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: Cygwin PostgreSQL CVS Patch Question

From
Tom Lane
Date:
Jason Tishler <Jason.Tishler@dothill.com> writes:
> Why did you remove the following from src/include/port/win.h?
>     #if (CYGWIN_VERSION_API_MAJOR >= 0) && (CYGWIN_VERSION_API_MINOR >= 8)
>     #define sys_nerr _sys_nerr
>     #endif
> This definitely breaks Cygwin.

Er, why should it?  With the code as it stands, configure should decide
that Cygwin doesn't HAVE_SYS_NERR, and so nothing will reference
sys_nerr.

            regards, tom lane

Re: Cygwin PostgreSQL CVS Patch Question

From
Jason Tishler
Date:
On Tue, Jan 09, 2001 at 04:34:57PM -0500, Tom Lane wrote:
> Jason Tishler <Jason.Tishler@dothill.com> writes:
> > Why did you remove the following from src/include/port/win.h?
> >     #if (CYGWIN_VERSION_API_MAJOR >= 0) && (CYGWIN_VERSION_API_MINOR >= 8)
> >     #define sys_nerr _sys_nerr
> >     #endif
> > This definitely breaks Cygwin.
>
> Er, why should it?  With the code as it stands, configure should decide
> that Cygwin doesn't HAVE_SYS_NERR, and so nothing will reference
> sys_nerr.

Unfortunately, the above is not true:

    $ configure
    ...
    checking for sys_nerr... yes
    ...

Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: Re: Cygwin PostgreSQL CVS Patch Question

From
Jason Tishler
Date:
Peter,

On Tue, Jan 09, 2001 at 05:55:39PM -0500, Jason Tishler wrote:
> On Tue, Jan 09, 2001 at 04:34:57PM -0500, Tom Lane wrote:
> > Jason Tishler <Jason.Tishler@dothill.com> writes:
> > > Why did you remove the following from src/include/port/win.h?
> > >     #if (CYGWIN_VERSION_API_MAJOR >= 0) && (CYGWIN_VERSION_API_MINOR >= 8)
> > >     #define sys_nerr _sys_nerr
> > >     #endif
> > > This definitely breaks Cygwin.
> >
> > Er, why should it?  With the code as it stands, configure should decide
> > that Cygwin doesn't HAVE_SYS_NERR, and so nothing will reference
> > sys_nerr.
>
> Unfortunately, the above is not true:
>
>     $ configure
>     ...
>     checking for sys_nerr... yes
>     ...

I have more details as to why the above is occurring.

Consider the following program, j.c, which is derived from the sys_nerr
test program in configure:

    extern int sys_nerr;
    int main() {
    int x = sys_nerr;
    ; return 0; }

Now compile it with the same (relevant) options used by configure:

    $ gcc -O2 -o j j.c

Now compile it without the -O2 option:

    $ gcc -o j j.c
    /tmp/ccf49pHw.o(.text+0xc):j.c: undefined reference to `sys_nerr'
    collect2: ld returned 1 exit status

The problem is gcc -O2 is being too smart, optimizing away the reference
to sys_nerr.

So when configure compiles it's sys_nerr test program it compiles without
errors and returns 0 which fools configure into thinking that Cygwin has
sys_nerr.

Any ideas on what is the best way to solve this problem?

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: Cygwin PostgreSQL CVS Patch Question

From
Tom Lane
Date:
Jason Tishler <Jason.Tishler@dothill.com> writes:
>> Er, why should it?  With the code as it stands, configure should decide
>> that Cygwin doesn't HAVE_SYS_NERR, and so nothing will reference
>> sys_nerr.

> Unfortunately, the above is not true:

>     $ configure
>     ...
>     checking for sys_nerr... yes

That's pretty darn interesting.  The configure check looks bulletproof
to me:

    [AC_TRY_LINK([extern int sys_nerr;],
      [int x = sys_nerr;],

Would you poke into it and figure out how this is succeeding, if there's
not any sys_nerr variable exported from the C library?

            regards, tom lane

Re: Re: Cygwin PostgreSQL CVS Patch Question

From
Tom Lane
Date:
Jason Tishler <Jason.Tishler@dothill.com> writes:
>     extern int sys_nerr;
>     int main() {
>     int x = sys_nerr;
>     ; return 0; }

> The problem is gcc -O2 is being too smart, optimizing away the reference
> to sys_nerr.

Oh, that's nasty.  Try a test program like so:

    extern int sys_nerr;
    int my_nerr;
    int main() {
    my_nerr = sys_nerr;
    return 0; }

I don't think gcc will try to optimize away an assignment to a global
variable.

            regards, tom lane

Re: Re: Cygwin PostgreSQL CVS Patch Question

From
Jason Tishler
Date:
Tom,

On Tue, Jan 09, 2001 at 06:37:31PM -0500, Tom Lane wrote:
> Jason Tishler <Jason.Tishler@dothill.com> writes:
> >     extern int sys_nerr;
> >     int main() {
> >     int x = sys_nerr;
> >     ; return 0; }
>
> > The problem is gcc -O2 is being too smart, optimizing away the reference
> > to sys_nerr.
>
> Oh, that's nasty.  Try a test program like so:
>
>     extern int sys_nerr;
>     int my_nerr;
>     int main() {
>     my_nerr = sys_nerr;
>     return 0; }
>
> I don't think gcc will try to optimize away an assignment to a global
> variable.

You are correct -- the above program fails as expected.

Are you going to change config/c-library.m4 to generate the above
program instead?

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: Re: Cygwin PostgreSQL CVS Patch Question

From
Tom Lane
Date:
Jason Tishler <Jason.Tishler@dothill.com> writes:
>> I don't think gcc will try to optimize away an assignment to a global
>> variable.

> You are correct -- the above program fails as expected.

Oh, good.

> Are you going to change config/c-library.m4 to generate the above
> program instead?

This is Peter's turf, but I will take care of it if he doesn't get to it
soon.  In the meantime, please work up a final Cygwin patch on the
assumption that configure will be fixed as above.

            regards, tom lane

Re: Re: Cygwin PostgreSQL CVS Patch Question

From
Jason Tishler
Date:
Tom,

On Tue, Jan 09, 2001 at 11:39:07PM -0500, Tom Lane wrote:
> This is Peter's turf, but I will take care of it if he doesn't get to it
> soon.  In the meantime, please work up a final Cygwin patch on the
> assumption that configure will be fixed as above.

Will do.

I can develop and test the patch by just hand editing config.h to #undef
HAVE_SYS_NERR after configure has been run.

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Cygwin PostgreSQL CVS Patch

From
Jason Tishler
Date:
Tom,

On Tue, Jan 09, 2001 at 11:39:07PM -0500, Tom Lane wrote:
> This is Peter's turf, but I will take care of it if he doesn't get to it
> soon.  In the meantime, please work up a final Cygwin patch on the
> assumption that configure will be fixed as above.

See attached for ChangeLog and patch.  The instructions to apply the
patch is as follows:

    $ cd pgsql # top of CVS working directory
    $ # save Cygwin-1.1.7-PostgreSQL-CVS-2.patch to current directory
    $ patch -p0 <Cygwin-1.1.7-PostgreSQL-CVS-2.patch

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Attachment

Re: Cygwin PostgreSQL CVS Patch

From
Tom Lane
Date:
Peter, any remaining objections, or does this look OK now?

            regards, tom lane

Re: Cygwin PostgreSQL CVS Patch

From
Peter Eisentraut
Date:
Jason Tishler writes:

> Tom,
>
> On Tue, Jan 09, 2001 at 11:39:07PM -0500, Tom Lane wrote:
> > This is Peter's turf, but I will take care of it if he doesn't get to it
> > soon.  In the meantime, please work up a final Cygwin patch on the
> > assumption that configure will be fixed as above.
>
> See attached for ChangeLog and patch.  The instructions to apply the
> patch is as follows:

The Makefile.shlib patch looks like a hack to me to get the regression
tests to work.  Surely you don't have to install *all* shared libraries
into bindir *and* libdir.  Pick one or fix the regression test driver or
do something else.

--
Peter Eisentraut      peter_e@gmx.net       http://yi.org/peter-e/


Re: Cygwin PostgreSQL CVS Patch

From
Bruce Momjian
Date:
Though I saw no email report, it seems this patch has been applied.

> Tom,
>
> On Tue, Jan 09, 2001 at 11:39:07PM -0500, Tom Lane wrote:
> > This is Peter's turf, but I will take care of it if he doesn't get to it
> > soon.  In the meantime, please work up a final Cygwin patch on the
> > assumption that configure will be fixed as above.
>
> See attached for ChangeLog and patch.  The instructions to apply the
> patch is as follows:
>
>     $ cd pgsql # top of CVS working directory
>     $ # save Cygwin-1.1.7-PostgreSQL-CVS-2.patch to current directory
>     $ patch -p0 <Cygwin-1.1.7-PostgreSQL-CVS-2.patch
>
> Thanks,
> Jason
>
> --
> Jason Tishler
> Director, Software Engineering       Phone: +1 (732) 264-8770 x235
> Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
> 82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
> Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

[ Attachment, skipping... ]

[ Attachment, skipping... ]


--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: Cygwin PostgreSQL CVS Patch

From
Jason Tishler
Date:
Peter,

On Fri, Jan 12, 2001 at 12:54:07AM +0100, Peter Eisentraut wrote:
> > On Tue, Jan 09, 2001 at 11:39:07PM -0500, Tom Lane wrote:
> > > This is Peter's turf, but I will take care of it if he doesn't get to it
> > > soon.  In the meantime, please work up a final Cygwin patch on the
> > > assumption that configure will be fixed as above.
> >
> > See attached for ChangeLog and patch.  The instructions to apply the
> > patch is as follows:
>
> The Makefile.shlib patch looks like a hack to me to get the regression
> tests to work.  Surely you don't have to install *all* shared libraries
> into bindir *and* libdir.  Pick one or fix the regression test driver or
> do something else.

My reasons for installing the "shared" libraries, DLLs under Cygwin,
into bindir and libdir are as follows:

    1. All of the DLLs except for plpgsql.dll need to be in the users
    PATH, otherwise Windows cannot start applications that depend on them.
    The canonical example is the regression test (really psql).  However,
    any app that is dependent on ecpg.dll, pgeasy.dll, pq.dll, or pq++.dll
    will have this problem too.  The only exception is plpgsql.dll which
    needs to be in LD_LIBRARY_PATH since it is dlopen-ed into
    postgres.exe, if appropriate.

    2. I would prefer to use symlinks but that would require the real
    files to be installed in bindir and symlinked back to libdir because
    Windows does *not* understand Cygwin symlinks.  Doing so would have
    caused more significant changes to the makefiles which I don't believe
    is desired for just one platform.

    3. I continued to install the DLLs to libdir because I was concerned
    that postgres.exe (or other executables) might be dlopen-ing them.
    If so, then LD_LIBRARY_PATH would have to be set to a different
    value on Cygwin than on other UNIX systems.

The only downside that I can see from installing to both bindir and
libdir is the possibility of the copies from getting out of sync and
wasted disk space.  The wasted disk space is really not an issue since
the total size of the five stripped DLLs on 7.0.3 is ~250K.

I admit that there is a "hacky" aspect to the Makefile.shlib patch.  I am
lucky that plpgsql.dll doesn't get installed to both bindir and libdir.
This is a desired side effect due to plpgsql.dll's install rule overriding
the standard one in Makefile.shlib.

Given more insight into my reasoning, will you reconsider and accept my
Makefile.shlib patch?

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: Cygwin PostgreSQL CVS Patch

From
Peter Eisentraut
Date:
Jason Tishler writes:

> My reasons for installing the "shared" libraries, DLLs under Cygwin,
> into bindir and libdir are as follows:
>
>     1. All of the DLLs except for plpgsql.dll need to be in the users
>     PATH, otherwise Windows cannot start applications that depend on them.

Okay, so you set your PATH to include libdir.  No problem there.

>     The canonical example is the regression test (really psql).

Okay, so we could change the regression test driver to set a PATH that
includes libdir.  No problem there.

>     However,
>     any app that is dependent on ecpg.dll, pgeasy.dll, pq.dll, or pq++.dll
>     will have this problem too.  The only exception is plpgsql.dll which
>     needs to be in LD_LIBRARY_PATH since it is dlopen-ed into
>     postgres.exe, if appropriate.

So you set your LD_LIBRARY_PATH to include libdir as you always had to.
No problem there.

>     3. I continued to install the DLLs to libdir because I was concerned
>     that postgres.exe (or other executables) might be dlopen-ing them.
>     If so, then LD_LIBRARY_PATH would have to be set to a different
>     value on Cygwin than on other UNIX systems.

On modern Unixes it is no longer necessary to set LD_LIBRARY_PATH at all
(and on those where it is the variable isn't necessarily called that), so
we're looking at platform-specific requirements anyway.

> Given more insight into my reasoning, will you reconsider and accept my
> Makefile.shlib patch?

I'd like to know what libtool does.  Diverging from libtool behaviour will
get us into trouble in the future.

--
Peter Eisentraut      peter_e@gmx.net       http://yi.org/peter-e/


Re: Cygwin PostgreSQL CVS Patch

From
Jason Tishler
Date:
Peter,

On Fri, Jan 12, 2001 at 04:59:17PM +0100, Peter Eisentraut wrote:
> Jason Tishler writes:
> >     The canonical example is the regression test (really psql).
>
> Okay, so we could change the regression test driver to set a PATH that
> includes libdir.  No problem there.

See attached patch for the above.

> > Given more insight into my reasoning, will you reconsider and accept my
> > Makefile.shlib patch?
>
> I'd like to know what libtool does.  Diverging from libtool behavior will
> get us into trouble in the future.

I don't know much about libtool so I can't help you here.  However, I
agree completely with your assessment.

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Attachment

Re: Cygwin PostgreSQL CVS Patch

From
Peter Eisentraut
Date:
Jason Tishler writes:

> > Okay, so we could change the regression test driver to set a PATH that
> > includes libdir.  No problem there.
>
> See attached patch for the above.

Installed.

Another issue you might be interested in is that of Unix domain sockets.
I understand that they now exist in Cygwin, so you might want to refine
this snippet in src/include/config.h[.in]:

/*
 * Define this if your operating system supports AF_UNIX family sockets.
 */
#if !defined(__CYGWIN__) && !defined(__QNX__) && !defined(__BEOS__)
# define HAVE_UNIX_SOCKETS 1
#endif

And take a look at doc/FAQ_MSWIN, if you like, to see if it's up to date.

Many thanks.

--
Peter Eisentraut      peter_e@gmx.net       http://yi.org/peter-e/


Re: Cygwin PostgreSQL CVS Patch

From
Jason Tishler
Date:
Peter,

On Sat, Jan 13, 2001 at 04:37:31AM +0100, Peter Eisentraut wrote:
> Jason Tishler writes:
>
> > > Okay, so we could change the regression test driver to set a PATH that
> > > includes libdir.  No problem there.
> >
> > See attached patch for the above.
>
> Installed.

Thanks.  I meant to surround the Cygwin specific stuff with a case
statement but forgot -- thanks for cleaning up after me.  Sigh...

> Another issue you might be interested in is that of Unix domain sockets.
> I understand that they now exist in Cygwin, so you might want to refine
> this snippet in src/include/config.h[.in]:
>
> /*
>  * Define this if your operating system supports AF_UNIX family sockets.
>  */
> #if !defined(__CYGWIN__) && !defined(__QNX__) && !defined(__BEOS__)
> # define HAVE_UNIX_SOCKETS 1
> #endif
>
> And take a look at doc/FAQ_MSWIN, if you like, to see if it's up to date.

See attached patch and ChangeLog.  This trivial patch enables UNIX
domain sockets for Cygwin.  This version of Cygwin PostgreSQL still
passes all regression tests.  However, there are two issues with
Cygwin's support of UNIX domain sockets:

    1. psql (and other clients) with hang if postmaster is not running
       and the socket file (e.g., /tmp/.s.PGSQL.5432) exists
    2. Cygwin's AF_UNIX sockets are really implemented as AF_INET
    sockets so they are inherently insecure.  See the follow for more
    details:

       http://sources.redhat.com/ml/cygwin/2000-12/msg01058.html

The procedure to apply the patch is as follows:

    $ cd pgsql
    $ # save attached patch to current directory
    $ patch -p0 <af_unix.patch

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Attachment

Re: Cygwin PostgreSQL CVS Patch

From
Peter Eisentraut
Date:
Jason Tishler writes:

> diff -u -r1.155 config.h.in
> --- src/include/config.h.in     2001/01/09 18:40:15     1.155
> +++ src/include/config.h.in     2001/01/16 14:20:10
> @@ -228,7 +228,7 @@
>  /*
>   * Define this if your operating system supports AF_UNIX family sockets.
>   */
> -#if !defined(__CYGWIN__) && !defined(__QNX__) && !defined(__BEOS__)
> +#if !defined(__QNX__) && !defined(__BEOS__)
>  # define HAVE_UNIX_SOCKETS 1
>  #endif

I could have figured as much ;-) but after we went through the trouble of
coping with old Cygwin's in dllinit.c we should do the same here.  The
question is again how to do that.

--
Peter Eisentraut      peter_e@gmx.net       http://yi.org/peter-e/


Re: Re: Cygwin PostgreSQL CVS Patch

From
Bruce Momjian
Date:
Can someone tell me the status of this patch?

> Peter,
>
> On Sat, Jan 13, 2001 at 04:37:31AM +0100, Peter Eisentraut wrote:
> > Jason Tishler writes:
> >
> > > > Okay, so we could change the regression test driver to set a PATH that
> > > > includes libdir.  No problem there.
> > >
> > > See attached patch for the above.
> >
> > Installed.
>
> Thanks.  I meant to surround the Cygwin specific stuff with a case
> statement but forgot -- thanks for cleaning up after me.  Sigh...
>
> > Another issue you might be interested in is that of Unix domain sockets.
> > I understand that they now exist in Cygwin, so you might want to refine
> > this snippet in src/include/config.h[.in]:
> >
> > /*
> >  * Define this if your operating system supports AF_UNIX family sockets.
> >  */
> > #if !defined(__CYGWIN__) && !defined(__QNX__) && !defined(__BEOS__)
> > # define HAVE_UNIX_SOCKETS 1
> > #endif
> >
> > And take a look at doc/FAQ_MSWIN, if you like, to see if it's up to date.
>
> See attached patch and ChangeLog.  This trivial patch enables UNIX
> domain sockets for Cygwin.  This version of Cygwin PostgreSQL still
> passes all regression tests.  However, there are two issues with
> Cygwin's support of UNIX domain sockets:
>
>     1. psql (and other clients) with hang if postmaster is not running
>        and the socket file (e.g., /tmp/.s.PGSQL.5432) exists
>     2. Cygwin's AF_UNIX sockets are really implemented as AF_INET
>     sockets so they are inherently insecure.  See the follow for more
>     details:
>
>        http://sources.redhat.com/ml/cygwin/2000-12/msg01058.html
>
> The procedure to apply the patch is as follows:
>
>     $ cd pgsql
>     $ # save attached patch to current directory
>     $ patch -p0 <af_unix.patch
>
> Thanks,
> Jason
>
> --
> Jason Tishler
> Director, Software Engineering       Phone: +1 (732) 264-8770 x235
> Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
> 82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
> Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

[ Attachment, skipping... ]

[ Attachment, skipping... ]


--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: Re: Cygwin PostgreSQL CVS Patch

From
Jason Tishler
Date:
Bruce,

On Wed, Jan 17, 2001 at 05:08:18PM -0500, Bruce Momjian wrote:
> Can someone tell me the status of this patch?

I believe all but the src/include/config.h.in part is acceptable to
Peter, but I would rather not speak for him.

Anyway, I have posted the following to the Cygwin list:

    http://cygwin.com/ml/cygwin/2001-01/msg00844.html

Unfortunately, I have not received any replies yet.

Assuming that I don't get any feedback, I am going to submit a patch
based on the above tomorrow morning.

Jason


--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: Cygwin PostgreSQL CVS Patch

From
Tom Lane
Date:
Jason Tishler <Jason.Tishler@dothill.com> writes:
> 1. Add the following cruft right before the above:

> #ifdef __CYGWIN__
> #include <cygwin/version.h>
> #endif

> 2. Move the including of os.h to somewhere before the above.

I don't like #2 since there may be (now or in the future) port header
files that depend on being able to override config.h choices.

A third alternative is to remove all mention of cygwin from the
test in config.h, allowing it to #define HAVE_UNIX_SOCKETS always,
and then in the cygwin os.h #undef it again if it's old cygwin.

            regards, tom lane

Re: Cygwin PostgreSQL CVS Patch

From
Jason Tishler
Date:
On Tue, Jan 16, 2001 at 04:49:18PM +0100, Peter Eisentraut wrote:
> Jason Tishler writes:
>
> > diff -u -r1.155 config.h.in
> > --- src/include/config.h.in     2001/01/09 18:40:15     1.155
> > +++ src/include/config.h.in     2001/01/16 14:20:10
> > @@ -228,7 +228,7 @@
> >  /*
> >   * Define this if your operating system supports AF_UNIX family sockets.
> >   */
> > -#if !defined(__CYGWIN__) && !defined(__QNX__) && !defined(__BEOS__)
> > +#if !defined(__QNX__) && !defined(__BEOS__)
> >  # define HAVE_UNIX_SOCKETS 1
> >  #endif
>
> I could have figured as much ;-) but after we went through the trouble of
> coping with old Cygwin's in dllinit.c we should do the same here.  The
> question is again how to do that.

After seeing that config.h.in already included os.h, I thought that the
following was a reasonable way to improved the above to deal with b20.1
and the Net Release:

#if !(defined (__CYGWIN__) && CYGWIN_VERSION_DLL_MAJOR < 1001) && \
    !defined(__QNX__) && !defined(__BEOS__)
# define HAVE_UNIX_SOCKETS 1
#endif

Unfortunately I didn't realize that os.h is included *after* the
AF_UNIX test, so the above will not work until cygwin/version.h is
included prior the test.

As I see it, I have two choices:

1. Add the following cruft right before the above:

#ifdef __CYGWIN__
#include <cygwin/version.h>
#endif

2. Move the including of os.h to somewhere before the above.

Any suggestions or guidance would be appreciated.

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: Cygwin PostgreSQL CVS Patch

From
Jason Tishler
Date:
Tom,

On Thu, Jan 18, 2001 at 01:03:18PM -0500, Tom Lane wrote:
> A third alternative is to remove all mention of cygwin from the
> test in config.h, allowing it to #define HAVE_UNIX_SOCKETS always,
> and then in the cygwin os.h #undef it again if it's old cygwin.

I like this approach best and I am going forward with it.

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Re: Cygwin PostgreSQL CVS Patch

From
Jason Tishler
Date:
Peter,

On Tue, Jan 16, 2001 at 04:49:18PM +0100, Peter Eisentraut wrote:
> I could have figured as much ;-) but after we went through the trouble of
> coping with old Cygwin's in dllinit.c we should do the same here.  The
> question is again how to do that.

See attached for another attempt.  The only difference is the addition
of a src/include/port/win.h patch.

Thanks,
Jason

--
Jason Tishler
Director, Software Engineering       Phone: +1 (732) 264-8770 x235
Dot Hill Systems Corp.               Fax:   +1 (732) 264-8798
82 Bethany Road, Suite 7             Email: Jason.Tishler@dothill.com
Hazlet, NJ 07730 USA                 WWW:   http://www.dothill.com

Attachment

Re: Cygwin PostgreSQL CVS Patch

From
Peter Eisentraut
Date:
Jason Tishler writes:

> > I could have figured as much ;-) but after we went through the trouble of
> > coping with old Cygwin's in dllinit.c we should do the same here.  The
> > question is again how to do that.
>
> See attached for another attempt.  The only difference is the addition
> of a src/include/port/win.h patch.

Patch installed.

--
Peter Eisentraut      peter_e@gmx.net       http://yi.org/peter-e/