MinGW patch

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

+ # We already have this in Makefile.win32, but configure needs it too
+ if test "$PORTNAME" = "win32"
+ then
+ 	CPPFLAGS="$CPPFLAGS -Isrc/include/port/win32"
+ fi

This seems unlikely to work when building outside the source tree.

regards, tom lane

Tom Lane wrote:

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

+ # We already have this in Makefile.win32, but configure needs it too
+ if test "$PORTNAME" = "win32"
+ then
+ 	CPPFLAGS="$CPPFLAGS -Isrc/include/port/win32"
+ fi

This seems unlikely to work when building outside the source tree.

Oh. Any ideas on how to handle that? The other reference uses:

override CPPFLAGS+="-I$(top_srcdir)/src/include/port/win32"

but top_srcdir isn't defined at this point. This is there only so
'configure' sees those include files. Does that relate to building
outside the tree?

-- 
  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:

but top_srcdir isn't defined at this point.

Perhaps $srcdir? Peter would probably know what to use.

regards, tom lane

Bruce Momjian writes:

Oh. Any ideas on how to handle that? The other reference uses:

override CPPFLAGS+="-I$(top_srcdir)/src/include/port/win32"

but top_srcdir isn't defined at this point.

If you put this in the port makefile, then it should work.

This is there only so 'configure' sees those include files. Does that
relate to building outside the tree?

Why does configure need to see those files? Surely you don't need
configure to test the capabilities of our own source tree?

--
Peter Eisentraut peter_e@gmx.net

Bruce Momjian writes:

In mbprint.c:

+ #ifndef PGSCRIPTS
+ #include "settings.h"
+ #endif

Why is this needed, considering that the rest of the code was not changed
to make use of any setting? The print modules in psql should not make use
of psql-specific settings. That's why there are extensive separate
structures to pass this information.

Also, I'm beginning to feel that there are too many places where the lack
of getpwuid() is being worked around. That needs to be centralized.

Lastly, since there is not going to be a Windows port in 7.4, why was this
committed to the head branch?

--
Peter Eisentraut peter_e@gmx.net

Peter Eisentraut <peter_e@gmx.net> writes:

Why does configure need to see those files? Surely you don't need
configure to test the capabilities of our own source tree?

Looking at the contents of src/include/port/win32, I would imagine the
problem is configure will decide we don't HAVE_SYS_SOCKET_H, etc.

regards, tom lane

Peter Eisentraut wrote:

Bruce Momjian writes:

Oh. Any ideas on how to handle that? The other reference uses:

override CPPFLAGS+="-I$(top_srcdir)/src/include/port/win32"

but top_srcdir isn't defined at this point.

If you put this in the port makefile, then it should work.

The problem is that the 'configure' doesn't find the include files
during its tests.

This is there only so 'configure' sees those include files. Does that
relate to building outside the tree?

Why does configure need to see those files? Surely you don't need
configure to test the capabilities of our own source tree?

As Tom mentioned, it doesn't find things like sys/socket.h, which we
have added to src/include/port/win32/. This was done to elimintate
#ifdef of the include files that are Win32-specific.

-- 
  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

Peter Eisentraut wrote:

Bruce Momjian writes:

In mbprint.c:

+ #ifndef PGSCRIPTS
+ #include "settings.h"
+ #endif

Why is this needed, considering that the rest of the code was not changed
to make use of any setting? The print modules in psql should not make use
of psql-specific settings. That's why there are extensive separate
structures to pass this information.

The problem is that mbvalidate() has this code:

if (GetVariableBool(pset.vars, "WIN32_CONSOLE"))
CharToOem(pwcs, pwcs);

That's why I needed the #ifdef so it only tests from psql. This code
was just broken without that test, and because no one else has done
Win32 compiles since it was added, no one saw it.

Also, I'm beginning to feel that there are too many places where the lack
of getpwuid() is being worked around. That needs to be centralized.

Yep. Let me add that to my list for 7.5.

Lastly, since there is not going to be a Windows port in 7.4, why was this
committed to the head branch?

Yes, I knew someone would ask, and debated it myself. Having these
patches in 7.4 allows us to have full-functional interfaces and client
binaries for MinGW. This allows us to show progress on the Win32 port,
and for folks to start using MinGW in client applications. Also, many
of these fixes will be needed for Cygwin and other Win32 platforms, so
it seemed best to fix them in head and just patch them back to the WIN32
branch.

-- 
  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:

Peter Eisentraut <peter_e@gmx.net> writes:

