bug#26488: [PATCH] gnu: Add crawl.

bug#26488: [PATCH] gnu: Add crawl.

[nee]

[-- Attachment #1: Type: text/plain, Size: 481 bytes --]

Hello, this is just the console version for now.
There is also a graphical version that I haven't build on the latest
release, yet.

* Multiple uris
When crawl makes a new release they move the previous tar archive into a
major-minor prefixed sub-directory. The current release only exists in
the higher directory.
I haven't seen any other package list multiple uris, so I hope I entered
it correctly.
I can only say that it builds, and got through guix lint like this.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-Add-crawl.patch --]
[-- Type: text/x-patch; name="0001-gnu-Add-crawl.patch", Size: 3775 bytes --]

From fd456a6f73fff5a2b9348ca1e43bd61fdf79fc6c Mon Sep 17 00:00:00 2001
From: nee <nee.git@cock.li>
Date: Thu, 13 Apr 2017 22:45:18 +0200
Subject: [PATCH] gnu: Add crawl.

* gnu/packages/games.scm (crawl): New variable.
---
 gnu/packages/games.scm | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/gnu/packages/games.scm b/gnu/packages/games.scm
index 710b2746c..522ad5366 100644
--- a/gnu/packages/games.scm
+++ b/gnu/packages/games.scm
@@ -115,6 +115,8 @@
   #:use-module (gnu packages messaging)
   #:use-module (gnu packages upnp)
   #:use-module (gnu packages wxwidgets)
+  #:use-module (gnu packages bison)
+  #:use-module (gnu packages flex)
   #:use-module (guix build-system gnu)
   #:use-module (guix build-system haskell)
   #:use-module (guix build-system python)
@@ -3649,3 +3651,68 @@ small robot living in the nano world, repair its maker.")
     ;; kiki-the-nano-bot/kiki-the-nano-bot_1.0.2+dfsg1-4_copyright>
     ;; for a statement from the author.
     (license license:public-domain)))
