new old winetests

[Ferenc Wagner]

Hi there,

I've got a more or less standalone version which can run the
tests and submit the results to a CGI script, if possible.
It also differentiates between libwine and cross builds.  I
would be grateful for a review.  One week point is its
integration into the build system: I hacked something
together which works, but not sure at all if that's the
right way.  One other is that I did not notice that the
tests return the number of tests failed as their exit
status.  It adds a couple of lines to the output...

Anyway: programs/winetests directory:

Cross-compiled .exe without the dsound tests:
http://afavant.elte.hu/~wferi/wine/tests.zip
Submission point (for now you won't see anything and
submissions overwrite each other, but this must be
contactable for automatic submission):
http://afavant.elte.hu/~wferi/cgi-bin/winetests.cgi

Pretty much deuglifying work remains, but it does something
(for me).  Sorry for the slow progress...

Feri.

[Dimitrie O. Paun]

On November 21, 2003 12:16 pm, Ferenc Wagner wrote:
> I've got a more or less standalone version which can run the
> tests and submit the results to a CGI script, if possible.
> It also differentiates between libwine and cross builds.  I
> would be grateful for a review.  One week point is its
> integration into the build system: I hacked something
> together which works, but not sure at all if that's the
> right way.

Hi Feri,

Nice work! It looks pretty clean, I think we're close
to submit it. A few comments:
  -- yes, I think we can do better in terms of build
     integration (see patch)
  -- I don't think we need that many header files, it's
     a small app. It is just a tiny little nit, but I'd
     simply stick all of them in the winetests.h file.
  -- please use 4 space indentation throughout. This is
     partly a personal request, partly for consistency.
     2 space indentation is slick, but it's hard on the
     eyes, and I find lately (hey, we're only growing old!)
     that I have a hard time following the code.
  -- I'm not sure about the WINELIB define. I'm afraid
     that one doesn't work as intended in the new scheme,
     we'll have to find something else. Anyway, we should
     just minimize it's use to the absolute minimum.

And here is the completely untested patch of trying to
better integrate the cross compilation with the build
system:

Index: configure.ac
===================================================================
RCS file: /var/cvs/wine/configure.ac,v
retrieving revision 1.209
diff -u -r1.209 configure.ac
--- configure.ac	20 Nov 2003 04:17:33 -0000	1.209
+++ configure.ac	21 Nov 2003 19:59:40 -0000
@@ -1633,6 +1633,7 @@
 programs/winemenubuilder/Makefile
 programs/winemine/Makefile
 programs/winepath/Makefile
+programs/winetests/Makefile
 programs/winevdm/Makefile
 programs/winhelp/Makefile
 programs/winver/Makefile
Index: Make.rules.in
===================================================================
RCS file: /var/cvs/wine/Make.rules.in,v
retrieving revision 1.163
diff -u -r1.163 Make.rules.in
--- Make.rules.in	11 Oct 2003 01:05:18 -0000	1.163
+++ Make.rules.in	21 Nov 2003 19:44:04 -0000
@@ -53,6 +53,8 @@
 MV        = mv
 LINT      = @LINT@
 LINTFLAGS = @LINTFLAGS@
+CROSSCC      = @CROSSCC@
+CROSSWINDRES = @CROSSWINDRES@
 INCLUDES     = -I$(SRCDIR) -I. -I$(TOPSRCDIR)/include -I$(TOPOBJDIR)/include $(EXTRAINCL)
 EXTRACFLAGS  = @EXTRACFLAGS@
 ALLCFLAGS    = $(INCLUDES) $(DEFS) $(DLLFLAGS) $(EXTRACFLAGS) $(CPPFLAGS) $(CFLAGS)
@@ -105,18 +107,22 @@
 CLEAN_FILES     = *.o *.a *.so *.ln *.$(LIBEXT) \#*\# *~ *% .\#* *.bak *.orig *.rej \
                   *.flc *.spec.c *.spec.def *.dbg.c y.tab.c y.tab.h @LEX_OUTPUT_ROOT@.c core

-OBJS = $(C_SRCS:.c=.o) $(EXTRA_OBJS)
+OBJS      = $(C_SRCS:.c=.o) $(EXTRA_OBJS)
+CROSSOBJS = $(C_SRCS:.c=.cross.o) $(EXTRA_OBJS:.o=.cross.o) $(RC_SRCS:.rc=.res.cross.o)

RCOBJS = $(RC_SRCS:.rc=.res.o)
 LINTS  = $(C_SRCS:.c=.ln)

# Implicit rules

-.SUFFIXES: .mc .rc .mc.rc .res .res.o .spec .spec.c .spec.def .ok
+.SUFFIXES: .mc .rc .mc.rc .res .res.o .spec .spec.c .spec.def .ok .cross.o .res.cross.o

.c.o:
    $(CC) -c $(ALLCFLAGS) -o $@ $<

+.c.cross.o:
+	$(CROSSCC) -c $(ALLCFLAGS) -o $@ $<
+
 .s.o:
    $(AS) -o $@ $<

@@ -128,6 +134,9 @@

.res.res.o:
    $(WINDRES) -i $< -o $@
+
+.res.res.cross.o:
+	$(CROSSWINDRES) -i $< -o $@

.spec.spec.c:
    $(WINEBUILD) $(DEFS) -o $@ --main-module $(MODULE) --spec $<
Index: dlls/Maketest.rules.in
===================================================================
RCS file: /var/cvs/wine/dlls/Maketest.rules.in,v
retrieving revision 1.28
diff -u -r1.28 Maketest.rules.in
--- dlls/Maketest.rules.in	11 Oct 2003 01:05:18 -0000	1.28
+++ dlls/Maketest.rules.in	21 Nov 2003 19:50:35 -0000
@@ -25,9 +25,6 @@
 IMPORTLIBS   = $(IMPORTS:%=$(DLLDIR)/lib%.$(IMPLIBEXT))

CROSSTEST    = $(TESTDLL:%.dll=%)_crosstest.exe
-CROSSOBJS    = $(C_SRCS:.c=.cross.o) $(RC_SRCS:.rc=.res.cross.o) $(TESTLIST:.c=.cross.o)
-CROSSCC      = @CROSSCC@
-CROSSWINDRES = @CROSSWINDRES@

@MAKE_RULES@

@@ -62,14 +59,6 @@
 $(TESTRESULTS): $(MODULE)$(DLLEXT) $(DLLDIR)/$(TESTDLL)$(DLLEXT)

# Rules for cross-compiling tests
-
-.SUFFIXES: .cross.o .res.cross.o
-
-.c.cross.o:
-	$(CROSSCC) -c $(ALLCFLAGS) -o $@ $<
-
-.res.res.cross.o:
-	$(CROSSWINDRES) -i $< -o $@

crosstest:: @CROSSTEST@

diff -u programs/winetests.feri/Makefile.in programs/winetests/Makefile.in
--- programs/winetests.feri/Makefile.in	2003-11-19 22:01:05.000000000 -0500
+++ programs/winetests/Makefile.in	2003-11-21 14:55:59.000000000 -0500
@@ -9,20 +9,18 @@

C_SRCS = main.c send.c util.c
 EXTRA_OBJS = winetests.o
-RC_SRCS = winetests.rc
-
-CROSS_GCC = i586-mingw32msvc-gcc
-CROSS_RC  = i586-mingw32msvc-windres
-CROSS_CFLAGS = -Wall -W -O2
-CROSS_OBJS = $(C_SRCS:.c=_cross.o) winetests_cross.o
+RC_SRCS = winetests_so.rc

@MAKE_PROG_RULES@

# Special rules

-winetests.rc: maketests tests.list
+winetests_so.rc: maketests tests.list
    $(SRCDIR)/maketests -r_so > $@

+winetests_pe.rc: maketests tests.list
+	$(SRCDIR)/maketests -r_pe > $@
+
 winetests.subtests: maketests tests.list
    $(SRCDIR)/maketests -s > $@

@@ -32,21 +30,12 @@
 winetests.c: winetests.tests winetests.subtests
    (echo '#include "winetests.h"'; cat winetests.{sub,}tests) > $@

-res_cross.rc: maketests tests.list
-	$(SRCDIR)/maketests -r_pe > $@
-
-$(CROSS_OBJS): %_cross.o: %.c
-	$(CROSS_GCC) $(CROSS_CFLAGS) -o $@ -c $<
-
-res_cross.o: res_cross.rc
-	$(CROSS_RC) -o $@ $<
-
-winetests_cross.exe: $(CROSS_OBJS) res_cross.o
-	$(CROSS_GCC) $^ -lws2_32 -o $@
+winetests_cross.exe: $(CROSSOBJS:_so.res.cross.o=_pe.res.cross.o)
+	$(CROSSCC) $^ -o $@ $(IMPORTS:%=-l%) $(LIBS)

clean::
    rm -f winetests.c winetests.rc winetests.subtests \
               winetests.tests winetests_cross.exe $(CROSS_OBJS) \
-              res_cross.rc res_cross.o
+              winetests_pe.rc winetests_pe.res.cross.o

### Dependencies:
diff -u programs/winetests.feri/util.h programs/winetests/util.h
--- programs/winetests.feri/util.h	2003-11-17 14:36:43.000000000 -0500
+++ programs/winetests/util.h	2003-11-21 14:23:43.000000000 -0500
@@ -2,9 +2,7 @@
 #define __UTIL_H

#include <stdio.h>
-#ifndef WINELIB
-#  include <stdlib.h>
-#endif
+#include <stdlib.h>

void fatal (const char* msg);
 void *xmalloc (size_t len);

[Dimitrie O. Paun]

On November 21, 2003 12:16 pm, Ferenc Wagner wrote:
> I would be grateful for a review.

OK,

One more time, this round with a patch incorporating
most of my suggestions. What it does:
   -- better integration in the build system
   -- supports out of tree builds
   -- cleans up file when winetests fails
   -- more uniform naming convention in the Makefile
   -- tested on Linux, it builds just fine
   -- cross compilation not tested at this point
   -- fix indentation and a few cosmetic things in main.c
   -- indent util.c to 4-space indent
   -- send.c not indented, I'll let you deal with it :)
   -- supplies an automatic build label for unattended builds
   -- remove the WINELIB flag (was always present anyway)
   -- tries to deal with the mkdir() problem
   -- renames removeDir to remove_dir for consistency
   -- rename IsDotDir to is_dot_dir for the same reason
   -- turn is_dot_dir into an inline function for neatness
   -- merge all headers into winetests.h

And here is the patch (relative to what you've send):

Index: configure.ac
===================================================================
RCS file: /var/cvs/wine/configure.ac,v
retrieving revision 1.210
diff -u -r1.210 configure.ac
--- configure.ac	22 Nov 2003 00:08:26 -0000	1.210
+++ configure.ac	22 Nov 2003 02:33:16 -0000
@@ -1638,6 +1638,7 @@
 programs/winemenubuilder/Makefile
 programs/winemine/Makefile
 programs/winepath/Makefile
+programs/winetests/Makefile
 programs/winevdm/Makefile
 programs/winhelp/Makefile
 programs/winver/Makefile
Index: Make.rules.in
===================================================================
RCS file: /var/cvs/wine/Make.rules.in,v
retrieving revision 1.163
diff -u -r1.163 Make.rules.in
--- Make.rules.in	11 Oct 2003 01:05:18 -0000	1.163
+++ Make.rules.in	21 Nov 2003 19:44:04 -0000
@@ -53,6 +53,8 @@
 MV        = mv
 LINT      = @LINT@
 LINTFLAGS = @LINTFLAGS@
+CROSSCC      = @CROSSCC@
+CROSSWINDRES = @CROSSWINDRES@
 INCLUDES     = -I$(SRCDIR) -I. -I$(TOPSRCDIR)/include -I$(TOPOBJDIR)/include $(EXTRAINCL)
 EXTRACFLAGS  = @EXTRACFLAGS@
 ALLCFLAGS    = $(INCLUDES) $(DEFS) $(DLLFLAGS) $(EXTRACFLAGS) $(CPPFLAGS) $(CFLAGS)
@@ -105,18 +107,22 @@
 CLEAN_FILES     = *.o *.a *.so *.ln *.$(LIBEXT) \#*\# *~ *% .\#* *.bak *.orig *.rej \
                   *.flc *.spec.c *.spec.def *.dbg.c y.tab.c y.tab.h @LEX_OUTPUT_ROOT@.c core

-OBJS = $(C_SRCS:.c=.o) $(EXTRA_OBJS)
+OBJS      = $(C_SRCS:.c=.o) $(EXTRA_OBJS)
+CROSSOBJS = $(C_SRCS:.c=.cross.o) $(EXTRA_OBJS:.o=.cross.o) $(RC_SRCS:.rc=.res.cross.o)

RCOBJS = $(RC_SRCS:.rc=.res.o)
 LINTS  = $(C_SRCS:.c=.ln)

# Implicit rules

-.SUFFIXES: .mc .rc .mc.rc .res .res.o .spec .spec.c .spec.def .ok
+.SUFFIXES: .mc .rc .mc.rc .res .res.o .spec .spec.c .spec.def .ok .cross.o .res.cross.o

.c.o:
    $(CC) -c $(ALLCFLAGS) -o $@ $<

+.c.cross.o:
+	$(CROSSCC) -c $(ALLCFLAGS) -o $@ $<
+
 .s.o:
    $(AS) -o $@ $<

@@ -128,6 +134,9 @@

.res.res.o:
    $(WINDRES) -i $< -o $@
+
+.res.res.cross.o:
+	$(CROSSWINDRES) -i $< -o $@

.spec.spec.c:
    $(WINEBUILD) $(DEFS) -o $@ --main-module $(MODULE) --spec $<
Index: dlls/Maketest.rules.in
===================================================================
RCS file: /var/cvs/wine/dlls/Maketest.rules.in,v
retrieving revision 1.28
diff -u -r1.28 Maketest.rules.in
--- dlls/Maketest.rules.in	11 Oct 2003 01:05:18 -0000	1.28
+++ dlls/Maketest.rules.in	21 Nov 2003 19:50:35 -0000
@@ -25,9 +25,6 @@
 IMPORTLIBS   = $(IMPORTS:%=$(DLLDIR)/lib%.$(IMPLIBEXT))

CROSSTEST    = $(TESTDLL:%.dll=%)_crosstest.exe
-CROSSOBJS    = $(C_SRCS:.c=.cross.o) $(RC_SRCS:.rc=.res.cross.o) $(TESTLIST:.c=.cross.o)
-CROSSCC      = @CROSSCC@
-CROSSWINDRES = @CROSSWINDRES@

@MAKE_RULES@

@@ -62,14 +59,6 @@
 $(TESTRESULTS): $(MODULE)$(DLLEXT) $(DLLDIR)/$(TESTDLL)$(DLLEXT)

# Rules for cross-compiling tests
-
-.SUFFIXES: .cross.o .res.cross.o
-
-.c.cross.o:
-	$(CROSSCC) -c $(ALLCFLAGS) -o $@ $<
-
-.res.res.cross.o:
-	$(CROSSWINDRES) -i $< -o $@

crosstest:: @CROSSTEST@

diff -uN programs/winetests.feri/main.c programs/winetests/main.c
--- programs/winetests.feri/main.c	2003-11-20 11:46:23.000000000 -0500
+++ programs/winetests/main.c	2003-11-21 19:10:02.000000000 -0500
@@ -23,22 +23,22 @@
  *
  */

-#ifdef WINELIB
-#  include "config.h"
-#  include "wine/port.h"
-#  define mkdir(d) mkdir (d, 0777)
-#  include <errno.h>
-#else
-#  include <stdlib.h>
-#  include <unistd.h>
-#endif
+#include "config.h"
+#include "wine/port.h"

#include <stdio.h>
+#include <stdlib.h>
 #include <wtypes.h>
+#include <errno.h>
+#ifdef HAVE_UNISTD_H
+#  include <unistd.h>
+#endif

#include "winetests.h"
-#include "send.h"
-#include "util.h"
+
+#ifndef _WIN32
+#  define mkdir(d) mkdir (d, 0777)
+#endif

void print_version ()
 {
@@ -58,43 +58,46 @@
              ver.dwMajorVersion, ver.dwMinorVersion, ver.dwBuildNumber,
              ver.dwPlatformId, ver.szCSDVersion);

-    if (ext)
-        xprintf ("    wServicePackMajor=%d\n    wServicePackMinor=%d\n"
-                 "    wSuiteMask=%d\n    wProductType=%d\n    wReserved=%d\n",
-                 ver.wServicePackMajor, ver.wServicePackMinor, ver.wSuiteMask,
-                 ver.wProductType, ver.wReserved);
-}
-
-#define IsDotDir(x)     ((x[0] == '.') && ((x[1] == 0) || ((x[1] == '.') && (x[2] == 0))))
-
-void removeDir (const char *dir)
-{
-  HANDLE  hFind;
-  WIN32_FIND_DATAA wfd;
-  char path[MAX_PATH];
-  size_t dirlen = strlen (dir);
-
-  /* Make sure the directory exists before going further */
-  memcpy (path, dir, dirlen);
-  strcpy (path + dirlen++, "\*");
-  hFind = FindFirstFileA (path, &wfd);
-  if (hFind == INVALID_HANDLE_VALUE)
-    return;
-
-  do {
-    char *lp = wfd.cFileName;
-
-    if (!lp[0]) lp = wfd.cAlternateFileName; /* ? FIXME not (!lp) ? */
-    if (IsDotDir (lp)) continue;
-    strcpy (path + dirlen, lp);
-    if (FILE_ATTRIBUTE_DIRECTORY & wfd.dwFileAttributes)
-        removeDir(path);
-    else if (!DeleteFileA (path))
-             fatal (strmake (NULL, "Can't delete file %s: error %d", path, GetLastError ()));
-  } while (FindNextFileA (hFind, &wfd));
-  FindClose (hFind);
-  if (!RemoveDirectoryA (dir))
-      fatal (strmake (NULL, "Can't remove directory %s: error %d", dir, GetLastError ()));
+    if (!ext) return;
+
+    xprintf ("    wServicePackMajor=%d\n    wServicePackMinor=%d\n"
+             "    wSuiteMask=%d\n    wProductType=%d\n    wReserved=%d\n",
+             ver.wServicePackMajor, ver.wServicePackMinor, ver.wSuiteMask,
+             ver.wProductType, ver.wReserved);
+}
+
+static inline int is_dot_dir(const char* x) 
+{
+    return ((x[0] == '.') && ((x[1] == 0) || ((x[1] == '.') && (x[2] == 0))));
+}
+
+void remove_dir (const char *dir)
+{
+    HANDLE  hFind;
+    WIN32_FIND_DATA wfd;
+    char path[MAX_PATH];
+    size_t dirlen = strlen (dir);
+
+    /* Make sure the directory exists before going further */
+    memcpy (path, dir, dirlen);
+    strcpy (path + dirlen++, "\*");
+    hFind = FindFirstFile (path, &wfd);
+    if (hFind == INVALID_HANDLE_VALUE) return;
+
+    do {
+        char *lp = wfd.cFileName;
+
+        if (!lp[0]) lp = wfd.cAlternateFileName; /* ? FIXME not (!lp) ? */
+        if (is_dot_dir (lp)) continue;
+        strcpy (path + dirlen, lp);
+        if (FILE_ATTRIBUTE_DIRECTORY & wfd.dwFileAttributes)
+            remove_dir(path);
+        else if (!DeleteFile (path))
+            fatal (strmake (NULL, "Can't delete file %s: error %d", path, GetLastError ()));
+    } while (FindNextFile (hFind, &wfd));
+    FindClose (hFind);
+    if (!RemoveDirectory (dir))
+        fatal (strmake (NULL, "Can't remove directory %s: error %d", dir, GetLastError ()));
 }

int count_tests()
@@ -108,7 +111,7 @@
     return total;
 }

