Session Menu

Merge lp:~ted/indicator-session/upstart-job into lp:indicator-session/14.04

  1. upstart-job
  2. Merge into trunk.14.04
Proposed by Ted Gould
Status: Merged
Approved by: Charles Kerr
Approved revision: 399
Merged at revision: 418
Proposed branch: lp:~ted/indicator-session/upstart-job
Merge into: lp:indicator-session/14.04
Diff against target: 111 lines (+57/-21)
5 files modified
.bzrignore (+1/-0)
data/CMakeLists.txt (+39/-18)
data/indicator-session.conf.in (+8/-0)
data/indicator-session.desktop.in (+9/-0)
data/indicator-session.service.in (+0/-3)
To merge this branch: bzr merge lp:~ted/indicator-session/upstart-job
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Charles Kerr (community) Approve
Sebastien Bacher Pending
Review via email: mp+192982@code.launchpad.net

This proposal supersedes a proposal from 2013-05-29.

Description of the change

Upstart job for indicator-session

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:395
http://jenkins.qa.ubuntu.com/job/indicator-session-ci/97/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-session-saucy-amd64-ci/38
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-session-saucy-armhf-ci/35
        deb: http://jenkins.qa.ubuntu.com/job/indicator-session-saucy-armhf-ci/35/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/indicator-session-ci/97/rebuild

review: Approve (continuous-integration)
Revision history for this message
Sebastien Bacher (seb128) wrote : Posted in a previous version of this proposal

Thanks for the work, some questions:

- why do we need/want to export G_MESSAGES_DEBUG from the job? Should that be the default in the source if that should be the default behaviour?

- do we win anything from the change today? (said differently: do we need this cycle? I'm for using upstart, but that indicator doesn't seem a candidate for much dynamic rules and we still have environment issues with upstart job atm)

- you restrict the session to Ubuntu, wasn't the indicator used by gnome-panel sessions as well?

review: Needs Information
Revision history for this message
Ted Gould (ted) wrote : Posted in a previous version of this proposal

On Tue, 2013-08-27 at 18:29 +0000, Sebastien Bacher wrote:

> - why do we need/want to export G_MESSAGES_DEBUG from the job? Should
> that be the default in the source if that should be the default
> behaviour?

I figured this was a good way to start, as the change may break
things :-) I think that we should look at removing it as we start to
get more stable.

> - do we win anything from the change today? (said differently: do we
> need this cycle? I'm for using upstart, but that indicator doesn't
> seem a candidate for much dynamic rules and we still have environment
> issues with upstart job atm)

The biggest driver for the change today is from QA. They want to be
able to shutdown the individual services without having them restart so
that they can replace them with mocks.

> - you restrict the session to Ubuntu, wasn't the indicator used by
> gnome-panel sessions as well?

Uhm, that's interesting. I'm not sure what the mask should be there, !=
ubuntu-touch ?

Revision history for this message
Sebastien Bacher (seb128) wrote : Posted in a previous version of this proposal

> I figured this was a good way to start, as the change may break things :-) I think that we should look at removing it as we start to get more stable.

you expect the upstart change to trigger code bugs? that's debugging mode and we don't want to have to patch all jobs at beta time ... having upstart exporting that variable for all jobs during unstable cycles seems like it would be a better way?

> The biggest driver for the change today is from QA. They want to be able to shutdown the individual services without having them restart so that they can replace them with mocks.

that's fair enough, still I would prefer if we could resolve the current env issues we have before adding extra jobs (we still have e.g indicator-session running gnome-control-center with the wrong XDG_CURRENT_DESKTOP sometimes, which leads to have unity controls not listed)

> Uhm, that's interesting. I'm not sure what the mask should be there, != ubuntu-touch ?

that's a good question, I'm not sure either what's the right set ... "!= ubuntu-touch" seems wrong though, afaik indicators are only used in Unity, gnome-panel, xfce (and maybe lxde), and I'm not sure if indicator-session is being using in xfce/lxde (they use e.g -sound I think). We should probably reach to flavor being doing those changes, as we might create issues for them otherwise

review: Needs Fixing
Revision history for this message
Ted Gould (ted) wrote : Posted in a previous version of this proposal

