Win32 defines

Bruce Momjian <pgman@candle.pha.pa.us> writes:

This adds some Win32 defines to cygwin defines --- this isn't all of
them, but it is most.

You got at least some of these backwards, eg

! #ifndef __CYGWIN__

! #if defined(__CYGWIN__) || defined(WIN32)

Also, are you sure that it's really appropriate to handle the native
port like Cygwin in each of these cases? Maybe so, I didn't look...

regards, tom lane

Bruce Momjian writes:

This adds some Win32 defines to cygwin defines --- this isn't all of
them, but it is most.

Index: src/backend/access/transam/xlog.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/access/transam/xlog.c,v
retrieving revision 1.112
diff -c -c -r1.112 xlog.c
*** src/backend/access/transam/xlog.c	21 Feb 2003 00:06:22 -0000	1.112
--- src/backend/access/transam/xlog.c	3 Apr 2003 06:09:26 -0000
***************
*** 1507,1513 ****
* overwrite an existing logfile.  However, there shouldn't be one, so
* rename() is an acceptable substitute except for the truly paranoid.
*/
! #if !defined(__BEOS__) && !defined(N_PLAT_NLM) && !defined(__CYGWIN__)
if (link(tmppath, path) < 0)
elog(PANIC, "link from %s to %s (initialization of log file %u, segment %u) failed: %m",
tmppath, path, log, seg);
--- 1507,1513 ----
* overwrite an existing logfile.  However, there shouldn't be one, so
* rename() is an acceptable substitute except for the truly paranoid.
*/
! #if !defined(__BEOS__) && !defined(N_PLAT_NLM) && !defined(__CYGWIN__) && !defined(WIN32)
if (link(tmppath, path) < 0)
elog(PANIC, "link from %s to %s (initialization of log file %u, segment %u) failed: %m",
tmppath, path, log, seg);

That list is getting pretty long. This should be replaced by something
like HAVE_WORKING_LINK and be handled liked HAVE_UNIX_SOCKETS.

Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/postmaster/postmaster.c,v
retrieving revision 1.309
diff -c -c -r1.309 postmaster.c
*** src/backend/postmaster/postmaster.c	24 Mar 2003 22:40:14 -0000	1.309
--- src/backend/postmaster/postmaster.c	3 Apr 2003 06:09:32 -0000