-LPVOID extract_rcdata (const char *name, DWORD* size)
+void* extract_rcdata (const char *name, DWORD* size)
 {
     HRSRC rsrc;
     HGLOBAL hdl;
@@ -147,9 +150,9 @@

xprintf ("%s:%s start\n", test->name, subtest);
     if (test->is_elf)
-      status = spawnvp (_P_WAIT, "/usr/local/src/wine/wine", argv);
+        status = spawnvp (_P_WAIT, "/usr/local/src/wine/wine", argv);
     else
-      status = spawnvp (_P_WAIT, test->exename, argv+1);
+        status = spawnvp (_P_WAIT, test->exename, argv+1);
     if (status)
         xprintf ("Exit status: %d, errno=%d: %s\n", status, errno, strerror (errno));
     xprintf ("%s:%s done\n", test->name, subtest);
@@ -196,13 +199,13 @@
     fclose (fp);
     close (1);                  /* Avoid sharing violation on Win */

-    removeDir (tempdir);
+    remove_dir (tempdir);

if (send_file (logfile))
-      fatal ("Can't submit logfile (network of file error).");
+        fatal ("Can't submit logfile (network of file error).");

if (remove (logfile))
-      fatal (strmake (NULL, "Can't remove logfile: %d.", errno));
+        fatal (strmake (NULL, "Can't remove logfile: %d.", errno));

return 0;
 }
