Merge lp:~ted/indicator-network/upstart-job into lp:indicator-network/13.10

Proposed by Ted Gould on 2013-04-24
Status: Merged
Approved by: Mathieu Trudel-Lapierre on 2013-04-29
Approved revision: 251
Merged at revision: 243
Proposed branch: lp:~ted/indicator-network/upstart-job
Merge into: lp:indicator-network/13.10
Prerequisite: lp:~ted/indicator-network/indicators-change
Diff against target: 134 lines (+45/-12) 9 files modified
To merge this branch: bzr merge lp:~ted/indicator-network/upstart-job
Reviewer Review Type Date Requested Status
Mathieu Trudel-Lapierre 2013-04-24 Approve on 2013-04-29
PS Jenkins bot continuous-integration Approve on 2013-04-24
Review via email: mp+160542@code.launchpad.net

Commit Message

Move from dbus activation to upstart user session job

Description of the Change

The main thrust of this branch is to move to upstart user session. Along the way we drop dbus activation and add an apport hook to pick up the new log file.

To post a comment you must log in.
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)

I understand the usefulness for debugging, but I strongly feel we should avoid having G_MESSAGES_DEBUG set in the upstart job. This is so likely to just get forgotten and left in ;)

No point in spamming .xsession-errors

review: Needs Fixing
Ted Gould (ted) wrote :

On Wed, 2013-04-24 at 14:10 +0000, Mathieu Trudel-Lapierre wrote:

> I understand the usefulness for debugging, but I strongly feel we should avoid having G_MESSAGES_DEBUG set in the upstart job. This is so likely to just get forgotten and left in ;)
>
> No point in spamming .xsession-errors

See, and now you see the beauty of upstart user session. It WON'T spam
xsession-errors because it only puts the log from that job in
~�.cache�upstart�indicator-network.log and we can then attach that in
the apport hook. So we can avoid the xession-errors issue *and* get
good bug reports!

Approve. Let's keep in mind to fix up the debug logs situation.

review: Approve

Preview Diff

