Ubuntu Push Notifications

Merge lp:~jonas-drange/ubuntu-push/fix-1628327 into lp:ubuntu-push

fix-1628327
Merge into trunk

Proposed by Jonas G. Drange on 2016-09-29

Status:	Merged
Approved by:	Jonas G. Drange on 2016-10-03
Approved revision:	184
Merged at revision:	173
Proposed branch:	lp:~jonas-drange/ubuntu-push/fix-1628327
Merge into:	lp:ubuntu-push
Diff against target:	194 lines (+32/-17) 12 files modified Makefile (+2/-2) bus/haptic/haptic.go (+2/-2) bus/notifications/raw.go (+1/-1) click/cnotificationsettings/cnotificationsettings.go (+1/-1) client/client.go (+17/-2) client/service/postal.go (+1/-1) client/session/seenstate/sqlseenstate.go (+1/-1) client/session/seenstate/sqlseenstate_test.go (+1/-1) debian/control (+3/-2) dependencies.tsv (+2/-2) logger/logger_test.go (+0/-1) server/api/handlers.go (+1/-1)
To merge this branch:	bzr merge lp:~jonas-drange/ubuntu-push/fix-1628327
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
system-apps-ci-bot	continuous-integration	2016-09-29	Needs Fixing on 2016-10-03
Guillermo Gonzalez			Approve on 2016-09-30
Łukasz Zemczak		2016-09-29	Approve on 2016-09-30
Review via email: mp+307159@code.launchpad.net

This proposal supersedes a proposal from 2016-09-28.

Commit Message

* Drops dead dependencies, replaces them with newer ones.
* Allows u-p to run without system image.
* Adds upstart dep to avoid dep wait on s390x.
* Fixes some formatting issues by use of go fmt.

system-apps-ci-bot (system-apps-ci-bot) wrote on 2016-09-28: Posted in a previous version of this proposal

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-push-ci/34/rebuild

review: Needs Fixing (continuous-integration)

system-apps-ci-bot (system-apps-ci-bot) wrote on 2016-09-28: Posted in a previous version of this proposal

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-push-ci/35/rebuild

review: Needs Fixing (continuous-integration)

system-apps-ci-bot (system-apps-ci-bot) wrote on 2016-09-29: Posted in a previous version of this proposal

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-push-ci/36/rebuild

review: Needs Fixing (continuous-integration)

system-apps-ci-bot (system-apps-ci-bot) wrote on 2016-09-29: Posted in a previous version of this proposal

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-push-ci/37/rebuild

review: Needs Fixing (continuous-integration)

system-apps-ci-bot (system-apps-ci-bot) wrote on 2016-09-29:

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-push-ci/38/rebuild

review: Needs Fixing (continuous-integration)

lp:~jonas-drange/ubuntu-push/fix-1628327 updated on 2016-09-29

181. By Jonas G. Drange on 2016-09-29: update deps tsv
182. By Jonas G. Drange on 2016-09-29: drop --with-install as it is redundant
183. By Jonas G. Drange on 2016-09-29: add upstart depends

system-apps-ci-bot (system-apps-ci-bot) wrote on 2016-09-29:

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-push-ci/39/rebuild

review: Needs Fixing (continuous-integration)

system-apps-ci-bot (system-apps-ci-bot) wrote on 2016-09-29:

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-push-ci/40/rebuild

review: Needs Fixing (continuous-integration)

Łukasz Zemczak (sil2100) wrote on 2016-09-30:

Looks good from my POV. But please note that I have no expertise in golang, so the GO bits would be nice to have reviewed by somone else as well. Packaging wise it's all good either way.
One thing: please mention the upstart dep addition to the commit message.

review: Approve

Guillermo Gonzalez (verterok) wrote on 2016-09-30:

looks good. but I'm a bit worried about not checking the error returned by systemimage, as it might be a valid error case for a system with system-image.
But I have no idea about the error types returned by the dbus stuff. My only request is to please double check that case.

Thanks.

review: Approve

Jonas G. Drange (jonas-drange) wrote on 2016-10-01:

Thanks, Guillermo. You are right, let me think about it. I guess we'd want to crash if 1) system-image runs and 2) system-image returns some valid error.

lp:~jonas-drange/ubuntu-push/fix-1628327 updated on 2016-10-03

184. By Jonas G. Drange on 2016-10-03: only allow SI to fail using a specific error message

system-apps-ci-bot (system-apps-ci-bot) wrote on 2016-10-03:

Click here to trigger a rebuild:
https://jenkins.canonical.com/system-apps/job/lp-ubuntu-push-ci/41/rebuild

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Jonas G. Drange

