Comment 4 for bug 1703690

Revision history for this message
fossfreedom (fossfreedom) wrote : Re: [Bug 1703690] Re: Add support for Budgie Desktop using GNOME Screensaver

Thanks Łukasz

With regards to the gnome-desktop3 patch. I had a look through the
gsettings API and there didnt seem to be a method to say "is this
gsettings key you are about to use valid" - in fact all the
documentation basically says it is up to the programmer to ensure that
a key being used is actually there ("programmer error"). GSettings
doesnt seem to-do any validation which is a shame.

I would prefer if the whole "draw-background" ubuntu patch could be
dropped (and hence make this patch unnecessary) but I don't know
really what the current impact that would have on the Unity desktop.

Ack with regards to the static functions in gnome-screensaver. I
suppose its a coding style to keep code as localised as possible.

I did ping on both #ubuntu-devel and #ubuntu-desktop recently but no
takers. I don't really know anyone on the GNOME team other than
JBicha.

On 3 August 2017 at 10:15, Łukasz Zemczak <email address hidden> wrote:
> Hello! Thanks for submitting the bug and patches!
>
> I looked briefly at the provided debdiffs and from the packaging POV
> they look good. I'm not sure about the contents though. I am not a GNOME
> developer so I'd like to get some opinion of someone that knows the code
> and would be able to say if it's the right way to go or not. Did you
> discuss these changes with the desktop team or anyone from the GNOME
> uploaders?
>
> Checking the patches I'm a bit worried about a few things. In the gnome-screensaver part, the patch introduces the static function in_desktop() in both gs-lock-plug.c and gs-window-x11.c. I know it's nice to have helper functions like these 'local' but I'm always reluctant whenever I see boilerplate code like this. Maybe it could be made global and somehow shared? I guess it's fine if there's a lot of boilerplate there already, would have to check the codebase.
> The gnome-desktop3 part seems good but a little bit hacky. Wonder if this could be done in a more generic way. Maybe it's not possible though...
>
> Anyway, let's get someone from the GNOME people to take a look before we
> decide about uploading it.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1703690
>
> Title:
> Add support for Budgie Desktop using GNOME Screensaver
>
> Status in gnome-desktop3 package in Ubuntu:
> In Progress
> Status in gnome-screensaver package in Ubuntu:
> In Progress
>
> Bug description:
> Ubuntu Budgie and Budgie Desktop uses GNOME Screensaver for its lock-
> screen.
>
> It also uses gnome-control-center to control both the desktop
> background and the lock-screen background.
>
> GNOME Screensaver does not support gnome-control-center lock screen
> dialog - GNOME has changed to a gsettings path
> org.gnome.desktop.screensaver. GNOME Screensaver is expecting the
> path org.gnome.desktop.background.
>
> To resolve this requires patches in two packages - gnome-screensaver
> and gnome-desktop3.
>
> 1. GNOME Screensaver has been patched to support Budgie Desktop and gnome-control-center.
> Note - I've taken the opportunity to reuse the existing "Unity" patchwork which is now defunct (I believe) since Unity uses an alternative locking mechanism. The lock-screen is styled as per the old "Unity" implementation before Unity 16.04 moved to the new lock screen.
>
> 2. The consequence of making the requisite changes to GNOME
> Screensaver has unfortunately impacted gnome-desktop3. gnome-desktop3
> has a Ubuntu specific patch to revert a GNOME upstream decision to
> remove a key called draw-background. GNOME Screensaver calls a public
> function in gnome-desktop3 - the gsettings path causes a segmentation
> fault since draw-background does not exist in
> org.gnome.desktop.screensaver. I have worked around this to check the
> path being called before pulling the draw-background key.
>
> All of this is explained in the dep3 headers of the two debdiff
> patches attached
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/ubuntu/+source/gnome-desktop3/+bug/1703690/+subscriptions