win32 build and test issues

I'm also thinking of creating a simple commandline interface
for pqkill
that can be used to make a clean shutdown.

See the win32 status page, there is a link to one already.

//Magnus

Magnus Hagander wrote:

I'm also thinking of creating a simple commandline interface
for pqkill
that can be used to make a clean shutdown.

See the win32 status page, there is a link to one already.

//Magnus

I will check it out. Meanwhile, there's promising news. With the patch
below applied to psql, I can run "make check" under MinGW and get all
but 12 tests passing. The remainder appear to be all related to the
timestamp issue, the floating point scientific format issue, and the
join sort order issue, which have been previously noted.

I'm not suggesting applying the patch yet - I'd like to know why the
line end thing cares what platform it is using - presumably regardless
of where it runs it is still using MSVCRT.

cheers

andrew

Index: startup.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/startup.c,v
retrieving revision 1.91
diff -c -w -r1.91 startup.c
*** startup.c   22 Apr 2004 14:34:38 -0000      1.91
--- startup.c   30 Apr 2004 16:44:45 -0000
***************
*** 124,129 ****
--- 124,132 ----
                }
        }

+ #ifdef WIN32
+       setvbuf(stderr,NULL,_IONBF,0);
+ #endif
        pset.cur_cmd_source = stdin;
        pset.cur_cmd_interactive = false;
        pset.encoding = PQenv2encoding();
Index: print.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/print.c,v
retrieving revision 1.46
diff -c -w -r1.46 print.c
*** print.c     24 Jan 2004 20:43:26 -0000      1.46
--- print.c     30 Apr 2004 16:44:46 -0000
***************
*** 388,394 ****
--- 388,396 ----
                for (ptr = footers; *ptr; ptr++)
                        fprintf(fout, "%s\n", *ptr);

+ #ifndef WIN32
        fputc('\n', fout);
+ #endif

/* clean up */
free(cell_w);

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

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

Andrew Dunstan wrote:

Magnus Hagander wrote:

I'm also thinking of creating a simple commandline interface
for pqkill
that can be used to make a clean shutdown.

See the win32 status page, there is a link to one already.

//Magnus

I will check it out. Meanwhile, there's promising news. With the patch
below applied to psql, I can run "make check" under MinGW and get all
but 12 tests passing. The remainder appear to be all related to the
timestamp issue, the floating point scientific format issue, and the
join sort order issue, which have been previously noted.

I'm not suggesting applying the patch yet - I'd like to know why the
line end thing cares what platform it is using - presumably regardless
of where it runs it is still using MSVCRT.

cheers

andrew

Index: startup.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/startup.c,v
retrieving revision 1.91
diff -c -w -r1.91 startup.c
*** startup.c   22 Apr 2004 14:34:38 -0000      1.91
--- startup.c   30 Apr 2004 16:44:45 -0000
***************
*** 124,129 ****
--- 124,132 ----
}
}

+ #ifdef WIN32
+       setvbuf(stderr,NULL,_IONBF,0);
+ #endif
pset.cur_cmd_source = stdin;
pset.cur_cmd_interactive = false;
pset.encoding = PQenv2encoding();
Index: print.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/print.c,v
retrieving revision 1.46
diff -c -w -r1.46 print.c
*** print.c     24 Jan 2004 20:43:26 -0000      1.46
--- print.c     30 Apr 2004 16:44:46 -0000
***************
*** 388,394 ****
--- 388,396 ----
for (ptr = footers; *ptr; ptr++)
fprintf(fout, "%s\n", *ptr);

+ #ifndef WIN32
fputc('\n', fout);
+ #endif

/* clean up */
free(cell_w);

---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to majordomo@postgresql.org)

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

the setvbuf patch for startup.c should be applied, as all it does is
ensure well understood and expected (i.e. = Unix) behaviour for stderr on
Win32.