diff -uN programs/winetests.feri/Makefile.in programs/winetests/Makefile.in
--- programs/winetests.feri/Makefile.in	2003-11-19 22:01:05.000000000 -0500
+++ programs/winetests/Makefile.in	2003-11-22 03:21:49.000000000 -0500
@@ -5,48 +5,36 @@
 MODULE    = winetests.exe
 APPMODE   = gui
 IMPORTS   = user32 ws2_32
-EXTRADEFS = -DWINELIB

C_SRCS = main.c send.c util.c
 EXTRA_OBJS = winetests.o
-RC_SRCS = winetests.rc
-
-CROSS_GCC = i586-mingw32msvc-gcc
-CROSS_RC  = i586-mingw32msvc-windres
-CROSS_CFLAGS = -Wall -W -O2
-CROSS_OBJS = $(C_SRCS:.c=_cross.o) winetests_cross.o
+RC_SRCS = winetests_so.rc

@MAKE_PROG_RULES@

# Special rules

-winetests.rc: maketests tests.list
-	$(SRCDIR)/maketests -r_so > $@
+winetests_so.rc: maketests tests.list
+	$(SRCDIR)/maketests -r_so $(SRCDIR)/tests.list > $@ || ( $(RM) $@ && exit 1 )
+
+winetests_pe.rc: maketests tests.list
+	$(SRCDIR)/maketests -r_so $(SRCDIR)/tests.list > $@ || ( $(RM) $@ && exit 1 )

