[bug#27083] screen-lockers: i3lock-color and i3lock-fancy

[Chris Marusich]

ng0 <address@hidden> writes:

> Hi Chris,
>
> thanks for your review. I'll have no time to address it before next week,
> just a couple of comments.

No worries!  Whenever you can update it, that's great!  We're almost
there; I think the next update will probably be the final one.

> Chris Marusich transcribed 9.7K bytes:
>>
>> ...
>> 
>> ng0 <address@hidden> writes:
>> 
>> > I've changed my mind. As there's also 'i3lock' (without the -color suffix)
>> > but we don't package it (yet), it would take away the freedom to
>> > decide which i3lock you want to use.
>> > The author of i3lock-fancy wantsus to use i3lock-color, but there's
>> > also a check and it falls back to the normal i3lock.
>> >
>> > Therefore I think it must be up to people using i3lock-fancy to
>> > install i3lock-color AND i3lock-fancy in their system profile
>> > (or just i3lock-color in systemprofile + suid it, and i3lock-fancy
>> > in the user profile).
>> 
>> I agree with your assessment.  For now, I think that's a fine plan,
>> although I dislike the fact that by adding the i3lock-fancy package by
>> itself, we will make it possible for someone to naively install just
>> that package to their profile, only to find that it doesn't work because
>> i3lock itself is not installed in the system as a screen locker program.
>> However, I can't think of a better solution at this time, and we're
>> already doing something similar for the other screen locker packages.
>> For example, if you install xscreensaver into your profile without also
>> installing it into the system as a screen locker service, xscreensaver
>> won't work because it won't be setuid-root.  So I think it's OK.
>
> We'll continue to have these problems with applications, maybe we should
> consider it a bug and find a common description. So far it's limited to
> screensavers, but I can't think of a reason why this problem would stay
> limited to applications of the screensaver class.

I've created an email thread to discuss this in more detail on
guix-devel.  The subject is: "Installing some packages results in
"incomplete deployment"".  Please look for it and reply there if you
have any other thoughts on that matter.

For this patch, though, I think it's fine to ask users to install
i3lock-fancy and also add to their operating system declaration a screen
locker service that uses i3lock.  That's how the existing screen lockers
have been packaged.

> For this specific application (i3lock-fancy), I could ask meskarune if she'd 
> like to add
> an error message if none is place already. At the top of the script
> check for the existence of the i3lock binary in the users path (simply
> 'i3lock', not (only) total paths). If not found, throw an error and abort
> with a message that "you need to install i3lock-color".

I think that would be nice.  I also think we can add i3lock-fancy to
Guix even before she makes those changes.

>> > i3lock-color should be removed before commiting this patch,
>> > and we should add a note about this to the description. Like:
>> > "You will need to install i3lock or one of its variants (like
>> > i3lock-color) to make use of i3lock-fancy."
>> 
>> I don't understand: why do we need to remove i3lock-color?
>
> i3lock-fancy is Bash script. I'm patching in the references it
> should keep. The 2 calls of i3lock in there must be just 'i3lock',
> this will call the binary with the suid, not for example the one
> in the store path of i3lock-color (which never worked).
> More correct would be this remark:
> i3lock-color should be removed from the inputs before commiting
> this patch, and we should add a note about this to the description.

I understand now, and I agree.

-- 
Chris

signature.asc
Description: PGP signature