|Summary:||[0.8, 0.9] be portable to Windows|
|Product:||Telepathy||Reporter:||Matti Reijonen <matti.reijonen>|
|Component:||tp-glib||Assignee:||Simon McVittie <smcv>|
|Status:||RESOLVED FIXED||QA Contact:||Telepathy bugs list <telepathy-bugs>|
|i915 platform:||i915 features:|
patch for making tp-glib build work on windows
Current patch against 08-portability branch
Description Matti Reijonen 2009-10-08 06:14:27 UTC
Created attachment 30157 [details] [review] patch for making tp-glib build work on windows I had to apply attached patch to telepathy_glib 0.7.37 for building it on windows (Vista). Patch was tested by building tp-glib with the cmakes that can be found in here: http://dev.realxtend.org/gf/project/viewerdeps/scmsvn/?action=browse&path=%2Ftrunk%2Ftelepathy-libs%2F Also tested with tp-qt4 roster & gabble 0.8.3
Comment 1 Simon McVittie 2009-10-13 05:59:56 UTC
Comment on attachment 30157 [details] [review] patch for making tp-glib build work on windows >diff --git a/telepathy-glib/debug.c b/telepathy-glib/debug.c >index 51e20ae..edc2c10 100755 >--- a/telepathy-glib/debug.c >+++ b/telepathy-glib/debug.c >@@ -71,6 +71,12 @@ > > #include "debug-internal.h" > >+#ifdef WIN32 >+#define STDIN_FILENO 0 /* standard input file descriptor */ >+#define STDOUT_FILENO 1 /* standard output file descriptor */ >+#define STDERR_FILENO 2 /* standard error file descriptor */ >+#endif I think it'd actually be cleaner to use 1 and 2 unconditionally, with a suitable comment, since these constants are universal - Unix shell scripts basically all rely on fd 1 and 2 being stdout and stderr, for instance, and GLib just uses 1 and 2. I've written a patch to do this. >@@ -457,13 +463,28 @@ tp_debug_timestamped_log_handler (const gchar *log_domain, I'd rather not have this #ifdef mess; I've done a patch that uses g_time_val_to_iso8601 instead (this is a small behaviour change - the format changes, and it's no longer in local time - but it's only debug output). >diff --git a/telepathy-glib/proxy.h b/telepathy-glib/proxy.h >index bd6b3a9..8bb3027 100755 >--- a/telepathy-glib/proxy.h >+++ b/telepathy-glib/proxy.h >@@ -25,8 +25,18 @@ > #include <dbus/dbus-glib.h> > #include <glib-object.h> > >+#ifdef WIN32 >+#include <glib/gquark.h> >+#endif Why is this needed? <glib-object.h> includes <glib.h>, which includes <glib/gquark.h>... so why do you need to include this header explicitly? >+#ifdef WIN32 >+#ifdef interface >+#undef interface >+#endif >+#endif Ugh. What is interface defined to in Win32-land? :-( In most of the cases where we mention "interface" as an identifier, this is easy to work around by renaming the function parameter to "iface". However, TpProxyClass has a member called "interface", which is part of our API... We could work around this like so: #define _TP_CPP_PASTE_TOKENS(a,b) a ## b GQuark _TP_CPP_PASTE_TOKENS(i,nterface); or we could just #undef interface. However, #undef'ing interface seems likely to break whatever API defines it... Anyone wanting to access the ->interface member would also have to perform a similar hack, unless we add a tp_proxy_class_set_interface() accessor (which might not be a bad idea anyway). When we break API we should probably rename this to dbus_interface or interface_name, but we're not doing that yet. >+#if WIN32 >+#undef interface; >+#endif That semicolon isn't syntactically right. I've fixed this file by renaming the "interface" argument to "iface", instead.
Comment 2 Simon McVittie 2009-10-13 06:10:12 UTC
Could you try building the 08-portability branch from my git repository on Windows, please? It's based on telepathy-glib 0.8.0 (which is in turn basically identical to 0.7.37 - there were some minor bugfixes in the examples, and no changes in the library). http://git.collabora.co.uk/?p=user/smcv/telepathy-glib-smcv.git;a=shortlog;h=refs/heads/08-portability git://git.collabora.co.uk/git/user/smcv/telepathy-glib-smcv.git For the moment, until we decide how to deal with it, you'll still need to do the "#undef interface" hack in proxy.h, but you shouldn't need to make any other changes now.
Comment 3 Matti Reijonen 2009-10-15 08:42:45 UTC
(In reply to comment #2) Tested that 08-portability branch and it did work with just #undef interface" in proxy.h and changing STDOUT_FILENO, STDERR_FILENO to 1, 2 in debug.c
Comment 4 Matti Reijonen 2009-10-16 00:43:36 UTC
Created attachment 30472 [details] [review] Current patch against 08-portability branch Attached this to keep track of changes, that are needed to make 08-portability branch work in windows.
Comment 5 Siraj Razick 2009-10-16 01:50:16 UTC
Comment on attachment 30472 [details] [review] Current patch against 08-portability branch isn't it better if we define, if STDOUT_FILENO...etc are not defined under windows, and not replace these with magical intergers like 1,2..etc ?
Comment 6 Siraj Razick 2009-10-16 02:35:55 UTC
(In reply to comment #5) > (From update of attachment 30472 [details] [review]) > isn't it better if we define, if STDOUT_FILENO...etc are not defined under > windows, and not replace these with magical intergers like 1,2..etc ? > Oops. I missed the 1# comment about this from Simon, So.. :) it's fine I guess
Comment 7 Simon McVittie 2009-10-16 06:21:46 UTC
(Removing patch keyword; this isn't ready for review until we have a git branch that compiles without changes.)
Comment 8 Simon McVittie 2009-10-16 07:03:09 UTC
Snapshot of the 08-portability branch, which I've rebased onto 0.8.1: http://people.collabora.co.uk/~smcv/telepathy-glib-0.8.1+08-portability.tar.gz
Comment 9 Simon McVittie 2009-10-16 07:47:10 UTC
I think this is ready for review. The one remaining issue is 'interface' which I'm fairly sure isn't our fault: 15:44 < mattire> to me it looks telepathy-glib 0.8.1.1 is windows compatible tested with roster app 15:44 < smcv> mattire: \o/ 15:44 < mattire> :) 15:44 < smcv> mattire: I wonder why interface isn't #define'd any more... 15:45 < smcv> mattire: oh argh I might know 15:45 < smcv> mattire: were you compiling tp-qt4 against it? 15:45 < smcv> mattire: it wouldn't surprise me if Qt defines 'interface' under some circumstances 15:45 < mattire> yes, i have qt-tp4 messing things
Comment 10 Matti Reijonen 2009-10-22 04:32:41 UTC
(In reply to comment #9) I recently found out that interface is defined in ObjBase.h in windows header, it is defined as struct.. it gets sometimes included and sometimes not, when just building telepathy-glib dll interface keyword does not cause problems, when continuing to build telepathy-gabble compatibility branch it comes back to haunt me again, when compiling ft-channel that includes connection.h that includes dbus-properties-mixin.h. also i had to do #undef interface in dbus-properties-mixin.h, the #undef in proxy.h seemed not to have effect, I'm not sure how to deal with this issue, any suggestions are welcome..
Comment 11 Simon McVittie 2009-10-22 12:05:57 UTC
(In reply to comment #10) > (In reply to comment #9) > I recently found out that interface is defined in ObjBase.h in windows header, > it is defined as struct.. In that case, I consider this namespace pollution to be not our bug; if you're including ObjBase.h somewhere (e.g. Gabble) then the inclusion of that header should do the #undef. dbus-properties-mixin.h doesn't include proxy.h (I don't think) so one #undef won't affect the other file. dbus-properties-mixin.h no longer mentions 'interface' so this shouldn't be a problem for you any more. When one of the other telepathy-glib core developers reviews and approves this, I think we can integrate this branch.
Comment 12 Simon McVittie 2009-10-28 09:40:46 UTC
(In reply to comment #11) > (In reply to comment #10) > > (In reply to comment #9) > > I recently found out that interface is defined in ObjBase.h in windows header, > > it is defined as struct.. For reviewers' benefit: in the corresponding Gabble bug, it has been noted that you have to #undef interface if you've included winsock2.h.
Comment 13 Simon McVittie 2009-11-13 08:54:53 UTC
Fixed in master, will be in either 0.9.2 or 0.10.0, whichever happens first (most likely 0.10.0). Since we have a new stable-branch release on the way soon anyway, I'm not going to merge this to 0.8.x; we can consider 0.10.x to be the one to use for Windows. A cmake build system might follow in 0.11.x, depending how we get on with it in telepathy-qt4 (which is our nominated cmake early-adopter).