Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> writes:
> > I modified pg_regress.c to use just the return code to determine if the
> > diff worked, but I added in a WIN32-specific test for the file size. I
> > think that is the cleanest solution. Attached.
>
> It really needs a comment, along the lines of
>
> /*
> * On Windows we'll get exit status 1 if the diff invocation
> * failed; so we need a way to distinguish failure from "files
> * are different". Check to make sure that a diff file was
> * created and is nonempty.
> */
>
> Also the test ought to account for file_size returning -1 if file's
> not there, so
>
> + #ifdef WIN32
> + if (WEXITSTATUS(r) == 1 && file_size(filename) <= 0)
> ...
I had already added a comment this morning, but didn't post the updated
patch. I have made the adjustment for -1.
Patch attached and applied.
--
Bruce Momjian [EMAIL PROTECTED]
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 30 Jul 2006 01:41:46 -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 30 Jul 2006 01:41:47 -0000
***************
*** 799,827 ****
}
/*
! * Run a "diff" command and check that it didn't crash
*/
! static void
! run_diff(const char *cmd)
{
int r;
r = system(cmd);
- /*
- * XXX FIXME: it appears that include/port/win32.h's definitions of
- * WIFEXITED and related macros may be wrong. They certainly don't
- * 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);
}
}
/*
--- 799,830 ----
}
/*
! * Run a "diff" command and also check that it didn't crash
*/
! static int
! run_diff(const char *cmd, const char *filename)
{
int r;
r = system(cmd);
if (!WIFEXITED(r) || WEXITSTATUS(r) > 1)
{
fprintf(stderr, _("diff command failed with status %d: %s\n"), r, cmd);
exit_nicely(2);
}
+ #ifdef WIN32
+ /*
+ * On WIN32, if the 'diff' command cannot be found, system() returns
+ * 1, but produces nothing to stdout, so we check for that here.
+ */
+ if (WEXITSTATUS(r) == 1 && file_size(filename) <= 0)
+ {
+ fprintf(stderr, _("diff command not found: %s\n"), cmd);
+ exit_nicely(2);
+ }
+ #endif
+
+ return WEXITSTATUS(r);
}
/*
***************
*** 844,850 ****
int best_line_count;
int i;
int l;
!
/* Check in resultmap if we should be looking at a different file */
expectname = testname;
for (rm = resultmap; rm != NULL; rm = rm->next)
--- 847,853 ----
int best_line_count;
int i;
int l;
!
/* Check in resultmap if we should be looking at a different file */
expectname = testname;
for (rm = resultmap; rm != NULL; rm = rm->next)
***************
*** 872,883 ****
snprintf(cmd, sizeof(cmd),
SYSTEMQUOTE "diff %s \"%s\" \"%s\" > \"%s\"" SYSTEMQUOTE,
basic_diff_opts, expectfile, resultsfile, diff);
- run_diff(cmd);
/* Is the diff file empty? */
! if (file_size(diff) == 0)
{
- /* No diff = no changes = good */
unlink(diff);
return false;
}
--- 875,884 ----
snprintf(cmd, sizeof(cmd),
SYSTEMQUOTE "diff %s \"%s\" \"%s\" > \"%s\"" SYSTEMQUOTE,
basic_diff_opts, expectfile, resultsfile, diff);
/* Is the diff file empty? */
! if (run_diff(cmd, diff) == 0)
{
unlink(diff);
return false;
}
***************
*** 896,906 ****
snprintf(cmd, sizeof(cmd),
SYSTEMQUOTE "diff %s \"%s\" \"%s\" > \"%s\"" SYSTEMQUOTE,
basic_diff_opts, expectfile, resultsfile, diff);
- run_diff(cmd);
! if (file_size(diff) == 0)
{
- /* No diff = no changes = good */
unlink(diff);
return false;
}
--- 897,905 ----
snprintf(cmd, sizeof(cmd),
SYSTEMQUOTE "diff %s \"%s\" \"%s\" > \"%s\"" SYSTEMQUOTE,
basic_diff_opts, expectfile, resultsfile, diff);
! if (run_diff(cmd, diff) == 0)
{
unlink(diff);
return false;
}
***************
*** 921,927 ****
snprintf(cmd, sizeof(cmd),
SYSTEMQUOTE "diff %s \"%s\" \"%s\" >> \"%s\"" SYSTEMQUOTE,
pretty_diff_opts, best_expect_file, resultsfile, difffilename);
! run_diff(cmd);
/* And append a separator */
difffile = fopen(difffilename, "a");
--- 920,926 ----
snprintf(cmd, sizeof(cmd),
SYSTEMQUOTE "diff %s \"%s\" \"%s\" >> \"%s\"" SYSTEMQUOTE,
pretty_diff_opts, best_expect_file, resultsfile, difffilename);
! run_diff(cmd, difffilename);
/* And append a separator */
difffile = fopen(difffilename, "a");
---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
