GNOME Bugzilla – Bug 342926
Win32 build fixes
Last modified: 2012-03-29 14:09:13 UTC
I've been trying to get Totem to build on Windows, so I can build RB. I'll attach build fixes to the bug.
Created attachment 66195 [details] [review] totem-plparser compilation fixes realpath() doesn't exist on windows, so I had to replace the symlink-resolving code. I've replaced it with the gnomevfs-using code from RB (which I wrote, so can be relicenced gpl+exception). There are also two missing #ifdef HAVE_HAL bits. With this patch, all that it needed to compile totem-plparser is some autofoo changes, which I'll attach once I clean them up.
I would prefer if realpath was just replaced. The rest of the code should still work, and shouldn't be replaced. I've committed the 2 HAVE_HAL fixes already.
Created attachment 67035 [details] [review] new patch This patch contains all the changes needed for me to compile on Windows, although I haven't tried running it yet (due to GStreamer issues). I can split up the patch if needed. configure.in, src/backend/Makefile.am: two groups of changes. The first is checking what target GTK is build against and using that instead of hard-coding gtk+-x11-2.0, and adding HAVE_GTK_X11 or HAVE_GTK_WIN32 so we can use #ifdefs. The second putting the GStreamer/Xine CFLAGS in BACKEND_CFLAGS and equivalent for LIBS; this means that totem-plparser doesn't get linked against them. src/plparse/totem-disc.c: I realised that Windows doesn't have device nodes as files, so there obviously isn't going to be any symlinks to them. A simple #ifdef to return NULL on windows. Other files: Putting the X-specific code in #ifdefs. There will probably need to be equivalent code for Windows in many of them, but I don't know what it is.
Comment on attachment 67035 [details] [review] new patch I'm not very happy with the build changes, but will need to review them properly later. >diff -u -p -u -r1.71 bacon-video-widget-gst-0.10.c >--- src/backend/bacon-video-widget-gst-0.10.c 23 May 2006 08:55:23 -0000 1.71 >+++ src/backend/bacon-video-widget-gst-0.10.c 9 Jun 2006 11:49:46 -0000 >@@ -51,7 +51,9 @@ > #include <stdio.h> > > /* gtk+/gnome */ >+#if HAVE_GTK_X11 > #include <gdk/gdkx.h> >+#endif > #include <gtk/gtk.h> > #include <glib/gi18n.h> > #include <gconf/gconf-client.h> >@@ -483,7 +485,9 @@ bacon_video_widget_expose_event (GtkWidg > BaconVideoWidget *bvw = BACON_VIDEO_WIDGET (widget); > GstXOverlay *xoverlay; > gboolean draw_logo; >+#if HAVE_GTK_X11 > XID window; >+#endif This should be GdkNativeWindow instead of an XID, and cast properly when used. > if (event && event->count > 0) > return TRUE; >@@ -499,10 +503,12 @@ bacon_video_widget_expose_event (GtkWidg > > g_mutex_unlock (bvw->priv->lock); > >+#if HAVE_GTK_X11 > window = GDK_WINDOW_XWINDOW (bvw->priv->video_window); > > if (xoverlay != NULL && GST_IS_X_OVERLAY (xoverlay)) > gst_x_overlay_set_xwindow_id (xoverlay, window); >+#endif Please replace the closing #endif by something like: #else #warning Portion of code not implemented for this platform #endif so that it's clear it's not supposed to work. >diff -u -p -u -r1.33 totem-disc.c >--- src/plparse/totem-disc.c 25 May 2006 15:37:48 -0000 1.33 >+++ src/plparse/totem-disc.c 9 Jun 2006 11:49:47 -0000 >@@ -36,7 +36,9 @@ > #include <errno.h> > #include <string.h> > >+#ifndef HAVE_GTK_WIN32 > #include <sys/ioctl.h> >+#endif It's not used anymore, even on Linux IIRC. > #include <sys/stat.h> > > #include <glib.h> >@@ -71,6 +73,7 @@ typedef struct _CdCache { > gboolean mounted; > } CdCache; > >+#ifndef HAVE_GTK_WIN32 This has nothing to do with the windowing system being used. Please use G_OS_WIN32 instead. > { >+#ifndef HAVE_GTK_WIN32 Same here
I just noticed we can also use "GDK_WINDOWING_X11" to check for X11 stuff. James, would you be able to test an updated patch?
Created attachment 74869 [details] [review] build fixes This contains two basic build fixes, correcting a type in a Makefile.am and a missing semicolon which breaks --enable-gtk
2006-10-17 Bastien Nocera <hadess@hadess.net> * browser-plugin/idl/Makefile.am: * src/totem.c: (totem_callback_connect): Fix typos spotted by James Livingston <jrl@ids.org.au> Fixed in HEAD and gnome-2-16, thanks.
Created attachment 74872 [details] [review] basic build fixes This patch has some basic build fixes: * don't link against gtk+-x11-2.0, since gtk+-{target}-2.0 gets pulled in automatically by gtk+-2.0 * use a few glib functions instead of unix ones (e.g. mkdir) * #ifdef XInitThreads and move g_thread_init() before gtk_init() * remove unused headers
Created attachment 74873 [details] [review] more porting * replaces use of lstat() to determine if a file is a symlink with g_file_test * provide implementation of realpath() on windows * use TerminateProcess() instead of kill() on windows
Created attachment 74874 [details] [review] screensaver port puts #ifdefs around the X11-specific screensaver code and emits a #warning about being unimplemented on other platforms. Potentially someone could actually implement the win32 screensaver blocking stuff.
Created attachment 74876 [details] [review] bacon-video-widget #ifdefs out all of the X overlay bits. AFAICT there isn't an equivalent on windows and nothing should happen, so I haven't put #warnings in for other platforms.
James, for the overlay, a warning should be added. The problem is that no interface for embedding on win32 exists yet, that has to be implemented in addition to the application bit that calls the embedding API (XOverlay on X, something-which-doesn't-exist-yet on win32. The warning should probably make that clear.
Comment on attachment 74873 [details] [review] more porting >diff -u -p -r1.5 totem-gromit.c >--- src/totem-gromit.c 8 Feb 2005 18:04:14 -0000 1.5 >+++ src/totem-gromit.c 17 Oct 2006 13:36:39 -0000 Just ifdef what you can out of this code, Gromit only exists on X11. <snip> >+#ifdef G_OS_UNIX >+ kill ((p-id_t) pid, SIGKILL); ^^^^^^ typo though ;) <snip> >diff -u -p -r1.43 totem-disc.c >--- src/plparse/totem-disc.c 30 Sep 2006 16:07:03 -0000 1.43 >+++ src/plparse/totem-disc.c 17 Oct 2006 13:36:42 -0000 >@@ -80,6 +80,27 @@ typedef struct _CdCache { > * Resolve relative paths > */ > >+#ifdef G_OS_WIN32 >+static char * >+realpath (const char *pathname, char resolved_path[PATH_MAX]) Could you work on getting that added to glib as well? <snip>
Comment on attachment 74874 [details] [review] screensaver port >diff -u -p -r1.24 totem-scrsaver.c >--- src/totem-scrsaver.c 16 Oct 2006 13:12:39 -0000 1.24 >+++ src/totem-scrsaver.c 17 Oct 2006 14:58:56 -0000 <snip> > #ifdef HAVE_XTEST >- if (scr->priv->have_xtest == True) >+ if (scr->priv->have_xtest) I would prefer: if (scr->priv->have_xtest != FALSE) <snip> >+ if (scr->priv->have_xtest) Here as well <snip>
Comment on attachment 74876 [details] [review] bacon-video-widget Same comment as Ronald for the bacon-video-widget stuff. The totem-interface bits look good (already filed to be added to GTK+, hopefully, so we can remove it soon). Don't forget to return NULL though.
I've committed the first and screensaver patches (after the boolean if test changes).
2007-03-19 Bastien Nocera <hadess@hadess.net> * src/plparse/totem-disc.c: (totem_resolve_symlink): remove home-made canonicalise function, helps Win32 support (Helps: #342926)
Created attachment 86418 [details] [review] updated patch This patch contains all the changed needed to get totem to compile and link on windows (for me, anyway). There are four pieces: * use rlimit on unix systems only, do nothing on windows (we might be able to do something, though this only affects the thumbnailer). * add a new function get_gdk_window_native_window, which does either GDK_WINDOW_XWINDOW or GDK_WINDOW_HWND depending on the platform, and use it. gst_x_overlay_set_xwindow_id() is still used on Windows, but what you pass it is slightly different. * #ifdef'd and stubbed out the bit in totem-interface which gets the top-level of the GtkPlug. It'd be nice if Gtk had something to do this, and I'm not sure how to do it in Windows. * bacon-message-connection can't use Unix domain sockets. I've just #ifdef'd the implementation out with stubs, GtkUnique or similar would be nice :)
Comment on attachment 86418 [details] [review] updated patch >Index: src/totem-resources.c >=================================================================== >--- src/totem-resources.c (revision 4231) >+++ src/totem-resources.c (working copy) >@@ -35,7 +35,10 @@ > #include <stdlib.h> > #include <sys/types.h> > #include <sys/stat.h> >+ >+#ifdef G_OS_UNIX > #include <sys/resource.h> >+#endif Wouldn't it be better to check for rlimit in configure.in? <snip> > static gpointer >Index: src/backend/bacon-video-widget-gst-0.10.c >=================================================================== >--- src/backend/bacon-video-widget-gst-0.10.c (revision 4231) >+++ src/backend/bacon-video-widget-gst-0.10.c (working copy) >@@ -59,7 +59,6 @@ > #include <stdio.h> > > /* gtk+/gnome */ >-#include <gdk/gdkx.h> > #include <gtk/gtk.h> > #include <glib/gi18n.h> > #include <gconf/gconf-client.h> >@@ -231,6 +230,28 @@ static int bvw_signals[LAST_SIGNAL] = { > GST_DEBUG_CATEGORY (_totem_gst_debug_cat); > #define GST_CAT_DEFAULT _totem_gst_debug_cat > >+ >+ >+#if defined (GDK_WINDOWING_X11) >+#include <gdk/gdkx.h> >+static gulong >+get_gdk_window_native_window (GdkWindow *window) >+{ >+ return GDK_WINDOW_XWINDOW (window); >+} >+#elif defined (GDK_WINDOWING_WIN32) >+#include <gdk/gdkwin32.h> >+static gulong >+get_gdk_window_native_window (GdkWindow *window) >+{ >+ return (gulong)GDK_WINDOW_HWND (window); >+} >+#else >+#warning unimplemented >+#endif Looks good except it should return GdkNativeWindows instead of gulong. >Index: src/bacon-message-connection.c >=================================================================== >--- src/bacon-message-connection.c (revision 4231) >+++ src/bacon-message-connection.c (working copy) >@@ -19,14 +19,18 @@ > > #include "config.h" > >+#include <glib.h> >+ > #include <sys/types.h> > #include <sys/stat.h> > #include <fcntl.h> > #include <stdio.h> > #include <string.h> > #include <unistd.h> >+#ifdef G_OS_UNIX > #include <sys/socket.h> > #include <sys/un.h> >+#endif > #include <errno.h> > > #include "bacon-message-connection.h" >@@ -57,6 +61,7 @@ struct BaconMessageConnection { > gpointer data; > }; > >+#ifdef G_OS_UNIX > static gboolean > test_is_socket (const char *path) > { >@@ -285,6 +290,7 @@ try_client (BaconMessageConnection *conn > > return setup_connection (conn); > } >+#endif > > BaconMessageConnection * > bacon_message_connection_new (const char *prefix) >@@ -294,6 +300,7 @@ bacon_message_connection_new (const char > g_return_val_if_fail (prefix != NULL, NULL); > > conn = g_new0 (BaconMessageConnection, 1); >+#ifdef G_OS_UNIX > conn->path = socket_filename (prefix); > > if (test_is_socket (conn->path) == FALSE) >@@ -321,6 +328,9 @@ bacon_message_connection_new (const char > conn->is_server = TRUE; > return conn; > } >+#else >+ conn->fd = -1; >+#endif > > conn->is_server = FALSE; > return conn; >@@ -381,10 +391,12 @@ bacon_message_connection_send (BaconMess > g_return_if_fail (conn != NULL); > g_return_if_fail (message != NULL); > >+#ifdef G_OS_UNIX > g_io_channel_write_chars (conn->chan, message, strlen (message), > NULL, NULL); > g_io_channel_write_chars (conn->chan, "\n", 1, NULL, NULL); > g_io_channel_flush (conn->chan, NULL); >+#endif > } Quite hackish, but fair enough. > gboolean >Index: src/totem-interface.c >=================================================================== >--- src/totem-interface.c (revision 4231) >+++ src/totem-interface.c (working copy) >@@ -34,7 +34,9 @@ > #include <glib.h> > #include <glib/gi18n.h> > #include <gtk/gtk.h> >+#ifdef GDK_WINDOWING_X11 > #include <gdk/gdkx.h> >+#endif > > #include "totem-interface.h" > >@@ -181,16 +183,18 @@ totem_interface_get_full_path (const cha > static GdkWindow * > totem_gtk_plug_get_toplevel (GtkPlug *plug) > { >- Window root, parent, *children; >- guint nchildren; > GdkNativeWindow xid; > > g_return_val_if_fail (GTK_IS_PLUG (plug), NULL); > > xid = gtk_plug_get_id (plug); > >+#ifdef GDK_WINDOWING_X11 > do > { >+ Window root, parent, *children; >+ guint nchildren; >+ > /* FIXME: multi-head */ > if (XQueryTree (GDK_DISPLAY (), xid, &root, > &parent, &children, &nchildren) == 0) >@@ -208,6 +212,10 @@ totem_gtk_plug_get_toplevel (GtkPlug *pl > xid = parent; > } > while (TRUE); >+#else >+#warning unimplemented >+ return NULL; >+#endif Just replace the same function by an empty one, it's easier.
Created attachment 128862 [details] [review] define strtok_r by strtok on win32
Created attachment 128863 [details] [review] set workable fallbacks for audio and video drivers on win32
Created attachment 128864 [details] [review] add G_MODULE_EXPORT for signal handlers and *_get_type
Created attachment 128865 [details] [review] 0004-remove-x-window-specific-functions
Created attachment 128866 [details] [review] 0005-conditionally-compile-some-code-only-on-UNIX-systems.patch
Created attachment 128867 [details] [review] 0006-add-a-sigaction-sturct-and-a-stub-implementation-of.patch
With the above fixes and some build stuff fixes, it partially works on win32. if you're interested, please check the branch on http://github.com/zsx/totem branch master is from preview/totem on git.gnome.org, which is 3 weeks old branch win32 contains some fixes should apply to the source code, including all above patches branch win32_build contains various hacks to build totem on mingw32
Comment on attachment 128862 [details] [review] define strtok_r by strtok on win32 You should file a bug against libegg, and have the strtok_r code replaced by g_strdelimit() instead.
Comment on attachment 128863 [details] [review] set workable fallbacks for audio and video drivers on win32 I don't think we should ever be calling gconfaudiosink on Win32 platforms. Can instantiating directsoundsink or dshowvideosink fail? If so, we should probably error out instead of using fake sinks.
Comment on attachment 128864 [details] [review] add G_MODULE_EXPORT for signal handlers and *_get_type All those look fine.
Comment on attachment 128865 [details] [review] 0004-remove-x-window-specific-functions This should really be replaced by a couple of #defines rather than functions.
Comment on attachment 128866 [details] [review] 0005-conditionally-compile-some-code-only-on-UNIX-systems.patch I'd rather see bacon-message-connection go away and be replaced by libunique (given that it's just about as useful for us). There's no GtkPlug on anything but X11, so the whole code should be ifdef'ed. The changes to totem-resources look fine.
Comment on attachment 128867 [details] [review] 0006-add-a-sigaction-sturct-and-a-stub-implementation-of.patch What do we (really) need this for?
Created attachment 129200 [details] [review] 0002-set-workable-fallbacks-for-audio-and-video-drivers-o.patch updated
Created attachment 129201 [details] [review] 0004-x-window-specific-stuff-cleanups.patch remove functions in favor of conditional compilation
(In reply to comment #32) > (From update of attachment 128867 [details] [review] [edit]) > What do we (really) need this for? > I don't know, but it doesn't compile on my system without this patch.
(In reply to comment #35) > (In reply to comment #32) > > (From update of attachment 128867 [details] [review] [edit] [edit]) > > What do we (really) need this for? > > > I don't know, but it doesn't compile on my system without this patch. What error does it give when compilation fails without the patch?
(In reply to comment #31) > (From update of attachment 128866 [details] [review] [edit]) > I'd rather see bacon-message-connection go away and be replaced by libunique > (given that it's just about as useful for us). There's no GtkPlug on anything > but X11, so the whole code should be ifdef'ed. The changes to totem-resources > look fine. > I had a look at libunique, but it doesn't seem to have a win32 implementation. So I just splitted this patch into three, every patch only changes one file.
Created attachment 129202 [details] [review] 0005-comment-out-GtkPlug-on-Non-X11-platforms.patch
Created attachment 129203 [details] [review] 0006-comment-out-set_resource_limits-on-Non-UNIX-system.patch
Created attachment 129204 [details] [review] 0007-conditionally-compile-UNIX-specific-stuffs.patch
Comment on attachment 129201 [details] [review] 0004-x-window-specific-stuff-cleanups.patch >+#ifdef GDK_WINDOWING_X11 >+#include <gdk/gdkx.h> >+#elif defined (GDK_WINDOWING_WIN32) >+#include <gdk/gdkwin32.h> >+#else >+#error unimplemented >+#endif define GDK_WINDOW_NATIVE() here, and use it below instead of ifdefs in the code.
Created attachment 129226 [details] [review] 0004-x-window-specific-stuff-cleanups.patch use a macro to replace #if's in the code
Created attachment 129227 [details] [review] 0004-x-window-specific-stuff-cleanups.patch Sorry, this is the correct one replace #if's in the codes with a macro
(In reply to comment #41) > (From update of attachment 129201 [details] [review] [edit]) > >+#ifdef GDK_WINDOWING_X11 > >+#include <gdk/gdkx.h> > >+#elif defined (GDK_WINDOWING_WIN32) > >+#include <gdk/gdkwin32.h> > >+#else > >+#error unimplemented > >+#endif > > define GDK_WINDOW_NATIVE() here, and use it below instead of ifdefs in the > code. > That's what I wanted, but I thought you preferred #ifdef's. ;) Anyway, I updated the patch as you suggested.
(In reply to comment #36) > (In reply to comment #35) > > (In reply to comment #32) > > > (From update of attachment 128867 [details] [review] [edit] [edit] [edit]) > > > What do we (really) need this for? > > > > > I don't know, but it doesn't compile on my system without this patch. > > What error does it give when compilation fails without the patch? > totem-python-module.c: In function `totem_python_module_init_python': totem-python-module.c:114: error: storage size of 'old_sigint' isn't known totem-python-module.c:128: warning: implicit declaration of function `sigaction' totem-python-module.c:128: warning: nested extern declaration of `sigaction' totem-python-module.c:149: warning: passing arg 2 of `PySys_SetArgv' from incompatible pointer type totem-python-module.c:114: warning: unused variable `old_sigint' make[3]: *** [libtotemmodule_la-totem-python-module.lo] Error 1
Committed the patch marked as commit-after-freeze. Additionally, the patch to BaconMessageConnection is now obsolete, as trunk has switched to libunique. 2009-04-08 Philip Withnall <philip@tecnocode.co.uk> * src/plugins/totem-plugin-manager.c: * src/totem-fullscreen.c: * src/totem-menu.c: * src/totem-object.c: * src/totem-playlist.c: * src/totem-preferences.c: * src/totem-statusbar.c: * src/totem-time-label.c: * src/totem-video-list.c: Patch from Shixin Zeng <zeng.shixin@gmail.com> to add G_MODULE_EXPORT to signal handlers and *_get_type functions. (Helps: #342926)
Comment on attachment 128867 [details] [review] 0006-add-a-sigaction-sturct-and-a-stub-implementation-of.patch Marking as obsolete, that code doesn't exist anymore.
Comment on attachment 129202 [details] [review] 0005-comment-out-GtkPlug-on-Non-X11-platforms.patch Already fixed in. commit 7da72628680b9a64f7a6918975a49991e83315cc Author: Bastien Nocera <hadess@hadess.net> Date: Thu Mar 29 04:11:03 2012 +0200 main: Add guards around GtkPlug parenting hack
Comment on attachment 129203 [details] [review] 0006-comment-out-set_resource_limits-on-Non-UNIX-system.patch Will commit this.
Comment on attachment 129227 [details] [review] 0004-x-window-specific-stuff-cleanups.patch Not relevant anymore
Review of attachment 129200 [details] [review]: We now use autoaudiosink and clutter now.
All pushed. Let me know if you manage to get something out of it.