1=== modified file '.bzrignore'
2--- .bzrignore 2012-10-31 11:50:11 +0000
3+++ .bzrignore 2013-04-24 03:49:29 +0000
4@@ -53,3 +53,4 @@
5 data/com.canonical.settings.network.service
6 data/com.canonical.settings.sound.service
7 secret-agent/chewie-secret-agent
8+data/indicator-network.conf
9
10=== modified file 'data/Makefile.am'
11--- data/Makefile.am 2013-04-24 03:49:29 +0000
12+++ data/Makefile.am 2013-04-24 03:49:29 +0000
13@@ -1,13 +1,13 @@
14-dbusservicedir = $(DBUS_SERVICE_DIR)
15-dbusservice_DATA = \
16- com.canonical.indicator.network.service
17+upstartjobdir = $(datadir)/upstart/sessions/
18+upstartjob_DATA = \
19+ indicator-network.conf
20
21 EXTRA_DIST = \
22- com.canonical.indicator.network.service.in
23-
24-CLEANFILES = $(dbusservice_DATA)
25-
26-com.canonical.indicator.network.service: com.canonical.indicator.network.service.in Makefile
27+ indicator-network.conf.in
28+
29+CLEANFILES = $(upstartjob_DATA)
30+
31+indicator-network.conf: indicator-network.conf.in Makefile
32 sed -e 's|@NETWORK_SERVICE[@]|$(pkglibdir)/indicator-network-menu-server|g' $< >$@
33
34 indicatordir = $(INDICATOR_DIR)
35
36=== removed file 'data/com.canonical.indicator.network.service.in'
37--- data/com.canonical.indicator.network.service.in 2013-04-24 03:49:29 +0000
38+++ data/com.canonical.indicator.network.service.in 1970-01-01 00:00:00 +0000
39@@ -1,3 +0,0 @@
40-[D-BUS Service]
41-Name=com.canonical.indicator.network
42-Exec=@NETWORK_SERVICE@
43
44=== added file 'data/indicator-network.conf.in'
45--- data/indicator-network.conf.in 1970-01-01 00:00:00 +0000
46+++ data/indicator-network.conf.in 2013-04-24 03:49:29 +0000
47@@ -0,0 +1,12 @@
48+description "Indicator Network Backend"
49+author "Ted Gould <ted@canonical.com>"
50+
51+start on indicators-loaded
52+stop on desktop-end
53+
54+env G_MESSAGES_DEBUG=all
55+export G_MESSAGES_DEBUG
56+
57+respawn
58+
59+exec @NETWORK_SERVICE@
60
61=== modified file 'debian/control'
62--- debian/control 2013-04-23 14:35:48 +0000
63+++ debian/control 2013-04-24 03:49:29 +0000
64@@ -28,7 +28,6 @@
65 Architecture: any
66 Depends: ${shlibs:Depends},
67 ${misc:Depends},
68- indicators-client,
69 Description: Systems settings menu service - Network indicator
70 The Indicator-network service is responsible for exporting the system settings
71 menu through dbus.
72
73=== added file 'debian/indicator-network-crashdb.conf'
74--- debian/indicator-network-crashdb.conf 1970-01-01 00:00:00 +0000
75+++ debian/indicator-network-crashdb.conf 2013-04-24 03:49:29 +0000
76@@ -0,0 +1,6 @@
77+
78+indicator_network = {
79+ 'impl': 'launchpad',
80+ 'project': 'indicator-network',
81+ 'bug_pattern_base': None,
82+}
83
84=== modified file 'debian/rules'
85--- debian/rules 2013-04-12 19:25:53 +0000
86+++ debian/rules 2013-04-24 03:49:29 +0000
87@@ -14,6 +14,10 @@
88 override_dh_install:
89 find $(CURDIR) -name \*.la -delete
90 find $(CURDIR) -name \*.a -delete
91+ mkdir -p debian/indicator-network/etc/apport/crashdb.conf.d
92+ cp debian/indicator-network-crashdb.conf debian/indicator-network/etc/apport/crashdb.conf.d
93+ mkdir -p debian/indicator-network/usr/share/apport/package-hooks
94+ cp debian/source_indicator-network.py debian/indicator-network/usr/share/apport/package-hooks
95 dh_install --list-missing
96
97 override_dh_auto_test:
98
99=== added file 'debian/source_indicator-network.py'
100--- debian/source_indicator-network.py 1970-01-01 00:00:00 +0000
101+++ debian/source_indicator-network.py 2013-04-24 03:49:29 +0000
102@@ -0,0 +1,10 @@
103+import os.path
104+from apport.hookutils import *
105+from xdg.BaseDirectory import xdg_cache_home
106+
107+def add_info(report):
108+ if not apport.packaging.is_distro_package(report['Package'].split()[0]):
109+ report['ThirdParty'] = 'True'
110+ report['CrashDB'] = 'indicator_network'
111+
112+ attach_file_if_exists(report, os.path.join(xdg_cache_home, 'upstart', 'indicator-network.log'), 'indicator-network.log')
113
114=== modified file 'network/network-action-manager.vala'
115--- network/network-action-manager.vala 2013-04-24 03:49:29 +0000
116+++ network/network-action-manager.vala 2013-04-24 03:49:29 +0000
117@@ -172,6 +172,8 @@
118 if (app.lookup_action (ap.get_path ()) != null)
119 return;
120
121+ GLib.debug("Adding new access point: " + ap.get_bssid() + " at " + ap.get_path());
122+
123 //TODO: Add actions for each AP NM connection
124 var strength_action_id = ap.get_path () + "::strength";
125 ap.notify.connect (ap_strength_changed);
126@@ -193,6 +195,8 @@
127
128 private void remove_ap (NM.AccessPoint ap)
129 {
130+ GLib.debug("Removing access point: " + ap.get_bssid() + " at " + ap.get_path());
131+
132 //TODO: Check if AP has connection action
133 app.remove_action (ap.get_path ());
134 app.remove_action (ap.get_path () + "::strength");

Subscribers

People subscribed via source and target branches