Subject: Re: bug#27083: screen-lockers: i3lock-color and i3lock-fancy

From: Chris Marusich <[email-masked]>
Date: Fri, 17 Nov 2017 01:55:52 -0800

<[email-masked]> writes:

> I have no idea about the error of i3lock-fancy (password is not
> accepted).  An commented phase in i3lock-color is still included where
> I tried to fix this, but I don't know much about PAM at this point.

When I checked using "guix system vm", the reason i3lock refused to
accept the password was because it could not read /etc/shadow.  One
solution to this problem is to do both of the following:

1) Do not wrap i3lock in a wrapper script.

2) Install the i3lock executable as setuid-root.

When installing i3lock as a setuid-root program, it is important not to
wrap it in a script.  It is common in UNIX-like systems to ignore the
setuid bit when the program is an interpreted script instead of an
executable (a simple Internet search for a phrase like "setuid bash
script" will provide lots of supporting information about this).  If you
do (2) without also removing the wrapping logic that you put into the
package definition, you will find that i3lock still rejects your
password - this is because it still can't read /etc/shadow.

The attached patch is a revised version of your first patch to add
i3lock-color.  I have basically just removed the wrapping phase.  To
install it setuid-root along with an appropriate PAM configuration file,
just add a screen-locker service like the following to your services in
your operating system declaration:

    (screen-locker-service i3lock-color "i3lock")

Note that this will create an appropriate PAM configuration file at
/etc/pam.d/i3lock, so you don't need to bother messing around with the
PAM configuration file that comes with i3lock-color.

I haven't looked at the fancy version yet.  I'll check it out when I get
a little more time.

> +  %D%/packages/patches/hypre-doc-tables.patch			\
> +  %D%/packages/patches/hypre-ldflags.patch			\

Did you intend to include these in the patch?  They aren't used, so I've
removed them in my version of the patch.

> +---
> + Makefile | 7 +------
> + 1 file changed, 1 insertion(+), 6 deletions(-)
> +
> +diff --git a/Makefile b/Makefile
> +index c0fe888..b4b6cd9 100644
> +--- a/Makefile
> ++++ b/Makefile
> +@@ -6,11 +6,6 @@ SYSCONFDIR=/etc
> + PKG_CONFIG=pkg-config
> + MANDIR=/usr/share/man
> + 
> +-# Check if pkg-config is installed, we need it for building CFLAGS/LIBS
> +-ifeq ($(shell which $(PKG_CONFIG) 2>/dev/null 1>/dev/null || echo 1),1)
> +-$(error "$(PKG_CONFIG) was not found")
> +-endif
> +-
> + CFLAGS += -std=c99
> + CFLAGS += -pipe
> + CFLAGS += -Wall
> +@@ -18,7 +13,7 @@ CFLAGS += -O2
> + SIMD_CFLAGS += -funroll-loops
> + SIMD_CFLAGS += -msse2
> + CPPFLAGS += -D_GNU_SOURCE
> +-CPPFLAGS += -DXKBCOMPOSE=$(shell if test -e /usr/include/xkbcommon/xkbcommon-compose.h ; then echo 1 ; else echo 0 ; fi )
> ++CPPFLAGS += -DXKBCOMPOSE=1
> + CFLAGS += $(shell $(PKG_CONFIG) --cflags cairo xcb-composite xcb-xinerama xcb-atom xcb-image xcb-xkb xkbcommon xkbcommon-x11)
> + LIBS += $(shell $(PKG_CONFIG) --libs cairo xcb-composite xcb-xinerama xcb-atom xcb-image xcb-xkb xkbcommon xkbcommon-x11)
> + LIBS += -lpam
> +-- 
> +2.13.0

FYI, it looks like maybe we could probably remove this patch file
entirely, and just use substitute* to modify the Makefile, if we
provided the "which" program as an input.  Specifically, if the which
program were present, then probably the check you removed would
succeed.  I haven't tried it myself, though.

> -;;; Copyright (c) 2016, 2017 ng0 <[email-masked]>
> +;;; Copyright (c) 2016, 2017 ng0 <[email-masked]>

FYI, the [email-masked] email address is still mentioned in
multiple other files, too.

> +    (description
> +     "I3lock-color is a screen locker.  It is a re-patched version of
> +i3lock, which is a simple screen locker like slock.  Features include:
> +
> +@enumerate
> +@item forking process, the locked screen is preserved when you suspend from RAM
> +@item specify background color or PNG image to be displayed in the lock screen
> +@item many additional color options
> +@end enumerate\n")

I'm not sure if the trailing newline is necessary.  Unless you beat me
to it, I'll check on this detail the next time I return to this thread.

-- 
Chris
From 53edb9d43966ec9fbea8292608875d0c894adaea Mon Sep 17 00:00:00 2001
From: ng0 <[email-masked]>
Date: Fri, 17 Nov 2017 01:38:14 -0800
Subject: [PATCH] gnu: Add i3lock-color.

* gnu/local.mk (dist_patch_DATA): Add i3lock-color-fix-makefile.patch.
* gnu/packages/patches/i3lock-color-fix-makefile.patch: New file.
* gnu/packages/wm.scm: Add i3lock-color.

Signed-off-by: Chris Marusich <[email-masked]>
---
 gnu/local.mk                                       |  1 +
 .../patches/i3lock-color-fix-makefile.patch        | 38 +++++++++++++
 gnu/packages/wm.scm                                | 66 ++++++++++++++++++++++
 3 files changed, 105 insertions(+)
 create mode 100644 gnu/packages/patches/i3lock-color-fix-makefile.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index 61bebe662..575d88506 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -738,6 +738,7 @@ dist_patch_DATA =						\
   %D%/packages/patches/hubbub-sort-entities.patch		\
   %D%/packages/patches/hurd-fix-eth-multiplexer-dependency.patch        \
   %D%/packages/patches/hydra-disable-darcs-test.patch		\
+  %D%/packages/patches/i3lock-color-fix-makefile.patch          \
   %D%/packages/patches/icecat-avoid-bundled-libraries.patch	\
   %D%/packages/patches/icu4c-CVE-2017-7867-CVE-2017-7868.patch	\
   %D%/packages/patches/icu4c-CVE-2017-14952.patch		\
diff --git a/gnu/packages/patches/i3lock-color-fix-makefile.patch b/gnu/packages/patches/i3lock-color-fix-makefile.patch
new file mode 100644
index 000000000..8243dc548
--- /dev/null
+++ b/gnu/packages/patches/i3lock-color-fix-makefile.patch
@@ -0,0 +1,38 @@
+From eaca36ed78bafb82bd51a9e83c6445ef8f1698c8 Mon Sep 17 00:00:00 2001
+From: ng0 <[email-masked]>
+Date: Mon, 22 May 2017 18:22:19 +0000
+Subject: [PATCH] This fix to the Makefile is required to make i3lock-color
+ build on Guix.
+
+---
+ Makefile | 7 +------
+ 1 file changed, 1 insertion(+), 6 deletions(-)
+
+diff --git a/Makefile b/Makefile
+index c0fe888..b4b6cd9 100644
+--- a/Makefile
++++ b/Makefile
+@@ -6,11 +6,6 @@ SYSCONFDIR=/etc
+ PKG_CONFIG=pkg-config
+ MANDIR=/usr/share/man
+ 
+-# Check if pkg-config is installed, we need it for building CFLAGS/LIBS
+-ifeq ($(shell which $(PKG_CONFIG) 2>/dev/null 1>/dev/null || echo 1),1)
+-$(error "$(PKG_CONFIG) was not found")
+-endif
+-
+ CFLAGS += -std=c99
+ CFLAGS += -pipe
+ CFLAGS += -Wall
+@@ -18,7 +13,7 @@ CFLAGS += -O2
+ SIMD_CFLAGS += -funroll-loops
+ SIMD_CFLAGS += -msse2
+ CPPFLAGS += -D_GNU_SOURCE
+-CPPFLAGS += -DXKBCOMPOSE=$(shell if test -e /usr/include/xkbcommon/xkbcommon-compose.h ; then echo 1 ; else echo 0 ; fi )
++CPPFLAGS += -DXKBCOMPOSE=1
+ CFLAGS += $(shell $(PKG_CONFIG) --cflags cairo xcb-composite xcb-xinerama xcb-atom xcb-image xcb-xkb xkbcommon xkbcommon-x11)
+ LIBS += $(shell $(PKG_CONFIG) --libs cairo xcb-composite xcb-xinerama xcb-atom xcb-image xcb-xkb xkbcommon xkbcommon-x11)
+ LIBS += -lpam
+-- 
+2.13.0
+
diff --git a/gnu/packages/wm.scm b/gnu/packages/wm.scm
index 09c10eaa0..3d5a0ef7a 100644
--- a/gnu/packages/wm.scm
+++ b/gnu/packages/wm.scm
@@ -66,6 +66,7 @@
   #:use-module (gnu packages gperf)
   #:use-module (gnu packages imagemagick)
   #:use-module (gnu packages lua)
+  #:use-module (gnu packages linux)
   #:use-module (gnu packages suckless)
   #:use-module (guix download)
   #:use-module (guix git-download))
@@ -333,6 +334,71 @@ and locate windows on all your workspaces, using an interactive dmenu
 prompt.")
       (license (license:non-copyleft "http://www.wtfpl.net/txt/copying/")))))
 