+
+(define-public crawl
+  (package
+    (name "crawl")
+    (version "0.19.5")
+    (source (origin
+              (method url-fetch)
+              (uri (list
+                    ;; Older releases get moved into a versioned directory
+                    (string-append "http://crawl.develz.org/release/"
+                                   (version-major+minor version) "/stone_soup-"
+                                   version "-nodeps.tar.xz")
+                    ;; Only the latest release is in this directory
+                    (string-append "http://crawl.develz.org/release/stone_soup-"
+                                   version "-nodeps.tar.xz")))
+              (sha256
+               (base32
+                "00yl2lb2shglxlxzpyk99zvglfx4amjybqwnzdcasvbiggb4cj18"))))
+    (arguments
+     '(#:tests? #f
+       #:make-flags
+       (let* ((sqlite (assoc-ref %build-inputs "sqlite"))
+              (out (assoc-ref %outputs "out")))
+         (list (string-append "sqlite=" sqlite)
+               (string-append "prefix=" out)
+               "SAVEDIR=~/.crawl"
+               "dejavu_fonts="
+               "TILES="
+               "BUILD_LUA="
+               "BUILD_SQLITE="
+               "BUILD_ZLIB="))
+       #:phases
+       (modify-phases
+           %standard-phases
+         (add-after
+          'unpack 'prepare-before-make
+          (lambda* (#:key inputs #:allow-other-keys)
+            (chdir "source")))
+         (add-before
+          'configure 'patch-makefile
+          (lambda* (#:key inputs #:allow-other-keys)
+            (substitute*
+                "Makefile"
+              (("SQLITE_INCLUDE_DIR := /usr/include")
+               "SQLITE_INCLUDE_DIR := ${sqlite}/include"))
+            (substitute*
+                "Makefile"
+              (("/usr/share/fonts /usr/local/share/fonts /usr/*/lib/X11/fonts;")
+               "${dejavu_fonts}/share/fonts;"))))
+         (delete 'configure))))
+    (build-system gnu-build-system)
+    (inputs `(("ncurses" ,ncurses)
+              ("sqlite" ,sqlite)
+              ("bison" ,bison)
+              ("flex" ,flex)
+              ("perl" ,perl)
+              ("zlib" ,zlib)
+              ("lua51" ,lua-5.1)))
+    (native-inputs `(("pkg-config" ,pkg-config)))
+    (synopsis "Roguelike dungeon crawler game")
+    (description "A roguelike adventure through dungeons filled with dangerous
+monsters in a quest to find the mystifyingly fabulous Orb of Zot.
+The game is also known under the name 'Dungeon Crawl Stone Soup'.")
+    (home-page "https://crawl.develz.org")
+    (license license:gpl2+)))
-- 
2.12.2

bug#26488: [PATCH] gnu: Add crawl.

[Arun Isaac]

Thanks for the patch!

>> +              (uri (list
>> +                    ;; Older releases get moved into a versioned directory
>> +                    (string-append "http://crawl.develz.org/release/"
>> +                                   (version-major+minor version) "/stone_soup-"
>> +                                   version "-nodeps.tar.xz")
>> +                    ;; Only the latest release is in this directory
>> +                    (string-append "http://crawl.develz.org/release/stone_soup-"
>> +                                   version "-nodeps.tar.xz")))

Why do we need two URIs? Shouldn't the latest release alone be enough?
Isn't that the only release we need to build?

>> +    (arguments
>> +     '(#:tests? #f

The release tarball does seem to come with tests. Could you package them
as well?

>> +       #:phases
>> +       (modify-phases
>> +           %standard-phases

Please put modify-phases and %standard-phases on the same line like:

(modify-phases %standard-phases

>> +         (add-after
>> +          'unpack 'prepare-before-make
>> +          (lambda* (#:key inputs #:allow-other-keys)
>> +            (chdir "source")))

I haven't actually tried building the package, but is this phase
necessary? If you do want to change directory before running make, you
can pass "-C" and "source" as #:make-flags.

>> +         (add-before
>> +          'configure 'patch-makefile
>> +          (lambda* (#:key inputs #:allow-other-keys)
>> +            (substitute*
>> +                "Makefile"
>> +              (("SQLITE_INCLUDE_DIR := /usr/include")
>> +               "SQLITE_INCLUDE_DIR := ${sqlite}/include"))
>> +            (substitute*
>> +                "Makefile"
>> +              (("/usr/share/fonts /usr/local/share/fonts /usr/*/lib/X11/fonts;")
>> +               "${dejavu_fonts}/share/fonts;"))))
>> +         (delete 'configure))))

You should be able to do this using #:make-flags. See
https://www.gnu.org/software/make/manual/html_node/Overriding.html

>> +    (description "A roguelike adventure through dungeons filled with dangerous
>> +monsters in a quest to find the mystifyingly fabulous Orb of Zot.
>> +The game is also known under the name 'Dungeon Crawl Stone Soup'.")

Could you make this "Dungeon Crawl Stone Soup is a roguelike adventure
... "? Also, remove the last sentence "The game is also known as ...".

>> +    (license license:gpl2+)))

licence.txt mentions multiple licenses. Could you mention them all as a
list of licenses?

bug#26488: [PATCH] gnu: Add crawl.

[nee]

[-- Attachment #1: Type: text/plain, Size: 3661 bytes --]

Hello, thank you for reviewing my patch.

Am 14.04.2017 um 10:07 schrieb Arun Isaac:
> 
> Thanks for the patch!
> 
>>> +              (uri (list
>>> +                    ;; Older releases get moved into a versioned directory
>>> +                    (string-append "http://crawl.develz.org/release/"
>>> +                                   (version-major+minor version) "/stone_soup-"
>>> +                                   version "-nodeps.tar.xz")
>>> +                    ;; Only the latest release is in this directory
>>> +                    (string-append "http://crawl.develz.org/release/stone_soup-"
>>> +                                   version "-nodeps.tar.xz")))
> 
> Why do we need two URIs? Shouldn't the latest release alone be enough?
> Isn't that the only release we need to build?
> 
My thought is that this way the build won't break in the same moment a
new version is released, since the referenced tar 404s when they move it.

>>> +    (arguments
>>> +     '(#:tests? #f
> 
> The release tarball does seem to come with tests. Could you package them
> as well?
> 
Ah, I missed those because they are in make test and not make check.
I tried to package them now, but the tests need to create a directory in
home. This fails and I don't know how to handle this with guix.
I need some help here.

>>> +       #:phases
>>> +       (modify-phases
>>> +           %standard-phases
> 
> Please put modify-phases and %standard-phases on the same line like:
> 
> (modify-phases %standard-phases
> 
Okay, done.

>>> +         (add-after
>>> +          'unpack 'prepare-before-make
>>> +          (lambda* (#:key inputs #:allow-other-keys)
>>> +            (chdir "source")))
> 
> I haven't actually tried building the package, but is this phase
> necessary? If you do want to change directory before running make, you
> can pass "-C" and "source" as #:make-flags.
> 
Yes, it works great!
I didn't know about that before, thank you.

>>> +         (add-before
>>> +          'configure 'patch-makefile
>>> +          (lambda* (#:key inputs #:allow-other-keys)
>>> +            (substitute*
>>> +                "Makefile"
>>> +              (("SQLITE_INCLUDE_DIR := /usr/include")
>>> +               "SQLITE_INCLUDE_DIR := ${sqlite}/include"))
>>> +            (substitute*
>>> +                "Makefile"
>>> +              (("/usr/share/fonts /usr/local/share/fonts /usr/*/lib/X11/fonts;")
>>> +               "${dejavu_fonts}/share/fonts;"))))
>>> +         (delete 'configure))))
> 
> You should be able to do this using #:make-flags. See
> https://www.gnu.org/software/make/manual/html_node/Overriding.html
> 
100% right!
I also removed the second substitute, since fonts are only required for
the graphical version.

I started packaging this more than half a year ago and only recently
decided to give it another try and remove the graphical version. Seems
like I still missed out on removing a lot of the mess.

>>> +    (description "A roguelike adventure through dungeons filled with dangerous
>>> +monsters in a quest to find the mystifyingly fabulous Orb of Zot.
>>> +The game is also known under the name 'Dungeon Crawl Stone Soup'.")
> 
> Could you make this "Dungeon Crawl Stone Soup is a roguelike adventure
> ... "? Also, remove the last sentence "The game is also known as ...".
> 
>>> +    (license license:gpl2+)))
> 
> licence.txt mentions multiple licenses. Could you mention them all as a
> list of licenses?
> 
> 
> 
I did both now. I hope this is the right way.

I'm appending my current WIP version.
The test phase is broken and I'll need help, as I said above.
When I disable the tests, the game builds and can be played.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-Add-crawl.patch --]
[-- Type: text/x-patch; name="0001-gnu-Add-crawl.patch", Size: 3851 bytes --]

From ddb53e51bbc8592e32b81cfcc4773de1a6ba2b6c Mon Sep 17 00:00:00 2001
From: nee <nee.git@cock.li>
Date: Thu, 13 Apr 2017 22:45:18 +0200
Subject: [PATCH] gnu: Add crawl.

* gnu/packages/games.scm (crawl): New variable.
---
 gnu/packages/games.scm | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/gnu/packages/games.scm b/gnu/packages/games.scm
index 710b2746c..c145ba830 100644
--- a/gnu/packages/games.scm
+++ b/gnu/packages/games.scm
@@ -115,6 +115,8 @@
   #:use-module (gnu packages messaging)
   #:use-module (gnu packages upnp)
   #:use-module (gnu packages wxwidgets)
+  #:use-module (gnu packages bison)
+  #:use-module (gnu packages flex)
   #:use-module (guix build-system gnu)
   #:use-module (guix build-system haskell)
   #:use-module (guix build-system python)
@@ -3649,3 +3651,71 @@ small robot living in the nano world, repair its maker.")
     ;; kiki-the-nano-bot/kiki-the-nano-bot_1.0.2+dfsg1-4_copyright>
     ;; for a statement from the author.
     (license license:public-domain)))
+
+(define-public crawl
+  (package
+    (name "crawl")
+    (version "0.19.5")
+    (source (origin
+              (method url-fetch)
+              (uri (list
+                    ;; Older releases get moved into a versioned directory
+                    (string-append "http://crawl.develz.org/release/"
+                                   (version-major+minor version) "/stone_soup-"
+                                   version "-nodeps.tar.xz")
+                    ;; Only the latest release is in this directory
+                    (string-append "http://crawl.develz.org/release/stone_soup-"
+                                   version "-nodeps.tar.xz")))
+              (sha256
+               (base32
+                "00yl2lb2shglxlxzpyk99zvglfx4amjybqwnzdcasvbiggb4cj18"))))
+    (arguments
+     '(#:make-flags
+       (let* ((sqlite (assoc-ref %build-inputs "sqlite"))
+              (out (assoc-ref %outputs "out")))
+         (list (string-append "SQLITE_INCLUDE_DIR=" sqlite "/include")
+               (string-append "prefix=" out)
+               "SAVEDIR=~/.crawl"
+               "TILES="
+               "BUILD_LUA="
+               "BUILD_SQLITE="
+               "BUILD_ZLIB="
+               "-Csource"))
+       #:phases
+       (modify-phases %standard-phases
+         (delete 'configure)
+         (replace
+          'check
+          (lambda* (#:key inputs outputs #:allow-other-keys)
+            (let* ((out (assoc-ref outputs "out")))
+              (and
+               (mkdir-p "~/.crawl/saves")
+               (zero? (system*
+                       "make" "test"
+                       (string-append "prefix=" out)
+                       "TILES="
+                       "BUILD_LUA="
+                       "BUILD_SQLITE="
+                       "BUILD_ZLIB="
+                       "-Csource")))))))))
+    (build-system gnu-build-system)
+    (inputs `(("ncurses" ,ncurses)
+              ("sqlite" ,sqlite)
+              ("bison" ,bison)
+              ("flex" ,flex)
+              ("zlib" ,zlib)
+              ("lua51" ,lua-5.1)))
+    (native-inputs `(("pkg-config" ,pkg-config)
+                     ("perl" ,perl)))
+    (synopsis "Roguelike dungeon crawler game")
+    (description "Dungeon Crawl Stone Soup is a roguelike adventure through
+dungeons filled with dangerous monsters in a quest to find the mystifyingly
+fabulous Orb of Zot.")
+    (home-page "https://crawl.develz.org")
+    (license (list license:gpl2+
+                   license:bsd-2
+                   license:bsd-3
+                   license:cc0
+                   license:x11
+                   license:zlib
+                   license:asl2.0))))
-- 
2.12.2

bug#26488: [PATCH] gnu: Add crawl.

[Danny Milosavljevic]

> Ah, I missed those because they are in make test and not make check.
> I tried to package them now, but the tests need to create a directory in
> home. This fails and I don't know how to handle this with guix.
> I need some help here.

         (add-before 'check 'set-check-up
           (lambda _
             (setenv "HOME" (getcwd)) ; or maybe /tmp or something
             #t))

bug#26488: [PATCH] gnu: Add crawl.

[Arun Isaac]

>> Why do we need two URIs? Shouldn't the latest release alone be enough?
>> Isn't that the only release we need to build?
>>
> My thought is that this way the build won't break in the same moment a
> new version is released, since the referenced tar 404s when they move it.

Ok, that's fine, then.

>>>> +    (arguments
>>>> +     '(#:tests? #f
>>
>> The release tarball does seem to come with tests. Could you package them
>> as well?
>>
> Ah, I missed those because they are in make test and not make check.
> I tried to package them now, but the tests need to create a directory in
> home. This fails and I don't know how to handle this with guix.
> I need some help here.

Try (setenv "HOME" "/tmp") as Danny mentioned.

>> licence.txt mentions multiple licenses. Could you mention them all as a
>> list of licenses?
>>
> I did both now. I hope this is the right way.

Yes, the licenses are correct now.

> +    (arguments
> +     '(#:make-flags
> +       (let* ((sqlite (assoc-ref %build-inputs "sqlite"))
> +              (out (assoc-ref %outputs "out")))
> +         (list (string-append "SQLITE_INCLUDE_DIR=" sqlite "/include")
> +               (string-append "prefix=" out)
> +               "SAVEDIR=~/.crawl"
> +               "TILES="
> +               "BUILD_LUA="
> +               "BUILD_SQLITE="
> +               "BUILD_ZLIB="
> +               "-Csource"))

Only a matter of aesthetics, but you could split "-C" and "source" into
separate strings.

> +       #:phases
> +       (modify-phases %standard-phases
> +         (delete 'configure)
> +         (replace
> +          'check

Put 'check on the same line as replace. In emacs, you can use
guix-devel-mode from emacs-guix to help you with indenting correctly.

> +          (lambda* (#:key inputs outputs #:allow-other-keys)
> +            (let* ((out (assoc-ref outputs "out")))
> +              (and
> +               (mkdir-p "~/.crawl/saves")
> +               (zero? (system*
> +                       "make" "test"
> +                       (string-append "prefix=" out)
> +                       "TILES="
> +                       "BUILD_LUA="
> +                       "BUILD_SQLITE="
> +                       "BUILD_ZLIB="
> +                       "-Csource")))))))))

You can drop the `and' function call, and just make it individual calls
to `mkdir-p' and `zero?'

Looking good so far! Only the check phase remains...

bug#26488: [PATCH] gnu: Add crawl.

[nee]

[-- Attachment #1: Type: text/plain, Size: 1255 bytes --]

Am 14.04.2017 um 20:04 schrieb Arun Isaac:
> Try (setenv "HOME" "/tmp") as Danny mentioned.
> 
Works, thanks to both of you.

> Only a matter of aesthetics, but you could split "-C" and "source" into
> separate strings.
> 
All the packages in games.scm do it as one string, so I didn't change it
for now. It should be changed for all packages at once.

> Put 'check on the same line as replace. In emacs, you can use
> guix-devel-mode from emacs-guix to help you with indenting correctly.
> 
Thanks, I installed emacs-guix and it indents correctly now.
Previously I assumed the .dir-locals.el would be enough.

> Looking good so far! Only the check phase remains...
> 
Good news everyone, I managed to package the check phase and all the
tests pass! The downside is that in order to run the tests the whole
game must be rebuild with a -DDEBUG define that seems to enable asserts
and debug outputs, otherwise the tests won't run. So the build times are
doubled.
That rebuild overwrites the objects, executable, and the .cflags file
from the build phase, so install would build it a 3rd time.
I worked around this by moving the check phase before the build phase.

I also added myself to the copyright header, since you mentioned that in
the mcomix thread.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-Add-crawl.patch --]
[-- Type: text/x-patch; name="0001-gnu-Add-crawl.patch", Size: 4970 bytes --]

From 72aa4b81e767edd451c5859be7e1d67f3c4ceedf Mon Sep 17 00:00:00 2001
From: nee <nee.git@cock.li>
Date: Thu, 13 Apr 2017 22:45:18 +0200
Subject: [PATCH] gnu: Add crawl.

* gnu/packages/games.scm (crawl): New variable.
---
 gnu/packages/games.scm | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/gnu/packages/games.scm b/gnu/packages/games.scm
index 710b2746c..502fa74ce 100644
--- a/gnu/packages/games.scm
+++ b/gnu/packages/games.scm
@@ -27,6 +27,7 @@
 ;;; Copyright © 2017 Adonay "adfeno" Felipe Nogueira <https://libreplanet.org/wiki/User:Adfeno> <adfeno@openmailbox.org>
 ;;; Copyright © 2017 Arun Isaac <arunisaac@systemreboot.net>
 ;;; Copyright © 2017 Tobias Geerinckx-Rice <me@tobias.gr>
+;;; Copyright © 2017 nee <nee-git@hidamari.blue>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -115,6 +116,8 @@
   #:use-module (gnu packages messaging)
   #:use-module (gnu packages upnp)
   #:use-module (gnu packages wxwidgets)
+  #:use-module (gnu packages bison)
+  #:use-module (gnu packages flex)
   #:use-module (guix build-system gnu)
   #:use-module (guix build-system haskell)
   #:use-module (guix build-system python)
@@ -3649,3 +3652,79 @@ small robot living in the nano world, repair its maker.")
     ;; kiki-the-nano-bot/kiki-the-nano-bot_1.0.2+dfsg1-4_copyright>
     ;; for a statement from the author.
     (license license:public-domain)))
+
+(define-public crawl
+  (package
+    (name "crawl")
+    (version "0.19.5")
+    (source (origin
+              (method url-fetch)
+              (uri (list
+                    ;; Older releases get moved into a versioned directory
+                    (string-append "http://crawl.develz.org/release/"
+                                   (version-major+minor version) "/stone_soup-"
+                                   version "-nodeps.tar.xz")
+                    ;; Only the latest release is in this directory
+                    (string-append "http://crawl.develz.org/release/stone_soup-"
+                                   version "-nodeps.tar.xz")))
+              (sha256
+               (base32
+                "00yl2lb2shglxlxzpyk99zvglfx4amjybqwnzdcasvbiggb4cj18"))))
+    (arguments
+     '(#:make-flags
+       (let* ((sqlite (assoc-ref %build-inputs "sqlite"))
+              (out (assoc-ref %outputs "out")))
+         (list (string-append "SQLITE_INCLUDE_DIR=" sqlite "/include")
+               (string-append "prefix=" out)
+               "SAVEDIR=~/.crawl"
+               "TILES="
+               "BUILD_LUA="
+               "BUILD_SQLITE="
+               "BUILD_ZLIB="
+               "-Csource"))
+       #:phases
+       (modify-phases %standard-phases
+         (delete 'configure)
+         (delete 'check)
+         ;; Test cases require the source to be rebuild with the -DDEBUG define.
+         ;; Do 'check before 'build to avoid a 3rd build on make install.
+         (add-before 'build 'check
+           (lambda* (#:key inputs outputs make-flags
+                     parallel-build? parallel-tests? #:allow-other-keys)
+             (let* ((parallel-flag (format #f "-j~d" (parallel-job-count)))
+                    (test-flags-build (if parallel-build?
+                                          (cons parallel-flag
+                                                make-flags)
+                                          make-flags))
+                    (test-flags-run (if parallel-tests?
+                                        (cons parallel-flag
+                                              make-flags)
+                                        make-flags)))
+               (setenv "HOME" (getcwd))
+               ;; Fake a terminal for the test cases.
+               (setenv "TERM" "xterm-256color")
+               (setenv "COLUMNS" "80")
+               (setenv "LINES" "24")
+               (apply system* (cons* "make" "debug" test-flags-build))
+               (zero? (apply system* (cons* "make" "test" test-flags-run)))))))))
+    (build-system gnu-build-system)
+    (inputs `(("ncurses" ,ncurses)
+              ("sqlite" ,sqlite)
+              ("bison" ,bison)
+              ("flex" ,flex)
+              ("zlib" ,zlib)
+              ("lua51" ,lua-5.1)))
+    (native-inputs `(("pkg-config" ,pkg-config)
+                     ("perl" ,perl)))
+    (synopsis "Roguelike dungeon crawler game")
+    (description "Dungeon Crawl Stone Soup is a roguelike adventure through
+dungeons filled with dangerous monsters in a quest to find the mystifyingly
+fabulous Orb of Zot.")
+    (home-page "https://crawl.develz.org")
+    (license (list license:gpl2+
+                   license:bsd-2
+                   license:bsd-3
+                   license:cc0
+                   license:x11
+                   license:zlib
+                   license:asl2.0))))
-- 
2.12.2

bug#26488: [PATCH] gnu: Add crawl.

[Arun Isaac]

>> Only a matter of aesthetics, but you could split "-C" and "source" into
>> separate strings.
>>
> All the packages in games.scm do it as one string, so I didn't change it
> for now. It should be changed for all packages at once.

Fair enough...

> +       #:phases
> +       (modify-phases %standard-phases
> +         (delete 'configure)
> +         (delete 'check)
> +         ;; Test cases require the source to be rebuild with the -DDEBUG define.
> +         ;; Do 'check before 'build to avoid a 3rd build on make install.
> +         (add-before 'build 'check
> +           (lambda* (#:key inputs outputs make-flags
> +                     parallel-build? parallel-tests? #:allow-other-keys)
> +             (let* ((parallel-flag (format #f "-j~d" (parallel-job-count)))
> +                    (test-flags-build (if parallel-build?
> +                                          (cons parallel-flag
> +                                                make-flags)
> +                                          make-flags))
> +                    (test-flags-run (if parallel-tests?
> +                                        (cons parallel-flag
> +                                              make-flags)
> +                                        make-flags)))

The parallel-build? and parallel-tests? arguments are only to disable
parallel builds and tests for packages whose build procedures fail when
run parallely. crawl's build and tests work fine when run in
parallel. So, you don't have to allow for sequential builds in your
'check phase. You can just assume the build is always going to be
parallel. No need to test for parallel-build? and parallel-test?.

> +               (setenv "HOME" (getcwd))
> +               ;; Fake a terminal for the test cases.
> +               (setenv "TERM" "xterm-256color")
> +               (setenv "COLUMNS" "80")
> +               (setenv "LINES" "24")

It looks like COLUMNS and LINES are not needed to fake a terminal. I was
able to build successfully without them. Please check.

> +               (apply system* (cons* "make" "debug" test-flags-build))
> +               (zero? (apply system* (cons* "make" "test" test-flags-run)))))))))

You can combine the two make commands into one.

(zero? (apply system* "make" "debug" "test" flags))

Also note that only the last argument of apply needs to be a list. No
need to cons* together to construct a list like you have done.

> +    (build-system gnu-build-system)
> +    (inputs `(("ncurses" ,ncurses)
> +              ("sqlite" ,sqlite)
> +              ("bison" ,bison)
> +              ("flex" ,flex)

bison and flex are native-inputs. The bison and flex executables are
required only at build time.

> +              ("zlib" ,zlib)
> +              ("lua51" ,lua-5.1)))
> +    (native-inputs `(("pkg-config" ,pkg-config)
> +                     ("perl" ,perl)))

It would be nice if you could sort all inputs and native-inputs in
alphabetical order. Not all package definitions do it. But, it does look
neater.

bug#26488: [PATCH] gnu: Add crawl.

[nee]

[-- Attachment #1: Type: text/plain, Size: 1703 bytes --]

Am 17.04.2017 um 20:37 schrieb Arun Isaac:
> The parallel-build? and parallel-tests? arguments are only to disable
> parallel builds and tests for packages whose build procedures fail when
> run parallely. crawl's build and tests work fine when run in
> parallel. So, you don't have to allow for sequential builds in your
> 'check phase. You can just assume the build is always going to be
> parallel. No need to test for parallel-build? and parallel-test?.
> 
Okay, removed it.
>> +               (setenv "HOME" (getcwd))
>> +               ;; Fake a terminal for the test cases.
>> +               (setenv "TERM" "xterm-256color")
>> +               (setenv "COLUMNS" "80")
>> +               (setenv "LINES" "24")
> 
> It looks like COLUMNS and LINES are not needed to fake a terminal. I was
> able to build successfully without them. Please check.
> 
I checked it and you are right. I assumed it would impact the tests,
because I know that crawl won't run in too small terminals. That doesn't
seem to be the case, so I removed it now.

> You can combine the two make commands into one.
> 
> Also note that only the last argument of apply needs to be a list. No
> need to cons* together to construct a list like you have done.
> 
Always nice to learn how I can make things shorter :)

> bison and flex are native-inputs. The bison and flex executables are
> required only at build time.
> 
Okay, I moved them into native and it still works.

> It would be nice if you could sort all inputs and native-inputs in
> alphabetical order. Not all package definitions do it. But, it does look
> neater.
> 
Okay, I reordered them.

Thank you very much, for your intensive help with cleaning up this package!

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-Add-crawl.patch --]
[-- Type: text/x-patch; name="0001-gnu-Add-crawl.patch", Size: 4475 bytes --]

From 868d777e1856019c0037c43647cfa245f0d0fb5b Mon Sep 17 00:00:00 2001
From: nee <nee.git@cock.li>
Date: Thu, 13 Apr 2017 22:45:18 +0200
Subject: [PATCH] gnu: Add crawl.

* gnu/packages/games.scm (crawl): New variable.
---
 gnu/packages/games.scm | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/gnu/packages/games.scm b/gnu/packages/games.scm
index 710b2746c..451500a04 100644
--- a/gnu/packages/games.scm
+++ b/gnu/packages/games.scm
@@ -27,6 +27,7 @@
 ;;; Copyright © 2017 Adonay "adfeno" Felipe Nogueira <https://libreplanet.org/wiki/User:Adfeno> <adfeno@openmailbox.org>
 ;;; Copyright © 2017 Arun Isaac <arunisaac@systemreboot.net>
 ;;; Copyright © 2017 Tobias Geerinckx-Rice <me@tobias.gr>
+;;; Copyright © 2017 nee <nee-git@hidamari.blue>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -115,6 +116,8 @@
   #:use-module (gnu packages messaging)
   #:use-module (gnu packages upnp)
   #:use-module (gnu packages wxwidgets)
+  #:use-module (gnu packages bison)
+  #:use-module (gnu packages flex)
   #:use-module (guix build-system gnu)
   #:use-module (guix build-system haskell)
   #:use-module (guix build-system python)
@@ -3649,3 +3652,71 @@ small robot living in the nano world, repair its maker.")
     ;; kiki-the-nano-bot/kiki-the-nano-bot_1.0.2+dfsg1-4_copyright>
     ;; for a statement from the author.
     (license license:public-domain)))
+
+(define-public crawl
+  (package
+    (name "crawl")
+    (version "0.19.5")
+    (source (origin
+              (method url-fetch)
+              (uri (list
+                    ;; Older releases get moved into a versioned directory
+                    (string-append "http://crawl.develz.org/release/"
+                                   (version-major+minor version) "/stone_soup-"
+                                   version "-nodeps.tar.xz")
+                    ;; Only the latest release is in this directory
+                    (string-append "http://crawl.develz.org/release/stone_soup-"
+                                   version "-nodeps.tar.xz")))
+              (sha256
+               (base32
+                "00yl2lb2shglxlxzpyk99zvglfx4amjybqwnzdcasvbiggb4cj18"))))
+    (arguments
+     '(#:make-flags
+       (let* ((sqlite (assoc-ref %build-inputs "sqlite"))
+              (out (assoc-ref %outputs "out")))
+         (list (string-append "SQLITE_INCLUDE_DIR=" sqlite "/include")
+               (string-append "prefix=" out)
+               "SAVEDIR=~/.crawl"
+               ;; disable graphical client
+               "TILES="
+               ;; don't build any bundeled dependencies
+               "BUILD_LUA="
+               "BUILD_SQLITE="
+               "BUILD_ZLIB="
+               "-Csource"))
+       #:phases
+       (modify-phases %standard-phases
+         (delete 'configure)
+         (delete 'check)
+         ;; Test cases require the source to be rebuild with the -DDEBUG define.
+         ;; Do 'check before 'build to avoid a 3rd build on make install.
+         (add-before 'build 'check
+           (lambda* (#:key inputs outputs make-flags
+                     parallel-build? parallel-tests? #:allow-other-keys)
+             (setenv "HOME" (getcwd))
+             ;; Fake a terminal for the test cases.
+             (setenv "TERM" "xterm-256color")
+             (zero? (apply system* "make" "debug" "test"
+                           (cons (format #f "-j~d" (parallel-job-count))
+                                 make-flags))))))))
+    (build-system gnu-build-system)
+    (inputs `(("lua51" ,lua-5.1)
+              ("ncurses" ,ncurses)
+              ("sqlite" ,sqlite)
+              ("zlib" ,zlib)))
+    (native-inputs `(("bison" ,bison)
+                     ("flex" ,flex)
+                     ("pkg-config" ,pkg-config)
+                     ("perl" ,perl)))
+    (synopsis "Roguelike dungeon crawler game")
+    (description "Dungeon Crawl Stone Soup is a roguelike adventure through
+dungeons filled with dangerous monsters in a quest to find the mystifyingly
+fabulous Orb of Zot.")
+    (home-page "https://crawl.develz.org")
+    (license (list license:gpl2+
+                   license:bsd-2
+                   license:bsd-3
+                   license:cc0
+                   license:x11
+                   license:zlib
+                   license:asl2.0))))
-- 
2.12.2

bug#26488: [PATCH] gnu: Add crawl.

[Arun Isaac]

> Thank you very much, for your intensive help with cleaning up this package!

No problem! That's what I'm here for... :-) And, thank you for your
patience with all my corrections!

> +       #:phases
> +       (modify-phases %standard-phases
> +         (delete 'configure)
> +         (delete 'check)
> +         ;; Test cases require the source to be rebuild with the -DDEBUG define.
> +         ;; Do 'check before 'build to avoid a 3rd build on make install.
> +         (add-before 'build 'check
> +           (lambda* (#:key inputs outputs make-flags
> +                     parallel-build? parallel-tests? #:allow-other-keys)

Since you are not using the parallel-build? and parallel-tests? keyword
arguments anymore, why keep them in the argument list?

> +             (zero? (apply system* "make" "debug" "test"
> +                           (cons (format #f "-j~d" (parallel-job-count))
> +                                 make-flags))))))))

As noted earlier, only the last argument of apply needs to be a
list. So, this can be shortened to something like:

(zero? (apply system* "make" (format #f "-j~d"
(parallel-job-count)) "debug" "test" make-flags))

Everything else LGTM! Only the graphical build remains now.

bug#26488: [PATCH] gnu: Add crawl.

[Christopher Allan Webber]

Arun Isaac writes:

> Everything else LGTM! Only the graphical build remains now.

I assume that the graphical build will inherit from the console version
anyway?  So maybe the graphical build shouldn't hold up the console
version if this will take a while?  What do you think?

bug#26488: [PATCH] gnu: Add crawl.

[Arun Isaac]

Christopher Allan Webber writes:

> Arun Isaac writes:
>
>> Everything else LGTM! Only the graphical build remains now.
>
> I assume that the graphical build will inherit from the console version
> anyway?  So maybe the graphical build shouldn't hold up the console
> version if this will take a while?  What do you think?

Alright, I have pushed crawl with a few changes. We can deal with the
graphical build in a separate patch.

bug#26488: [PATCH] gnu: Add crawl.

[Arun Isaac]

> +               "TILES="

This disables the building of the graphical interface, if I'm not
wrong. Could you build the graphical interface as well?

> +               "BUILD_LUA="
> +               "BUILD_SQLITE="
> +               "BUILD_ZLIB="

Also, could you explain what these three BUILD_* make-flags do? Is it to
disable the use of their bundled lua, sqlite and zlib libraries? I can't
for the life of me, read and understand their Makefile. They seem to
have basically reinvented autoconf in a large hand-written Makefile.

Thanks!

bug#26488: [PATCH] gnu: Add crawl.

[nee]

Am 17.04.2017 um 21:29 schrieb Arun Isaac:
> 
>> +               "TILES="
> 
> This disables the building of the graphical interface, if I'm not
> wrong. Could you build the graphical interface as well?
> 
Yes, I'll definitely try that once the terminal package is good enough.

>> +               "BUILD_LUA="
>> +               "BUILD_SQLITE="
>> +               "BUILD_ZLIB="
> 
> Also, could you explain what these three BUILD_* make-flags do? Is it to
> disable the use of their bundled lua, sqlite and zlib libraries? I can't
> for the life of me, read and understand their Makefile. They seem to
> have basically reinvented autoconf in a large hand-written Makefile.
> 
> Thanks!
> 
> 
> 
That's exactly what it's for. By default it will look for the bundled
libraries and fail since I download the -nodeps.tar
I'll add a comment.