Re: [HACKERS] pg_regress breaks on msys - Mailing list pgsql-patches

Tom Lane wrote:
> "Hiroshi Saito" <z-saito@guitar.ocn.ne.jp> writes:
> > This is very strange.!!
> >      boolean              ... ok
> >      char                 ... diff command failed with status 1: "diff -w "./expected/char.out"
> > "./results/char.out" >"./results/char.diff""
> > server stopped
>
> Yes, I believe the problem is that our Windows versions of the
> WIFEXITED/WEXITSTATUS macros are wrong. pg_regress is trying to verify
> that the diff call didn't fail entirely (eg, diff not there or failed
> to read one of the files), but the status code diff is returning for
> "files not the same" is confusing it.
>
> Can anyone check into what the result status conventions really are on
> Windows?  I am tempted to change the macros to just swap the bytes,
> but I dunno what that will do to their existing usages to check the
> result of pclose() or win32_waitpid().

I checked on MinGW and system() just returns the value returned by the
application.  There isn't any special two-values-in-one layering like is
done on Unix for wait() and the return value from system().  It seems if
the child dies from a signal, the parent dies too, at least in my C
tests.

I propose the following patch which just makes Win32 behave as returning
a single value from system(), and cleans up pg_regress.c.  We don't call
wait() or pipe() on Win32.  Though this is clearly a bug fix, I am
hesitant to backpatch it to 8.1 or 8.0 because of no problem reports.

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: src/include/port/win32.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/port/win32.h,v
retrieving revision 1.53
diff -c -c -r1.53 win32.h
*** src/include/port/win32.h    16 Jul 2006 20:17:04 -0000    1.53
--- src/include/port/win32.h    28 Jul 2006 19:01:07 -0000
***************
*** 109,120 ****


  /*
!  * Signal stuff
   */
! #define WEXITSTATUS(w)    (((w) >> 8) & 0xff)
! #define WIFEXITED(w)    (((w) & 0xff) == 0)
! #define WIFSIGNALED(w)    (((w) & 0x7f) > 0 && (((w) & 0x7f) < 0x7f))
! #define WTERMSIG(w)        ((w) & 0x7f)

  #define sigmask(sig) ( 1 << ((sig)-1) )

--- 109,125 ----


  /*
!  *    Signal stuff
!  *    WIN32 doesn't have wait(), so the return value for children
!  *    is simply the return value specified by the child, without
!  *    any additional information on whether the child terminated
!  *    on its own or via a signal.  These macros are also used
!  *    to interpret the return value of system().
   */
! #define WEXITSTATUS(w)    (w)
! #define WIFEXITED(w)    (true)
! #define WIFSIGNALED(w)    (false)
! #define WTERMSIG(w)        (0)

  #define sigmask(sig) ( 1 << ((sig)-1) )

Index: src/test/regress/pg_regress.c
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/pg_regress.c,v
retrieving revision 1.16
diff -c -c -r1.16 pg_regress.c
*** src/test/regress/pg_regress.c    27 Jul 2006 15:37:19 -0000    1.16
--- src/test/regress/pg_regress.c    28 Jul 2006 19:01:12 -0000
***************
*** 813,823 ****
       * work for inspecting the results of system().  For the moment,
       * hard-wire the check on Windows.
       */
- #ifndef WIN32
      if (!WIFEXITED(r) || WEXITSTATUS(r) > 1)
- #else
-     if (r != 0 && r != 1)
- #endif
      {
          fprintf(stderr, _("diff command failed with status %d: %s\n"), r, cmd);
          exit_nicely(2);
--- 813,819 ----