+;; The last release is from 2015, use the git commit.
+(define-public i3lock-color
+  (let ((commit "275ff68f6a6b7985c65fbaac537f4c018b47942f")
+        (revision "1"))
+  (package
+    (name "i3lock-color")
+    (version (string-append "2.8-" revision "." (string-take commit 7)))
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/chrjguill/i3lock-color")
+             (commit commit)))
+       (file-name (string-append name "-" version "-checkout"))
+       (snippet
+        ;; The Makefile is read-only, we are going to patch it.
+        '(chmod "Makefile" #o755))
+       (patches (search-patches "i3lock-color-fix-makefile.patch"))
+       (sha256
+        (base32
+         "1qhxqj1gwmf473b4b1bfqxdxiw1y6gq9rf97d77jc9s9z267fjm5"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:tests? #f ; No tests included.
+       #:make-flags (list "CC=gcc")
+       #:phases
+       (modify-phases %standard-phases
+         (replace 'configure
+           (lambda* (#:key outputs #:allow-other-keys)
+             (let* ((out (assoc-ref outputs "out"))
+                    (man (string-append out "/share/man"))
+                    (etc (string-append out "/etc")))
+             (substitute* "Makefile"
+               (("PREFIX=/usr")
+                (string-append "PREFIX=" out))
+               (("SYSCONFDIR=/etc")
+                (string-append "SYSCONFDIR=" etc))
+               (("MANDIR=/usr/share/man")
+                (string-append "MANDIR=" man)))
+             #t))))))
+    (inputs
+     `(("xcb-util-image" ,xcb-util-image)
+       ("xcb-util-keysyms" ,xcb-util-keysyms)
+       ("xcb-util" ,xcb-util)
+       ("libxcb" ,libxcb)
+       ("linux-pam" ,linux-pam)
+       ("libev" ,libev)
+       ("libx11" ,libx11)
+       ("cairo" ,cairo)))
+    (native-inputs
+     `(("libxkbcommon" ,libxkbcommon)
+       ("pkg-config" ,pkg-config)))
+    (home-page "https://github.com/chrjguill/i3lock-color")
+    (synopsis "Screenlocker with color configuration support")
+    (description
+     "I3lock-color is a screen locker.  It is a re-patched version of
+i3lock, which is a simple screen locker like slock.  Features include:
+
+@enumerate
+@item forking process, the locked screen is preserved when you suspend from RAM
+@item specify background color or PNG image to be displayed in the lock screen
+@item many additional color options
+@end enumerate\n")
+    (license license:bsd-3))))
+
 (define-public xmonad
   (package
     (name "xmonad")
-- 
2.14.2