all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Arun Isaac <arunisaac@systemreboot.net>
To: 26488@debbugs.gnu.org
Subject: bug#26488: [PATCH] gnu: Add crawl.
Date: Tue, 18 Apr 2017 00:07:55 +0530	[thread overview]
Message-ID: <b75ed791.AEQAJYdmpaQAAAAAAAAAAAO02gcAAAACwQwAAAAAAAW9WABY9QuM@mailjet.com> (raw)
In-Reply-To: <8ac92fe4-14dd-3933-8901-bea4ff8673ac@cock.li>


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

  reply	other threads:[~2017-04-17 18:39 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-13 21:26 bug#26488: [PATCH] gnu: Add crawl nee
     [not found] ` <cu7shlbed1g.fsf@systemreboot.net>
2017-04-14  8:07   ` Arun Isaac
2017-04-14 16:10     ` nee
2017-04-14 17:23       ` Danny Milosavljevic
2017-04-14 18:04       ` Arun Isaac
     [not found]       ` <c09ea559.AEMAJMF3UlcAAAAAAAAAAAO0_XAAAAACwQwAAAAAAAW9WABY8Q9P@mailjet.com>
2017-04-17  4:46         ` nee
2017-04-17 18:37           ` Arun Isaac [this message]
2017-04-18 22:15             ` nee
2017-04-19  6:11               ` Arun Isaac
2017-04-21 17:10                 ` Christopher Allan Webber
2017-04-21 20:25                   ` Arun Isaac
2017-04-17 19:29           ` Arun Isaac
2017-04-17 21:29             ` nee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b75ed791.AEQAJYdmpaQAAAAAAAAAAAO02gcAAAACwQwAAAAAAAW9WABY9QuM@mailjet.com \
    --to=arunisaac@systemreboot.net \
    --cc=26488@debbugs.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.