On Tue, 2013-08-27 at 19:01 +0000, Sebastien Bacher wrote:

> > I figured this was a good way to start, as the change may break
> things :-) I think that we should look at removing it as we start to
> get more stable.
>
>
>
> you expect the upstart change to trigger code bugs? that's debugging
> mode and we don't want to have to patch all jobs at beta time ...
> having upstart exporting that variable for all jobs during unstable
> cycles seems like it would be a better way?

No, but I expect there were a lot of messages and bugs that we can debug
today that we couldn't before because all the log files were
intermingled and the messages and warnings got lost. I'm expecting
apport bugs to be much more useful and I'm trying to optimize that as we
start to fix and refine.

I'm not against making it a global setting. My only comment would be
there that probably most applications aren't looking at the logs, but we
would be for indicators. I have no problem generating them though. I
don't feel like that's a decision I can make, my field of influence is
the indicators :-)

> > The biggest driver for the change today is from QA. They want to be
> able to shutdown the individual services without having them restart
> so that they can replace them with mocks.
>
>
>
> that's fair enough, still I would prefer if we could resolve the
> current env issues we have before adding extra jobs (we still have e.g
> indicator-session running gnome-control-center with the wrong
> XDG_CURRENT_DESKTOP sometimes, which leads to have unity controls not
> listed)

We should fix those bugs, I agree. These branches have been around for
a while, and this change has been planned. It is later than I'd like,
but I'd rather get it done and move on.

> > Uhm, that's interesting. I'm not sure what the mask should be
> there, != ubuntu-touch ?
>
>
>
> that's a good question, I'm not sure either what's the right set ...
> "!= ubuntu-touch" seems wrong though, afaik indicators are only used
> in Unity, gnome-panel, xfce (and maybe lxde), and I'm not sure if
> indicator-session is being using in xfce/lxde (they use e.g -sound I
> think). We should probably reach to flavor being doing those changes,
> as we might create issues for them otherwise

So let's leave it is not filtering right now. The reality is that the
phone won't have indicator-session installed in most cases, and even if
it was, it would only take a bit of RAM not really hurt anything.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:396
http://jenkins.qa.ubuntu.com/job/indicator-session-ci/98/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-session-saucy-amd64-ci/39
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-session-saucy-armhf-ci/36
        deb: http://jenkins.qa.ubuntu.com/job/indicator-session-saucy-armhf-ci/36/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/indicator-session-ci/98/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:397
http://jenkins.qa.ubuntu.com/job/indicator-session-ci/99/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-session-saucy-amd64-ci/40
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-session-saucy-armhf-ci/37
        deb: http://jenkins.qa.ubuntu.com/job/indicator-session-saucy-armhf-ci/37/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/indicator-session-ci/99/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:399
http://jenkins.qa.ubuntu.com/job/indicator-session-ci/105/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-session-trusty-amd64-ci/1
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-session-trusty-armhf-ci/1
        deb: http://jenkins.qa.ubuntu.com/job/indicator-session-trusty-armhf-ci/1/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/indicator-session-ci/105/rebuild

