GNU bug report logs - #48220
[PATCH] gnu: xfce4-session: Add xset to propagated-inputs.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 48220 in the body.
You can then email your comments to 48220 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Brendan Tildesley <mail <at> brendan.scot>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: xfce4-session: Add xset to propagated-inputs.
Date: Tue,  4 May 2021 19:20:51 +1000

* gnu/packages/xfce.scm (xfce4-session):[propagated-inputs]: Add xset so
that xflock4 can turn off the monitor.
---
 gnu/packages/xfce.scm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gnu/packages/xfce.scm b/gnu/packages/xfce.scm
index 9655d8ccf7..f1e40a94b8 100644
--- a/gnu/packages/xfce.scm
+++ b/gnu/packages/xfce.scm
@@ -718,6 +718,8 @@ your system in categories, so you can quickly find and launch them.")
        ("libsm" ,libsm)
        ("libwnck" ,libwnck)
        ("libxfce4ui" ,libxfce4ui)))
+    (propagated-inputs
+     `(("xset" ,xset)))
     (home-page "https://www.xfce.org/")
     (synopsis "Xfce session manager")
     (description
-- 
2.31.1

Message #8 received at 48220 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Brendan Tildesley <mail <at> brendan.scot>
Cc: 48220 <at> debbugs.gnu.org
Subject: Re: bug#48220: [PATCH] gnu: xfce4-session: Add xset to
 propagated-inputs.
Date: Wed, 05 May 2021 15:23:50 +0200

Hi Brendan,

Brendan Tildesley <mail <at> brendan.scot> skribis:

> * gnu/packages/xfce.scm (xfce4-session):[propagated-inputs]: Add xset so
> that xflock4 can turn off the monitor.

Can’t we instead patch ‘scripts/xflock4’ so that it refers to ‘xset’ by
its absolute file name?

However, my understanding is that the xset code is already a fallback:

--8<---------------cut here---------------start------------->8---
# else run another access locking utility, if installed
for lock_cmd in \
  "xlock -mode blank" \
  "slock"
  do
    set -- $lock_cmd
    if command -v -- $1 >/dev/null 2>&1; then
        $lock_cmd >/dev/null 2>&1 &
	# turn off display backlight:
	xset dpms force off
        exit
    fi
done
--8<---------------cut here---------------end--------------->8---

Probably we should ensure the first ‘for’ loop works as expected.
Perhaps we need to replace ‘xfce4-screensaver-command’ by its absolute
file name there.

WDYT?

Thanks,
Ludo’.

Message #11 received at 48220 <at> debbugs.gnu.org (full text, mbox):

From: Brendan Tildesley <mail <at> brendan.scot>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 48220 <at> debbugs.gnu.org
Subject: Re: bug#48220: [PATCH] gnu: xfce4-session: Add xset to
 propagated-inputs.
Date: Thu, 6 May 2021 04:38:58 +0200 (CEST)

[Message part 1 (text/plain, inline)]

> On 05/05/2021 3:23 PM Ludovic Courtès <ludo <at> gnu.org> wrote:
> 
>  
> Hi Brendan,
> 
> Brendan Tildesley <mail <at> brendan.scot> skribis:
> 
> > * gnu/packages/xfce.scm (xfce4-session):[propagated-inputs]: Add xset so
> > that xflock4 can turn off the monitor.
> 
> Can’t we instead patch ‘scripts/xflock4’ so that it refers to ‘xset’ by
> its absolute file name?
> 

I can. It felt weird just patching one command and not others though.

> However, my understanding is that the xset code is already a fallback:

> 
> --8<---------------cut here---------------start------------->8---
> # else run another access locking utility, if installed
> for lock_cmd in \
>   "xlock -mode blank" \
>   "slock"
>   do
>     set -- $lock_cmd
>     if command -v -- $1 >/dev/null 2>&1; then
>         $lock_cmd >/dev/null 2>&1 &
> 	# turn off display backlight:
> 	xset dpms force off
>         exit
>     fi
> done
> --8<---------------cut here---------------end--------------->8---
> 
> Probably we should ensure the first ‘for’ loop works as expected.
> Perhaps we need to replace ‘xfce4-screensaver-command’ by its absolute
> file name there.
> 

Currently xfce4-screensaver is not installed in the xfce package at all by default,
so it isn't used. I could add it, but for me it was glitchy/flickering and I would keep 
clearing the password as I was entering it so I couldn't log back in. It even does it
in a VM. I do have a recent amd graphics card with proprietary linux though, my computer
can't boot otherwise :(.

If you run guix environment --ad-hoc xfce4-screensaver
then
 xfce4-screensaver &; xfce4-screensaver-command --lock
does it work for you?

I'm not sure what is best, should we be maximal and include xfce4-screensaver in the xfce
package, or be minimal and make people have to install it manually?
> WDYT?
> 
> Thanks,
> Ludo’.

[0001-gnu-xfce4-session-Add-xset-to-propagated-inputs.patch (text/x-patch, attachment)]

[0001-gnu-xfce4-session-Allow-xflock4-to-use-xset.patch (text/x-patch, attachment)]

Message #14 received at 48220 <at> debbugs.gnu.org (full text, mbox):

From: Efraim Flashner <efraim <at> flashner.co.il>
To: Brendan Tildesley <mail <at> brendan.scot>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 48220 <at> debbugs.gnu.org
Subject: Re: [bug#48220] [PATCH] gnu: xfce4-session: Add xset to
 propagated-inputs.
Date: Thu, 6 May 2021 15:03:06 +0300

[Message part 1 (text/plain, inline)]

On Thu, May 06, 2021 at 04:38:58AM +0200, Brendan Tildesley wrote:
> 
> > On 05/05/2021 3:23 PM Ludovic Courtès <ludo <at> gnu.org> wrote:
> > 
> >  
> > Hi Brendan,
> > 
> > Brendan Tildesley <mail <at> brendan.scot> skribis:
> > 
> > > * gnu/packages/xfce.scm (xfce4-session):[propagated-inputs]: Add xset so
> > > that xflock4 can turn off the monitor.
> > 
> > Can’t we instead patch ‘scripts/xflock4’ so that it refers to ‘xset’ by
> > its absolute file name?
> > 
> 
> I can. It felt weird just patching one command and not others though.
> 
> > However, my understanding is that the xset code is already a fallback:
> 
> > 
> > --8<---------------cut here---------------start------------->8---
> > # else run another access locking utility, if installed
> > for lock_cmd in \
> >   "xlock -mode blank" \
> >   "slock"
> >   do
> >     set -- $lock_cmd
> >     if command -v -- $1 >/dev/null 2>&1; then
> >         $lock_cmd >/dev/null 2>&1 &
> > 	# turn off display backlight:
> > 	xset dpms force off
> >         exit
> >     fi
> > done
> > --8<---------------cut here---------------end--------------->8---
> > 
> > Probably we should ensure the first ‘for’ loop works as expected.
> > Perhaps we need to replace ‘xfce4-screensaver-command’ by its absolute
> > file name there.
> > 
> 
> Currently xfce4-screensaver is not installed in the xfce package at all by default,
> so it isn't used. I could add it, but for me it was glitchy/flickering and I would keep 
> clearing the password as I was entering it so I couldn't log back in. It even does it
> in a VM. I do have a recent amd graphics card with proprietary linux though, my computer
> can't boot otherwise :(.
> 
> If you run guix environment --ad-hoc xfce4-screensaver
> then
>  xfce4-screensaver &; xfce4-screensaver-command --lock
> does it work for you?
> 
> I'm not sure what is best, should we be maximal and include xfce4-screensaver in the xfce
> package, or be minimal and make people have to install it manually?
> > WDYT?
> > 
> > Thanks,
> > Ludo’.

Are you using %desktop-services in your OS config? By default they come
with a screen-locker-service for both slock and xlockmore.

-- 
Efraim Flashner   <efraim <at> flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[signature.asc (application/pgp-signature, inline)]

Message #17 received at 48220 <at> debbugs.gnu.org (full text, mbox):

From: Brendan Tildesley <mail <at> brendan.scot>
To: Efraim Flashner <efraim <at> flashner.co.il>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 48220 <at> debbugs.gnu.org
Subject: Re: [bug#48220] [PATCH] gnu: xfce4-session: Add xset to
 propagated-inputs.
Date: Thu, 6 May 2021 14:13:12 +0200 (CEST)

> On 05/06/2021 2:03 PM Efraim Flashner <efraim <at> flashner.co.il> wrote:
> 
>  
> On Thu, May 06, 2021 at 04:38:58AM +0200, Brendan Tildesley wrote:
> > 
> > > On 05/05/2021 3:23 PM Ludovic Courtès <ludo <at> gnu.org> wrote:
> > > 
> > >  
> > > Hi Brendan,
> > > 
> > > Brendan Tildesley <mail <at> brendan.scot> skribis:
> > > 
> > > > * gnu/packages/xfce.scm (xfce4-session):[propagated-inputs]: Add xset so
> > > > that xflock4 can turn off the monitor.
> > > 
> > > Can’t we instead patch ‘scripts/xflock4’ so that it refers to ‘xset’ by
> > > its absolute file name?
> > > 
> > 
> > I can. It felt weird just patching one command and not others though.
> > 
> > > However, my understanding is that the xset code is already a fallback:
> > 
> > > 
> > > --8<---------------cut here---------------start------------->8---
> > > # else run another access locking utility, if installed
> > > for lock_cmd in \
> > >   "xlock -mode blank" \
> > >   "slock"
> > >   do
> > >     set -- $lock_cmd
> > >     if command -v -- $1 >/dev/null 2>&1; then
> > >         $lock_cmd >/dev/null 2>&1 &
> > > 	# turn off display backlight:
> > > 	xset dpms force off
> > >         exit
> > >     fi
> > > done
> > > --8<---------------cut here---------------end--------------->8---
[...]
> Are you using %desktop-services in your OS config? By default they come
> with a screen-locker-service for both slock and xlockmore.
> 

Yep, those ones work but they merely produce a black screen with the backlight on,
then the xset command in the above script actually turns off the screen to save power.
if any of xfce4-screensaver, gnome-screensaver, or xscreensaver are available, then it
will use those first.

Message #22 received at 48220-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Brendan Tildesley <mail <at> brendan.scot>
Cc: 48220-done <at> debbugs.gnu.org
Subject: Re: bug#48220: [PATCH] gnu: xfce4-session: Add xset to
 propagated-inputs.
Date: Sat, 08 May 2021 12:23:21 +0200

Hi!

Brendan Tildesley <mail <at> brendan.scot> skribis:

[...]

>> However, my understanding is that the xset code is already a fallback:
>
>> 
>> --8<---------------cut here---------------start------------->8---
>> # else run another access locking utility, if installed
>> for lock_cmd in \
>>   "xlock -mode blank" \
>>   "slock"
>>   do
>>     set -- $lock_cmd
>>     if command -v -- $1 >/dev/null 2>&1; then
>>         $lock_cmd >/dev/null 2>&1 &
>> 	# turn off display backlight:
>> 	xset dpms force off
>>         exit
>>     fi
>> done
>> --8<---------------cut here---------------end--------------->8---
>> 
>> Probably we should ensure the first ‘for’ loop works as expected.
>> Perhaps we need to replace ‘xfce4-screensaver-command’ by its absolute
>> file name there.
>> 
>
> Currently xfce4-screensaver is not installed in the xfce package at all by default,
> so it isn't used. I could add it, but for me it was glitchy/flickering and I would keep 
> clearing the password as I was entering it so I couldn't log back in. It even does it
> in a VM. I do have a recent amd graphics card with proprietary linux though, my computer
> can't boot otherwise :(.

Hmm OK.  We should address this in a separate issue.

> If you run guix environment --ad-hoc xfce4-screensaver
> then
>  xfce4-screensaver &; xfce4-screensaver-command --lock
> does it work for you?

I get:

--8<---------------cut here---------------start------------->8---
$ xfce4-screensaver & xfce4-screensaver-command --lock
[1] 19108
** Message: 12:20:44.361: Screensaver is not running! Start xfce4-screensaver first
--8<---------------cut here---------------end--------------->8---

… but I’m not running Xfce.  I should prolly try in a VM.

> I'm not sure what is best, should we be maximal and include xfce4-screensaver in the xfce
> package, or be minimal and make people have to install it manually?

I’d lean towards including xfce4-screensaver, since that’s the intended
use of Xfce, but only once it actually works.

> From ed66cf50a3b9294effc8bbae04b0f2564bd55c10 Mon Sep 17 00:00:00 2001
> From: Brendan Tildesley <mail <at> brendan.scot>
> Date: Thu, 6 May 2021 12:34:55 +1000
> Subject: [PATCH] gnu: xfce4-session: Allow xflock4 to use xset.
>
> * gnu/packages/xfce.scm (xfce4-session):
> [inputs]: Add xset.
> [arguments]: Add a phase to use exact store path to xset in xflock4.

[...]

> +         (add-after 'unpack 'patch-xflock
> +           (lambda _
> +             (substitute* "scripts/xflock4"
> +               (("xset") (which "xset"))))))))

Applied after changing ‘which’ to refer to the “xset” input so that it
works correctly when cross-compiling.

Closing, and let’s discuss xfce4-screensaver separately!

Thanks,
Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.