Re: [PATCH] gnu: Add denemo.

[ludo@gnu.org (Ludovic Courtès)]

Kei Kebreau <kei@openmailbox.org> skribis:

> Here is an updated patch for GNU Denemo.

Nice work!

> Everything seems fine except for grafting (i.e. disabling grafting
> renders the issue invisible). For some reason, "find-files"
> does not recognize a file with a Unicode-encoded filename when called
> inside "rename-matching-files" from guix/build/graft.scm. When
> "find-files" is used on its own, the file is recognized properly.
> Is anyone familiar with the grafting code available to help figure out
> what is happening to the file name?

Problem is that the grafting code (‘graft-derivation/shallow’ in (guix
grafts)) is running in the C locale, so it expects file names to be
ASCII.  I’ll look into it.

Some comments on the package:

> From 6bd5843bef06a02ecf1235090350562c8b096aca Mon Sep 17 00:00:00 2001
> From: Kei Kebreau <kei@openmailbox.org>
> Date: Thu, 8 Dec 2016 14:00:43 -0500
> Subject: [PATCH] gnu: Add denemo.
>
> * gnu/packages/music.scm (denemo): New variable.

[...]

> +    (arguments
> +     `(#:phases
> +       (modify-phases %standard-phases
> +         (replace 'check
> +           (lambda _
> +             (zero? (system* "make" "-C" "tests" "check")))))))

Is this really needed?  Perhaps leave a comment explaining whether/why
“make check” at the top level is broken (and perhaps report it as a bug
upstream!).

> +    (native-inputs
> +     `(("autoconf" ,autoconf)
> +       ("automake" ,automake)

This is not needed (or it’s a bug too ;-)).

> +    (license (list license:cc-by-sa3.0
> +                   license:lgpl2.1+
> +                   license:gpl2
> +                   license:gpl2+
> +                   license:gpl3
> +                   license:gpl3+))))

I think ‘gpl3+’ is enough here since it “wins”.  You can leave a comment
explaining where the other licenses appear, though.

Thanks!

Ludo’.