Re: [PATCH] Add st

[amirouche]

I attached two patches. The second patch is to rename dwm.scm

On 2015-06-12 00:15, Mark H Weaver wrote:
> 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.

Sorry. I did not commit before generating the patch.

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

No it is not needed, since it's only useful to install the terminfo 
file. I removed it.

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

0001-gnu-Add-st.patch
Description: Text Data

0002-gnu-Rename-gnu-packages-dwm-to-gnu-packages-suckless.patch
Description: Text Data