[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Add st
From: |
Mark H Weaver |
Subject: |
Re: [PATCH] Add st |
Date: |
Thu, 11 Jun 2015 18:15:37 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
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