Bug 42530

Created attachment 53077 [details] [review]
Support for Android patch

Created attachment 53078 [details] [review]
Support building gabble plugins

Created attachment 53079 [details] [review]
make sure the proper GABBLE,SALUT define  is given

Created attachment 53080 [details] [review]
gabble: copy plugin-base files to gabble directory

Android doesn't like building this stuff in a common directory to the
salut plugin because it stores the object files in the same place, but
salut != gabble...

Comment on attachment 53077 [details] [review]
Support for Android patch

Review of attachment 53077 [details] [review]:
-----------------------------------------------------------------

::: salut/Makefile.am
@@ +37,5 @@
> +	 -:CFLAGS $(DEFS) $(CFLAGS) $(DEFAULT_INCLUDES) $(INCLUDES) \
> +	  $(AM_CFLAGS) -I../../wocky \
> +	 -:CPPFLAGS $(CPPFLAGS) $(AM_CPPFLAGS) \
> +	 -:LDFLAGS $(AM_LDFLAGS) $(ytstenut_salut_la_LIBADD) -lwocky \
> +	  -ltelepathy-salut -lxml2 \

Hardcoded link flags :(

Comment on attachment 53078 [details] [review]
Support building gabble plugins

Review of attachment 53078 [details] [review]:
-----------------------------------------------------------------

::: gabble/Makefile.am
@@ +37,5 @@
> +	 -:CFLAGS $(DEFS) $(CFLAGS) $(DEFAULT_INCLUDES) $(INCLUDES) \
> +	  $(AM_CFLAGS) -I../../wocky \
> +	 -:CPPFLAGS $(CPPFLAGS) $(AM_CPPFLAGS) \
> +	 -:LDFLAGS $(AM_LDFLAGS) $(ytstenut_gabble_la_LIBADD) -lwocky \
> +	  -ltelepathy-gabble -lxml2 \

Ditto.

Comment on attachment 53080 [details] [review]
gabble: copy plugin-base files to gabble directory

Review of attachment 53080 [details] [review]:
-----------------------------------------------------------------

Doesn't this break the build on non-Android?

Created attachment 53833 [details] [review]
Support for Android - fixed

This fixes the hardcoded libs.

It requires patches to salut and gabble, which I'll submit next

Created attachment 53839 [details] [review]
Support for Android - fixed

I had to reintroduce the -ltelepathy-x hardcode because neither gabble nor salut are declared as libs in PKG_CONFIG and the patches I had submitted to add that were rejected because it is not safe to do that.

As this is a special case, I'd rather leave these 2 hardcodes in this case.

Please commit, thanks.

This is the link to the branch with the commits: http://cgit.freedesktop.org/~asoliverez/ytstenut-plugins/log/?h=android

> Support for building on Android, modified from patches by Derek Foreman and Johnny Lamb

That is not my name.

Created attachment 56323 [details] [review]
Support for Android - fixed

Fixed the Jonny's name. Extremely sorry about that.

Created attachment 56806 [details] [review]
Support to build on Android

Patch updated to latest master

Created attachment 56807 [details] [review]
Fix for gabble plugin refactor

Please, can someone with permissions push this after it's reviewed? Thanks!

(In reply to comment #15)
> Created attachment 56807 [details] [review] [review]
> Fix for gabble plugin refactor

GABBLE_LIBS already contains -lgabble-plugins. Please remove the redundant mention, so you're not tied to the exact library file name.

(In reply to comment #14)
> Created attachment 56806 [details] [review] [review]
> Support to build on Android
> 
> Patch updated to latest master

Jonny's name reverted back!

 ytstenut_gabble_la_SOURCES = \
...
-	$(top_srcdir)/plugin-base/ytstenut.h \
...
-	$(top_srcdir)/plugin-base/caps-manager.h \
...
-	message-channel.h \

Headers are still sources. If it's troublesome to refer to the headers in the other directory as SOURCES, please at least include the ones in the current directory.

Created attachment 56843 [details] [review]
Support for Android patch

Patch with corrections from Olli's review

Okay, apparently it's troublesome to mark stuff from other directories as sources. Jonny's name is fixed, and the CM libs aren't mentioned explicitly anymore. That's enough for me. Please merge.

(In reply to comment #20)
> Okay, apparently it's troublesome to mark stuff from other directories as sources. 

That's because in Android, the stuff from other directories is copied, because it can't be compiled multiple times. It's the copied_files you see there. Jonny had to work around that when adding the plugins

commit 9ceff79bbfa423452249298684543f5f2dbe581b
Author: Alvaro Soliverez <alvaro.soliverez@collabora.co.uk>
Date:   Tue Dec 13 16:37:54 2011 -0300

    Support for building on Android, modified from patches by Derek Foreman and Johnny Lamb

commit 1e8ce5770f6a908b133aa2b0401b2e4f7dc9c1b5
Author: Derek Foreman <derek.foreman@collabora.co.uk>
Date:   Mon May 16 10:22:21 2011 -0400

    respect HAVE_GETTEXT

(In reply to comment #13)
> Created attachment 56323 [details] [review] [review]
> Support for Android - fixed
> 
> Fixed the Jonny's name. Extremely sorry about that.

(In reply to comment #22)
> commit 9ceff79bbfa423452249298684543f5f2dbe581b
> Author: Alvaro Soliverez <alvaro.soliverez@collabora.co.uk>
> Date:   Tue Dec 13 16:37:54 2011 -0300
> 
>     Support for building on Android, modified from patches by Derek Foreman and
> Johnny Lamb