The first code has been pushed to jk/applet-editor-libnm-port.

It builds and run. However, it still needs some work to check that everything works as before, especially secret handling. And it needs extensive testing, of course.

oh, wow

the libnm API isn't finalized yet... besides the gdbus porting (which I'm about to land), I'm also going to be changing the APIs to be more gio-like (having both sync and async APIs in places where we currently only have one or the other, and using GCancellables and GAsyncReadyCallback), and merging NMRemoteSettings into NMClient.

so we won't want to actually commit anything to network-manager-applet until all that's done

I'd actually started a port too back in early September just so I could understand the implications of libnm more :)  But I got side-tracked with the applet's D-Bus service for single-instance (and tried to port that to GtkApplication but failed) and dropped it.  jklimes saves the day! :)

I finished/updated the porting, squashed it, did some other stuff, and pushed to danw/libnm.

The bulk of the "other stuff" is dealing with the fact that we are presumabably going to have to ship both libnm-glib-based and libnm-based versions of libnm-gtk until gnome-shell and gnome-control-center have been ported to libnm. So I did that. The libnm-based version is called "libnma", since that matches the namespacing of the code anyway. Although in the case of libutils.la and libwireless-security.la, I was able to build both the libnm-glib and libnm-based versions from the same sources, I decided to create a separate copy of the libnm-gtk sources to become libnma, in particular because we can't really have ifdefs in the installed headers.

One thing to note is that if we moved NMAMobileProvidersDatabase somewhere else, then gnome-shell could drop its dependency on libnm-gtk when it ported to libnm. ("Somewhere else" would presumably be either libnm or libmm-glib.) And then we wouldn't need to build NMA.gir/.typelib.

(Although also, gnome-shell only makes very minimal use of NMAMobileProvidersDatabase, so maybe it could just stop using it and parse the XML itself...)

Please also look at bug #740574 and check which one should get merged first.

rebased and repushed; the previous push was after the NMSecretAgent->NMSecretAgentOld rename, so there weren't actually any changes needed for libnm API changes, just for merging network-manager-applet changes.

Looks like the port wasn't quite complete... I've rebased onto git master again (due to the appindicator changes) and fixed up some errors, and added a couple new cleanup patches.

The only 2 things left to get rid of dbus-glib are to port the applet and editor over to use GDBus and GtkApplication for their single-instance features.  The editor implements some custom command-line passing to the master instance which could get converted to GtkApplication easily I think, while the applet uses dbus to provide the GNOME control center with some convenience functions like connecting to 802.1x wifi.

I've pushed dcbw/libnm-bgo736911 with my fixes.  *dcbw* not *danw* :)

GApplication has some DBus hooks that we could use with the applet, see the GApplication docs for more details.  It basically hands us a GDBusConnection that we can use to register our exported object; see the dbus_register/dbus_unregister class methods.

This requires glib2 2.34+ though.  However, Ubuntu 12.10, Fedora 18, Debian Sid & Jessie, and OpenSUSE 12.3 all have glib >= 2.34 so I think it's OK to bump the glib requirement.

>    build: split dbus-glib CFLAGS/LIBS from NMA

>@@ -79,6 +79,8 @@ nm_applet_LDADD = \
> 	-lm \
> 	$(GTK_LIBS) \
> 	$(NMA_LIBS) \
>+	$(DBUS_GLIB_LIBS) \
>+	$(LIBNM_LIBS) \

You can't have both NMA_LIBS and LIBNM_LIBS. (Likewise in nm-connection-editor.)

(Maybe NMA_LIBS should be renamed to LIBNM_GLIB_LIBS now?)


>    editor: convert firewall zones retrieval to GDBus

>+		g_warning ("Failed to get zones from FirewallD: (%d) %s", error->code, error->message);

At one point I started writing a patch to drop error->code from error messages everywhere; the code is not useful without also knowing the domain, and if the message alone isn't sufficient for debugging then we ought to improve the message anyway. So we should just print the message, not the code.

(Likewise elsewhere in the patch.)


>    applet: use g_unix_signal_add() for signal handling

Is signal handling even necessary? All of the applet's "non-local" cleanup (removing icon, unregistering secret agent, etc) should happen automatically when its X/D-Bus/etc connections are lost, so can't we just remove the signal handling entirely and let it be killed by the signal?


>    applet: use nm_connection_get_id()

The second "fixup! applet: port to libnm" patch (919fd37b) seems like it should be part of this patch.

(In reply to Dan Winship from comment #9)
> >    build: split dbus-glib CFLAGS/LIBS from NMA
> 
> >@@ -79,6 +79,8 @@ nm_applet_LDADD = \
> > 	-lm \
> > 	$(GTK_LIBS) \
> > 	$(NMA_LIBS) \
> >+	$(DBUS_GLIB_LIBS) \
> >+	$(LIBNM_LIBS) \
> 
> You can't have both NMA_LIBS and LIBNM_LIBS. (Likewise in
> nm-connection-editor.)

Fixed, I think this was due to bad rebasing.  They don't need LIBNM_LIBS in this patch, but they might need it later.

> (Maybe NMA_LIBS should be renamed to LIBNM_GLIB_LIBS now?)

Yeah, probably.  Done in a fixup.

> >    editor: convert firewall zones retrieval to GDBus
> 
> >+		g_warning ("Failed to get zones from FirewallD: (%d) %s", error->code, error->message);
> 
> At one point I started writing a patch to drop error->code from error
> messages everywhere; the code is not useful without also knowing the domain,
> and if the message alone isn't sufficient for debugging then we ought to
> improve the message anyway. So we should just print the message, not the
> code.
> 
> (Likewise elsewhere in the patch.)

Done in the two places I found.

> >    applet: use g_unix_signal_add() for signal handling
> 
> Is signal handling even necessary? All of the applet's "non-local" cleanup
> (removing icon, unregistering secret agent, etc) should happen automatically
> when its X/D-Bus/etc connections are lost, so can't we just remove the
> signal handling entirely and let it be killed by the signal?

Removed all signal handling in "applet: remove custom signal handling"

> >    applet: use nm_connection_get_id()
> 
> The second "fixup! applet: port to libnm" patch (919fd37b) seems like it
> should be part of this patch.

The fixup is to fix a bug in the original patch for the applet port, without which the applet port patch will segfault.  Then I fix up the other stuff later.

Repushed with fixups, please review.

(note: repushed to danw/libnm-bgo736911, not dcbw/libnm-bgo736911)

(In reply to Dan Williams from comment #10)
> > >    applet: use nm_connection_get_id()
> > 
> > The second "fixup! applet: port to libnm" patch (919fd37b) seems like it
> > should be part of this patch.
> 
> The fixup is to fix a bug in the original patch for the applet port, without
> which the applet port patch will segfault.  Then I fix up the other stuff
> later.

Oh, didn't notice the bugfix part. But you switch it from get_connection_id() to nm_connection_get_id() at the same time. You could fix the bug without making that change.

Otherwise, looks good to go.

Merged to git master.