Why does configure need to see those files? Surely you don't need
configure to test the capabilities of our own source tree?

Looking at the contents of src/include/port/win32, I would imagine the
problem is configure will decide we don't HAVE_SYS_SOCKET_H, etc.

Yep. Can anyone suggest a better solution?

-- 
  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 writes:

The problem is that mbvalidate() has this code:

if (GetVariableBool(pset.vars, "WIN32_CONSOLE"))
CharToOem(pwcs, pwcs);

That's why I needed the #ifdef so it only tests from psql. This code
was just broken without that test, and because no one else has done
Win32 compiles since it was added, no one saw it.

Interesting, considering that WIN32_CONSOLE isn't being set anywhere.

Anyway, that code needs to be removed/fixed, for the mentioned reasons.

--
Peter Eisentraut peter_e@gmx.net

Peter Eisentraut wrote:

Bruce Momjian writes:

The problem is that mbvalidate() has this code:

if (GetVariableBool(pset.vars, "WIN32_CONSOLE"))
CharToOem(pwcs, pwcs);

That's why I needed the #ifdef so it only tests from psql. This code
was just broken without that test, and because no one else has done
Win32 compiles since it was added, no one saw it.

Interesting, considering that WIN32_CONSOLE isn't being set anywhere.

Anyway, that code needs to be removed/fixed, for the mentioned reasons.

Yikes, I see no doc mention of it.

Here is the submission email:

c) Problem: 8bit characters were displayed/interpreted wrong in psql
This is due to the fact that the Win32 "console" uses a
different encoding than the rest of the Windows system
Solution: Introduced a new psql variable WIN32_CONSOLE
When set with "\set WIN32_console", the function OemToChar()

I have CC'ed the author.

Is this a valid feature? Should I document 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

Bruce Momjian writes:

Is this a valid feature? Should I document it?

Maybe it's a valid feature, but the implementation is not valid.
Other applications also make use of the print routines (e.g., createlang),
and they cannot interactively set variables. So I repeat: the print
routines may not access the psql settings.

I think we ought to detect the console type automatically anyway.
Certainly we don't want people to have to set this variables all the time.

--
Peter Eisentraut peter_e@gmx.net

Bruce Momjian writes:

As Tom mentioned, it doesn't find things like sys/socket.h, which we
have added to src/include/port/win32/. This was done to elimintate
#ifdef of the include files that are Win32-specific.

OK, then you can reference the top source directory as $srcdir in
configure.

--
Peter Eisentraut peter_e@gmx.net

Peter Eisentraut wrote:

c) Problem: 8bit characters were displayed/interpreted wrong in psql
This is due to the fact that the Win32 "console" uses a
different encoding than the rest of the Windows system
Solution: Introduced a new psql variable WIN32_CONSOLE
When set with "\set WIN32_console", the function OemToChar()

Bruce Momjian writes:

Is this a valid feature? Should I document it?

Maybe it's a valid feature, but the implementation is not valid.
Other applications also make use of the print routines (e.g., createlang),
and they cannot interactively set variables. So I repeat: the print
routines may not access the psql settings.

I think we ought to detect the console type automatically anyway.
Certainly we don't want people to have to set this variables all the time.

Agreed. Automatic is ideal. Any ideas out there?

-- 
  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

Peter Eisentraut wrote:

Bruce Momjian writes:

As Tom mentioned, it doesn't find things like sys/socket.h, which we
have added to src/include/port/win32/. This was done to elimintate
#ifdef of the include files that are Win32-specific.

OK, then you can reference the top source directory as $srcdir in
configure.

OK, new code is:

if test "$PORTNAME" = "win32"
then
CPPFLAGS="$CPPFLAGS -I$srcdir/src/include/port/win32"
fi

Thanks.

-- 
  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 wrote:

I think we ought to detect the console type automatically anyway.
Certainly we don't want people to have to set this variables all the time.

Agreed. Automatic is ideal. Any ideas out there?

Does input from the console work correctly right now?

What about setting the code page of the console to the ansi code page?
Or to the code page that postgresql uses internally.

SetConsoleCP(GetACP());
SetConsoleOutputCP(GetACP());

might be sufficient.

--
Manfred

On Sun, 7 Sep 2003 12:34:02 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us> wrote:

Peter Eisentraut wrote:

c) Problem: 8bit characters were displayed/interpreted wrong in psql
This is due to the fact that the Win32 "console" uses a
different encoding than the rest of the Windows system
Solution: Introduced a new psql variable WIN32_CONSOLE
When set with "\set WIN32_console", the function OemToChar()