winetests.subtests: maketests tests.list
-	$(SRCDIR)/maketests -s > $@
+	$(SRCDIR)/maketests -s $(SRCDIR)/tests.list > $@ || ( $(RM) $@ && exit 1 )

winetests.tests: maketests tests.list
-	$(SRCDIR)/maketests -t $(BUILD) > $@
+	$(SRCDIR)/maketests -t $(SRCDIR)/tests.list > $@ || ( $(RM) $@ && exit 1 )

winetests.c: winetests.tests winetests.subtests
    (echo '#include "winetests.h"'; cat winetests.{sub,}tests) > $@

-res_cross.rc: maketests tests.list
-	$(SRCDIR)/maketests -r_pe > $@
-
-$(CROSS_OBJS): %_cross.o: %.c
-	$(CROSS_GCC) $(CROSS_CFLAGS) -o $@ -c $<
-
-res_cross.o: res_cross.rc
-	$(CROSS_RC) -o $@ $<
-
-winetests_cross.exe: $(CROSS_OBJS) res_cross.o
-	$(CROSS_GCC) $^ -lws2_32 -o $@
+winetests_cross.exe: $(CROSSOBJS:_so.res.cross.o=_pe.res.cross.o)
+	$(CROSSCC) $^ -o $@ $(IMPORTS:%=-l%) $(LIBS)

clean::
    rm -f winetests.c winetests.rc winetests.subtests \
-              winetests.tests winetests_cross.exe $(CROSS_OBJS) \
-              res_cross.rc res_cross.o
+              winetests.tests winetests_cross.exe $(CROSSOBJS) \
+              winetests_pe.rc winetests_pe.res.cross.o

### Dependencies:
diff -uN programs/winetests.feri/maketests programs/winetests/maketests
--- programs/winetests.feri/maketests	2003-11-21 08:15:09.000000000 -0500
+++ programs/winetests/maketests	2003-11-22 03:20:52.000000000 -0500
@@ -3,16 +3,16 @@
 MODE="$1"

BINDIR="../.."
-TEST_EXES=`sed 's/#.*//' tests.list`
+TEST_EXES=`sed 's/#.*//' "$2"`

echo "/* Automatically generated -- do not edit! */"
 case $MODE in
     -t)
-	if [ -z "$2" ]; then
-	    echo Please supply a build tag. >/dev/stderr
-	    exit 1
+	if [ -z "$WINE_BUILD" ]; then
+	    WINE_BUILD="`date`"
+	    echo "warning: using automatically generated BUILD tag: $WINE_BUILD" >/dev/stderr
    fi
-        echo "const char build_tag[] = "$2";"
+        echo "const char build_tag[] = "$WINE_BUILD";"
    echo "struct wine_test wine_tests[] = {"
     ;;
 esac
diff -uN programs/winetests.feri/send.c programs/winetests/send.c
--- programs/winetests.feri/send.c	2003-11-19 21:12:21.000000000 -0500
+++ programs/winetests/send.c	2003-11-21 19:11:01.000000000 -0500
@@ -1,8 +1,7 @@
 #include <winsock.h>
 #include <stdio.h>

-#include "util.h"
-#include "send.h"
+#include "winetests.h"

SOCKET
 open_http (const char *ipnum)
diff -uN programs/winetests.feri/send.h programs/winetests/send.h
--- programs/winetests.feri/send.h	2003-11-15 09:31:51.000000000 -0500
+++ programs/winetests/send.h	1969-12-31 19:00:00.000000000 -0500
@@ -1,6 +0,0 @@
-#ifndef __SEND_H
-#define __SEND_H
-
-int send_file (const char *name);
-
-#endif /* __SEND_H */
diff -uN programs/winetests.feri/util.c programs/winetests/util.c
--- programs/winetests.feri/util.c	2003-11-17 14:36:33.000000000 -0500
+++ programs/winetests/util.c	2003-11-22 03:33:15.000000000 -0500
@@ -1,70 +1,70 @@
 #include <windows.h>

-#include "util.h"
+#include "winetests.h"

void fatal (const char* msg)
 {
-  MessageBox (NULL, msg, "Fatal Error", MB_ICONERROR | MB_OK);
-  exit (1);
+    MessageBox (NULL, msg, "Fatal Error", MB_ICONERROR | MB_OK);
+    exit (1);
 }

void *xmalloc (size_t len)
 {
-  void *p = malloc (len);
+    void *p = malloc (len);

-  if (!p) fatal ("Out of memory.");
-  return p;
+    if (!p) fatal ("Out of memory.");
+    return p;
 }

void *xrealloc (void *op, size_t len)
 {
-  void *p = realloc (op, len);
+    void *p = realloc (op, len);

-  if (!p) fatal ("Out of memory.");
-  return p;
+    if (!p) fatal ("Out of memory.");
+    return p;
 }

void xprintf (const char *fmt, ...)
 {
-  va_list ap;
+    va_list ap;

-  va_start (ap, fmt);
-  if (vprintf (fmt, ap) < 0) fatal ("Can't write logs.");
-  va_end (ap);
+    va_start (ap, fmt);
+    if (vprintf (fmt, ap) < 0) fatal ("Can't write logs.");
+    va_end (ap);
 }

