[bug#57984] [PATCH] gnu: Update zynaddsubfx to 3.0.6

[Maxim Cournoyer <maxim.cournoyer@gmail.com>]

Hello,

Sughosha <Sughosha@proton.me> writes:

> Sorry, one small typos in the commit message.
> From 77f364a304e4510baafe86f5a641bdf84952bf06 Mon Sep 17 00:00:00 2001
> From: Sughosha <sughosha@proton.me>
> Date: Wed, 21 Sep 2022 19:15:47 +0200
> Subject: [PATCH] gnu: Update zynaddsubfx to 3.0.6
>
> * gnu/packages/music.scm (mruby-zest): New variable.
>
> * gnu/packages/music.scm (zynaddsubfx): Update to 3.0.6.
>
>   Upgrade to Zyn-Fusion with new UI called zest.
> ---
>  gnu/packages/music.scm | 96 ++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 88 insertions(+), 8 deletions(-)
>
> diff --git a/gnu/packages/music.scm b/gnu/packages/music.scm
> index 66b4beae0c..c9081bfb69 100644
> --- a/gnu/packages/music.scm
> +++ b/gnu/packages/music.scm
> @@ -167,6 +167,7 @@ (define-module (gnu packages music)
>    #:use-module (gnu packages rdf)
>    #:use-module (gnu packages readline)
>    #:use-module (gnu packages rsync)
> +  #:use-module (gnu packages ruby)
>    #:use-module (gnu packages sdl)
>    #:use-module (gnu packages serialization)
>    #:use-module (gnu packages sphinx)
> @@ -2989,10 +2990,69 @@ (define-public vmpk
>  instrument or MIDI file player.")
>      (license license:gpl3+)))
>  
> +(define mruby-zest
> +  (package
> +    (name "mruby-zest")
> +    (version "3.0.6")
> +    (source (origin
> +              (method git-fetch)
> +              (uri (git-reference
> +                    (url "https://github.com/mruby-zest/mruby-zest-build")
> +                    (commit version)
> +                    (recursive? #t)))

Recursive git clones are typically synonymous with bundled libraries or
other undesired, hence it needs to be explained in a comment.  If there
are bundled libraries, they should be packaged separately ideally, or if
that's too much work to do, at least a TODO should be added mentioning
which bundled libraries are currently used.

> +              (file-name (git-file-name name version))
> +              (sha256
> +               (base32
> +                "0dz4zv1km9805lji2q2qqdd8s8hgfd723dxdzcivbhm612szm1mc"))))
> +    (build-system gnu-build-system)
> +    (arguments

Please use gexps for new packages, e.g. (arguments (list #:make-flags
#~(list "CC=gcc" ...) ...))

> +     `(#:tests? #f

Disabling the tests warrants a comment, which can be as simple as the ";no
test suite" inline comment.

> +       #:make-flags `("CC=gcc" "CONFIG_SHELL=bash")
> +       #:phases (modify-phases %standard-phases
> +                  ;; no configure rule available

Please use complete sentences for standalone comments, e.g. "There is no
configure script."
> +                  (delete 'configure)
> +                  ;; no install rule available

Ditto.

> +                  (replace 'install
> +                    (lambda* (#:key outputs #:allow-other-keys)
> +                      (let* ((out (string-append (assoc-ref outputs "out")
> +                                                 "/opt/zest")))

After this is turned into a gexp, you can use #$output to refer to the
main output directly.

> +                        (install-file "zest" out)
> +                        (install-file "libzest.so" out)
> +                        (install-file "mruby/bin/mruby" out)
> +                        (install-file "deps/nanovg/example/entypo.ttf"
> +                                      (string-append out "/font"))
> +                        (install-file "deps/nanovg/example/Roboto-Bold.ttf"
> +                                      (string-append out "/font"))
> +                        (install-file "deps/nanovg/example/Roboto-Light.ttf"
> +                                      (string-append out "/font"))
> +                        (install-file "deps/nanovg/example/Roboto-Regular.ttf"
> +                         (string-append out "/font"))

Can't it use the fonts provided by the system?  Packages shouldn't
include any font, ideally, as they're heavy and should be
user-configurable.

> +                        (copy-recursively "src/mruby-zest/qml"
> +                                          (string-append out "/qml"))
> +                        (copy-recursively "src/mruby-zest/example"
> +                                          (string-append out "/qml"))
> +                        (invoke "touch"
> +                                (string-append out "/qml/MainWindow.qml"))

Why is this empty file needed?  A comment would be useful.

> +                        (copy-recursively "src/osc-bridge/schema"
> +                                          (string-append out "/schema"))
> +                        (copy-file "completions/zyn-fusion"
> +                                   (string-append out "/completions"))
> +                        (wrap-program (string-append out "/zest")
> +                                      `("LD_LIBRARY_PATH" =
> +                                        (,out)))))))))
> +    (native-inputs (list ruby libuv pkg-config))
> +    (inputs (list libx11 mesa bash-minimal))

Please order dependencies lexicographically.

> +    (home-page "https://github.com/mruby-zest/mruby-zest-build")
> +    (synopsis "Widget classes for the mruby-zest framework")
> +    (description
> +     "This repository contains all of the widgets needed to create the
> +@code{zyn-fusion} user interface for ZynAddSubFX.")
> +    (license license:lgpl2.1)))
> +
>  (define-public zynaddsubfx
>    (package
>      (name "zynaddsubfx")
> -    (version "3.0.5")
> +    (version (package-version mruby-zest))
>      (source (origin
>                (method url-fetch)
>                (uri (string-append
> @@ -3000,10 +3060,11 @@ (define-public zynaddsubfx
>                      version "/zynaddsubfx-" version ".tar.bz2"))
>                (sha256
>                 (base32
> -                "0qwzg14h043rmyf9jqdylxhyfy4sl0vsr0gjql51wjhid0i34ivl"))))
> +                "1bkirvcg0lz1i7ypnz3dyh218yhrqpnijxs8n3wlgwbcixvn1lfb"))))
>      (build-system cmake-build-system)
>      (arguments
> -     `(#:phases
> +     `(#:configure-flags `("-DGuiModule=zest")
> +       #:phases
>         (modify-phases %standard-phases
>           ;; Move SSE compiler optimization flags from generic target to
>           ;; athlon64 and core2 targets, because otherwise the build would fail
> @@ -3014,20 +3075,39 @@ (define-public zynaddsubfx
>                (("-msse -msse2 -mfpmath=sse") "")
>                (("-march=(athlon64|core2)" flag)
>                 (string-append flag " -msse -msse2 -mfpmath=sse")))
> -            #t)))))
> +            #t))

Trailing #t are no longer needed; remove them.

> +         (add-after 'install 'link-zest
> +          (lambda* (#:key inputs outputs #:allow-other-keys)
> +            (let ((zestdir (string-append (assoc-ref inputs
> +                                                     "mruby-zest")
> +                                          "/opt/zest"))
> +                  (bindir (string-append (assoc-ref outputs "out")
> +                                         "/bin"))
> +                  (libdir (string-append (assoc-ref outputs "out")
> +                                         "/lib")))
> +              (symlink (string-append zestdir "/libzest.so")
> +                       (string-append libdir "/libzest.so"))
> +              (symlink (string-append zestdir "/zest")
> +                       (string-append bindir "/zyn-fusion"))
> +              (symlink (string-append zestdir "/font")
> +                       (string-append libdir "/font"))
> +              (symlink (string-append zestdir "/qml")
> +                       (string-append libdir "/qml"))
> +              (symlink (string-append zestdir "/schema")
> +                       (string-append libdir "/schema"))))))))
>      (inputs
>       (list liblo
> -           ntk
> +           mruby-zest
>             mesa
>             alsa-lib
>             jack-1
> -           fftw
> +           fftwf
>             minixml
>             libxpm
>             zlib))
>      (native-inputs
> -     (list pkg-config))
> -    (home-page "http://zynaddsubfx.sf.net/")
> +     (list pkg-config ruby doxygen))

Please mind the ordering.

> +    (home-page "https://zynaddsubfx.sourceforge.io/")
>      (synopsis "Software synthesizer")
>      (description
>       "ZynAddSubFX is a feature heavy realtime software synthesizer.  It offers

Could you send a v2 with the requested changes?  You can do so with 'git
send-email -v2 --to=57984@debbugs.gnu.org -1' from the checkout which
has this v2 as last commit.

Thank you!

Maxim