Bruce Momjian writes:

Is this a valid feature? Should I document it?

We have documented it and included a patch for the psql documentation.
Was it applied?

Maybe it's a valid feature, but the implementation is not valid.
Other applications also make use of the print routines (e.g., createlang),
and they cannot interactively set variables. So I repeat: the print
routines may not access the psql settings.

I think we ought to detect the console type automatically anyway.
Certainly we don't want people to have to set this variables all the time.

Agreed. Automatic is ideal. Any ideas out there?

Automatic detection does not work in all situation. The problem is a Microsoft
insanity: the win32 "console" uses a different encoding *only on stdin and
stdout*, not when accessing files. Thus it must be possible to set the parameter
WIN32_CONSOLE manually. Sample psql session:

psql> insert into tabelle values 'bäh'; -- here we need OemToChar()
psql> select * from tabelle; -- here we need CharToOem()
psql> \i script.sql -- here CharToOem() must NOT be used

Note that this will not be observed in English speaking countries! The problem
only occurs in other languages, which is the reason why most authors of win32
software do not even know about this problem.

Another soulution would be, not to compile psql as a console application, but
as a windows application and write an own terminal emulator. This solution
however faces another win32 insanity: windows applications have no stdout.

Christoph Dalitz

----- Original Message -----
From: "Bruce Momjian" <pgman@candle.pha.pa.us>
To: "Tom Lane" <tgl@sss.pgh.pa.us>
Cc: "Peter Eisentraut" <peter_e@gmx.net>; "PostgreSQL-patches"
<pgsql-patches@postgresql.org>; "PostgreSQL Win32 port list"
<pgsql-hackers-win32@postgresql.org>
Sent: Sunday, September 07, 2003 5:40 PM
Subject: Re: [pgsql-hackers-win32] [PATCHES] MinGW patch

Tom Lane wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

Why does configure need to see those files? Surely you don't need
configure to test the capabilities of our own source tree?

Looking at the contents of src/include/port/win32, I would imagine the
problem is configure will decide we don't HAVE_SYS_SOCKET_H, etc.

Yep. Can anyone suggest a better solution?

I am seeing problem with HAVE_RL_FILENAME_COMPLETION_FUNCTION also on my
system. When I manually remove it from pg_config.h, psql compiles OK.

Regards !

Christoph Dalitz wrote:

On Sun, 7 Sep 2003 12:34:02 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us> wrote:

Peter Eisentraut wrote:

c) Problem: 8bit characters were displayed/interpreted wrong in psql
This is due to the fact that the Win32 "console" uses a
different encoding than the rest of the Windows system
Solution: Introduced a new psql variable WIN32_CONSOLE
When set with "\set WIN32_console", the function OemToChar()

Bruce Momjian writes:

Is this a valid feature? Should I document it?

We have documented it and included a patch for the psql documentation.
Was it applied?

Oh, sorry, I see it now. It is already in CVS:

<varlistentry>
<term><varname>WIN32_CONSOLE</varname></term>
<listitem>
<para>
This variable is only useful when working under the Win32 command
console. As the Win32 command console uses a different encoding than
the rest of the Windows system. Eight-bit characters (e.g. German Umlauts)
are corrupted. When this variable is set the command console encoding will
be translated into ASCII encoding for input and output.
</para>
</listitem>
</varlistentry>

I must have forgotten to do a case-insenstive grep.

Maybe it's a valid feature, but the implementation is not valid.
Other applications also make use of the print routines (e.g., createlang),
and they cannot interactively set variables. So I repeat: the print
routines may not access the psql settings.

I think we ought to detect the console type automatically anyway.
Certainly we don't want people to have to set this variables all the time.

Agreed. Automatic is ideal. Any ideas out there?

Automatic detection does not work in all situation. The problem is a Microsoft
insanity: the win32 "console" uses a different encoding *only on stdin and
stdout*, not when accessing files. Thus it must be possible to set the parameter
WIN32_CONSOLE manually. Sample psql session:

psql> insert into tabelle values 'b?h'; -- here we need OemToChar()
psql> select * from tabelle; -- here we need CharToOem()
psql> \i script.sql -- here CharToOem() must NOT be used

Note that this will not be observed in English speaking countries! The problem
only occurs in other languages, which is the reason why most authors of win32
software do not even know about this problem.

Another soulution would be, not to compile psql as a console application, but
as a windows application and write an own terminal emulator. This solution
however faces another win32 insanity: windows applications have no stdout.

Got 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