char *vstrmake (size_t *lenp, const char *fmt, va_list ap)
 {
-  size_t size = 1000;
-  char *p, *q;
-  int n;
-
-  p = malloc (size);
-  if (!p) return NULL;
-  while (1) {
-    n = vsnprintf (p, size, fmt, ap);
-    if (n < 0) size *= 2;   /* Windows */
-    else if ((unsigned)n >= size) size = n+1; /* glibc */
-    else break;
-    q = realloc (p, size);
-    if (!q) {
-      free (p);
-      return NULL;
+    size_t size = 1000;
+    char *p, *q;
+    int n;
+
+    p = malloc (size);
+    if (!p) return NULL;
+    while (1) {
+        n = vsnprintf (p, size, fmt, ap);
+        if (n < 0) size *= 2;   /* Windows */
+        else if ((unsigned)n >= size) size = n+1; /* glibc */
+        else break;
+        q = realloc (p, size);
+        if (!q) {
+          free (p);
+          return NULL;
+       }
+       p = q;
     }
-    p = q;
-  }
-  if (lenp) *lenp = n;
-  return p;
+    if (lenp) *lenp = n;
+    return p;
 }

char *strmake (size_t *lenp, const char *fmt, ...)
 {
-  va_list ap;
-  char *p;
+    va_list ap;
+    char *p;

-  va_start (ap, fmt);
-  p = vstrmake (lenp, fmt, ap);
-  if (!p) fatal ("Out of memory.");
-  va_end (ap);
-  return p;
+    va_start (ap, fmt);
+    p = vstrmake (lenp, fmt, ap);
+    if (!p) fatal ("Out of memory.");
+    va_end (ap);
+    return p;
 }
diff -uN programs/winetests.feri/util.h programs/winetests/util.h
--- programs/winetests.feri/util.h	2003-11-17 14:36:43.000000000 -0500
+++ programs/winetests/util.h	1969-12-31 19:00:00.000000000 -0500
@@ -1,16 +0,0 @@
-#ifndef __UTIL_H
-#define __UTIL_H
-
-#include <stdio.h>
-#ifndef WINELIB
-#  include <stdlib.h>
-#endif
-
-void fatal (const char* msg);
-void *xmalloc (size_t len);
-void *xrealloc (void *op, size_t len);
-void xprintf (const char *fmt, ...);
-char *vstrmake (size_t *lenp, const char *fmt, va_list ap);
-char *strmake (size_t *lenp, const char *fmt, ...);
-
-#endif /* __UTIL_H */
diff -uN programs/winetests.feri/winetests.h programs/winetests/winetests.h
--- programs/winetests.feri/winetests.h	2003-11-21 08:15:59.000000000 -0500
+++ programs/winetests/winetests.h	2003-11-22 03:05:10.000000000 -0500
@@ -1,6 +1,10 @@
 #ifndef __WINETESTS_H
 #define __WINETESTS_H

+#include <stdio.h>
+#include <stdlib.h>
+#include <stdarg.h>
+
 struct wine_test
 {
     const char *name;
@@ -17,4 +21,13 @@

extern const char build_tag[];

+void fatal (const char* msg);
+void *xmalloc (size_t len);
+void *xrealloc (void *op, size_t len);
+void xprintf (const char *fmt, ...);
+char *vstrmake (size_t *lenp, const char *fmt, va_list ap);
+char *strmake (size_t *lenp, const char *fmt, ...);
+
+int send_file (const char *name);
+
 #endif /* __WINETESTS_H */

[Ferenc Wagner]

Hi Dimi,

I appreciate your support very much.  Now it's just a quick
reflection on the patch you sent, before I can take a closer
look and testing.  Btw, if it's you who ran the program
under Wine, you may be interested in the results:

Tests from build Sat Nov 22 03:29:11 EST 2003
Operating system version:
    dwMajorVersion=4
    dwMinorVersion=0
    dwBuildNumber=67109814
    PlatformId=1
    szCSDVersion=
    wServicePackMajor=0
    wServicePackMinor=0
    wSuiteMask=0
    wProductType=0
    wReserved=64
Test output:
advapi32:registry start
Exit status: 1, errno=0: Success
advapi32:registry done
[...]
msvcrt:cpp
 start
Exit status: 1, errno=0: Success
msvcrt:cpp
 done
msvcrt:file
 start
Exit status: 1, errno=0: Success
msvcrt:file
 done
msvcrt:heap
 start
Exit status: 1, errno=0: Success
msvcrt:heap
 done
msvcrt:scanf
 start
Exit status: 1, errno=0: Success
msvcrt:scanf
 done
netapi32:access start
Exit status: 1, errno=0: Success
netapi32:access done
[...]

It is strange how the msvcrt test is different, but as a
whole it looks like a resource error (missing/wrong test
files).  I arrange the last report to be available at
http://afavant.elte.hu/~wferi/wine/report.

"Dimitrie O. Paun"  writes:
> -- better integration in the build system

You convinced me that this system is utterly and absolutely
far beyond me.  I'd have never invented the amazing
.res.res.cross.o quadruple extension...  Btw, did you take
into account that cross resources require a separate
resource script, too?  I can't tell it from your patch...
You've got RC_SRCS = winetests_so.rc, but what about _pe.rc?
Also, make depend stopped on generated source files for me
in previous experiments.  It may not be a problem.
> -- supports out of tree builds

What was needed for this?
> -- cleans up file when winetests fails

May I ask you to point it out?  I can't see it on fatals.
> -- supplies an automatic build label for unattended builds

Might the CVS date of ChangeLog not be better perhaps?
> -- remove the WINELIB flag (was always present anyway)

:) Not before I put the crosscompile commands into the
Makefile.  It definitely made a difference, but sure can be
traded for some extra #includes.
> -- renames removeDir to remove_dir for consistency

That function may be an overkill anyway.  All we have to do
is delete some files and a directory, all of which we know
the name of.  It that's not enough, at least one test is
buggy.

Thanks again, and will change that pesky indentation.

Feri.

[Dimitrie O. Paun]

On November 22, 2003 11:07 am, Ferenc Wagner wrote:
>            Hi Dimi,
>
> I appreciate your support very much.  Now it's just a quick
> reflection on the patch you sent, before I can take a closer
> look and testing.  Btw, if it's you who ran the program
> under Wine, you may be interested in the results:

Yes, it was me, thanks!
> It is strange how the msvcrt test is different, but as a
> whole it looks like a resource error (missing/wrong test
> files).

Yes, there was a problem. For unknown reasons, for msvcrt
I get this:
const char* subtests_for_test5[] = {
    "cpp^M",
    "file^M",
    "heap^M",
    "scanf^M",
};

That is, a <CR> at the end of the subtests name. I know
it's a problem, and it should be fixed, but I wanted
to send the patch out, it was way too late already.
Maybe you can look into that. If I discover something,
I'll send out a note.
> "Dimitrie O. Paun"  writes:
> > -- better integration in the build system
>
> You convinced me that this system is utterly and absolutely
> far beyond me.  I'd have never invented the amazing
> .res.res.cross.o quadruple extension...

It's not a quadruple extension. The rule describes how to
get from a .res file to a res.cross.o. The reason we need
that is that MinGW doesn't know how to link in a .res file
(as everybody does), they need to compile first the thing
to an object file. We can not use just the .cross.o ext
because it may conflict with something else (many projects
have a xxx.c file and a xxx.rc file). Hence the longer
extension.
> Btw, did you take into account that cross resources require 
> a separate resource script, too?  I can't tell it from your patch...

I hope I did. Namely this part:
    $(CROSSOBJS:_so.res.cross.o=_pe.res.cross.o)
> > -- supports out of tree builds
>
> What was needed for this?

We need to explicitly give the maketests script the
tests.list file. I did so as the second parameter:

$(SRCDIR)/maketests -r_so $(SRCDIR)/tests.list > $@
> > -- cleans up file when winetests fails
>
> May I ask you to point it out?  I can't see it on fatals.

This bit:
    ... || ( $(RM) $@ && exit 1 )

This is needed because the redirection always creates the output
file, even if maketests fails. So on failure we need to cleanup.
> > -- supplies an automatic build label for unattended builds
>
> Might the CVS date of ChangeLog not be better perhaps?

Maybe. But this one was simple. Problem is that the ChangeLog
doesn't tell you if the tree is modified or not, so it may be
misleading. This way at least we know that something may be
changed from the official version. Maybe a combination of the
two, but I don't think it's that important.
> > -- remove the WINELIB flag (was always present anyway)
>
> :) Not before I put the crosscompile commands into the
> Makefile.  It definitely made a difference, but sure can be
> traded for some extra #includes.

You are correct, it was making a difference, but not anymore
as it will get passed in when cross compiling as well 
(with my scheme of using the Make.rules.in rules).
> > -- renames removeDir to remove_dir for consistency
>
> That function may be an overkill anyway.  All we have to do
> is delete some files and a directory, all of which we know
> the name of.  It that's not enough, at least one test is
> buggy.

Feel free to do whatever you think it's more appropriate.
I don't mind having a more general function, it's not
that big, and at least it removes all possible future 
headaches associated with cleanup. I'd keep it.
> Thanks again, and will change that pesky indentation.

Thank you! :)

[Dimitrie O. Paun]

On November 22, 2003 11:45 am, Dimitrie O. Paun wrote:
> Yes, there was a problem. For unknown reasons, for msvcrt
> I get this:
> const char* subtests_for_test5[] = {
>     "cpp^M",
>     "file^M",
>     "heap^M",
>     "scanf^M",
> };

Here is the fix for this one (to maketests):

-           wine $filename | sed '1,/^Valid/d;s/    (.*)/    "\1",/'
+           wine $filename | sed '1,/^Valid/d;s/    ([0-9a-zA-Z_]*)/    "\1",/'

BTW, there's no need to make the script bash specific,
we can use expr instead:

