Bug 46345 - tp-salut does not build on windows/mingw32
Summary: tp-salut does not build on windows/mingw32
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: salut (show other bugs)
Version: unspecified
Hardware: Other Windows (All)
: medium normal
Assignee: Siraj Razick
QA Contact: Telepathy bugs list
URL: http://cgit.collabora.com/git/user/si...
Whiteboard: review+
Keywords: patch
Depends on:
Blocks:
 
Reported: 2012-02-20 07:11 UTC by Siraj Razick
Modified: 2012-02-22 11:31 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Siraj Razick 2012-02-20 07:11:45 UTC
tp-salut doesn't compile on windows or mingw32 since it still uses unix sockets headers and functions unconditionally, and Avahi does not support windows.

so we have planed the following

1.) port tp-salut to windows using the dummy backend
   * this includes supporting a new switch --with-backend=salut/dummy/dns_sd
   * make sure tp-salut compiles on windows and mingw32 with the dummy backend
2.) add a new backend to support libdns_sd so we can use Apple's mdnsresponder 
    in windows.
Comment 1 Siraj Razick 2012-02-20 15:51:13 UTC
First patch which ports telepathy-salut to windows, with a dummy backend.

the patch set does the following

.1) support defining which backend to use salut/dummy or libdns_sd (mdns responder ). the mdns backed is work in progress so once it's ready the backend will be supported

2.) fix socket stuff which fails when we cross compile tp-salut for windows.

3.) And other fixes, which solves compile errors on windows and finally 
build system fixes which makes tp-salut completely build on windows and mingw32 with a dummy backend.

Next I'll be working on adding libdns_sd support to tp-salut so that we can have a working telepathy-salut on Windows.
Comment 2 Olli Salli 2012-02-21 04:42:17 UTC
> #ifdef G_OS_WIN32
> typedef uint32_t u_int32_t;
> typedef uint16_t u_int16_t;
> #endif

uint32_t and uint16_t come from stdint.h, which you don't explicitly include, and which is a C99 header not supported by all Windows compilers anyway. Please rather change the code to use guint32 and guint16 and let GLib worry about defining them.

Otherwise the Gibber fixes look OK.

Leaving FT out is OK for now but eventually we need to fix bug 18530 and include it in the Windows port.

> +AM_CONDITIONAL(USE_BACKEND_DNS_SD, [test "x$with_backend" = "xlibdns_sd"])

This backend doesn't exist yet. So don't add build system foo for it here please.

> +    AC_HELP_STRING([--with-backend=[dummy/avahi/libdns_sd]],

Typically --with args are used in configure for when you want to do --with-some-dependency-lib --without-some-dependency-lib . 

Thus, I'd make this --zeroconf-backend=dummy/avahi for now.

> +if test x$with_backend = xavahi; then
> PKG_CHECK_MODULES(AVAHI, [avahi-gobject, avahi-client])

Indent stuff that now has become the body of an `if` branch, please. (This is not the only occurrence).

Generally, the build system should have a consistent coding style as well.
Comment 3 Simon McVittie 2012-02-21 05:49:07 UTC
(In reply to comment #2)
> Typically --with args are used in configure for when you want to do
> --with-some-dependency-lib --without-some-dependency-lib . 
> 
> Thus, I'd make this --zeroconf-backend=dummy/avahi for now.

The documented convention in Autoconf is that --with is for optional packages, whereas --enable is for including or omitting parts of this package, and the docs specifically say "--enable-FEATURE should never make a feature behave differently or cause one feature to replace another".

As far as I can see, the Autoconf docs would want you to use --with-avahi and --with-libdns-sd, have the default be --with-avahi, make --with-avahi --with-libdns-sd be an error (or use Avahi because it's better?), and make --without-avahi use the dummy backend? Or something.

I think --with-backend=[avahi|libdns_sd|no] would be fine, tbh. If you make the "dummy" case be "no" instead of "dummy", then people wanting the dummy backend can use --without-backend (which is exactly equivalent to --with-backend=no).
Comment 4 Siraj Razick 2012-02-21 18:20:50 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > Typically --with args are used in configure for when you want to do
> > --with-some-dependency-lib --without-some-dependency-lib . 
> > 
> > Thus, I'd make this --zeroconf-backend=dummy/avahi for now.
> 
> The documented convention in Autoconf is that --with is for optional packages,
> whereas --enable is for including or omitting parts of this package, and the
> docs specifically say "--enable-FEATURE should never make a feature behave
> differently or cause one feature to replace another".
> 
> As far as I can see, the Autoconf docs would want you to use --with-avahi and
> --with-libdns-sd, have the default be --with-avahi, make --with-avahi
> --with-libdns-sd be an error (or use Avahi because it's better?), and make
> --without-avahi use the dummy backend? Or something.
> 
> I think --with-backend=[avahi|libdns_sd|no] would be fine, tbh. If you make the
> "dummy" case be "no" instead of "dummy", then people wanting the dummy backend
> can use --without-backend (which is exactly equivalent to --with-backend=no).


the branch is now updated with the suggested changes.
Comment 5 Olli Salli 2012-02-22 11:31:08 UTC
as caught by check-coding-style, all the casts like:

> +  ret = setsockopt (fd, SOL_SOCKET, SO_REUSEADDR, (char *)&yes, sizeof (int));

are wrong. In Telepathy coding style, there is a space between "(type)" and what_to_cast.

So:

CORRECT:
(char *) &yes

INCORRECT:
(char *)&yes

I fixed these and merged. Please try to keep in mind from now on.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.