Created attachment 235166 [details] [review]
loginManager: Make suspend() a NOP in the ConsoleKit path

UPower will remove its suspend support eventually, and g-s-d already
depends on logind for power management.

Review of attachment 235166 [details] [review]:

Uhm... Ok, but Upower has not removed its suspend support yet, and gsd's plugin can be disabled.
Is there a particular reason for this, other than upsetting Gentoo and Debian?

(In reply to comment #2)
> Review of attachment 235166 [details] [review]:
> 
> Uhm... Ok, but Upower has not removed its suspend support yet

According to Bastien it is going to happen "soon".


> Is there a particular reason for this, other than upsetting Gentoo and Debian?

"support for suspend relies on the use of logind and won't be available otherwise" is easier to communicate than "automatic suspend will only work when using logind; suspend is still available from the user menu, although your screen may not be locked at all or only after coming back from suspend"

Attachment 235166 [details] pushed as b91d9c2 - loginManager: Make suspend() a NOP in the ConsoleKit path

For the record, I think this is fine.

I'm still not convinced that "power management = non-basic functionality". Fortunately this commit was easy enough to revert as a distro patch once I got annoyed enough at having to run "sudo pm-suspend".

To be slightly more constructive, shouldn't this commit also have removed the Suspend button completely on non-systemd systems rather than having a dummy button?

Created attachment 235489 [details] [review]
loginManager: Fix canSuspend in non-logind path

It is supposed to be a NOP, but commit b91d9c28679 got it wrong.


(In reply to comment #6)
> I'm still not convinced that "power management = non-basic functionality".
> Fortunately this commit was easy enough to revert as a distro patch once I got
> annoyed enough at having to run "sudo pm-suspend".

I suspect UPower dropping support for suspend later this cycle (unless I misunderstood the time frame for this), at which point a simple revert won't do anymore.


> To be slightly more constructive, shouldn't this commit also have removed the
> Suspend button completely on non-systemd systems rather than having a dummy
> button?

Yes, that's what the patch was supposed to do. Apparently I messed it up, fix attached.

Still, I don't understand this whole "upower is dropping support for suspend".
Ubuntu is going to systemd any time soon, AFAIUI, so they'll keep using and maintaining suspend code in upower, even if it is their own fork.

(In reply to comment #8)
> Still, I don't understand this whole "upower is dropping support for suspend".
> Ubuntu is going to systemd any time soon, AFAIUI, so they'll keep using and
> maintaining suspend code in upower, even if it is their own fork.

That's fine, but we won't need to support any issues in the non-logind path upstream (ensuring the screen is locked before suspending being the most prominent one)

Review of attachment 235489 [details] [review]:

OK.

Attachment 235489 [details] pushed as 1bf996c - loginManager: Fix canSuspend in non-logind path