-    i=$(($i+1))
+    i=`expr $i + 1`
> > > -- renames removeDir to remove_dir for consistency
> >
> > That function may be an overkill anyway.

I think we need it. Tests will/may create test dirs, with test files,
and so on. We need to be able to wipe the slate clean.

[David Laight]

> BTW, there's no need to make the script bash specific,
> we can use expr instead:
> - i=$(($i+1))
> - i=`expr $i + 1`

That isn't a bashism, it is required to be supported by any shell
that claims to be Posix compliant.
That probably means everything except an AT&T/SYSV bourne shell.

If you are in a loop then running expr will slow things down.
It is possible to count in the old bourne shell by doing:
    in="$in i"
    set -- $in
    i=$#
But unless you actually have a real problem it isn't worth while.
Solaris users need to put /usr/xpg4/bin before /usr/bin in PATH
to get a Posix compiant shell (aka ksh).

David

[Ferenc Wagner]

David Laight  writes:
> > BTW, there's no need to make the script bash specific,
> > we can use expr instead:
> > - i=$(($i+1))
> > - i=`expr $i + 1`
>
> That isn't a bashism, it is required to be supported by
> any shell that claims to be Posix compliant.  That
> probably means everything except an AT&T/SYSV bourne
> shell.

There we have it, the first Solaris user complaining.  We
seem to need the above patch after all...  Will submit.

Feri.

[David Laight]

On Sat, Dec 20, 2003 at 08:57:29PM +0100, Ferenc Wagner wrote:
> David Laight  writes:
> > > BTW, there's no need to make the script bash specific,
> > > we can use expr instead:
> > > - i=$(($i+1))
> > > - i=`expr $i + 1`
> >
> > That isn't a bashism, it is required to be supported by
> > any shell that claims to be Posix compliant.  That
> > probably means everything except an AT&T/SYSV bourne
> > shell.
>
> There we have it, the first Solaris user complaining.  We
> seem to need the above patch after all...  Will submit.

Just tell the Solaris user to put /usr/xpg4/bin on $PATH before /usr/bin.
Then 'sh' will be 'ksh' and posix compliant...

David

[Dimitrie O. Paun]

On December 20, 2003 05:08 pm, David Laight wrote:
> Just tell the Solaris user to put /usr/xpg4/bin on $PATH before /usr/bin.
> Then 'sh' will be 'ksh' and posix compliant...

This would be just us trying to be nasty.
There absolutely _zero_ reason to not use expr in this
case. And don't tell me the performance story, it's
just BS in this case (in fact, I'll bet you can't measure
_any_ significant difference between the build times of
winetests with and without 'expr').

[Ferenc Wagner]

David Laight  writes:
> Just tell the Solaris user to put /usr/xpg4/bin on $PATH
> before /usr/bin.  Then 'sh' will be 'ksh' and posix
> compliant...

We are talking about #!/bin/sh now.  $PATH is out of
consideration here, I'm afraid.

Feri.

[Alexandre Julliard]

"Dimitrie O. Paun"  writes:
> Here is the fix for this one (to maketests):
> -
>       wine $filename | sed '1,/^Valid/d;s/    \(.*\)/    "\1",/'
> -
>       wine $filename | sed '1,/^Valid/d;s/    \([0-9a-zA-Z_]*\)/    "\1",/'

This should really be done at run time, you shouldn't invoke wine
during the build process, it creates lots of annoying dependencies.

[Ferenc Wagner]

Alexandre Julliard  writes:
> "Dimitrie O. Paun"  writes:
> > Here is the fix for this one (to maketests):
> > -
> >       wine $filename | sed '1,/^Valid/d;s/    \(.*\)/    "\1",/'
> > -
> >       wine $filename | sed '1,/^Valid/d;s/    \([0-9a-zA-Z_]*\)/    "\1",/'
>
> This should really be done at run time, you shouldn't invoke wine
> during the build process, it creates lots of annoying dependencies.

What about this:

makename="$BINDIR/`dirname $test`/Makefile.in"
    [...]
            sed -n '/^CTESTS =/,/[^]$/{s/^CTESTS =//;s/\$//;s/([0-9a-zA-Z_]*).c/"\1",/g;p;}' $makename

that is, extracting the subtest names from the corresponding
Makefile.in?

Feri.

[Alexandre Julliard]

Ferenc Wagner  writes:
> What about this:
> makename="$BINDIR/`dirname $test`/Makefile.in"
> [...]
>         sed -n '/^CTESTS =/,/[^\]$/{s/^CTESTS =//;s/\\$//;s/\([0-9a-zA-Z_]*\)\.c/"\1",/g;p;}' $makename
>
> that is, extracting the subtest names from the corresponding
> Makefile.in?

It's still fairly ugly IMO. Doing this at run time would be a lot
cleaner.

[Ferenc Wagner]

Alexandre Julliard  writes:
> Ferenc Wagner  writes:
> > What about this:
> > makename="$BINDIR/`dirname $test`/Makefile.in"
> > [...]
> >         sed -n '/^CTESTS =/,/[^\]$/{s/^CTESTS =//;s/\\$//;s/\([0-9a-zA-Z_]*\)\.c/"\1",/g;p;}' $makename
> >
> > that is, extracting the subtest names from the corresponding
> > Makefile.in?
>
> It's still fairly ugly IMO. Doing this at run time would be a lot
> cleaner.

String processing in C is seldom clean, especially if you
want to make it portable...  But for the challenge, I gave
it a shot.

It took 79 (not exactly clean) lines (get_subtests).  If
you still prefer this, be it.  The build definitely got
simpler.

But anyway, please tell all your objections, so that we can
work towards making it suitable for inclusion!  I am sending
a tarball of the present state to this thread shortly.

Feri.

[Dimitrie O. Paun]

On November 25, 2003 02:09 pm, Ferenc Wagner wrote:
> I am sending a tarball of the present state to this 
> thread shortly.

... the suspense it's killin' me! :)

[Ferenc Wagner]

"Dimitrie O. Paun"  writes:
> On November 25, 2003 02:09 pm, Ferenc Wagner wrote:
> > I am sending a tarball of the present state to this 
> > thread shortly.
>
> ... the suspense it's killin' me! :)

Sorry, I really sent everything a long time ago, but a spam
flood seemingly broke the university mail services.  It's
starting to get better, though, so the wait may not be that
long any more...  :)
                                                Feri.

[Ferenc Wagner]

"Dimitrie O. Paun"  writes:
> For unknown reasons, for msvcrt I get this:
> const char* subtests_for_test5[] = {
>     "cpp^M",
>     "file^M",
>     "heap^M",
>     "scanf^M",
> };

Oh, silly me, I forgot that I already noticed this problem
and get rid of it by putting
sed '1,/^Valid/d;s/^M$//;s/^    (.*)$/    "\1",/'
into maketests.  I think I understand this: the msvcrt test
must be linked with msvcrt (:), so printf emits a CR at the
end of the lines.  If we ran the cross compiled tests to get
the names of the subtests, we'd get this all the time.

Thanks for the detailed explanations; I never use implicit
rules (prefer static patters).
> This bit:
>     ... || ( $(RM) $@ && exit 1 )

Ah, I needed that bad!  It was driving me crazy!
> I find the program too quite. I wasn't sure (until I got
> your email) that it ran at all! We should print the name
> of the currently running test, and when it completes the
> status (pass/fail).

I didn't want to bother with this until we decide on the
interface.  I don't know what it depends on whether a
Windows programs opens a console when run, and whether it
closes it instantly when finishes.  Basically, we have to
decide on the interface.  As a quick drop-in it may be
useful anyway.

Feri.

