After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 705525 - Uses deprecated UPower functionality
Uses deprecated UPower functionality
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: power
3.8.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
Depends on:
Blocks:
 
 
Reported: 2013-08-05 16:46 UTC by Jan Alexander Steffens (heftig)
Modified: 2013-08-06 14:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (3.19 KB, patch)
2013-08-05 16:46 UTC, Jan Alexander Steffens (heftig)
reviewed Details | Review

Description Jan Alexander Steffens (heftig) 2013-08-05 16:46:35 UTC
Created attachment 250894 [details] [review]
patch

UPower's can_hibernate/can_suspend is deprecated and will return an error unless upower is built with --enable-deprecated.

Attached a half-assed patch porting the power plugin to use the systemd-logind
interface instead, following a similar patch to gnome-control-center (bug 705275). Unlike that patch, this one wasn't tested much at all beyond warning-free compilation, so please review.
Comment 1 Bastien Nocera 2013-08-06 09:52:37 UTC
Comment on attachment 250894 [details] [review]
patch

Thanks for the patch. I've tweaked it a little, and took very long to write tests ;)
Comment 2 Bastien Nocera 2013-08-06 09:52:53 UTC
commit ef4ae20315de08a41e9c06402d3fad9bdb5983e6
Author: Bastien Nocera <hadess@hadess.net>
Date:   Tue Aug 6 11:48:09 2013 +0200

    power: Add test case for falling back on critical
    
    When hibernate isn't available, make sure that we end up
    suspending instead. This also tests the removal of deprecated
    UPower functions, replaced by logind ones.

commit 099b16dfc9b50c06be2751dcfb7dc1dba8249dba
Author: Bastien Nocera <hadess@hadess.net>
Date:   Tue Aug 6 11:22:30 2013 +0200

    tests: Add support for CanHibernate/CanSuspend

commit 0c5fd82a6ce25abca9a9d7018889bad8fc1c6057
Author: Jan Alexander Steffens (heftig) <jan.steffens@gmail.com>
Date:   Mon Aug 5 18:15:00 2013 +0200

    power: Use logind to discover critical action availability
    
    The upower functionality is deprecated and will return an
    error unless upower is built with --enable-deprecated.
    
    Follows a similar patch to gnome-control-center.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=705525
Comment 3 Jan Alexander Steffens (heftig) 2013-08-06 10:00:07 UTC
Are you sure that's right? I created the additional "setpolicy" variable for a reason. 

Your code only falls back from SUSPEND/HIBERNATE to SHUTDOWN if logind successfully returns something else than "yes". The old code also fell back when the dbus call failed, or, more importantly, in the case of SUSPEND and is_ups==TRUE.
Comment 4 Bastien Nocera 2013-08-06 10:11:18 UTC
That's enough, isn't it?

diff --git a/plugins/power/gsd-power-manager.c b/plugins/power/gsd-power-manager.c
index 346bd9f..c7e8e25 100644
--- a/plugins/power/gsd-power-manager.c
+++ b/plugins/power/gsd-power-manager.c
@@ -1194,6 +1194,8 @@ manager_critical_action_get (GsdPowerManager *manager,
                 if (g_strcmp0 (s, "yes") != 0)
                         policy = GSD_POWER_ACTION_SHUTDOWN;
                 g_variant_unref (result);
+        } else {
+                policy = GSD_POWER_ACTION_SHUTDOWN;
         }
 
         return policy;
Comment 5 Jan Alexander Steffens (heftig) 2013-08-06 10:15:01 UTC
Yes, I think so. You can also remove the initialization from the policy variable, as you're overwriting it immediately afterwards, anyway.
Comment 6 Bastien Nocera 2013-08-06 14:29:31 UTC
commit e7b2835443d4e0bbb4d379de0b9dcae5c36bebfe
Author: Bastien Nocera <hadess@hadess.net>
Date:   Tue Aug 6 16:25:36 2013 +0200

    power: Handle critical action for UPSes
    
    And the case where logind doesn't respond.
    
    Spotted by Jan Alexander Steffens (heftig) <jan.steffens@gmail.com>
    
    https://bugzilla.gnome.org/show_bug.cgi?id=705525