Re: Autoconfiscation patch for GTK+ on Win32

[Owen Taylor <otaylor redhat com>]

OK, I took a look through your patch now. It generally looks fine
(as much as I can tell), and should be fine to commit. Various comments 
are below.

I'm planning to do a 1.3.10 release at the beginning of next week.
Should we be thinking about testing those tarballs on Win32 as
well as on X11, or are we still not at that point yet?

Regards,
                                        Owen

> +AC_MSG_CHECKING([for native Win32])
> +case "$host" in
> +  *-*-mingw*)
> +    os_win32=yes
> +
> +    AS="$CC"
> +    ASFLAGS="$CFLAGS"
> +    AC_SUBST(AS)
> +    AC_SUBST(ASFLAGS)

Something to this effect was committed by James Henstridge yesterday 
cross-platform.

> -AC_ARG_WITH(gdktarget, [  --with-gdktarget=[x11/linux-fb] select GDK target [default=x11] ],
> +if test "$platform_win32" = yes; then
> +  gdktarget=win32
> +  gdktargetlib=libgdk-win32-1.3.la
> +  gtktargetlib=libgtk-win32-1.3.la
> +else
> +  gdktarget=x11
> +  gdktargetlib=libgdk-x11-1.3.la
> +  gtktargetlib=libgtk-x11-1.3.la
> +fi

I think we can just do:

 gdktargetlib=libgdk-$gdktarget-1.3.la
 gtktargetlib=libgtk-$gdktarget-1.3.la

And handle these two and linux-fb as well.

> -# libtool option to control which symbols are exported
> -# right now, symbols starting with _ are not exported
> -LIBTOOL_EXPORT_OPTIONS='-export-symbols-regex "^[[^_]].*"'
> +if test "$os_win32" != yes; then
> +    # libtool option to control which symbols are exported
> +    # right now, symbols starting with _ are not exported
> +    LIBTOOL_EXPORT_OPTIONS='-export-symbols-regex "^[[^_]].*"'
> +else
> +    LIBTOOL_EXPORT_OPTIONS=
> +fi
>  AC_SUBST(LIBTOOL_EXPORT_OPTIONS)

I think long term we probably need to get rid of the manually
maintained .def files - it seems like an uneccessary maintainence
burden. I think the right approach is probably to have a perl script
that scans the headers for functions, removes underscore prefixed
functions, and generates export lists for libtool and def files
for Windows. 

(Main problem here is that we probably can't depend on Perl
existing even for people building from CVS on Windows, so there
is a bit of a bootstrap problem.) 

The current scheme and the above should be OK for now.
  
> +if test "x$gdktarget" = "xwin32"; then
> +  # We start off with the libraries from Pango
> +
> +  ## be sure we also have Pango built with win32 or ft2 support
> +  if $PKG_CONFIG --exists pangoft2 ; then
> +    PANGO_PACKAGES="pangowin32 pangoft2"
> +    have_ft2=true
> +    AC_DEFINE(HAVE_FT2)

Why are we pulling in pangoft2 library here? This doesn't make
sense to me. (Also, it looks to me like PANGO_PACKAGES gets 
overwritten further down.)

> +  win32_libs="`$PKG_CONFIG --libs $PANGO_PACKAGES`"
> +  win32_cflags="`$PKG_CONFIG --cflags $PANGO_PACKAGES`"
> +  win32_extra_libs=
> +
> +  ## Strip the .la files
> + 
> +  win32_libs_for_checks=""
> +  for I in $win32_libs ; do
> +    case $I in 
> +      *.la) ;;
> +      *) win32_libs_for_checks="$win32_libs_for_checks $I" ;;
> +    esac
> +  done
> +
> +  # Sanity check for the gdi32 library
> +  AC_CHECK_LIB(gdi32, CreateDCA 16, :,
> +            AC_MSG_ERROR([*** libgdi32 not found. Check 'config.log' for more details.]),
> +            $win32_libs_for_checks)
> +
> +  GDK_PIXBUF_WIN32_EXTRA_CFLAGS="$win32_cflags"
> +  GDK_PIXBUF_WIN32_EXTRA_LIBS="$win32_extra_libs `$PKG_CONFIG --libs pangowin32`"

Do we have a gdk-pixbuf-win32 library? GDK_PIXBUF_XLIB_EXTRA_CFLAGS/LIBS
are for the gdk-pixbuf-xlib library in contrib/ for using GdkPixbuf
without Gdk.