***************
*** 323,330 ****
* be proper support for Unix-y file permissions.  Need to think of a
* reasonable check to apply on Windows.
*/
! #ifndef __CYGWIN__
!
if (stat(checkdir, &stat_buf) == -1)
{
if (errno == ENOENT)
--- 323,329 ----
* be proper support for Unix-y file permissions.  Need to think of a
* reasonable check to apply on Windows.
*/
! #if !defined(__CYGWIN__) && !defined(WIN32)
if (stat(checkdir, &stat_buf) == -1)
{
if (errno == ENOENT)

The comment tells the tale. What is a reasonable check to apply to
Windows here?

Index: src/include/c.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/c.h,v
retrieving revision 1.135
diff -c -c -r1.135 c.h
*** src/include/c.h	9 Jan 2003 18:00:24 -0000	1.135
--- src/include/c.h	3 Apr 2003 06:09:40 -0000
***************
*** 63,69 ****
#endif
#include <sys/types.h>

! #ifdef __CYGWIN__
#include <errno.h>
#include <sys/fcntl.h>			/* ensure O_BINARY is available */
#endif
--- 63,69 ----
#endif
#include <sys/types.h>

! #if defined(__CYGWIN__) || defined(WIN32)
#include <errno.h>
#include <sys/fcntl.h> /* ensure O_BINARY is available */
#endif

I think this should be killed and <errno.h> and <fcntl.h> always included.

Index: src/include/utils/datetime.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/utils/datetime.h,v
retrieving revision 1.36
diff -c -c -r1.36 datetime.h
*** src/include/utils/datetime.h	20 Feb 2003 05:24:55 -0000	1.36
--- src/include/utils/datetime.h	3 Apr 2003 06:09:41 -0000
***************
*** 217,223 ****
#endif

/* Global variable holding time zone information. */
! #if defined(__CYGWIN__) || defined(N_PLAT_NLM)
#define TIMEZONE_GLOBAL _timezone
#else
#define TIMEZONE_GLOBAL timezone
--- 217,223 ----
#endif

/* Global variable holding time zone information. */
! #if defined(__CYGWIN__) || defined(WIN32) || defined(N_PLAT_NLM)
#define TIMEZONE_GLOBAL _timezone
#else
#define TIMEZONE_GLOBAL timezone

We should move that to the port-specific include files, like

#define timezone _timezone

Index: src/interfaces/ecpg/pgtypeslib/dt.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/interfaces/ecpg/pgtypeslib/dt.h,v
retrieving revision 1.3
diff -c -c -r1.3 dt.h
*** src/interfaces/ecpg/pgtypeslib/dt.h	1 Apr 2003 14:37:25 -0000	1.3
--- src/interfaces/ecpg/pgtypeslib/dt.h	3 Apr 2003 06:09:41 -0000

(same here)

Index: src/tools/entab/entab.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/tools/entab/entab.c,v
retrieving revision 1.11
diff -c -c -r1.11 entab.c
*** src/tools/entab/entab.c	9 Jan 2002 18:21:46 -0000	1.11
--- src/tools/entab/entab.c	3 Apr 2003 06:09:44 -0000
***************
*** 94,100 ****
in_file = stdin;
else
{
! #ifndef __CYGWIN__
if ((in_file = fopen(*argv, "r")) == NULL)
#else
if ((in_file = fopen(*argv, "rb")) == NULL)
--- 94,100 ----
in_file = stdin;
else
{
! #if defined(__CYGWIN__) || defined(WIN32)
if ((in_file = fopen(*argv, "r")) == NULL)
#else
if ((in_file = fopen(*argv, "rb")) == NULL)

This should use the macros defined in c.h.

--
Peter Eisentraut peter_e@gmx.net

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

This adds some Win32 defines to cygwin defines --- this isn't all of
them, but it is most.

You got at least some of these backwards, eg

! #ifndef __CYGWIN__

! #if defined(__CYGWIN__) || defined(WIN32)

Thanks. Fixed. I reviewed them all, but hacked at this one after
checking.

Also, are you sure that it's really appropriate to handle the native
port like Cygwin in each of these cases? Maybe so, I didn't look...

I am goiing to back off some of these until it is compiled on Win32 ---
I am not positive if some of these are cygwin-specific changes and which
are Win32 also.

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

On Fri, 2003-04-04 at 15:48, Bruce Momjian wrote:

I am goiing to back off some of these until it is compiled on Win32 ---
I am not positive if some of these are cygwin-specific changes and which
are Win32 also.

If we finish the native Win32 port, is there any need to keep the cygwin
stuff around?

Cheers,

Neil

Neil Conway wrote:

On Fri, 2003-04-04 at 15:48, Bruce Momjian wrote:

I am goiing to back off some of these until it is compiled on Win32 ---
I am not positive if some of these are cygwin-specific changes and which
are Win32 also.

If we finish the native Win32 port, is there any need to keep the cygwin
stuff around?

They do ship PostgreSQL with cygwin, so we may need to keep it around,
at least for a few releases, and it isn't that big a port.

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

On Sat, Apr 05, 2003 at 12:38:51PM -0500, Bruce Momjian wrote:

Neil Conway wrote:

If we finish the native Win32 port, is there any need to keep the cygwin
stuff around?

They do ship PostgreSQL with cygwin, so we may need to keep it around,
at least for a few releases, and it isn't that big a port.

Surely there's no need for a less performant, less reliable Cygwin port
when a native Win32 one is available? If they ship it now, they
probably won't need to later when the Win32 port is finished. Cygwin is
already said to be "experimental" or non-commercial quality, AFAIR.

Not that I care though...

--
Alvaro Herrera (<alvherre[a]dcc.uchile.cl>)
Y una voz del caos me hablo y me dijo
"Sonrie y se feliz, podria ser peor".
Y sonrei. Y fui feliz.
Y fue peor.

Alvaro Herrera wrote:

On Sat, Apr 05, 2003 at 12:38:51PM -0500, Bruce Momjian wrote:

Neil Conway wrote:

If we finish the native Win32 port, is there any need to keep the cygwin
stuff around?

They do ship PostgreSQL with cygwin, so we may need to keep it around,
at least for a few releases, and it isn't that big a port.

Surely there's no need for a less performant, less reliable Cygwin port
when a native Win32 one is available? If they ship it now, they
probably won't need to later when the Win32 port is finished. Cygwin is
already said to be "experimental" or non-commercial quality, AFAIR.

Not that I care though...

I can remove it anytime people want it removed --- maybe once we have
Win32 stabalized and working 100%, we can remove it.

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

On Sat, 2003-04-05 at 12:53, Bruce Momjian wrote:

Alvaro Herrera wrote:

On Sat, Apr 05, 2003 at 12:38:51PM -0500, Bruce Momjian wrote:

Neil Conway wrote:

If we finish the native Win32 port, is there any need to keep the cygwin
stuff around?

They do ship PostgreSQL with cygwin, so we may need to keep it around,
at least for a few releases, and it isn't that big a port.

Surely there's no need for a less performant, less reliable Cygwin port
when a native Win32 one is available? If they ship it now, they
probably won't need to later when the Win32 port is finished. Cygwin is
already said to be "experimental" or non-commercial quality, AFAIR.

Not that I care though...

I can remove it anytime people want it removed --- maybe once we have
Win32 stabalized and working 100%, we can remove it.

Even wine has the occasional update to ensure it works on Cygwin -- so
I'm sure there are a number of good reasons that we just don't know yet.

--
Rod Taylor <rbt@rbt.ca>

PGP Key: http://www.rbt.ca/rbtpub.asc

Neil Conway writes:

If we finish the native Win32 port, is there any need to keep the cygwin
stuff around?

Yes, because some people use it.

Why, you ask? For the same reason they are using Cygwin in the first
place.

--
Peter Eisentraut peter_e@gmx.net

OK, I have modified the patch to match your suggestions. I also removed
the N_PLAT_NLM define because I think that just got in when we were
thinking about the Novell port that we cancelled. Is that correct?

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

Peter Eisentraut wrote:

Bruce Momjian writes:

This adds some Win32 defines to cygwin defines --- this isn't all of
them, but it is most.

Index: src/backend/access/transam/xlog.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/access/transam/xlog.c,v
retrieving revision 1.112
diff -c -c -r1.112 xlog.c
*** src/backend/access/transam/xlog.c	21 Feb 2003 00:06:22 -0000	1.112
--- src/backend/access/transam/xlog.c	3 Apr 2003 06:09:26 -0000
***************
*** 1507,1513 ****
* overwrite an existing logfile.  However, there shouldn't be one, so
* rename() is an acceptable substitute except for the truly paranoid.
*/
! #if !defined(__BEOS__) && !defined(N_PLAT_NLM) && !defined(__CYGWIN__)
if (link(tmppath, path) < 0)
elog(PANIC, "link from %s to %s (initialization of log file %u, segment %u) failed: %m",
tmppath, path, log, seg);
--- 1507,1513 ----
* overwrite an existing logfile.  However, there shouldn't be one, so
* rename() is an acceptable substitute except for the truly paranoid.
*/
! #if !defined(__BEOS__) && !defined(N_PLAT_NLM) && !defined(__CYGWIN__) && !defined(WIN32)
if (link(tmppath, path) < 0)
elog(PANIC, "link from %s to %s (initialization of log file %u, segment %u) failed: %m",
tmppath, path, log, seg);

That list is getting pretty long. This should be replaced by something
like HAVE_WORKING_LINK and be handled liked HAVE_UNIX_SOCKETS.

Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/postmaster/postmaster.c,v
retrieving revision 1.309
diff -c -c -r1.309 postmaster.c
*** src/backend/postmaster/postmaster.c	24 Mar 2003 22:40:14 -0000	1.309
--- src/backend/postmaster/postmaster.c	3 Apr 2003 06:09:32 -0000

***************
*** 323,330 ****
* be proper support for Unix-y file permissions.  Need to think of a
* reasonable check to apply on Windows.
*/
! #ifndef __CYGWIN__
!
if (stat(checkdir, &stat_buf) == -1)
{
if (errno == ENOENT)
--- 323,329 ----
* be proper support for Unix-y file permissions.  Need to think of a
* reasonable check to apply on Windows.
*/
! #if !defined(__CYGWIN__) && !defined(WIN32)
if (stat(checkdir, &stat_buf) == -1)
{
if (errno == ENOENT)

The comment tells the tale. What is a reasonable check to apply to
Windows here?

Index: src/include/c.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/c.h,v
retrieving revision 1.135
diff -c -c -r1.135 c.h
*** src/include/c.h	9 Jan 2003 18:00:24 -0000	1.135
--- src/include/c.h	3 Apr 2003 06:09:40 -0000
***************
*** 63,69 ****
#endif
#include <sys/types.h>

! #ifdef __CYGWIN__
#include <errno.h>
#include <sys/fcntl.h>			/* ensure O_BINARY is available */
#endif
--- 63,69 ----
#endif
#include <sys/types.h>

! #if defined(__CYGWIN__) || defined(WIN32)
#include <errno.h>
#include <sys/fcntl.h> /* ensure O_BINARY is available */
#endif

I think this should be killed and <errno.h> and <fcntl.h> always included.

Index: src/include/utils/datetime.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/utils/datetime.h,v
retrieving revision 1.36
diff -c -c -r1.36 datetime.h
*** src/include/utils/datetime.h	20 Feb 2003 05:24:55 -0000	1.36
--- src/include/utils/datetime.h	3 Apr 2003 06:09:41 -0000
***************
*** 217,223 ****
#endif

/* Global variable holding time zone information. */
! #if defined(__CYGWIN__) || defined(N_PLAT_NLM)
#define TIMEZONE_GLOBAL _timezone
#else
#define TIMEZONE_GLOBAL timezone
--- 217,223 ----
#endif

/* Global variable holding time zone information. */
! #if defined(__CYGWIN__) || defined(WIN32) || defined(N_PLAT_NLM)
#define TIMEZONE_GLOBAL _timezone
#else
#define TIMEZONE_GLOBAL timezone

We should move that to the port-specific include files, like

#define timezone _timezone

Index: src/interfaces/ecpg/pgtypeslib/dt.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/interfaces/ecpg/pgtypeslib/dt.h,v
retrieving revision 1.3
diff -c -c -r1.3 dt.h
*** src/interfaces/ecpg/pgtypeslib/dt.h	1 Apr 2003 14:37:25 -0000	1.3
--- src/interfaces/ecpg/pgtypeslib/dt.h	3 Apr 2003 06:09:41 -0000

(same here)

Index: src/tools/entab/entab.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/tools/entab/entab.c,v
retrieving revision 1.11
diff -c -c -r1.11 entab.c
*** src/tools/entab/entab.c	9 Jan 2002 18:21:46 -0000	1.11
--- src/tools/entab/entab.c	3 Apr 2003 06:09:44 -0000
***************
*** 94,100 ****
in_file = stdin;
else
{
! #ifndef __CYGWIN__
if ((in_file = fopen(*argv, "r")) == NULL)
#else
if ((in_file = fopen(*argv, "rb")) == NULL)
--- 94,100 ----
in_file = stdin;
else
{
! #if defined(__CYGWIN__) || defined(WIN32)
if ((in_file = fopen(*argv, "r")) == NULL)
#else
if ((in_file = fopen(*argv, "rb")) == NULL)

This should use the macros defined in c.h.

--
Peter Eisentraut peter_e@gmx.net

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

Why is this removing the use of the TIMEZONE_GLOBAL macro?

regards, tom lane

Tom Lane wrote:

Why is this removing the use of the TIMEZONE_GLOBAL macro?

It was Peter's suggestion. Once we had a port-specific define, we
should use timezone directly, no?

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

Index: src/include/utils/datetime.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/utils/datetime.h,v
retrieving revision 1.36
diff -c -c -r1.36 datetime.h
*** src/include/utils/datetime.h      20 Feb 2003 05:24:55 -0000      1.36
--- src/include/utils/datetime.h      3 Apr 2003 06:09:41 -0000
***************
*** 217,223 ****
#endif

/* Global variable holding time zone information. */
! #if defined(__CYGWIN__) || defined(N_PLAT_NLM)
#define TIMEZONE_GLOBAL _timezone
#else
#define TIMEZONE_GLOBAL timezone
--- 217,223 ----
#endif

/* Global variable holding time zone information. */
! #if defined(__CYGWIN__) || defined(WIN32) || defined(N_PLAT_NLM)
#define TIMEZONE_GLOBAL _timezone
#else
#define TIMEZONE_GLOBAL timezone

We should move that to the port-specific include files, like

#define timezone _timezone

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

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Tom Lane wrote:

Why is this removing the use of the TIMEZONE_GLOBAL macro?

It was Peter's suggestion. Once we had a port-specific define, we
should use timezone directly, no?

#define timezone _timezone

Oh, I missed that part. A bit risky IMHO --- the macro might expand in
places you didn't expect/want. TIMEZONE_GLOBAL is unlikely to cause
any such conflict ...

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Tom Lane wrote:

Why is this removing the use of the TIMEZONE_GLOBAL macro?

It was Peter's suggestion. Once we had a port-specific define, we
should use timezone directly, no?

#define timezone _timezone

Oh, I missed that part. A bit risky IMHO --- the macro might expand in
places you didn't expect/want. TIMEZONE_GLOBAL is unlikely to cause
any such conflict ...

Agreed. A 'timezone' macro will expand as a structure member, while a
timezone variable will not. Here is a new version that defines a
variable timezone to equal _timezone. This seems cleaner, and removes
the need for TIMEZONE_GLOBAL.

I am still researching the stat() handling in postmaster.c. SRA has a
porting document that I translated from Japanese (via Babelfish) which
looks very helpful.

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

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Here is a new version that defines a
variable timezone to equal _timezone.

Surely that will not work. It won't even compile, let alone track
run-time changes in _timezone.

regards, tom lane

OK, here is the newest version of the patch, with a new file for Win32
macros.

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

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Tom Lane wrote:

Why is this removing the use of the TIMEZONE_GLOBAL macro?

It was Peter's suggestion. Once we had a port-specific define, we
should use timezone directly, no?

#define timezone _timezone

Oh, I missed that part. A bit risky IMHO --- the macro might expand in
places you didn't expect/want. TIMEZONE_GLOBAL is unlikely to cause
any such conflict ...

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 5: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faqs/FAQ.html

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

Bruce Momjian <pgman@candle.pha.pa.us> writes:

[ patch removes #ifndef CYGWIN around permissions checks ]

Has Windows' filesystem gotten better since we put those #ifdefs in
there? IIRC, they're there because PG refused to boot on Windows
without 'em.

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

[ patch removes #ifndef CYGWIN around permissions checks ]

Has Windows' filesystem gotten better since we put those #ifdefs in
there? IIRC, they're there because PG refused to boot on Windows
without 'em.

Is this the check you are asking about:

+ #if !defined(__CYGWIN__) && !defined(WIN32)
if (stat_buf.st_mode & (S_IRWXG | S_IRWXO))
elog(FATAL, "data directory %s has group or world access; permissions should be u=rwx (0700)",
checkdir);
! #endif

The check is still there, but it does the stat() and checks the return
code first, then skips the group/other checks.

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

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

[ patch removes #ifndef CYGWIN around permissions checks ]

Has Windows' filesystem gotten better since we put those #ifdefs in
there? IIRC, they're there because PG refused to boot on Windows
without 'em.

Actually, it would compile with the new win32defs.h file, but it would
be useless, so I skip it.

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