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 669495 - Make NetworkManager support optional
Make NetworkManager support optional
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: network-indicator
3.2.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
: 679871 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2012-02-06 17:57 UTC by Michael Biebl
Modified: 2014-02-12 00:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] Make NetworkManager support optional (8.00 KB, patch)
2012-02-06 17:58 UTC, Michael Biebl
reviewed Details | Review
Display summary at the end of configure (855 bytes, patch)
2012-11-06 10:33 UTC, Guillaume Desmottes
none Details | Review
Make NM optional (7.65 KB, patch)
2012-11-06 10:33 UTC, Guillaume Desmottes
none Details | Review
make NM optional (3.10) (8.40 KB, patch)
2013-10-12 14:47 UTC, Antoine Jacoutot
reviewed Details | Review
make NM optional (8.49 KB, patch)
2013-12-14 09:52 UTC, Antoine Jacoutot
none Details | Review
make NetworkManager optional (8.52 KB, patch)
2014-02-11 23:06 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Michael Biebl 2012-02-06 17:57:16 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
Comment 1 Michael Biebl 2012-02-06 17:58:07 UTC
Created attachment 206922 [details] [review]
[PATCH] Make NetworkManager support optional
Comment 2 Colin Walters 2012-02-06 20:11:19 UTC
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).
Comment 3 Michael Biebl 2012-02-06 22:15:08 UTC
(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!
Comment 4 Florian Müllner 2012-02-06 22:30:52 UTC
(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.
Comment 5 Michael Biebl 2012-02-06 22:33:09 UTC
(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.
Comment 6 Giovanni Campagna 2012-04-29 20:23:00 UTC
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)
Comment 7 Guillaume Desmottes 2012-11-06 10:33:01 UTC
Created attachment 228229 [details] [review]
Display summary at the end of configure

Based on Michael Biebl's patch from bgo#669495.
Comment 8 Guillaume Desmottes 2012-11-06 10:33:05 UTC
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. ]
Comment 9 Guillaume Desmottes 2012-11-06 10:33:36 UTC
I rebased the original patch on top of current master.
Comment 10 Travis Reitter 2013-06-10 22:50:47 UTC
Should this be resolved fixed or has it not actually been merged?
Comment 11 Jasper Lievisse Adriaanse 2013-10-12 09:23:40 UTC
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?
Comment 12 Jasper Lievisse Adriaanse 2013-10-12 09:25:00 UTC
*** Bug 679871 has been marked as a duplicate of this bug. ***
Comment 13 Antoine Jacoutot 2013-10-12 14:47:09 UTC
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.
Comment 14 Giovanni Campagna 2013-10-12 15:13:15 UTC
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.
Comment 15 Antoine Jacoutot 2013-12-14 09:51:31 UTC
> 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).
Comment 16 Antoine Jacoutot 2013-12-14 09:52:11 UTC
Created attachment 264189 [details] [review]
make NM optional
Comment 17 Allison Karlitskaya (desrt) 2014-02-11 23:06:28 UTC
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.
Comment 18 Jasper St. Pierre (not reading bugmail) 2014-02-11 23:53:28 UTC
Review of attachment 268861 [details] [review]:

OK.

::: js/ui/sessionMode.js
@@ +101,1 @@
                      'keyring', 'autorunManager', 'automountManager'],

A bit disturbing, but OK.
Comment 19 Allison Karlitskaya (desrt) 2014-02-12 00:15:29 UTC
Attachment 268861 [details] pushed as 9f3499a - make NetworkManager optional

tvm!