I am not happy about the patch for print.c unless I can work out *why* it
works, or someone can explain it to me. (That's why i made the comment
below about not applying it, and didn't send it to -patches.)

cheers

andrew

Bruce Momjian said:

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

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

--

I can apply the last part of this patch, but we can't patch configure,
only configure.in. Can you think of a way to patch that instead?

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

Andrew Dunstan wrote:

Below are a couple of diffs.

The first is the fix I made in configure - on my W2K machine with the
current MinGW/MSys I was getting the previously reported symlink problem
every time. With the looping patch (yes I *know* it's ugly, and we have
to get to the root of the problem, but I wanted to get past it for now)
it never happened, and builds worked. In any case we should use
something like the last couple of lines at least, to *show* something
went wrong. As we have it now we say we are doing something and not
testing for failure.

The second allows "make check" to proceed to almost the end - you still
need to use the task manager to shut things down.

The regression tests themselves seem to fail for the most part in every
configuration I have tried. The problem seems to be mostly buffering
issues - error messages appear out of order from what is expected. I
have not made sure what passes if we ignore that issue.

cheers

andrew

Index: configure
===================================================================
RCS file: /projects/cvsroot/pgsql-server/configure,v
retrieving revision 1.351
diff -c -w -r1.351 configure
*** configure   27 Apr 2004 20:09:27 -0000      1.351
--- configure   29 Apr 2004 20:17:06 -0000
***************
*** 19141,19151 ****
--- 19141,19160 ----
esac

# Make a symlink if possible; otherwise try a hard link.
+ for linktry in 1 2 3 4 5; do
ln -s $ac_rel_source $ac_dest 2>/dev/null ||
ln $srcdir/$ac_source $ac_dest ||
{ { echo "$as_me:$LINENO: error: cannot link $ac_dest to $srcdir/$ac_source" >&5
echo "$as_me: error: cannot link $ac_dest to $srcdir/$ac_source" >&2;}
{ (exit 1); exit 1; }; }
+   test -e $ac_dest && break
+ done
+   test -e $ac_dest || 
+ { { echo "$as_me:$LINENO: error: failed to link $ac_dest to $srcdir/$ac_source"
+  >&5
+ echo "$as_me: error: failed to link $ac_dest to $srcdir/$ac_source" >&2;}
+    { (exit 1); exit 1; }; }
+ 
done
_ACEOF

Index: src/test/regress/pg_regress.sh
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/test/regress/pg_regress.sh,v
retrieving revision 1.38
diff -c -w -r1.38 pg_regress.sh
*** src/test/regress/pg_regress.sh      8 Jan 2004 20:04:41 -0000       1.38
--- src/test/regress/pg_regress.sh      29 Apr 2004 20:17:07 -0000
***************
*** 294,305 ****
if [ x"$temp_install" != x"" ]
then
if echo x"$temp_install" | grep -v '^x/' >/dev/null 2>&1; then
temp_install="`pwd`/$temp_install"
fi

bindir=$temp_install/install/$bindir
libdir=$temp_install/install/$libdir
- pkglibdir=$temp_install/install/$pkglibdir
datadir=$temp_install/install/$datadir
PGDATA=$temp_install/data

--- 294,313 ----
if [ x"$temp_install" != x"" ]
then
if echo x"$temp_install" | grep -v '^x/' >/dev/null 2>&1; then
+         case $host_platform in
+           *-*-mingw32*)
+                 pkglibdir="`pwd -W`/$temp_install/install/$pkglibdir"
+                 temp_install="`pwd`/$temp_install"
+                 ;;
+           *)
temp_install="`pwd`/$temp_install"
+                 pkglibdir=$temp_install/install/$pkglibdir
+                 ;;
+         esac
fi

bindir=$temp_install/install/$bindir
libdir=$temp_install/install/$libdir
datadir=$temp_install/install/$datadir
PGDATA=$temp_install/data

***************
*** 336,342 ****
# executables, not dlopen'ed ones)
# ----------
case $host_platform in
!         *-*-cygwin*)
PATH=$libdir:$PATH
export PATH
;;
--- 344,350 ----
# executables, not dlopen'ed ones)
# ----------
case $host_platform in
!         *-*-cygwin* | *-*-mingw32*)
PATH=$libdir:$PATH
export PATH
;;

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

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

OK will apply only the first part.

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

Andrew Dunstan wrote:

Bruce,