> +  win32_cflags="$WIN32_CFLAGS"
> +  win32_ldflags="$WIN32_LDFLAGS"

What's the purpose of setting these variables ... they don't 
seem to be used anywhere.

> +if USE_WIN32
> +libgdk_win32_includedir = $(includedir)/gtk-2.0/gdk
> +
> +libgdk_win32_1_3_la_LIBADD = \
> +	$(gtarget)/libgdk-$(gtarget).la
> +
> +libgdk_win32_include_HEADERS = $(gdk_headers)
> +libgdk_win32_1_3_la_SOURCES = $(gdk_c_sources) gdkenumtypes.c
> +
> +libgdk_win32_1_3_la_LDFLAGS = $(gdk_win32_symbols) -lgdi32 -user32 -limm32 -lshell32 -lole32 -luuid

Should these be added in configure.in? For X11, dependency libraries
definitely have to be in configure.in so they can make their way 
into the .pc file, but if we have reliable inter-library dependencies
on Win32 that may not be necessary.

> +GDKVAR
>  GMutex           *gdk_threads_mutex = NULL;          /* Global GDK lock */

Should be on one line here.

> @@ -253,4 +259,23 @@
>  #ifdef G_OS_WIN32
>  
> +/* DllMain function needed to tuck away the GLib DLL name */
> +
> +static char dll_name[MAX_PATH];
> +
> +BOOL WINAPI
> +DllMain (HINSTANCE hinstDLL,
> +	 DWORD     fdwReason,
> +	 LPVOID    lpvReserved)
> +{
> +  switch (fdwReason)
> +    {
> +    case DLL_PROCESS_ATTACH:
> +      GetModuleFileName ((HMODULE) hinstDLL, dll_name, sizeof (dll_name));
> +      break;
> +    }
> +
> +  return TRUE;
> +}
> +

Perhaps we should have something like:

 G_WIN32_DEFINE_DLL_NAME (static, dll_name)

This seems like a lot of magic code to cut-and-paste into multiple
places. 

(I guess the windows.h include is a bit of a pain ... is using
'extern' or something like that to define the one function possible?
Even if you had to make the macro include windows.h, I think it might
be better than cut-and-paste.)

> +libgtk_win32_1_3_la_LDFLAGS = $(gtk_win32_symbols) -lwsock32

Same question as for GDK.


> Index: gtk/gtkfilesel.c
> ===================================================================
> RCS file: /cvs/gnome/gtk+/gtk/gtkfilesel.c,v
> retrieving revision 1.85
> diff -u -2 -r1.85 gtkfilesel.c
> --- gtk/gtkfilesel.c	2001/09/19 00:49:51	1.85
> +++ gtk/gtkfilesel.c	2001/10/09 23:58:16
> @@ -45,4 +45,7 @@
>  #include <pwd.h>
>  #endif
> +#if 1 /* def HAVE_WINSOCK_H */

Please fix :-)

> +#include <winsock.h>		/* For gethostname */
> +#endif


> +static const gchar *
> +gtk_win32_libdir (void)
> +{
> +  static char *gtk_libdir = NULL;
> +  if (gtk_libdir == NULL)
> +    gtk_libdir = g_win32_get_package_installation_subdirectory
> +      (GETTEXT_PACKAGE, _gtk_win32_dll_name, "lib");
> +
> +  return gtk_libdir;
> +}
> +
> +static const gchar *
> +gtk_win32_localedir (void)
> +{
> +  static char *gtk_localedir = NULL;
> +  if (gtk_localedir == NULL)
> +    gtk_localedir = g_win32_get_package_installation_subdirectory
> +      (GETTEXT_PACKAGE, _gtk_win32_dll_name, "lib\\locale");
> +
> +  return gtk_localedir;
> +}
> +
> +#undef GTK_LIBDIR
> +#define GTK_LIBDIR gtk_win32_libdir()
> +
> +#undef GTK_LOCALEDIR
> +#define GTK_LOCALEDIR gtk_win32_localedir()

Instead of this sort of hackery, why don't we just define

 _gtk_get_libdir()
 _gtk_get_localedir()
 _gtk_get_sysconfdir()
 _gtk_get_data_prefix()

in gtkmain.c, put them in gtkprivate.h and then use them where needed
instead of the macros. Yes, it is marginally less efficient, but
code cleanliness is more important.