[Dimitrie O. Paun]

On November 22, 2003 12:32 pm, Ferenc Wagner wrote:
> Thanks for the detailed explanations; I never use implicit
> rules (prefer static patters).

Same here, but Alexandre prefers them. Why is that Alexandre?
> I didn't want to bother with this until we decide on the
> interface.  I don't know what it depends on whether a
> Windows programs opens a console when run, and whether it
> closes it instantly when finishes.  Basically, we have to
> decide on the interface.  As a quick drop-in it may be
> useful anyway.

Yes, plus it's trivial to implement, and we should always
have _some_ feedback, otherwise we'll confuse a lot of people.

[Alexandre Julliard]

"Dimitrie O. Paun"  writes:
> On November 22, 2003 12:32 pm, Ferenc Wagner wrote:
> > Thanks for the detailed explanations; I never use implicit
> > rules (prefer static patters).
>
> Same here, but Alexandre prefers them. Why is that Alexandre?

Portability, some versions of make don't have pattern rules.

[Dimitrie O. Paun]

On November 22, 2003 11:07 am, Ferenc Wagner wrote:
> Btw, if it's you who ran the program
> under Wine, you may be interested in the results:

Speaking of which, I find the program too quite. I wasn't
sure (until I got your email) that it ran at all! We should
print the name of the currently running test, and when it
completes the status (pass/fail). The output would look like:

....
msvcrt
    cpp: fail
    file: pass
    heap:

Whatdayathink?

[Ferenc Wagner]

"Dimitrie O. Paun"  writes:
> -- cross compilation not tested at this point

Has a problem, indeed:
../../include/wine/port.h:87: dlfcn.h: No such file or directory

Understandable, as gcc has dlfcn.h, but the MinGW gcc does
not.  But commenting out the offending #define in config.h
results in successful compilation.  Impressive work! (modulo
one c&p typo in Makefile.in and winetests_so.rc not cleaned)

Btw, msvcrt:file leaves a file called wne* in %TEMP% and
shlwapi:string takes forever (probably not, but I killed it).

State of affairs attached.  I didn't use popen() because Wine
doesn't support it.

Feri.

[Alexandre Julliard]

Ferenc Wagner  writes:
> Has a problem, indeed:
> ../../include/wine/port.h:87: dlfcn.h: No such file or directory

That's because if you want to cross-compile you need to
cross-configure too. The crosstest stuff is a hack that works
because the tests don't use Wine-specific features, but it cannot
be generalized to the rest of the tree.

The right way to do it is to create two separate build trees, one in
which you compile normally and one in which you cross-compile.
Something like this (assuming your Wine source is in wine-src):

mkdir wine-elf; cd wine-elf
../wine-src/configure
make depend; make
cd ..
mkdir wine-pe; cd wine-pe
../wine-src/configure --host=i586-mingw32msvc --with-wine-tools=../wine-elf
make depend; make

(a few dlls won't build with mingw, you can just skip them).

[Ferenc Wagner]

Alexandre Julliard  writes:
> Ferenc Wagner  writes:
> > Has a problem, indeed:
> > ../../include/wine/port.h:87: dlfcn.h: No such file or directory
>
> That's because if you want to cross-compile you need to
> cross-configure too. The crosstest stuff is a hack that works
> because the tests don't use Wine-specific features, but it cannot
> be generalized to the rest of the tree.
>
> The right way to do it is to create two separate build trees, one in
> which you compile normally and one in which you cross-compile.
> Something like this (assuming your Wine source is in wine-src):
>
> mkdir wine-elf; cd wine-elf
> ../wine-src/configure
> make depend; make
> cd ..
> mkdir wine-pe; cd wine-pe
> ../wine-src/configure --host=i586-mingw32msvc --with-wine-tools=../wine-elf
> make depend; make
>
> (a few dlls won't build with mingw, you can just skip them).

Is not

#ifdef _WIN32
#  include <io.h>
#  include <process.h>
#else
#  include "config.h"
#  include "wine/port.h"
#  define mkdir(d) mkdir (d, 0777)
#endif

an acceptable workaround?  This should be compilable even
with MSVC after all, and Dimi has already invented all the
magic needed for a successful make winetests_cross.exe.

Feri.

[Alexandre Julliard]

Ferenc Wagner  writes:
> Is not
>
> #ifdef _WIN32
> #  include <io.h>
> #  include <process.h>
> #else
> #  include "config.h"
> #  include "wine/port.h"
> #  define mkdir(d) mkdir (d, 0777)
> #endif
>
> an acceptable workaround?  This should be compilable even
> with MSVC after all, and Dimi has already invented all the
> magic needed for a successful make winetests_cross.exe.

I'm not sure why you don't want to use the normal mechanism for
that. Dimi's patch cannot do the right thing, as you have noticed
yourself, since it doesn't use the right config.h; and I don't think
we want to add such #ifdefs to all the programs.

[Ferenc Wagner]

Alexandre Julliard  writes:
> Ferenc Wagner  writes:
> > #ifdef _WIN32
> > #  include <io.h>
> > #  include <process.h>
> > #else
> > #  include "config.h"
> > #  include "wine/port.h"
> > #  define mkdir(d) mkdir (d, 0777)
> > #endif
>
> I'm not sure why you don't want to use the normal
> mechanism for that.

Well, I did try the method you suggested (cross configuring
in a separate build tree), and to my amazement it almost
worked without even issuing make in the top directory (I am
jumpy to do that on my 200 MHz machine with no free space on
the HD).  Of course it did not find -lwine and -lwine_port,
but did not need them either, so copying the failed linker
command from make's output worked after omitting those two
libs.

This clean and elegant method obviously saves on the number
of lines in maketests and Makefile.in, but has a rather big
space and time overhead.  And, basically, this program
belongs to the tests, so why not use the same solution for
the same problem again?
> I don't think we want to add such #ifdefs to all the
> programs.

That #ifdef is necessary anyway because of the mkdir()
prototype mismatch, I am afraid.  And it's only a single
program...

Having said that, a cross configuring version in the tree is
better than three in my hands, but please give this a last
consideration before your final decision!

Thanks,
Feri.

[Dimitrie O. Paun]

On November 25, 2003 02:53 pm, Ferenc Wagner wrote:
> State of affairs attached.  I didn't use popen() because Wine
> doesn't support it.

OK,

This looks a lot cleaner. Very, very nice! There are still
a few nits, but I think it should go in so we can send small
incremental patches. Here are the few problems that I still see:

-- the mkdir() issue is still not 100% solved. We currently have:
#ifndef _WIN32
#  define mkdir(d) mkdir (d, 0777)
#endif
     which is not too bad, but it will not work if we compile with
     winegcc. We don't need to solve it right now, but it would be
     nice to do so. Why don't we just link with msvcrt? Or we can
     change the test to:
#if defined(__unix__) && !defined(__MSVCRT__)
     which should work with all: gcc/winegcc/mingw/msvc

-- we use fatal() in remove_dir()
     Again, not a problem, but I'm wondering if we're not too strict.
     Maybe we should just issue warnings for those case, no need to
     kill the entire run, is it?

-- running ELF tests
     When we do so, we use a hardcoded path for wine 
    /usr/local/src/wine/wine
     This is not going to work in general, but it's something we can
     live with for now, as running ELF tests is mainly interesting
     for developers like me who don't have a Windows box easily
     accessible.
  -- we're linking against ws2_32
     Is this DLL available on all versions of Windows we want to run on?
     Remember, we want this to run on all versions of Windows...

In all honesty, I can't see any showstoppers -- the code looks
very neat and clean (thanks Feri!), and the remaining issues can
be solved after inclusion. We have a lot of work to do to close
the loop: web-site integration, getting someone to do builds
and post them on SourceForge (and notify the testers), signing
up testers, etc.

[Ferenc Wagner]

"Dimitrie O. Paun"  writes:
> -- the mkdir() issue is still not 100% solved. We currently have:
> #ifndef _WIN32
> #  define mkdir(d) mkdir (d, 0777)
> #endif
>      [...] Why don't we just link with msvcrt?

"Dimitrie O. Paun"  also wrote:
> "Doctor, it hurts when I do that..." :))) I'd say, don't
> do it.  Just use libc calls, don't link against msvcrt.

