[Sugar-devel] [sugar] Unset gnome keyring environment variables

[Caspar Bothmer]

Hello Daniel,

first of all, as Simon mentioned, this patch is the proper solution to make gnome-keyring-daemon start a new instance within the windowed sugar session.

That said, here are a few suggestions.

> Subject: [sugar] Unset gnome keyring environment variables
Ok, the patch does this.  But why?  What about "Enable gnome-keyring-daemon to run within sugar-emulator"?

> Rather than starting it manually. As suggested on
> https://bugzilla.gnome.org/show_bug.cgi?id=628302

It's great you included the link to the bug report.  It helped me find the necessary information to understand the patch.  But why didn't you explain the patch, either in your own words or by including the explanation Stef Walter gave within the bug report?  It explains both the problem and the solution.


> @@ -120,6 +120,11 @@ def _start_window_manager():
>
>
>  def _setup_env(display, scaling, emulator_pid):
> +    for variable in ['GPG_AGENT_INFO', 'SSH_AUTH_SOCK',
> +                     'GNOME_KEYRING_CONTROL', 'GNOME_KEYRING_PID']:
> +        if variable in os.environ:
> +            del os.environ[variable]
> +
>      os.environ['SUGAR_EMULATOR'] = 'yes'

This patch is straight forward, it does its job.  But the code is not self-explaining as it's not clear from the code, that this allows, gnome-keyring-daemon to start a new instance within the sugar-emulator.  So you need to step in and provide a comment explaining the reasoning behind.


Ok, that's it.  Nice job.  I am looking forward to more patches from you.

Caspar