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

From: Arun Isaac <[email-masked]>
Date: Fri, 14 Apr 2017 23:34:50 +0530

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