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

[ng0 <ng0@infotropique.org>]

[-- Attachment #1: Type: text/plain, Size: 11977 bytes --]

Hi Chris,

thanks for your input. I think I was just stuck by too many
repetive attempts. Thanks for a fresh view :)
I'll send applied changes as soon as I can (probably next week).

Chris Marusich transcribed 11K bytes:
> <ng0@pragmatique.xyz> 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.

Oh, 2 was what I was thinking of that might fix
it. I just had no time to test it.

> 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.

The fancy version is basically a fork with some more fancy visual elements.
It should be similar.

> > +  %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.

Many weeks ago, I don't know what happened there.
Maybe a mistake in rebase.

> > +---
> > + 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.

Okay, I will give it a try.

> > -;;; Copyright (c) 2016, 2017 ng0 <contact.ng0@cryptolab.net>
> > +;;; Copyright (c) 2016, 2017 ng0 <ng0@no-reply.pragmatique.xyz>
> 
> FYI, the contact.ng0@cryptolab.net email address is still mentioned in
> multiple other files, too.

Yes. I'm only changing addresses once I touch the file. So old
addresses might be in there for a long time. Otoh, I will
prepare a patch and change all my header addresses.

> > +    (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.

I've seen this multiple times before and have just followed this style.

> -- 
> Chris

> From 53edb9d43966ec9fbea8292608875d0c894adaea Mon Sep 17 00:00:00 2001
> From: ng0 <ng0@no-reply.pragmatique.xyz>
> 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 <cmmarusich@gmail.com>
> ---
>  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 <ng0@no-reply.pragmatique.xyz>
> +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
> 




-- 
GnuPG: A88C8ADD129828D7EAC02E52E22F9BBFEE348588
GnuPG: https://dl.n0.is/dist/keys/
  WWW: https://we.make.ritual.n0.is

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]