review: Approve (continuous-integration)
Revision history for this message
Charles Kerr (charlesk) :
review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file '.bzrignore'
--- .bzrignore 2013-05-02 17:09:51 +0000
+++ .bzrignore 2013-10-29 00:37:54 +0000
@@ -253,3 +253,4 @@
253dbus-user.c253dbus-user.c
254dbus-user.h254dbus-user.h
255tests/test-service255tests/test-service
256indicator-session.conf
256257
=== modified file 'data/CMakeLists.txt'
--- data/CMakeLists.txt 2013-07-02 02:00:17 +0000
+++ data/CMakeLists.txt 2013-10-29 00:37:54 +0000
@@ -17,24 +17,45 @@
1717
1818
19##19##
20## DBus Service File20## Upstart Config File
21##21##
2222
23# where to install23# where to install
24set (DBUS_SERVICE_DIR "${CMAKE_INSTALL_FULL_DATADIR}/dbus-1/services")24set (UPSTART_JOB_DIR "${CMAKE_INSTALL_FULL_DATADIR}/upstart/sessions")
25message (STATUS "${DBUS_SERVICE_DIR} is the DBus Service File install dir")25message (STATUS "${UPSTART_JOB_DIR} is the Upstart Job install dir")
2626
27set (SERVICE_NAME "${CMAKE_PROJECT_NAME}.service")27set (UPSTART_JOB_NAME "${CMAKE_PROJECT_NAME}.conf")
28set (SERVICE_FILE "${CMAKE_CURRENT_BINARY_DIR}/${SERVICE_NAME}")28set (UPSTART_JOB_FILE "${CMAKE_CURRENT_BINARY_DIR}/${UPSTART_JOB_NAME}")
29set (SERVICE_FILE_IN "${CMAKE_CURRENT_SOURCE_DIR}/${SERVICE_NAME}.in")29set (UPSTART_JOB_FILE_IN "${CMAKE_CURRENT_SOURCE_DIR}/${UPSTART_JOB_NAME}.in")
3030
31# build it31# build it
32set (pkglibexecdir "${CMAKE_INSTALL_FULL_PKGLIBEXECDIR}")32set (pkglibexecdir "${CMAKE_INSTALL_FULL_PKGLIBEXECDIR}")
33configure_file ("${SERVICE_FILE_IN}" "${SERVICE_FILE}")33configure_file ("${UPSTART_JOB_FILE_IN}" "${UPSTART_JOB_FILE}")
3434
35# install it35# install it
36install (FILES "${SERVICE_FILE}"36install (FILES "${UPSTART_JOB_FILE}"
37 DESTINATION "${DBUS_SERVICE_DIR}")37 DESTINATION "${UPSTART_JOB_DIR}")
38
39
40##
41## XDG Autostart Config File
42##
43
44# where to install
45set (XDG_AUTOSTART_DIR "/etc/xdg/autostart")
46message (STATUS "${XDG_AUTOSTART_DIR} is the XDG Autostart install dir")
47
48set (XDG_AUTOSTART_NAME "${CMAKE_PROJECT_NAME}.desktop")
49set (XDG_AUTOSTART_FILE "${CMAKE_CURRENT_BINARY_DIR}/${XDG_AUTOSTART_NAME}")
50set (XDG_AUTOSTART_FILE_IN "${CMAKE_CURRENT_SOURCE_DIR}/${XDG_AUTOSTART_NAME}.in")
51
52# build it
53set (pkglibexecdir "${CMAKE_INSTALL_FULL_PKGLIBEXECDIR}")
54configure_file ("${XDG_AUTOSTART_FILE_IN}" "${XDG_AUTOSTART_FILE}")
55
56# install it
57install (FILES "${XDG_AUTOSTART_FILE}"
58 DESTINATION "${XDG_AUTOSTART_DIR}")
3859
3960
40##61##
4162
=== added file 'data/indicator-session.conf.in'
--- data/indicator-session.conf.in 1970-01-01 00:00:00 +0000
+++ data/indicator-session.conf.in 2013-10-29 00:37:54 +0000
@@ -0,0 +1,8 @@
1description "Indicator Session Service"
2
3start on indicators-loaded or indicator-services-start
4stop on desktop-end or indicator-services-end
5
6respawn
7
8exec @pkglibexecdir@/indicator-session-service
09
=== added file 'data/indicator-session.desktop.in'
--- data/indicator-session.desktop.in 1970-01-01 00:00:00 +0000
+++ data/indicator-session.desktop.in 2013-10-29 00:37:54 +0000
@@ -0,0 +1,9 @@
1[Desktop Entry]
2Type=Application
3Name=Indicator Session
4Exec=@pkglibexecdir@/indicator-session-service
5NotShowIn=Unity;
6NoDisplay=true
7StartupNotify=false
8Terminal=false
9
010
=== removed file 'data/indicator-session.service.in'
--- data/indicator-session.service.in 2013-06-20 19:43:23 +0000
+++ data/indicator-session.service.in 1970-01-01 00:00:00 +0000
@@ -1,3 +0,0 @@
1[D-BUS Service]
2Name=com.canonical.indicator.session
3Exec=@pkglibexecdir@/indicator-session-service

Subscribers

People subscribed via source and target branches