the setvbuf patch for startup.c should be applied, as all it does is
ensure well understood and expected (i.e. = Unix) behaviour for stderr on
Win32.

I am not happy about the patch for print.c unless I can work out *why* it
works, or someone can explain it to me. (That's why i made the comment
below about not applying it, and didn't send it to -patches.)

cheers

andrew

Bruce Momjian said:

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

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

--

Andrew Dunstan wrote:

Magnus Hagander wrote:

I'm also thinking of creating a simple commandline interface
for pqkill
that can be used to make a clean shutdown.

See the win32 status page, there is a link to one already.

//Magnus

I will check it out. Meanwhile, there's promising news. With the patch
below applied to psql, I can run "make check" under MinGW and get all
but 12 tests passing. The remainder appear to be all related to the
timestamp issue, the floating point scientific format issue, and the
join sort order issue, which have been previously noted.

I'm not suggesting applying the patch yet - I'd like to know why the
line end thing cares what platform it is using - presumably regardless
of where it runs it is still using MSVCRT.

cheers

andrew

Index: startup.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/startup.c,v
retrieving revision 1.91
diff -c -w -r1.91 startup.c
*** startup.c   22 Apr 2004 14:34:38 -0000      1.91
--- startup.c   30 Apr 2004 16:44:45 -0000
***************
*** 124,129 ****
--- 124,132 ----
}
}

+ #ifdef WIN32
+       setvbuf(stderr,NULL,_IONBF,0);
+ #endif
pset.cur_cmd_source = stdin;
pset.cur_cmd_interactive = false;
pset.encoding = PQenv2encoding();
Index: print.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/print.c,v
retrieving revision 1.46
diff -c -w -r1.46 print.c
*** print.c     24 Jan 2004 20:43:26 -0000      1.46
--- print.c     30 Apr 2004 16:44:46 -0000
***************
*** 388,394 ****
--- 388,396 ----
for (ptr = footers; *ptr; ptr++)
fprintf(fout, "%s\n", *ptr);

+ #ifndef WIN32
fputc('\n', fout);
+ #endif

/* clean up */
free(cell_w);

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

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

This wasn't posted with a view to application. More a progress report.

Claudio has submitted a patch dealing with part of it.

I will look into the configure.in question in due course.

cheers

andrew

Bruce Momjian said:

I can apply the last part of this patch, but we can't patch configure,
only configure.in. Can you think of a way to patch that instead?

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

--

The really horrid hack below fixes for me a MINGW/MSys problem that
otherwise occurs inconsistently (fails on different links, and pretends
to have succeeded), but reliably (every run at least one link will not
actually have happened).

There are 2 parts - first we loop a few times until we succeed, and
second after the loop we test that we have actually succeeded, and
complain loudly otherwise.

The second part seems well worth doing. Nobody has yet come up with a
reasonable alternative to the first part (other than making the user do
it by hand, which defeats the whole purpose of configure).

So, the questions are: what parts of this should we do?
1) failure test only or
2) loop plus failure test or
3) nothing

And if not 3), is there some autoconf wizard out there who can help do
this properly? It would probably take me many hours to work out, as I
have never touched the beast.

cheers

andrew

Index: configure
===================================================================
RCS file: /projects/cvsroot/pgsql-server/configure,v
retrieving revision 1.351
diff -c -w -r1.351 configure
*** configure   27 Apr 2004 20:09:27 -0000      1.351
--- configure   29 Apr 2004 20:17:06 -0000
***************
*** 19141,19151 ****
--- 19141,19160 ----
   esac

   # Make a symlink if possible; otherwise try a hard link.
+ for linktry in 1 2 3 4 5; do
   ln -s $ac_rel_source $ac_dest 2>/dev/null ||
     ln $srcdir/$ac_source $ac_dest ||
     { { echo "$as_me:$LINENO: error: cannot link $ac_dest to 
$srcdir/$ac_source" >&5
 echo "$as_me: error: cannot link $ac_dest to $srcdir/$ac_source" >&2;}
    { (exit 1); exit 1; }; }
