GNOME Bugzilla – Bug 669495
Make NetworkManager support optional
Last modified: 2014-02-12 00:15:32 UTC
Hi, to make gnome-shell compile on our Debian kfreebsd ports, we apply the patch at [1], which makes it possible to enable NetworkManager support via a --enable-networkmanager configure switch (the default is autodetect). There are other distros, like Gentoo, which also apply this patch so apparently there is some demand for this. I thought I share this patch and forward it so ideally it is merged upstream. Please review and apply if the patch is acceptable. Cheers, Michael [1] http://patch-tracker.debian.org/patch/series/view/gnome-shell/3.2.2.1-1/10-make-NetworkManager-optional.patch [2] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=652482
Created attachment 206922 [details] [review] [PATCH] Make NetworkManager support optional
Review of attachment 206922 [details] [review]: ::: gnome-shell-3.2.2.1.orig/configure.ac @@ +96,3 @@ telepathy-glib >= $TELEPATHY_GLIB_MIN_VERSION telepathy-logger-0.2 >= $TELEPATHY_LOGGER_MIN_VERSION + polkit-agent-1 >= $POLKIT_MIN_VERSION xfixes) This patch loses the check for gnome-keyring-1. Bad rebase? @@ +105,3 @@ + AS_HELP_STRING([--disable-networkmanager], + [disable NetworkManager support @<:@default=auto@:>@]),, + [enable_networkmanager=auto]) Ok, I don't mind this under the premise that it's reasonable to use GNOME in e.g. a "legacy thin client" case where the user has no control over the local network stack. So thin client builds should allow running without it. But I'd be pretty opposed to growing a network stack abstraction layer - we chose NetworkManager in GNOME (and developed it here in fact).
(In reply to comment #2) > Review of attachment 206922 [details] [review]: > > ::: gnome-shell-3.2.2.1.orig/configure.ac > @@ +96,3 @@ > telepathy-glib >= $TELEPATHY_GLIB_MIN_VERSION > telepathy-logger-0.2 >= > $TELEPATHY_LOGGER_MIN_VERSION > + polkit-agent-1 >= $POLKIT_MIN_VERSION xfixes) > > This patch loses the check for gnome-keyring-1. Bad rebase? The gnome-keyring build dependency was added as part of http://git.gnome.org/browse/gnome-shell/commit/?id=2af5e851b37d036de3d30d3165cdd57f63a2f79a And a grep for gnome-keyring only yields src/shell-network-agent.c:#include <gnome-keyring.h> That's why I included it. Thanks, for the review!
(In reply to comment #3) > And a grep for gnome-keyring only yields > src/shell-network-agent.c:#include <gnome-keyring.h> That's going to change, see bug 652459.
(In reply to comment #4) > (In reply to comment #3) > > And a grep for gnome-keyring only yields > > src/shell-network-agent.c:#include <gnome-keyring.h> > > That's going to change, see bug 652459. Ah, ok. Didn't know about that. I can send attach an updated patch (but it's probably trivial enough to fix up manually). Whatever you prefer, just let me know.
If we go down this route and make NM support optional (which kind of makes sense), it could be an occasion to remove the try/catch around loading NMApplet. (See 668388 for more discussion on this)
Created attachment 228229 [details] [review] Display summary at the end of configure Based on Michael Biebl's patch from bgo#669495.
Created attachment 228230 [details] [review] Make NM optional [ Alexandre Rostovtsev <tetromino@gentoo.org> : * use config.js (and AC_SUBST HAVE_NETWORKMANAGER appropriately); * take care to not import ui.status.network if nm is disabled; * do not try to reassign to const variables; * no point really in fiddling with the list of installed js files; * don't build shell-mobile-providers if nm is disabled; * use "networkmanager" instead of "network_manager" because THE BIKESHED SHOULD BE BLUE, also because the upstream package name is NetworkManager, not Network_Manager. ]
I rebased the original patch on top of current master.
Should this be resolved fixed or has it not actually been merged?
Could this please go in? Other platforms (such as OpenBSD) need to keep applying this patch from Gentoo. Guillaume, could you please attach a new patch rebased against master?
*** Bug 679871 has been marked as a duplicate of this bug. ***
Created attachment 257107 [details] [review] make NM optional (3.10) Here you go, patch against master. It would be nice indeed to have this or a variation of it to be pushed. Thanks for looking.
Review of attachment 257107 [details] [review]: ::: configure.ac @@ +171,3 @@ + libnm-util >= $NETWORKMANAGER_MIN_VERSION + libnm-gtk >= $NETWORKMANAGER_MIN_VERSION + gnome-keyring-1], Why gnome-keyring-1? We should use libsecret, not gnome-keyring.
> Why gnome-keyring-1? We should use libsecret, not gnome-keyring. You are absolutely right, that was a left-over from an older version. Patch against HEAD updated (and tested on OpenBSD).
Created attachment 264189 [details] [review] make NM optional
Created attachment 268861 [details] [review] make NetworkManager optional NetworkManager is only available on Linux. Rebased to apply on master again. Also added another instance of the 'if (this._network)' check to prevent an exception on startup. Seems to work fine now.
Review of attachment 268861 [details] [review]: OK. ::: js/ui/sessionMode.js @@ +101,1 @@ 'keyring', 'autorunManager', 'automountManager'], A bit disturbing, but OK.
Attachment 268861 [details] pushed as 9f3499a - make NetworkManager optional tvm!