From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34887) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eKfdC-0001op-Vp for guix-patches@gnu.org; Fri, 01 Dec 2017 02:21:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eKfd8-00087v-40 for guix-patches@gnu.org; Fri, 01 Dec 2017 02:21:06 -0500 Received: from debbugs.gnu.org ([208.118.235.43]:58457) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1eKfd7-00087g-V0 for guix-patches@gnu.org; Fri, 01 Dec 2017 02:21:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1eKfd7-0002XE-Jy for guix-patches@gnu.org; Fri, 01 Dec 2017 02:21:01 -0500 Subject: [bug#27083] screen-lockers: i3lock-color and i3lock-fancy Resent-Message-ID: From: Chris Marusich References: <871skx9ql3.fsf@gmail.com> <20171117101835.whd5f7lincppxlgy@abyayala> <20171117201705.75ldk4yxnpd7m3qa@abyayala> <20171117205734.63tbjhxqbcw6sis7@abyayala> <20171117210211.vdauzbvhrjubwamt@abyayala> <20171117211901.eor5yjlvj3icgbsd@abyayala> Date: Thu, 30 Nov 2017 23:20:07 -0800 In-Reply-To: <20171117211901.eor5yjlvj3icgbsd@abyayala> (ng0@infotropique.org's message of "Fri, 17 Nov 2017 21:19:01 +0000") Message-ID: <87374uc3tk.fsf@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: ng0 Cc: 27083@debbugs.gnu.org --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi ng0, I'm glad to hear i3lock works for you, too! I was able to find time to draft feedback for your latest i3lock-color patch, but not for i3lock-fancy (although it looks very simple, and I will check it in the next few days). In the interest of not moving too slowly, I'm sending what I have to you now. ng0 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. >> > > > > > -;;; Copyright (c) 2016, 2017 ng0 >> > > > > > +;;; Copyright (c) 2016, 2017 ng0 >> > > > >=20 >> > > > > FYI, the contact.ng0@cryptolab.net email address is still mentio= ned in >> > > > > multiple other files, too. >> > > >=20 >> > > > 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. Understood. >> > > > > > + (description >> > > > > > + "I3lock-color is a screen locker. It is a re-patched ve= rsion of >> > > > > > +i3lock, which is a simple screen locker like slock. Features= include: >> > > > > > + >> > > > > > +@enumerate >> > > > > > +@item forking process, the locked screen is preserved when yo= u suspend from RAM >> > > > > > +@item specify background color or PNG image to be displayed i= n the lock screen >> > > > > > +@item many additional color options >> > > > > > +@end enumerate\n") >> > > > >=20 >> > > > > I'm not sure if the trailing newline is necessary. Unless you b= eat me >> > > > > to it, I'll check on this detail the next time I return to this = thread. >> > > >=20 >> > > > I've seen this multiple times before and have just followed this s= tyle. I looked into this, and it seems that newlines are elided from the output. Check "guix package --show=3Dmaxflow" and "guix edit maxflow" to see a particularly striking example. > diff --git a/gnu/local.mk b/gnu/local.mk > index 54d1ac91c..2ec5844dc 100644 > --- a/gnu/local.mk > +++ b/gnu/local.mk > @@ -744,6 +744,7 @@ dist_patch_DATA =3D \ > %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/icecat-bug-1348660-pt5.patch \ > %D%/packages/patches/icecat-bug-1415133.patch \ Now that the Makefile is patched using substitute*, we can remove this added line from gnu/local.mk. > diff --git a/gnu/packages/wm.scm b/gnu/packages/wm.scm > index 62a5b5460..e4db72a6f 100644 > --- a/gnu/packages/wm.scm > +++ b/gnu/packages/wm.scm > @@ -68,6 +68,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)) > @@ -335,6 +336,73 @@ and locate windows on all your workspaces, using an = interactive dmenu > prompt.") > (license (license:non-copyleft "http://www.wtfpl.net/txt/copying/"= ))))) >=20=20 > +(define-public i3lock-color > + (package > + (name "i3lock-color") > + (version "2.9.1-c") According to "guix lint", version 2.10.1-c is now available. Should we use it? > + (source > + (origin > + (method url-fetch) > + (uri (string-append "https://github.com/chrjguill/i3lock-color/" > + "archive/" version ".tar.gz")) > + (file-name (string-append name "-" version ".tar.gz")) > + (sha256 > + (base32 > + "18ql0kb24qlqsijs6j99algf2lprl78mzsz53b1hshmc8jjd4m27")))) > + (build-system gnu-build-system) > + (arguments > + `(#:tests? #f ; No tests included. > + #:make-flags (list "CC=3Dgcc") Why is this #:make-flags argument necessary? I'm not suggesting it isn't; I just don't know whether it is or not, and I'm curious. > + #:phases > + (modify-phases %standard-phases > + (replace 'configure > + (lambda* (#:key outputs inputs #:allow-other-keys) > + (let* ((out (assoc-ref outputs "out")) > + (etc (string-append out "/etc")) > + (man (string-append out "/share/man")) > + (xkb (assoc-ref inputs "libxkbcommon")) > + (xkbheader (string-append xkb > + "/include/xkbcommon/" > + "xkbcommon-compose.h"))) > + (substitute* "Makefile" > + (("PKG_CONFIG=3Dpkg-config") > + (string-append "PKG_CONFIG=3D" > + (which "pkg-config"))) > + (("/usr/include/xkbcommon/xkbcommon-compose.h") > + xkbheader) > + (("PREFIX=3D/usr") > + (string-append "PREFIX=3D" out)) > + (("SYSCONFDIR=3D/etc") > + (string-append "SYSCONFDIR=3D" etc)) > + (("MANDIR=3D/usr/share/man") > + (string-append "MANDIR=3D" man))) > + #t)))))) I like that we were able to remove the patch file by using substitute*! Hopefully once you upstream your changes, we can simplify this further. > + (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) > + ("which" ,which))) i3lock-color retains the following references when built: =2D-8<---------------cut here---------------start------------->8--- $ guix gc --references /gnu/store/kfymvwgm4g0avs3b97micq8477sa0if6-i3lock-c= olor-2.9.1-c | sort -t - -k 2 -k 1,1 /gnu/store/nwxv9s2q8pi0m6gn6fyidpj8442dwp6f-cairo-1.14.10 /gnu/store/6wyjls0q2c9gjskkplsr1ad09p3d8gzg-gcc-5.4.0-lib /gnu/store/3h31zsqxjjg52da5gp3qmhkh4x8klhah-glibc-2.25 /gnu/store/8zyry9663xgcrgg7fkrdrw40511d1gyz-libev-4.24 /gnu/store/3cxhfkh0n1naan9db0z302mwqpxqkry6-libxcb-1.12 /gnu/store/vyip2r21g65q90jy79nm5gsz6yl1s7gp-libxkbcommon-0.7.1 /gnu/store/jf7sby087sakbsirdxrfi5cizf3ya4md-linux-pam-1.3.0 /gnu/store/f4y2v0ynyr1rhzjxv1m6xlglgg0grxll-xcb-util-0.4.0 /gnu/store/x67589ylz75jadmhab8z6z03f1i5l5rv-xcb-util-image-0.4.0 =2D-8<---------------cut here---------------end--------------->8--- Specifically, libxkbcommon is referenced. Therefore, it might be a runtime dependency of the program. Are you sure it should be a native-input? If it really is required at runtime, it should be an input, not a native-input. In addition, xcb-util-keysyms and libx11 are both unreferenced. Does i3lock-color build correctly without them? If so, can we just remove them? If not, should they be native-inputs? > + (synopsis "Screenlocker with color configuration support") Could you change this phrase to "screen locker" instead of "screenlocker"? That is what you use in the description, and it's also what we use in the synopses/descriptions of the other screen lockers we've already packaged (and the manual). The consistency might make this package more discoverable. > 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? =2D-=20 Chris --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEy/WXVcvn5+/vGD+x3UCaFdgiRp0FAlohAqcACgkQ3UCaFdgi Rp3i1A/6AuGa3NA5ctVdB7jqju5pkYRrNQHJ2SA2TNOX3Q2u1H7TXkamFhPQZ+gv V7CI9GXKLYth5r8KFLsfYxaGnU8XS+jeoCfI69JxPq9rMV33Nbx239MQtmOdLUO6 VJGPpP1u6bmwv6a8fCnTrBAwxJHvue1YR+KFWezt7XzbgAzhOHJbgUPsSk5ylG2a rf8qpbxD5chAWnmriWbjW+1eOh45xQlFtQSe6qq2VuovbSGJSvn8VSfbZxyE/AV+ v/mpJ4KZoDuYwR2OShdwlLLz+ewlFsrx7bynL2TE2C1GedXc/2ioMpyAK1nhSl8/ gE1GJ4IeXyF3MweVGyHtvfF8iCNdJdkhqd13lWSpUHBh8UEKYG1RgtMM0zJbPysj YwJEC8VJ2VpClI5PaRPLO/YLhfT+c9TZbkWIxBTTf0tKrmiF1dn1W2EAGyxfMDPs XvOiC3QbziIsfYTgduMhGWRV9J1PPnt5zO9R0ipqB8BN9yWOtNCDdEbhs0nfaSq+ QR7+6/TYwZoY16vym+e4Yk77ew6JLmpw4aBzRl7F08yeM14CsYDYsrs8WiYQgALS Tik8DrBW6+hGvwZ3y9axKGeGKiy2vLLbg7LifD2jCE7Jgttr6C7FSxi2+bOMF7Rd FV2g1M3YSAElabL4gFrsi1uBrdC3b4UBcIEWYM1Pe/l98SFcXrQ= =0M/q -----END PGP SIGNATURE----- --=-=-=--