GNOME Bugzilla – Bug 705525
Uses deprecated UPower functionality
Last modified: 2013-08-06 14:29:31 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 on attachment 250894 [details] [review] patch Thanks for the patch. I've tweaked it a little, and took very long to write tests ;)
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
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.
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;
Yes, I think so. You can also remove the initialization from the policy variable, as you're overwriting it immediately afterwards, anyway.
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