+   test -e $ac_dest && break
+ done
+   test -e $ac_dest || + { { echo "$as_me:$LINENO: error: failed to 
link $ac_dest to $srcdir/$ac_source"
+  >&5
+ echo "$as_me: error: failed to link $ac_dest to $srcdir/$ac_source" >&2;}
+    { (exit 1); exit 1; }; }
   done
 _ACEOF

Andrew Dunstan <andrew@dunslane.net> writes:

And if not 3), is there some autoconf wizard out there who can help do
this properly? It would probably take me many hours to work out, as I
have never touched the beast.

Obviously, or you would know that configure is a generated file that
there is no point in editing by hand.

The real issue in my mind is why is "ln" unreliable in mingw? I cannot
see any point in a retry kluge when we do not know what's really going
on.

regards, tom lane

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

And if not 3), is there some autoconf wizard out there who can help do
this properly? It would probably take me many hours to work out, as I
have never touched the beast.

Obviously, or you would know that configure is a generated file that
there is no point in editing by hand.

er ... that's why I asked how to do it properly. I simply included the
diff to show what I had been able to make work, not because I wanted it
applied.

The real issue in my mind is why is "ln" unreliable in mingw? I cannot
see any point in a retry kluge when we do not know what's really going
on.

I'm still trying to find out. But I don't see why this is different from
the kludge we already have for unlink, and that one is right inside
postgresql. In fact. it's more or less the same solution.

At the very least, until we can find a better solution we should have
something like the checking part of what I did. We've seen quite a
number of obscure failure reports that have all been traced back to this
failure, which is currently quite unreported by configure.

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:

Tom Lane wrote:

The real issue in my mind is why is "ln" unreliable in mingw? I cannot
see any point in a retry kluge when we do not know what's really going
on.

I'm still trying to find out. But I don't see why this is different from
the kludge we already have for unlink, and that one is right inside
postgresql.

It's different because we know why we need that one: we understand the
cause of the behavior and we therefore can have some confidence that the
kluge will fix it (or not, as the case may be). I have zero confidence
in looping five times around an "ln" call.

regards, tom lane

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

Tom Lane wrote:

The real issue in my mind is why is "ln" unreliable in mingw? I cannot
see any point in a retry kluge when we do not know what's really going
on.

I'm still trying to find out. But I don't see why this is different from
the kludge we already have for unlink, and that one is right inside
postgresql.

It's different because we know why we need that one: we understand the
cause of the behavior and we therefore can have some confidence that the
kluge will fix it (or not, as the case may be). I have zero confidence
in looping five times around an "ln" call.

Even if we don't do that can we *please* put in something that detects
the error, and tells the user what they will have to do to fix it?
Failing in a situation which we know we can detect and not telling the
user is intolerable, IMNSHO.

cheers

andrew

Andrew Dunstan wrote:

It's different because we know why we need that one: we understand the
cause of the behavior and we therefore can have some confidence that the
kluge will fix it (or not, as the case may be). I have zero confidence
in looping five times around an "ln" call.

Even if we don't do that can we *please* put in something that detects
the error, and tells the user what they will have to do to fix it?
Failing in a situation which we know we can detect and not telling the
user is intolerable, IMNSHO.

Agreed. At a minium we have to throw an error and tell them to run it
again.

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

Andrew Dunstan wrote:

Bruce,

the setvbuf patch for startup.c should be applied, as all it does is
ensure well understood and expected (i.e. = Unix) behaviour for stderr on
Win32.

OK, this first part of the patch was applied. I will discard the second
part and wait for your continued research. Thanks.

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

I am not happy about the patch for print.c unless I can work out *why* it
works, or someone can explain it to me. (That's why i made the comment
below about not applying it, and didn't send it to -patches.)

cheers

andrew

Bruce Momjian said:

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

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

--

Andrew Dunstan wrote:

Magnus Hagander wrote:

I'm also thinking of creating a simple commandline interface
for pqkill
that can be used to make a clean shutdown.

See the win32 status page, there is a link to one already.

//Magnus

I will check it out. Meanwhile, there's promising news. With the patch
below applied to psql, I can run "make check" under MinGW and get all
but 12 tests passing. The remainder appear to be all related to the
timestamp issue, the floating point scientific format issue, and the
join sort order issue, which have been previously noted.

I'm not suggesting applying the patch yet - I'd like to know why the
line end thing cares what platform it is using - presumably regardless
of where it runs it is still using MSVCRT.

cheers

andrew

Index: startup.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/startup.c,v
retrieving revision 1.91
diff -c -w -r1.91 startup.c
*** startup.c   22 Apr 2004 14:34:38 -0000      1.91
--- startup.c   30 Apr 2004 16:44:45 -0000
***************
*** 124,129 ****
--- 124,132 ----
}
}

