After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 690225 - [patch] Make GOA optional
[patch] Make GOA optional
Status: RESOLVED FIXED
Product: libgdata
Classification: Platform
Component: General
git master
Other Linux
: Normal enhancement
: ---
Assigned To: libgdata-maint
libgdata-maint
Depends on:
Blocks:
 
 
Reported: 2012-12-14 21:16 UTC by Nirbheek Chauhan
Modified: 2012-12-17 23:19 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Make GOA optional (2.69 KB, patch)
2012-12-14 21:16 UTC, Nirbheek Chauhan
needs-work Details | Review
Make GOA optional, and add a goaenabled variable to libgdata.pc (3.38 KB, patch)
2012-12-16 19:54 UTC, Nirbheek Chauhan
accepted-commit_now Details | Review
Make GOA optional, and add a goa_enabled variable to libgdata.pc (3.39 KB, patch)
2012-12-17 17:49 UTC, Nirbheek Chauhan
committed Details | Review

Description Nirbheek Chauhan 2012-12-14 21:16:09 UTC
Created attachment 231594 [details] [review]
Make GOA optional

Hello,

The attached patch splits the --enable-gnome option into a sub-option --enable-goa, which can be disabled if wanted. This helps keep the dependency list small for applications which want to use libgdata but not pull in all of GOA and hence Webkit-Gtk, etc.

Thanks!

Behaviour:

* No parameters => same as before
* --enable-gnome => same as before
* --disable-gnome => same as before
* --enable-goa => enables both (gnome is on if unspecified, as before)
* --disable-goa => disables goa (gnome is on if unspecified, as before)
* --enable-gnome --enable-goa => enables both
* --enable-gnome --disable-goa => disables goa
* --disable-gnome --enable-goa => disables both
* --disable-gnome --disable-goa => disables both
Comment 1 Philip Withnall 2012-12-16 15:31:03 UTC
Review of attachment 231594 [details] [review]:

This looks good to me. However, there needs to be a way for packages (such as EDS) which depend on GDataGoaAuthorizer to detect whether libgdata was compile with --enable-goa at configure time. Something in libgdata.pc would be suitable.
Comment 2 Philip Withnall 2012-12-16 15:34:04 UTC
Also, please make sure you rebase on bug #690281. Thanks.
Comment 3 Nirbheek Chauhan 2012-12-16 19:54:31 UTC
Created attachment 231665 [details] [review]
Make GOA optional, and add a goaenabled variable to libgdata.pc

The attached patch is rebased, and adds a "goaenabled" variable to libgdata.pc — would this method work for EDS, etc? Or is there something else?
Comment 4 Philip Withnall 2012-12-17 15:37:40 UTC
Review of attachment 231665 [details] [review]:

Feel free to commit to master with the two modifications below. Thanks!

::: configure.ac
@@ +105,3 @@
+		GNOME_PACKAGES_PUBLIC="$GNOME_PACKAGES_PUBLIC"
+		GNOME_PACKAGES_PRIVATE="$GNOME_PACKAGES_PRIVATE goa-1.0 >= $GOA_REQS"
+		GOA_ENABLED="1"

GLib seems to use ‘true’ and ‘false’ for G_MODULE_SUPPORTED in http://git.gnome.org/browse/glib/tree/gmodule-export-2.0.pc.in#n6 (see http://git.gnome.org/browse/glib/tree/configure.ac#n1630).

Might be a good idea to follow its lead, although I can’t find any documentation on the canonical format for booleans in pkg-config files.

::: libgdata.pc.in
@@ +3,3 @@
 libdir=@libdir@
 includedir=@includedir@
+goaenabled=@GOA_ENABLED@

Could you call it ‘goa_enabled’ instead please?
Comment 5 Nirbheek Chauhan 2012-12-17 17:49:57 UTC
Created attachment 231753 [details] [review]
Make GOA optional, and add a goa_enabled variable to libgdata.pc

(In reply to comment #4)
> Review of attachment 231665 [details] [review]:
> 
> Feel free to commit to master with the two modifications below. Thanks!
> 

The attached patch has the two modifications — I agree that following precedent and using "true" makes sense.

I lost my commit access a long time ago, and haven't gotten it back yet. Could I ask you to commit the patch for me? Thanks! :)
Comment 6 Philip Withnall 2012-12-17 23:18:56 UTC
Committed!

commit b4d828e002adf21a5ef671ec1b62bed2f48382af
Author: Nirbheek Chauhan <nirbheek.chauhan@collabora.co.uk>
Date:   Mon Dec 17 23:18:12 2012 +0530

    build: Make gnome-online-accounts optional as well
    
    It already was optional under --enable-gnome; just split that out into its own
    configure option
    
    Closes: https://bugzilla.gnome.org/show_bug.cgi?id=690225

 Makefile.am    |  6 +++---
 configure.ac   | 16 +++++++++++++++-
 libgdata.pc.in |  1 +
 3 files changed, 19 insertions(+), 4 deletions(-)