GNU bug report logs - #32162
[PATCH] gnu: Add nethack

Previous Next

Full log

Message #8 received at 32162 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Anonymous <mcrfan96 <at> cock.li>
Cc: 32162 <at> debbugs.gnu.org
Subject: Re: [bug#32162] [PATCH] gnu: Add nethack
Date: Tue, 17 Jul 2018 14:45:31 +0200

Hello,

Anonymous <mcrfan96 <at> cock.li> skribis:

> I've created a package for nethack by basically copying the package
> from NixOS.
>
> This is my first time writing a guix package, so feel free to make
> corrections.

That’s a very good start, thanks for your patch and welcome!

I have a few comments below, but the package looks pretty good to me.

As a general comment: note that Nixpkgs uses Bash for the “build-side”
code (actions performed when building the derivation), whereas Guix uses
Scheme.  There’s a bunch of utility functions in the (guix build utils)
modules that usually allow us to not resort to Bash scripting.  Most of
my comments below are about using the Scheme equivalent to the Bash
script.

> +          (replace 'configure
> +            (lambda _
> +              (let ((bash (string-append
> +                            (assoc-ref %build-inputs "bash")
> +                            "/bin/bash")))
> +                (chdir "sys/unix")
> +                (substitute* "setup.sh" (("/bin/sh") bash))
> +                (invoke bash "setup.sh" "hints/linux")
> +                (chdir "../..")

I recommend writing it like this:

  (with-directory-excursion "sys/unix"
    (substitute* …)
    (invoke …))

It takes care of chdir’ing back and it’s somewhat easier to read IMO.

> +                (map
> +                  (lambda (i)
> +                    (invoke "mv"
> +                      (string-append output "/games/lib/nethackdir/" i)
> +                      (string-append output "/games/lib/nethackuserdir")))
> +                  '("xlogfile" "logfile" "perm" "record" "save"))

Rather:

  (for-each (lambda (file)
              (install-file file
                            (string-append … "/games/lib/nethackuserdir")))
            '(…))

> +                (let ((outfile (open-file nethack-script "w"))
> +                      (user-dir "~/.config/nethack"))
> +                  (map
> +                    (lambda (line)
> +                      (display (string-append line "\n") outfile))
> +                    `(,(string-append "#!" (assoc-ref %build-inputs "bash")
> +                                      "/bin/bash")
> +                      ,(string-append
> +                          "PATH="
> +                          (list->search-path-as-string
> +                            (list (string-append
> +                                    (assoc-ref %build-inputs "coreutils")
> +                                    "/bin")
> +                                  (string-append
> +                                    (assoc-ref %build-inputs "less")
> +                                    "/bin"))
> +                            ":"))
> +                      ,(string-append "if [ ! -d " user-dir " ]; then")
> +                      ,(string-append "  mkdir -p " user-dir)
> +                      ,(string-append "  cp -r " output
> +                                      "/games/lib/nethackuserdir/* " user-dir)
> +                      ,(string-append "  chmod -R +w " user-dir)
> +                      "fi"
> +                      "RUNDIR=$(mktemp -d)"
> +                      "cleanup() {"
> +                      "  rm -rf $RUNDIR"
> +                      "}"
> +                      "trap cleanup EXIT"
> +                      "cd $RUNDIR"
> +                      ,(string-append "for i in " user-dir "/*; do")
> +                      "  ln -s $i $(basename $i)"
> +                      "done"
> +                      ,(string-append "for i in " output
> +                                      "/games/lib/nethackdir/*; do")
> +                      "  ln -s $i $(basename $i)"
> +                      "done"
> +                      ,(string-append output "/games/nethack"))))

For improved readability, how about:

  (call-with-output-file nethack-script
    (lambda (port)
      (format port "#!~a/bin/sh
first line
second line
…\n"
              (assoc-ref inputs "bash"))))

?

Could you send an updated patch?  If that’s fine with you, please use:

  git format-patch master

in your branch to produce the patch, and then:

  git send-email --to=32162 <at> debbugs.gnu.org 000*.patch

to send the patch.  That way the commit will get proper attribution.
See <https://www.gnu.org/software/guix/manual/en/html_node/Submitting-Patches.html>.

Let me know if you have any questions.

Thank you!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.