Merge lp:~attente/mir/1634508 into lp:mir

[William Hua]

Use the default socket location if MIR_SOCKET is "".
(LP: #1634508)

Use the default socket location if MIR_SOCKET is "".

[Cemil Azizoglu (cemil-azizoglu)]

Makes sense

review: Approve

[Alan Griffiths (alan-griffiths)]

*Needs Discussion*

I'd prefer adding the endpoint name to the "Failed to connect" error message. That will help with other stupid mistakes specifying the endpoint.

Amusingly, mir_demo_client_basic seems to be the only place we do that!

review: Needs Information

[Chris Halse Rogers (raof)]

I'm +1 for making the error message more useful; I think this would be more than just the endpoint name we tried to connect to, but also the source of that name (default, environment variable, command-line parameter).

I'm +0 on this; option validation is not unsensible, but the only *correct* validation is provided by the result of connect().

review: Abstain

[William Hua (attente)]

The unity8 upstart script will never be able to sanitize the environment properly because dbus-update-activation-environment is unable to actually unset MIR_SOCKET after it has been set once (which I guess is why it sets it to "" causing the bug in the first place). You could improve the error message, but there is no other way to prevent this particular problem other than to make the mir client library more robust in its interpretation of MIR_SOCKET.

[Alan Griffiths (alan-griffiths)]

OK as a workaround

review: Approve

[Chris Halse Rogers (raof)]

Is there a reason why we don't just fix unity8's upstart script?
Neither X11 nor Wayland handle having DISPLAY="" or WAYLAND_DISPLAY="".

[Iain Lane (laney)]

DBus UpdateActivationEnvironment doesn't let you unset variables completely, and Unity8's script messes with MIR_SOCKET, so it can only set it to an empty variable.

If there were a case in hand where this broke for X11 or Wayland I think those would want to be fixed too.

Or DBus could be, but it would have to be a new API since UAE takes an a{ss} (Variable → Value) so a longer term thing.

[Mir CI Bot (mir-ci-bot)]

review: Approve (continuous-integration)

=== modified file 'src/client/mir_connection_api.cpp'
--- src/client/mir_connection_api.cpp	2016-10-12 06:03:15 +0000
+++ src/client/mir_connection_api.cpp	2016-10-18 13:40:08 +0000
@@ -64,7 +64,7 @@
             else
             {
                 auto socket_env = getenv("MIR_SOCKET");
-                if (socket_env)
+                if (socket_env && socket_env[0])
                     sock = socket_env;
                 else
                     sock = mir::default_server_socket;