That's why.  To preserve our sanity.  Maybe it's not worth it.
>  Or we can change the test to:
>
> #if defined(__unix__) && !defined(__MSVCRT__)
>      which should work with all: gcc/winegcc/mingw/msvc

It may be the easier path.  But this strongly depends on
Alexandre's verdict.  See the discussion on the other branch
of this thread.
> -- we use fatal() in remove_dir()
>      Again, not a problem, but I'm wondering if we're not too strict.

Sure, we are.  In the beginning, nothing useful happened
after remove_dir(), so I just didn't care.  Moving send_file
before this would be useful indeed.  Someone someday may
create a real UI for this and handle it properly.
> -- running ELF tests
>      When we do so, we use a hardcoded path for wine

Oh yes, that's for me who does not have wine in the path.
We could just remove the path, I could add wine to my PATH,
and that's it.
> -- we're linking against ws2_32
>      Is this DLL available on all versions of Windows we
>      want to run on?

As far I know
http://msdn.microsoft.com/library/en-us/winsock/winsock/windows_sockets_start_page_2.asp
yes.  But I am not sure whether it is installed in the base
system or only an option if you want to have networking
(like on Win3.1).  Probably not too many people would run
the tests on a standalone machine anyway, but who knows...
> I can't see any showstoppers

Good to you. :)  What do you think about Alexandre's cross-
configuring idea?

Feri.

[Dimitrie O. Paun]

On December 2, 2003 05:48 am, Ferenc Wagner wrote:
> "Dimitrie O. Paun"  writes:
> > -- the mkdir() issue is still not 100% solved. We currently have:
> > #ifndef _WIN32
> > #  define mkdir(d) mkdir (d, 0777)
> > #endif
> >      [...] Why don't we just link with msvcrt?
>
> "Dimitrie O. Paun"  also wrote:
> > "Doctor, it hurts when I do that..." :))) I'd say, don't
> > do it.  Just use libc calls, don't link against msvcrt.
>
> That's why.  To preserve our sanity.  Maybe it's not worth it.

:))) I know there was a reason, but I can't quite remember
what the problem was. :)
> >  Or we can change the test to:
> >
> > #if defined(__unix__) && !defined(__MSVCRT__)
> >      which should work with all: gcc/winegcc/mingw/msvc
>
> It may be the easier path.  But this strongly depends on
> Alexandre's verdict.  See the discussion on the other branch
> of this thread.

Well, unfortunately, the mkdir() bit is a ugly wart. I've run
into it in other projects (wxWindows comes to mind). If anyone
can figure out a solution that doesn't involve modifying the
app, let me know. Until then this should do.
> > -- we use fatal() in remove_dir()
> >      Again, not a problem, but I'm wondering if we're not too strict.
>
> Sure, we are.  In the beginning, nothing useful happened
> after remove_dir(), so I just didn't care.  Moving send_file
> before this would be useful indeed.  Someone someday may
> create a real UI for this and handle it properly.

Until then, how about we replace the fatal() call to a
warning() call which does not exit?
> > -- running ELF tests
> >      When we do so, we use a hardcoded path for wine
>
> Oh yes, that's for me who does not have wine in the path.
> We could just remove the path, I could add wine to my PATH,
> and that's it.

Well, let's do that then :)
> > -- we're linking against ws2_32
> >      Is this DLL available on all versions of Windows we
> >      want to run on?
>
> As far I know
> http://msdn.microsoft.com/library/en-us/winsock/winsock/windows_sockets_st
> art_page_2.asp yes.

It's OK then. We need to depend on _something_ to send out
the results, this seems like a decent dependency.
> > I can't see any showstoppers
>
> Good to you. :)  What do you think about Alexandre's cross-
> configuring idea?

Well, it's a clean idea. The CROSSCC stuff that I've adopted
from the tests it's a bit of a hack, and I do not understand
why we have it in the first place. It's convenient, but a bit
ugly (and I guess it survives since it's so small). However,
as you pointed out, if we do use it for tests, why not for
winetests, it fits in the same category, it's convenient, and
we can use it almost as is (this is the only thing we do in
our Makefile):

winetests_cross.exe: $(CROSSOBJS:_so.res.cross.o=_pe.res.cross.o)
        $(CROSSCC) $^ -o $@ $(IMPORTS:%=-l%) $(LIBS)

So Alexandre, what say you? :) We'd really like to get this in
and move forward, there still so much work to do on this front.
Let's not lose momentum.

[Alexandre Julliard]

"Dimitrie O. Paun"  writes:
> Well, unfortunately, the mkdir() bit is a ugly wart. I've run
> into it in other projects (wxWindows comes to mind). If anyone
> can figure out a solution that doesn't involve modifying the
> app, let me know. Until then this should do.

Without modifying the app I don't know, but in this case you could use
CreateDirectory instead.
> Well, it's a clean idea. The CROSSCC stuff that I've adopted
> from the tests it's a bit of a hack, and I do not understand
> why we have it in the first place. It's convenient, but a bit
> ugly (and I guess it survives since it's so small). However,
> as you pointed out, if we do use it for tests, why not for
> winetests, it fits in the same category, it's convenient, and
> we can use it almost as is (this is the only thing we do in
> our Makefile):
>
> winetests_cross.exe: $(CROSSOBJS:_so.res.cross.o=_pe.res.cross.o)
>         $(CROSSCC) $^ -o $@ $(IMPORTS:%=-l%) $(LIBS)
>
> So Alexandre, what say you? :) We'd really like to get this in
> and move forward, there still so much work to do on this front.
> Let's not lose momentum.

The crosstest stuff is a hack that's here to make it possible to do a
quick check under Windows when you modify a test. I don't really see
the same need for winetests, so I'd suggest we do things the clean way
first. If it turns out we really need the hack we can always add it
later.

Also a minor detail: I'd propose to rename winetests to winetest,
since we already have a directory by that name in CVS, it avoids
having too many ugly useless dirs in the repository (plus it fits
in a 8.3 file name ;-)

[Dimitrie O. Paun]

On December 2, 2003 02:39 pm, Alexandre Julliard wrote:
> Without modifying the app I don't know, but in this case you could use
> CreateDirectory instead.

Of course we can, for whatever reason we go stuck into using
the std C lib. Not a problem.
> The crosstest stuff is a hack that's here to make it possible to do a
> quick check under Windows when you modify a test. I don't really see
> the same need for winetests, so I'd suggest we do things the clean way
> first. If it turns out we really need the hack we can always add it
> later.

Sure, we'll do it. Thinking about it, it doesn't take that much to
have a different tree anyway, because you would need different obj
files anyway, so I don't think it takes any significant space overhead
to have a different tree, right? In which case the hack simply saves
you to switch to the console that is in the cross-tree...
> Also a minor detail: I'd propose to rename winetests to winetest,
> since we already have a directory by that name in CVS, it avoids
> having too many ugly useless dirs in the repository (plus it fits
> in a 8.3 file name ;-)

Old habits die hard, eh? :))) Consider it done.