+ #ifdef WIN32
+       setvbuf(stderr,NULL,_IONBF,0);
+ #endif
pset.cur_cmd_source = stdin;
pset.cur_cmd_interactive = false;
pset.encoding = PQenv2encoding();
Index: print.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/print.c,v
retrieving revision 1.46
diff -c -w -r1.46 print.c
*** print.c     24 Jan 2004 20:43:26 -0000      1.46
--- print.c     30 Apr 2004 16:44:46 -0000
***************
*** 388,394 ****
--- 388,396 ----
for (ptr = footers; *ptr; ptr++)
fprintf(fout, "%s\n", *ptr);

+ #ifndef WIN32
fputc('\n', fout);
+ #endif

/* clean up */
free(cell_w);

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

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

Andrew Dunstan wrote:

Even if we don't do that can we *please* put in something that
detects the error, and tells the user what they will have to do to
fix it? Failing in a situation which we know we can detect and not
telling the user is intolerable, IMNSHO.

Can you try a more recent version of autoconf and see if that behaves
more tolerably?

I have tried very hard to work out why this patch works for the
regression tests in MINGW/MSys, but it doesn't make any sense why this
one call should add an extra blank line. Does anyone have any bright
ideas? I'm reluctant to say I think we should apply it until the problem
is understood and the behaviour explained. But it does appear to work.

cheers

andrew

Andrew, can you illustrate to the group the exact difference in output?

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

Andrew Dunstan wrote:

I have tried very hard to work out why this patch works for the
regression tests in MINGW/MSys, but it doesn't make any sense why this
one call should add an extra blank line. Does anyone have any bright
ideas? I'm reluctant to say I think we should apply it until the problem
is understood and the behaviour explained. But it does appear to work.

cheers

andrew

Index: print.c
===================================================================
RCS file: /projects/cvsroot/pgsql-server/src/bin/psql/print.c,v
retrieving revision 1.46
diff -c -w -r1.46 print.c
*** print.c     24 Jan 2004 20:43:26 -0000      1.46
--- print.c     30 Apr 2004 16:44:46 -0000
***************
*** 388,394 ****
--- 388,396 ----
for (ptr = footers; *ptr; ptr++)
fprintf(fout, "%s\n", *ptr);

+ #ifndef WIN32
fputc('\n', fout);
+ #endif

/* clean up */
free(cell_w);

---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings

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

I will post a diff tomorrow. It's the extra blank lines in the
regression output thing that Claudio and others have remarked on. It
happens after every "(nnn rows)" line.

cheers

andrew

Bruce Momjian wrote:

Peter Eisentraut wrote:

Andrew Dunstan wrote:

Even if we don't do that can we *please* put in something that
detects the error, and tells the user what they will have to do to
fix it? Failing in a situation which we know we can detect and not
telling the user is intolerable, IMNSHO.

Can you try a more recent version of autoconf and see if that behaves
more tolerably?

tested with autoconf 2.59.

Unfortunately, it does not. It does try to copy if a link fails, unlike
what we have now:

ln -s $ac_rel_source $ac_dest 2>/dev/null ||
ln $srcdir/$ac_source $ac_dest 2>/dev/null ||
cp -p $srcdir/$ac_source $ac_dest ||

We don't have the last line, which must have been added since autoconf 2.53.

However, the problem is that the first line will actually appear to have
succeeded, i.e. MSys's ln is lying to us ;-(

This comes from the autoconf macro _AC_OUTPUT_LINKS defined in its
status.m4, which I guess is what we'd need to override (is that
possible?) if we are going to detect the failure, or maybe there's some
more magical way that in my unfamiliarity with autoconf I am unaware of.

cheers

andrew