Subject: Re: bug#33772: StumpWM broken commands?

From: Christopher Lemmer Webber <[email-masked]>
Date: Wed, 19 Dec 2018 08:42:35 -0500

Pierre Langlois writes:

> Alex Kost writes:
>
>> Pierre Langlois (2018-12-18 17:34 +0000) wrote:
>>
>>> Hello!
>>>
>>> Christopher Lemmer Webber writes:
>>>
>>>> Ludovic Courtès writes:
>>>>
>>>>> Hi,
>>>>>
>>>>> Christopher Lemmer Webber <[email-masked]> skribis:
>>>>>
>>>>>> It seems to me that StumpWM broke somehow, for some reason.  I can
>>>>>> confirm there was a version change:
>>>>>>
>>>>>> [email-masked]:out        /gnu/store/i0picr2xr2aq2a52nsaw67nvar1r3khw-sbcl-stumpwm-18.05
>>>>>> [email-masked]:out        /gnu/store/6h2iln76dx8pxdp1hsaqb1yncm8viczj-sbcl-stumpwm-18.11
>>>>>>
>>>>>> I am not sure if this is the cause or something else.  At any rate, when
>>>>>> I try to run commands such as "gnew" (the command to make a new
>>>>>> group/workstation) I get back:
>>>>>>
>>>>>>   Error In Command 'gnew': invalid number of arguments: 2
>>>>>>
>>>>>> I though I'd try running the command manually and check the error, but
>>>>>> this also happens with eval!  Some other commands are not affected.
>>>>>>
>>>>>> (I can try re-enabling the live REPL in StumpWM to play around with it,
>>>>>> but I currently have it off because they have the same live hacking
>>>>>> vulnerability in SBCL/SLIME that we had in Guile/Geiser some time ago.)
>>>>>
>>>>> Pierre, does that ring a bell?
>>>>>
>>>>> Others on the mailing list reported being happy with the upgrade, but
>>>>> maybe they use different bits of StumpWM.
>>>
>>> Ah yes, I can reproduce this :-/. Both in GuixSD and with Guix on top of
>>> ArchLinux.  The 'gnew' command does work if you pass it the name of the
>>> new group as an argument, that's why I didn't notice this.  It should
>>> interactively ask for a name if you don't pass one so that's still a
>>> regression.
>>>
>>> Maybe that's a bug upstream? It would be good to try StumpWM 18.11 on
>>> another distribution to see if this is specific to Guix. I cannot
>>> install it on ArchLinux though at the moment.
>>
>> I use StumpWM from git checkout (not from Guix) and I don't reproduce
>> this problem.  When I run "gnew", it asks me for a group name and
>> creates a new group without any problem.
>>
>> I have built stumpwm from the latest commit (which is one commit after
>> 18.11 release) and tried it on GuixSD and on ArchLinux.  Works for me
>> on both systems, so it's probably not an upstream bug.
>
> Hi Alex,
>
> I was investigating just now and thought I'd try that one commit that
> was pushed after the release. The description doesn't sound like it
> would fix the problem but it was easy to try that.  But it looks like it
> does fix the problem!
>
> Can you confirm the attached patch works for you?
>
>>>From 735bda15874bdb9209c171ae2b1a99256763697d Mon Sep 17 00:00:00 2001
> From: Pierre Langlois <[email-masked]>
> Date: Tue, 18 Dec 2018 21:17:16 +0000
> Subject: [PATCH] gnu: stumpwm: Fix gnew command.
>
> The new release of StumpWM broken the interactive 'gnew' command.  It was
> fixed upstream [0] and will be part of the next release.
>
> 0: https://github.com/stumpwm/stumpwm/pull/538
>
> * gnu/packages/patches/stumpwm-fix-broken-read-one-line.patch: New file.
> * gnu/packages/lisp.scm (stumpwm)[source]: Use it.
> * gnu/local.mk (dist_patch_DATA): Register it.
> ---
>  gnu/local.mk                                  |  1 +
>  gnu/packages/lisp.scm                         |  4 +-
>  .../stumpwm-fix-broken-read-one-line.patch    | 45 +++++++++++++++++++
>  3 files changed, 49 insertions(+), 1 deletion(-)
>  create mode 100644 gnu/packages/patches/stumpwm-fix-broken-read-one-line.patch
>
> diff --git a/gnu/local.mk b/gnu/local.mk
> index 0d60e9b75..8e1ce88bc 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -1174,6 +1174,7 @@ dist_patch_DATA =						\
>    %D%/packages/patches/swish-e-search.patch			\
>    %D%/packages/patches/swish-e-format-security.patch		\
>    %D%/packages/patches/synfigstudio-fix-ui-with-gtk3.patch 	\
> +  %D%/packages/patches/stumpwm-fix-broken-read-one-line.patch	\
>    %D%/packages/patches/t1lib-CVE-2010-2642.patch		\
>    %D%/packages/patches/t1lib-CVE-2011-0764.patch		\
>    %D%/packages/patches/t1lib-CVE-2011-1552+.patch		\
> diff --git a/gnu/packages/lisp.scm b/gnu/packages/lisp.scm
> index d8da2951d..e4669e34f 100644
> --- a/gnu/packages/lisp.scm
> +++ b/gnu/packages/lisp.scm
> @@ -976,7 +976,9 @@ from other CLXes around the net.")
>                      version ".tar.gz"))
>                (sha256
>                 (base32 "177gxfk4c127i9crghx6fmkipznhgylvzgnjb2pna38g21gg6s39"))
> -              (file-name (string-append "stumpwm-" version ".tar.gz"))))
> +              (file-name (string-append "stumpwm-" version ".tar.gz"))
> +              (patches
> +               (search-patches "stumpwm-fix-broken-read-one-line.patch"))))
>      (build-system asdf-build-system/sbcl)
>      (native-inputs `(("fiasco" ,sbcl-fiasco)))
>      (inputs `(("cl-ppcre" ,sbcl-cl-ppcre)
> diff --git a/gnu/packages/patches/stumpwm-fix-broken-read-one-line.patch b/gnu/packages/patches/stumpwm-fix-broken-read-one-line.patch
> new file mode 100644
> index 000000000..f8dac6130
> --- /dev/null
> +++ b/gnu/packages/patches/stumpwm-fix-broken-read-one-line.patch
> @@ -0,0 +1,45 @@
> +From a13db62a4da06426cf2eb2376d1a3723b5ee52d5 Mon Sep 17 00:00:00 2001
> +From: Vasily Postnicov <[email-masked]>
> +Date: Fri, 14 Dec 2018 20:01:53 +0300
> +Subject: [PATCH] READ-ONE-LINE: Turn COMPLETIONS into a keyword argument
> +
> +This keeps READ-ONE-line backwards compatible to changes prior
> +dae0422811771d179077b9336618f2b19be85b7b. Currently both
> +ARGUMENT-POP-OR-READ and ARGUMENT-POP-REST-OR-READ are still being
> +called with the previous lambda list. Update the calls to the
> +READ-ONE-LINE that used the 'new' lambda list, COMPLETING-READ and
> +YES-OR-NO-P.
> +
> +Closes #538
> +---
> + input.lisp | 8 ++++++--
> + 1 file changed, 6 insertions(+), 2 deletions(-)
> +
> +diff --git a/input.lisp b/input.lisp
> +index b698a368..7904b35f 100644
> +--- a/input.lisp
> ++++ b/input.lisp
> +@@ -307,10 +307,13 @@ passed the substring to complete on and is expected to return a list
> + of matches. If require-match argument is non-nil then the input must
> + match with an element of the completions."
> +   (check-type completions (or list function symbol))
> +-  (let ((line (read-one-line screen prompt completions :initial-input initial-input :require-match require-match)))
> ++  (let ((line (read-one-line screen prompt
> ++                             :completions completions
> ++                             :initial-input initial-input
> ++                             :require-match require-match)))
> +     (when line (string-trim " " line))))
> + 
> +-(defun read-one-line (screen prompt completions &key (initial-input "") require-match password)
> ++(defun read-one-line (screen prompt &key completions (initial-input "") require-match password)
> +   "Read a line of input through stumpwm and return it. Returns nil if the user aborted."
> +   (let ((*input-last-command* nil)
> +         (*input-completions* completions)
> +@@ -842,6 +845,7 @@ user presses 'y'"
> + user presses 'yes'"
> +   (loop for line = (read-one-line (current-screen)
> +                                   (format nil "~a(yes or no) " message)
> ++                                  :completions
> +                                   '("yes" "no"))
> +         until (find line '("yes" "no") :test 'string-equal)
> +         do (message "Please answer yes or no")

I haven't tried it yet but it makes a lot of sense that this would fix
it.  Both gnew and eval interactively pull up a request for a line of
input.