Marius Bakke <mbakke@fastmail.com> writes:

> Kei Kebreau <kei@openmailbox.org> writes:
>
>> * gnu/packages/games.scm (hyperrogue): Update to 9.4c.
>> [source]: Remove snippet.
>> [arguments]: Remove #:make-flags; adjust configure and install phases; add
>> install-data phase.
>> [inputs]: Add hyperrogue-data.
>
> Thanks!
>
> [...]
>
>> +         (add-after 'install 'install-data
>> +           (lambda* (#:key inputs outputs #:allow-other-keys)
>> +             (let* ((data (assoc-ref inputs "hyperrogue-data"))
>> +                    (out (assoc-ref outputs "out"))
>> +                    (share-dir (string-append out "/share/hyperrogue")))
>> +               (system* "tar" "xvf" data "-C" ".")
>> +               (with-directory-excursion "hyperrogue94c-win"
>
> You can access the "version" variable here by "unquoting" it (with
> comma). Another approach is skipping the "top folder" of the tarball
> with "--strip-components=1". Maybe the "sounds" subdirectory can be
> extracted directly to "out/share/sounds"? See e.g.
> "font-adobe-source-code-pro" for some tar trickery.
>
>> +                 (for-each
>> +                  (lambda (file)
>> +                    (install-file file (string-append share-dir "/sounds")))
>> +                  (append (find-files "." "\\.ogg$")
>> +                          (find-files "sounds" ".*$")))
>> +                 (for-each
>> +                  (lambda (file)
>> +                    (install-file file share-dir))
>> +                  '("papermodeldata.txt" "papermodeldata-help.txt"))))
>>               #t)))))
>>      (inputs
>>       `(("font-dejavu" ,font-dejavu)
>> @@ -3227,7 +3238,23 @@ throwing people around in pseudo-randomly generated buildings.")
>>         ("sdl-union" ,(sdl-union (list sdl
>>                                        sdl-gfx
>>                                        sdl-mixer
>> -                                      sdl-ttf)))))
>> +                                      sdl-ttf)))
>> +       ("hyperrogue-data"
>> +        ,(origin
>> +           (method url-fetch)
>> +           (uri
>> +            (string-append
>> +             "http://www.roguetemple.com/z/hyper/" name
>> +             (string-join (string-split version #\.) "")
>> +             "-win.zip"))
>> +           (sha256
>> +            (base32
>> +             "1cyyrsnrixygg3zyz97hpsm6jzwbhydiwk3kl0lm7qjnw2nzkhhh"))
>
> It would be good to have a comment near the source stating "remember to
> update hyperrogue-data as well". Otherwise, if the enterprising Guix
> contributor already have the old tarball in their store, they may not
> notice the impending version/hash mismatch.
>
> Also, this can be a "native-input" since it's downloaded, extracted and
> thrown away, and only carry platform-independent data (for us, anyway).
>
>> +           (modules '((guix build utils)))
>> +           (snippet
>> +            '(begin
>> +               ;; Remove .exe and .dll files.
>> +               (for-each delete-file (find-files "." "\\.(exe|dll)$"))))))))
>
> ...for the same reason, I don't think this is necessary, especially if
> we can avoid extracting these files to begin with. It will just use up
> *more* space in the store, since it creates a derivation in addition to
> the tarball. :-)
>
> The rest LGTM, thanks!
>
>>      (home-page "http://www.roguetemple.com/z/hyper/")
>>      (synopsis "Non-euclidean graphical rogue-like game")
>>      (description
>> -- 
>> 2.12.0

The new patch that I sent attempts to take care of these issues, but the
unzip pattern matching in the install-data phase is not working for some
reason. Any ideas?