Re: [PATCH] gnu: Add nethack.

[Kei Kebreau]

On Sat, 04 Jun 2016 23:15:57 +0200
address@hidden (Ludovic Courtès) wrote:

> Hi!
> 
> Kei Kebreau <address@hidden> skribis:
> 
> > From b728e078408f17136e8a4c3344b606e8f152b9e4 Mon Sep 17 00:00:00
> > 2001 From: Kei Kebreau <address@hidden>
> > Date: Tue, 31 May 2016 17:42:28 -0400
> > Subject: [PATCH] gnu: Add nethack.
> >
> > * gnu/packages/games.scm (nethack): New variable.  
> 
> You need to mention the new .patch file here (see ‘git log’ for
> examples.)
> 
> You also need to add the .patch file to gnu/local.mk, and to mention
> the change to gnu/local.mk in the commit log.
> 
> [...]
> 
> > +         (replace 'configure
> > +           (lambda* (#:key inputs outputs #:allow-other-keys)
> > +             (let ((out (assoc-ref outputs "out")))
> > +               (substitute* "sys/unix/hints/linux"
> > +                 (("^PREFIX=.*$")
> > +                  (string-append "PREFIX=" out "\n"))
> > +                 (("/bin/gzip") (which "gzip")))
> > +               (substitute* "sys/unix/setup.sh"
> > +                 (("/bin/sh") (which "bash"))))
> > +             (system* "sh" "sys/unix/setup.sh"
> > "sys/unix/hints/linux")))  
> 
> Should be: (zero? (system* …)), which returns #t on success (a phase
> must return a true value to be considered successful.)
> 
> > +         (add-after 'install 'move-state-files
> > +           (lambda* (#:key inputs outputs #:allow-other-keys)
> > +             (let* ((out (assoc-ref outputs "out")))
> > +               (mkdir (string-append out
> > "/games/lib/nethack-state-files"))
> > +               (chdir (string-append out "/games/lib/nethackdir"))
> > +               (for-each (lambda (file)
> > +                           (system* "mv" file
> > +                                    (string-append
> > +                                     out
> > "/games/lib/nethack-state-files")))  
> 
> Instead of using the ‘mv’ command (or any other Coreutils command, for
> that matter), use the Scheme equivalent.  Here it would be:
> 
>   (install-file file directory)
> 
> Besides, “/games” is unusual in the file system hierarchy.  Usually,
> state files go to the localstatedir, i.e., the var/PACKAGE
> subdirectory.
> 
> Thus, what about putting state files in OUT/var/nethack?
> 
> But again, OUT is immutable, so these files cannot be modified, so
> they’re not really “state.”
> 
> > +                         '("logfile" "perm" "record" "save"
> > "xlogfile")))))  
> 
> For clarity, have the phase return #t.
> 
> > +         (add-after 'move-state-files 'wrap-program
> > +           (lambda* (#:key inputs outputs #:allow-other-keys)
> > +             (let* ((out (assoc-ref outputs "out"))
> > +                    (bin (string-append out "/bin"))
> > +                    (nethack-user-dir "~/.nethack"))
> > +               (mkdir bin)
> > +               (with-directory-excursion bin
> > +                 (call-with-output-file "nethack"
> > +                   (lambda (port)
> > +                     (format port "#!~a/bin/sh -e
> > +# Create NetHack directory in user's $HOME if it isn't there
> > +if [ ! -d ~a ]; then
> > +  mkdir -p ~a
> > +  cp -r ~a/* ~a
> > +  chmod -R +w ~a
> > +fi
> > +
> > +RUNDIR=$(mktemp -d)
> > +
> > +cleanup() {
> > +  rm -rf $RUNDIR
> > +}
> > +trap cleanup EXIT
> > +
> > +cd $RUNDIR
> > +for i in ~a/*; do
> > +  ln -s $i $(basename $i)
> > +done
> > +for i in ~a/*; do
> > +  ln -s $i $(basename $i)
> > +done
> > +./nethack~%"  
> 
> Do we really need this wrapper?  Can’t we instead take it as a patch
> from Debian or something?  I’m not a fan of inline Bash code, and not
> very confident of scripts that do ‘rm -rf’.  :-)
> 
> 
> > +--- nethack-3.6.0.orig/include/config.h    2016-05-27
> > 17:20:03.062318307 -0400 ++++ nethack-3.6.0/include/config.h
> > 2016-05-31 16:48:04.283642766 -0400  
> 
> Patches must always start with a line or two indicating what they do
> and what their upstream status or origin is.
> 
> > +@@ -308,7 +308,6 @@
> > + #define INSURANCE /* allow crashed game recovery */
> > + 
> > + #ifndef MAC
> > +-#define CHDIR /* delete if no chdir() available */
> > + #endif  
> 
> Why?
> 
> > +-# CC = gcc
> > ++CC = gcc
> > + #
> > + # For Bull DPX/2 systems at B.O.S. 2.0 or higher use the
> > following:
> > + #
> > +@@ -104,11 +104,11 @@
> > + 
> > + # yacc/lex programs to use to generate *_comp.h, *_lex.c, and
> > *_yacc.c.
> > + # if, instead of yacc/lex you have bison/flex, comment/uncomment
> > the following. +-YACC     = yacc
> > +-LEX      = lex
> > +-# YACC     = bison -y
> > ++# YACC     = yacc
> > ++# LEX      = lex
> > ++YACC     = bison -y
> > + # YACC     = byacc
> > +-# LEX      = flex
> > ++LEX      = flex  
> 
> Would it work to, instead, do:
> 
>   #:make-flags '("CC=gcc" "LEX=flex" …)
> 
> If it does, I think it’s preferable.
> 
> Could you send an updated patch?
> 
> Thanks for working on this tricky package!  ;-)
> 
> Ludo’.

Unfortunately, Debian doesn't have any related patches because it's
state files are writable in the equivalent of our store directories. It
seems like the bash script will have to stay for the sake of
functionality unless someone comes up with something cleaner, though I
prefer to avoid them. Long ago, I lost a GNU/Linux installation to
"rm -rf"...

I've added comments to the patch file and replaced the calls to
coreutils with their Scheme equivalent (and cleaned up nethack's store
folder along the way).
-- 
Kei (GPG Key: 4096R/E6A5EE3C19467A0D)

0001-gnu-Add-nethack.patch
Description: Text document

pgp_P6VEdZ5QS.pgp
Description: OpenPGP digital signature