Merge ~sylvain-pineau/checkbox-ng:more-remote-fixes into checkbox-ng:master

Proposed by Sylvain Pineau
Status: Merged
Approved by: Sylvain Pineau
Approved revision: 819be75ccbdc6b76d0d05622c9c1f12795defae4
Merged at revision: 47229249ed71df88e7c8c0f9feec054fb60be4d9
Proposed branch: ~sylvain-pineau/checkbox-ng:more-remote-fixes
Merge into: checkbox-ng:master
Diff against target: 81 lines (+22/-21)
2 files modified
checkbox_ng/launcher/slave.py (+2/-1)
plainbox/impl/session/remote_assistant.py (+20/-20)
Reviewer Review Type Date Requested Status
Maciej Kisielewski (community) Approve
Review via email: mp+418023@code.launchpad.net

Description of the change

Two more patches for remote:

1. Ensure that the future services installed via snap install hooks will select the appropriate restart strategy (must be RemoteDebRestartStrategy)

2. XDG_SESSION_TYPE is added to the extra env var for jobs requiring a different command on wayland sessions.

To post a comment you must log in.
Revision history for this message
Maciej Kisielewski (kissiel) wrote :

Looks good, totally landable. The shell invocation to extract XDG_SESSION_TYPE using bash chain bothered me, so I proposed python version instead below

review: Approve
Revision history for this message
Sylvain Pineau (sylvain-pineau) wrote :

Excellent idea, I've reworked my patches accordingly

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/checkbox_ng/launcher/slave.py b/checkbox_ng/launcher/slave.py
index c7e79df..c184371 100644
--- a/checkbox_ng/launcher/slave.py
+++ b/checkbox_ng/launcher/slave.py
@@ -116,8 +116,9 @@ class RemoteSlave():
116 SessionAssistantSlave.session_assistant = RemoteSessionAssistant(116 SessionAssistantSlave.session_assistant = RemoteSessionAssistant(
117 lambda s: [sys.argv[0] + 'service'])117 lambda s: [sys.argv[0] + 'service'])
118 snap_data = os.getenv('SNAP_DATA')118 snap_data = os.getenv('SNAP_DATA')
119 snap_rev = os.getenv('SNAP_REVISION')
119 remote_restart_strategy_debug = os.getenv('REMOTE_RESTART_DEBUG')120 remote_restart_strategy_debug = os.getenv('REMOTE_RESTART_DEBUG')
120 if snap_data or remote_restart_strategy_debug or ctx.args.resume:121 if (snap_data and snap_rev) or ctx.args.resume:
121 if remote_restart_strategy_debug:122 if remote_restart_strategy_debug:
122 strategy = RemoteSnappyRestartStrategy(debug=True)123 strategy = RemoteSnappyRestartStrategy(debug=True)
123 else:124 else:
diff --git a/plainbox/impl/session/remote_assistant.py b/plainbox/impl/session/remote_assistant.py
index 5763e49..e3c6c11 100644
--- a/plainbox/impl/session/remote_assistant.py
+++ b/plainbox/impl/session/remote_assistant.py
@@ -27,7 +27,6 @@ from collections import namedtuple
27from contextlib import suppress27from contextlib import suppress
28from tempfile import SpooledTemporaryFile28from tempfile import SpooledTemporaryFile
29from threading import Thread, Lock29from threading import Thread, Lock
30from subprocess import CalledProcessError, check_output
3130
32from plainbox.impl.execution import UnifiedRunner31from plainbox.impl.execution import UnifiedRunner
33from plainbox.impl.session.assistant import SessionAssistant32from plainbox.impl.session.assistant import SessionAssistant
@@ -211,26 +210,26 @@ class RemoteSessionAssistant():
211 self._last_response = response210 self._last_response = response
212 self._state = Running211 self._state = Running
213212
213 def _set_envvar_from_proc(self, name):
214 for path in os.listdir('/proc/'):
215 with suppress(Exception):
216 env = open(
217 os.path.join('/proc/', path, 'environ')).read().split('\0')
218 for key, val in [item.split('=') for item in env]:
219 if key == name:
220 return val
221 return ''
222
214 def _prepare_display_without_psutil(self):223 def _prepare_display_without_psutil(self):
215 try:224 uid = pwd.getpwnam(self._normal_user).pw_uid
216 display_value = check_output(225 return {
217 'strings /proc/*/environ 2>/dev/null | '226 'DISPLAY': self._set_envvar_from_proc('DISPLAY'),
218 'grep -m 1 -oP "(?<=DISPLAY=).*"',227 'XAUTHORITY': self._set_envvar_from_proc('XAUTHORITY'),
219 shell=True, universal_newlines=True).rstrip()228 'XDG_SESSION_TYPE': self._set_envvar_from_proc('XDG_SESSION_TYPE'),
220 xauth_value = check_output(229 'XDG_RUNTIME_DIR': '/run/user/{}'.format(uid),
221 'strings /proc/*/environ 2>/dev/null | '230 'DBUS_SESSION_BUS_ADDRESS':
222 'grep -m 1 -oP "(?<=XAUTHORITY=).*"',231 'unix:path=/run/user/{}/bus'.format(uid)
223 shell=True, universal_newlines=True).rstrip()232 }
224 uid = pwd.getpwnam(self._normal_user).pw_uid
225 return {
226 'DISPLAY': display_value,
227 'XAUTHORITY': xauth_value,
228 'XDG_RUNTIME_DIR': '/run/user/{}'.format(uid),
229 'DBUS_SESSION_BUS_ADDRESS':
230 'unix:path=/run/user/{}/bus'.format(uid)
231 }
232 except CalledProcessError:
233 return {}
234233
235 def prepare_extra_env(self):234 def prepare_extra_env(self):
236 # If possible also set the DISPLAY env var235 # If possible also set the DISPLAY env var
@@ -257,6 +256,7 @@ class RemoteSessionAssistant():
257 return {256 return {
258 'DISPLAY': p_environ['DISPLAY'],257 'DISPLAY': p_environ['DISPLAY'],
259 'XAUTHORITY': p_environ['XAUTHORITY'],258 'XAUTHORITY': p_environ['XAUTHORITY'],
259 'XDG_SESSION_TYPE': p_environ['XDG_SESSION_TYPE'],
260 'XDG_RUNTIME_DIR': '/run/user/{}'.format(uid),260 'XDG_RUNTIME_DIR': '/run/user/{}'.format(uid),
261 'DBUS_SESSION_BUS_ADDRESS':261 'DBUS_SESSION_BUS_ADDRESS':
262 'unix:path=/run/user/{}/bus'.format(uid)262 'unix:path=/run/user/{}/bus'.format(uid)

Subscribers

People subscribed via source and target branches