[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] gnu: Add denemo.
From: |
Ludovic Courtès |
Subject: |
Re: [PATCH] gnu: Add denemo. |
Date: |
Fri, 09 Dec 2016 22:43:20 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) |
Kei Kebreau <address@hidden> 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 <address@hidden>
> 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’.