Stuart Bishop

Ubuntu Push Hackers

1	=== modified file 'Makefile'
2	--- Makefile 2016-03-09 16:12:39 +0000
3	+++ Makefile 2016-10-03 11:09:10 +0000
4	@@ -10,8 +10,8 @@
5	GODEPS = launchpad.net/gocheck
6	GODEPS += launchpad.net/go-dbus/v1
7	GODEPS += launchpad.net/go-xdg/v0
8	-GODEPS += code.google.com/p/gosqlite/sqlite3
9	-GODEPS += code.google.com/p/go-uuid/uuid
10	+GODEPS += github.com/mattn/go-sqlite3
11	+GODEPS += github.com/pborman/uuid
12
13	# cgocheck=0 is a workaround for lp:1555198
14	GOTEST := GODEBUG=cgocheck=0 ./scripts/goctest
15
16	=== modified file 'bus/haptic/haptic.go'
17	--- bus/haptic/haptic.go 2016-07-08 17:05:38 +0000
18	+++ bus/haptic/haptic.go 2016-10-03 11:09:10 +0000
19	@@ -61,8 +61,8 @@
20	return false
21	}
22
23	- if (haptic.acc.SilentMode()) {
24	- if (!vibrateInSilentMode()) {
25	+ if haptic.acc.SilentMode() {
26	+ if !vibrateInSilentMode() {
27	haptic.log.Debugf("[%s] vibrate disabled by user when in Silent Mode.", nid)
28	return false
29	}
30
31	=== modified file 'bus/notifications/raw.go'
32	--- bus/notifications/raw.go 2016-07-08 17:05:38 +0000
33	+++ bus/notifications/raw.go 2016-10-03 11:09:10 +0000
34	@@ -141,7 +141,7 @@
35	panic("please check notification is not nil before calling present")
36	}
37
38	- if (!canUseBubblesNotify(app)) {
39	+ if !canUseBubblesNotify(app) {
40	raw.log.Debugf("[%s] bubbles disabled by user for this app.", nid)
41
42	if raw.sound != nil {
43
44	=== modified file 'click/cnotificationsettings/cnotificationsettings.go'
45	--- click/cnotificationsettings/cnotificationsettings.go 2016-08-12 21:09:43 +0000
46	+++ click/cnotificationsettings/cnotificationsettings.go 2016-10-03 11:09:10 +0000
47	@@ -74,7 +74,7 @@
48	}
49
50	// Define notifications settings GSettings path
51	- if (strcmp(pkgname, "") == 0) {
52	+ if (strcmp(pkgname, "") == 0) {
53	// Use "dpkg" as package name for legacy apps
54	path = g_strconcat(SETTINGS_BASE_PATH, "dpkg/", appname, "/", NULL);
55	} else {
56
57	=== modified file 'client/client.go'
58	--- client/client.go 2016-03-09 16:09:34 +0000
59	+++ client/client.go 2016-10-03 11:09:10 +0000
60	@@ -50,6 +50,10 @@
61	"launchpad.net/ubuntu-push/util"
62	)
63
64	+const (
65	+ SI_NO_SERVICE_ERROR = "org.freedesktop.DBus.Error.ServiceUnknown: The name com.canonical.SystemImage was not provided by any .service files"
66	+)
67	+
68	// ClientConfig holds the client configuration
69	type ClientConfig struct {
70	connectivity.ConnectivityConfig // q.v.
71	@@ -293,11 +297,22 @@
72	util.NewAutoRedialer(client.systemImageEndp).Redial()
73	sysimg := systemimage.New(client.systemImageEndp, client.log)
74	info, err := sysimg.Information()
75	+
76	if err != nil {
77	- return err
78	+ /* SI is not running, so don't fail but rather provide unknown/empty details. See lp:1628522 */
79	+ if err.Error() == SI_NO_SERVICE_ERROR {
80	+ info = &systemimage.InfoResult{
81	+ BuildNumber: 0,
82	+ Device: "unknown",
83	+ Channel: "",
84	+ LastUpdate: "",
85	+ }
86	+ } else {
87	+ return err
88	+ }
89	}
90	client.systemImageInfo = info
91	- return err
92	+ return nil
93	}
94
95	// initSessionAndPoller creates the session and the poller objects
96
97	=== modified file 'client/service/postal.go'
98	--- client/service/postal.go 2016-07-08 17:05:38 +0000
99	+++ client/service/postal.go 2016-10-03 11:09:10 +0000
100	@@ -21,7 +21,7 @@
101	"os"
102	"sync"
103
104	- "code.google.com/p/go-uuid/uuid"
105	+ "github.com/pborman/uuid"
106
107	"launchpad.net/ubuntu-push/bus"
108	"launchpad.net/ubuntu-push/bus/accounts"
109
110	=== modified file 'client/session/seenstate/sqlseenstate.go'
111	--- client/session/seenstate/sqlseenstate.go 2015-01-29 09:49:36 +0000
112	+++ client/session/seenstate/sqlseenstate.go 2016-10-03 11:09:10 +0000
113	@@ -21,7 +21,7 @@
114	"fmt"
115	"strings"
116
117	- _ "code.google.com/p/gosqlite/sqlite3"
118	+ _ "github.com/mattn/go-sqlite3"
119
120	"launchpad.net/ubuntu-push/protocol"
121	)
122
123	=== modified file 'client/session/seenstate/sqlseenstate_test.go'
124	--- client/session/seenstate/sqlseenstate_test.go 2015-01-29 09:49:36 +0000
125	+++ client/session/seenstate/sqlseenstate_test.go 2016-10-03 11:09:10 +0000
126	@@ -19,7 +19,7 @@
127	import (
128	"database/sql"
129
130	- _ "code.google.com/p/gosqlite/sqlite3"
131	+ _ "github.com/mattn/go-sqlite3"
132	. "launchpad.net/gocheck"
133
134	"launchpad.net/ubuntu-push/protocol"
135
136	=== modified file 'debian/control'
137	--- debian/control 2015-11-30 20:59:30 +0000
138	+++ debian/control 2016-10-03 11:09:10 +0000
139	@@ -10,8 +10,8 @@
140	golang-go-dbus-dev,
141	golang-go-xdg-dev,
142	golang-gocheck-dev,
143	- golang-gosqlite-dev,
144	- golang-uuid-dev,
145	+ golang-github-mattn-go-sqlite3-dev,
146	+ golang-github-pborman-uuid-dev,
147	libgcrypt11-dev,
148	libglib2.0-dev (>= 2.31.6),
149	libmessaging-menu-dev,
150	@@ -24,6 +24,7 @@
151	libaccounts-glib-dev,
152	cmake,
153	python3,
154	+ upstart,
155	Standards-Version: 3.9.5
156	Homepage: http://launchpad.net/ubuntu-push
157	Vcs-Bzr: lp:ubuntu-push
158
159	=== modified file 'dependencies.tsv'
160	--- dependencies.tsv 2015-11-20 15:03:47 +0000
161	+++ dependencies.tsv 2016-10-03 11:09:10 +0000
162	@@ -1,5 +1,5 @@
163	-code.google.com/p/go-uuid hg 7dda39b2e7d5e265014674c5af696ba4186679e9 11
164	-code.google.com/p/gosqlite hg 74691fb6f83716190870cde1b658538dd4b18eb0 15
165	+github.com/mattn/go-sqlite3 git 4b0af852c17164dce48e6754e3094c55192e4934 2016-09-27T02:28:46Z
166	+github.com/pborman/uuid git b984ec7fa9ff9e428bd0cf0abf429384dfbe3e37 2016-08-24T21:06:00Z
167	launchpad.net/go-dbus/v1 bzr jlenton@gmail.com-20141023032446-s5icvsucwlv5o38a 129
168	launchpad.net/go-xdg/v0 bzr john.lenton@canonical.com-20140208094800-gubd5md7cro3mtxa 10
169	launchpad.net/gocheck bzr gustavo@niemeyer.net-20140225173054-xu9zlkf9kxhvow02 87
170
171	=== modified file 'logger/logger_test.go'
172	--- logger/logger_test.go 2016-07-18 11:12:23 +0000
173	+++ logger/logger_test.go 2016-10-03 11:09:10 +0000
174	@@ -140,7 +140,6 @@
175	baselog.Output(1, "foobar")
176	logger.Output(1, "foobaz")
177
178	-
179	out := buf.String()
180
181	/* Workaround for https://github.com/golang/go/issues/16406 which is
182
183	=== modified file 'server/api/handlers.go'
184	--- server/api/handlers.go 2014-11-17 19:44:15 +0000
185	+++ server/api/handlers.go 2016-10-03 11:09:10 +0000
186	@@ -27,7 +27,7 @@
187	"net/http"
188	"time"
189
190	- "code.google.com/p/go-uuid/uuid"
191	+ "github.com/pborman/uuid"
192
193	"launchpad.net/ubuntu-push/logger"
194	"launchpad.net/ubuntu-push/protocol"

Ubuntu Push Notifications

Merge lp:~jonas-drange/ubuntu-push/fix-1628327 into lp:ubuntu-push

Commit Message

Description of the Change

Preview Diff

Subscribers