Re: [PATCH] Add st

[Mark H Weaver]

address@hidden writes:

> On 2015-06-09 16:22, Andreas Enge wrote:
>>> +    (description
>>> +     "Xterm is bloated and unmaintainable.  It has over 65K lines
>>> of code and
>>
>> Please write a more neutral description. Something like
>> "St implements a simple and light-weight terminal emulator.  It
>> implements
>> 256 colors, most VT10X escape sequences, utf8, X11 copy/paste,
>> antialiased fonts (using fontconfig), fallback fonts, resizing,
>> and line drawing."
>> It is nicer to write what a programme does well than to complain about
>> what others do poorly.
>
> Agreed. I created a new patch with the changes you requested and I've
> run: ./pre-inst-env guix lint st.

Did you change the description at all?  At first glance, it looks the
same as before.  It should be more neutral, as Andreas said.

> From 24e374cd99adb8efc1d6a9c5ba0d7cfb1f71828f Mon Sep 17 00:00:00 2001
> From: amz3 <address@hidden>
> Date: Sun, 7 Jun 2015 19:04:28 +0200
> Subject: [PATCH] gnu: add st.
>
> * gnu/packages/dwm.scm (st): New variable.
> * gnu/packages/patches/st-0.5-do-not-install-terminfo.patch: New file.

You would need to add this patch to 'dist_patch_DATA' in gnu-system.am
as well, but in this case I think it's better to make this change in a
custom phase using 'substitute*'.  The reason is that if the user
requests the source code using 'guix build -S st', it should probably
not include that change, which is guix-specific.

It could be something like this: (untested)

--8<---------------cut here---------------start------------->8---
       #:phases
       (modify-phases %standard-phases
         (delete 'configure)
         (add-after 'unpack 'inhibit-terminfo-install
                    (lambda _
                      (substitute* "Makefile"
                        (("address@hidden -s st.info") ""))
                      #t)))))
--8<---------------cut here---------------end--------------->8---

Also, we should probably rename dwm.scm to suckless.scm.
GNU_SYSTEM_MODULES in gnu-system.am should be updated accordingly,
keeping it sorted.

> diff --git a/gnu/packages/dwm.scm b/gnu/packages/dwm.scm
> index 98fa122..8ce078b 100644
> --- a/gnu/packages/dwm.scm
> +++ b/gnu/packages/dwm.scm

Please add a copyright line for yourself to the top of the file.

> @@ -22,7 +22,11 @@
>    #:use-module (guix download)
>    #:use-module (guix build-system gnu)
>    #:use-module (gnu packages)
> -  #:use-module (gnu packages xorg))
> +  #:use-module (gnu packages xorg)
> +  #:use-module (gnu packages fonts)
> +  #:use-module (gnu packages ncurses)
> +  #:use-module (gnu packages pkg-config)
> +  #:use-module ((gnu packages fontutils) #:prefix font-utils:))

Instead of adding a prefix for (gnu package fontutils), better to add a
'license:' prefix for (guix licenses) and update the 'license' fields of
the existing packages accordingly.

>  
>  (define-public dwm
>    (package
> @@ -139,3 +143,47 @@ numbers of user-defined menu items efficiently.")
>      (description
>       "Simple X session lock with trivial feedback on password entry.")
>      (license x11)))
> +
> +
> +(define-public st

Just one blank line between packages please.

> +  (package
> +    (name "st")
> +    (version "0.5")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append "http://dl.suckless.org/st/st-";
> +                           version ".tar.gz"))
> +       (patches (list (search-patch "st-0.5-do-not-install-terminfo.patch")))
> +       (sha256
> +        (base32
> +         "0knxpzaa86pprng6hak8hx8bw22yw22rpz1ffxjpcvqlz3xdv05f"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     '(#:tests? #f ; no tests
> +       #:make-flags (list "CC=gcc"
> +                          (string-append "PREFIX=" %output))
> +       #:phases
> +       (alist-delete 'configure %standard-phases)))
> +    (inputs
> +     `(("libx11" ,libx11)
> +       ("libxft" ,libxft)
> +       ("libxcomposite" ,libxcomposite)
> +       ("compositeproto" ,compositeproto)
> +       ("libxext" ,libxext)
> +       ("xextproto" ,xextproto)
> +       ("libxrender" ,libxrender)
> +       ("fontconfig" ,font-utils:fontconfig)
> +       ("freetype" ,font-utils:freetype)
> +       ("font-liberation" ,font-liberation)))
> +    (native-inputs `(("pkg-config" ,pkg-config)
> +                     ("ncurses" ,ncurses)))

Is ncurses really needed here as a native-input?  If 'st' needs to be
linked to it for use at run time, then it should be a normal input.  It
should only be a native-input if it's needed at build time.  That would
be surprising to me, since the build process within guix-daemon is
obviously non-interactive.

> +    (home-page "http://st.suckless.org/";)
> +    (synopsis "Simple terminal emulator")
> +    (description
> +     "Xterm is bloated and unmaintainable.  It has over 65K lines of code and
> +      emulates obscure and obsolete terminals you will never need.  The 
> popular
> +      alternative, rxvt has only 32K lines of code.  This is just too much 
> for
> +      something as simple as a terminal emulator; it's yet another example of
> +      code complexity.  st fix that.")

See above.

Can you send an updated patch?

     Thanks!
       Mark