Merge lp:~3v1n0/unity/systemd-unit-fixes into lp:unity
| Status: | Merged |
|---|---|
| Approved by: | Marco Trevisan (Treviño) on 2016-09-08 |
| Approved revision: | 4219 |
| Merged at revision: | 4193 |
| Proposed branch: | lp:~3v1n0/unity/systemd-unit-fixes |
| Merge into: | lp:unity |
| Diff against target: |
967 lines (+538/-66) 31 files modified
data/CMakeLists.txt (+7/-1) data/unity7.conf.in (+2/-19) data/unity7.override (+1/-0) data/unity7.service.in (+15/-0) debian/control (+23/-21) debian/libunity-core-6.0-9.install (+3/-2) debian/unity-services.install (+4/-1) debian/unity-services.links (+13/-0) debian/unity.install (+4/-0) debian/unity.links (+1/-0) lockscreen/LockScreenController.cpp (+5/-0) lockscreen/LockScreenController.h (+3/-0) services/CMakeLists.txt (+31/-0) services/unity-panel-service-lockscreen.conf.in (+3/-1) services/unity-panel-service-lockscreen.override (+1/-0) services/unity-panel-service-lockscreen.service.in (+2/-1) services/unity-panel-service.conf.in (+3/-1) services/unity-panel-service.override (+1/-0) services/unity-panel-service.service.in (+10/-0) services/unity-screen-locked.target (+5/-0) tests/CMakeLists.txt (+1/-0) tests/test_lockscreen_controller.cpp (+5/-2) tests/test_systemd_wrapper.cpp (+110/-0) tools/CMakeLists.txt (+6/-0) tools/compiz-profile-selector.in (+19/-0) tools/systemd-prestart-check (+29/-0) tools/unity.cmake (+63/-17) tools/upstart-prestart-check (+17/-0) unity-shared/CMakeLists.txt (+1/-0) unity-shared/SystemdWrapper.cpp (+96/-0) unity-shared/SystemdWrapper.h (+54/-0) |
| To merge this branch: | bzr merge lp:~3v1n0/unity/systemd-unit-fixes |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Andrea Azzarone | Approve on 2016-09-08 | ||
| Ted Gould (community) | 2016-09-02 | Approve on 2016-09-07 | |
| Iain Lane | systemd-units | 2016-09-02 | Approve on 2016-09-07 |
| Marco Trevisan (Treviño) | Pending | ||
|
Review via email:
|
|||
This proposal supersedes a proposal from 2016-07-20.
Commit Message
Unity: add systemd units for the shell and related services, mark unity7 a requirement for ubuntu-session
At it's core this MR is about making unity7 work with the systemd user sessions.
It required a larger change than expected, so some descriptions :-)
It moves the prestart scripts from Upstart into a shared shell script so that both
systemd and Upstart can use the same code.
Added a signal wrapper for Systemd, today we are sending both signals as both
will be running for the time being. In the future the Upstart signals should be
droppable.
For lock screen we created a target for when the lock screen is enabled.
We put the panel job in there, but other jobs can add themselves by
putting a symbolic link into the "unity-
directory to their jobs if they want.
Description of the Change
This is mainly based on lp:~ted/unity/systemd-unit. with some fixes applied on top
| Marco Trevisan (Treviño) (3v1n0) wrote : | # |
| Marco Trevisan (Treviño) (3v1n0) wrote : | # |
I don't see unity-compiz-
Please move that into unity.install together with systemd (and unity7.conf file). I don't remember why they were added there in the past (likely for some gsettings and unity8 dependency on libunity-core), but I think that is not the nicest places to be.
| Marco Trevisan (Treviño) (3v1n0) wrote : | # |
Can you please merge this again with lp:~azzar1/unity/unity-active-plugins--safety-check?
It should be the last time :-)
| Marco Trevisan (Treviño) (3v1n0) wrote : | # |
As per channel discussion:
15:15:38 <Trevinho> tedg: also, I was thinking... using that code in the current yakkety causes the Start/Stop calls to not happen since the proxy is not connected... They get queued though... Maybe you could avoid to call start/stop if the systemd_proxy_ is not connected?
15:16:37 <Trevinho> tedg: at the beginning I had the idea of just provinding one of the two wrappers, depending on what is running in the session, but... It's a little to annoying, and I didn't want you to create an abstract class just for that, but... Maybe...
15:16:55 <tedg> Trevinho: Oh, because it's connecting to the session bus instead of the user bus right now?
15:17:04 <Trevinho> tedg: yes
15:17:14 <tedg> Trevinho: We won't have the Upstart stuff for long hopefully, so I don't think it's worth abstracting.
15:18:18 <Trevinho> tedg: yeah, same I thought...
15:18:36 <Trevinho> tedg: but currently guess we'd get some "Timed out waiting for proxy" erros in unity logs
15:18:37 <Trevinho> so...
15:19:11 <Trevinho> tedg: is an env var available when systemd is there?
15:19:55 <tedg> tedg: I think checking if it's connected is fine. We can drop that too once we get migrated over if it's taking too long.
15:20:03 <Trevinho> ok...
15:20:30 <Trevinho> tedg: I mean, it might be not connected at the very start of the session... Like if starting with unity already locked (because of a crash) though
15:20:48 <Trevinho> but... Yeah, as you say, this is something we can get rid of
15:21:08 <Trevinho> or, juts check for an env var presency for both upstart and systemd wrappers
15:23:01 <Trevinho> tedg: also /usr/share/
15:24:04 <Trevinho> tedg: and probably renamed unity-compiz-
15:25:05 <tedg> K, I was avoiding changing the codebase too much, but since andyrock moved the .conf file into the build system we can move it to lib.
15:33:32 <Trevinho> tedg: yeah, that's nicer
| Ted Gould (ted) wrote : | # |
K, I think that's everything :-)
| Marco Trevisan (Treviño) (3v1n0) wrote : | # |
Still some conflicts with the parent branch, merge again please...
| Marco Trevisan (Treviño) (3v1n0) wrote : | # |
Also, you want me to land this or you prefer to land the branch together with the other systemd branches?
| Marco Trevisan (Treviño) (3v1n0) wrote : | # |
Looks good, just still wondering if you can get rid of that static list from the .service :)
| Ted Gould (ted) wrote : | # |
So I removed the list and put it as a set of symbolic links in the packaging. At least then it is a packaging thing (when suggests change the links should change) more than an Upstream thing.
| Marco Trevisan (Treviño) (3v1n0) wrote : | # |
Mh, this is better... I would have guessed this was done at each indicator level, but I can go with this too.
One only minor thing, it shouldn't never happen that the lockscreen is there without ups running, but... For safety I think it would be better to also add
/usr/lib/
/usr/lib/
/usr/lib/
/usr/lib/
/usr/lib/
| Ted Gould (ted) wrote : | # |
On Tue, 2016-08-09 at 16:09 +0000, Marco Trevisan (Treviño) wrote:
> Mh, this is better... I would have guessed this was done at each
> indicator level, but I can go with this too.
Yes, systemd is "backwards" to Upstart in that you define a state and
how to achieve that state instead of an event that you listen for. So
the state is panel-service running and you achieve that by starting the
indicators instead of the panel service asking the indicators to start.
I miss Upstart, I think it made more sense too :-(
> One only minor thing, it shouldn't never happen that the lockscreen is there without ups running, but... For safety I think it would be better to also add
>
Yup, good idea, added.
| Marco Trevisan (Treviño) (3v1n0) wrote : | # |
Ouch, I'm getting this now:
dpkg: error processing archive /tmp/apt-
trying to overwrite '/usr/lib/
I guess this is because the unity.install +usr/lib/*/unity line, can you set it as it used to be please?
| Ted Gould (ted) wrote : | # |
Set them up to explicitly mention, looks like they end up in the right package now.
| Marco Trevisan (Treviño) (3v1n0) wrote : | # |
I didn't generated scripts have wrong path, you should use FULL_LIBDIR.
[/usr/lib/
| Marco Trevisan (Treviño) (3v1n0) wrote : | # |
The prefixes on the .service and .config files are still wrong... See the inline comments.
Also unity7 doesn't start automatically (it does if I use systemctl manually), and I also see this:
indicator-
● indicator-
● indicator-
● indicator-
● indicator-
● indicator-
● indicator-
● indicator-
unity-
unity-
unity-
unity7.service loaded inactive dead Unity Shell v7
indicators-
| Iain Lane (laney) wrote : | # |
Missing:
- symlink /usr/lib/
- Environment=
| Marco Trevisan (Treviño) (3v1n0) wrote : | # |
> - Environment=
This isn't true anymore, since the profile could also be ubnutu-lowgfx, so this has to be done from the pre-start script
| Marco Trevisan (Treviño) (3v1n0) wrote : | # |
> - Environment=
This isn't true anymore, since the profile could also be ubnutu-lowgfx, so this has to be done from the pre-start script
| Iain Lane (laney) wrote : | # |
On Wed, Aug 31, 2016 at 10:54:22AM -0000, Marco Trevisan (Treviño) wrote:
> > - Environment=
>
> This isn't true anymore, since the profile could also be ubnutu-lowgfx, so this has to be done from the pre-start script
Then make that call dbus-update-
--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]
| Iain Lane (laney) wrote : | # |
On Wed, Aug 31, 2016 at 11:03:38AM -0000, Iain Lane wrote:
> On Wed, Aug 31, 2016 at 10:54:22AM -0000, Marco Trevisan (Treviño) wrote:
> > > - Environment=
> >
> > This isn't true anymore, since the profile could also be ubnutu-lowgfx, so this has to be done from the pre-start script
>
> Then make that call dbus-update-
instead of systemctl set-environment, that is. Most jobs do
initctl set-env -g ...
dbus-
--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]
- 4207. By Marco Trevisan (Treviño) on 2016-09-02
-
unity-panel-
service- lockscreen. service: return proper value if upstart status is running - 4208. By Marco Trevisan (Treviño) on 2016-09-05
-
unity-panel-
service. conf: don't try to start if systemd is managing unity7 - 4209. By Marco Trevisan (Treviño) on 2016-09-05
-
Tools: add systemd/upstart pre-start scripts
- 4210. By Marco Trevisan (Treviño) on 2016-09-05
-
Systemd services: add pre-start to check if unity can run in this environment
- 4211. By Marco Trevisan (Treviño) on 2016-09-05
-
upstart conf: add pre-start to check if unity can run in this environment
- 4212. By Marco Trevisan (Treviño) on 2016-09-05
-
upstart-
prestart- check: return invalid values on failures - 4213. By Marco Trevisan (Treviño) on 2016-09-05
-
upstart-
prestart- check: remove the set -e. We want control things manually here
- 4214. By Marco Trevisan (Treviño) on 2016-09-05
-
systemd-
prestart- check: try to cancel the current running job if it's an unity one - 4215. By Marco Trevisan (Treviño) on 2016-09-05
-
upstart-
prestart- check: don't print anything from systemctl - 4216. By Marco Trevisan (Treviño) on 2016-09-05
-
systemd-
prestart- check: remove empty first line - 4217. By Marco Trevisan (Treviño) on 2016-09-05
-
systemd-
prestart- check: ouch, I forgot to rename var... - 4218. By Marco Trevisan (Treviño) on 2016-09-06
-
systemd-
prestart- check: use systemctl --user cancel to stop the currently running job - 4219. By Marco Trevisan (Treviño) on 2016-09-06
-
unity-panel-
service. service: set it back to After, or stopping it will stop the whole shell
| Iain Lane (laney) wrote : | # |
I think the systemd bits themselves are sane now (don't fully understand the locked stuff though).
As discussed, I think ups and maybe bamf could be Wants instead of Requires, so the session can still somewhat work if they get broken.


There have been changes recently to upstart job to change COMPIZ_ CONFIG_ PROFILE depending on the gfx environment. So please sync with upstream changes on that.
Also, please rebase on lp:~azzar1/unity/unity-active-plugins--safety-check, which will need to be ported too