diff mbox series

[bug#57984] gnu: Update zynaddsubfx to 3.0.6

Message ID 164b342a-f0ba-2e66-9b61-5e46c615a510@telenet.be
State New
Headers show
Series [bug#57984] gnu: Update zynaddsubfx to 3.0.6 | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git-branch success View Git branch
cbaines/applying patch fail
cbaines/issue success View issue

Commit Message

M Oct. 26, 2022, 12:04 p.m. UTC
> Regarding recursive clone, I packages the runtime dependencies
 > seperately but I left the mruby gems, since they seem to be needed
 > just as source (without needing to be compiled) as they are to be
 > configured by mruby for mruby-zest. [...]

Bundling things that aren't compiled is still bundling.  Also, now both 
mruby-zest and zynaddsubfx bundle rtosc, which is worse than a package 
bundling a dependent that isn't used anywhere else.

I've looked at the latest patches and did some changes (see attached 
patches), though the bundling still remains.


In nanovg, I changed the 'revision' to 0 -- its just a monotonically 
increasing number for each package update, it's not a 'how many commits 
have there been', see (guix)Version Numbers.

I also changed "ar" to #$(ar-for-target) -- using the 'wrong' ar 
sometimes works, but not always, e.g. IIRC it failed when 
cross-compiling from some 64-bit arch to a 32-bit ach?

Upstream appears to use 'premake4' as a build system, so I adjusted the 
package definition accordingly.

I set the 'file-name' field for to be more informative.

---

I corrected the revision in font-entypo as with nanovg.

I expanded the description a little, and added a TODO
on building the font from source (it seems to have been
neglected for font packages because of node dependencies).

The license combination seemed a bit odd to me so I added
some more text there.

---

mruby-zest:

I changed 'TODO: package mruby gems separately' to 'TODO: unbundle mruby 
gems' as that seems easier to search for.  Some things were actually 
packaged, so I removed those from 'deps' in a snippet.  It turned out 
that the Makefile tried to build mruby, so I patched that out.

As mruby isn't included anymore, you could give removing some of the
bundled mruby gems a try, especially given that they aren't actually
installed.

I removed '#:tests? #false' because a test suite exists -- the makefile 
has a 'test' and 'rtest' target.  I choose the 'rtest' target (because 
'test' is for testing mruby, not mruby-zest).  It turned out the test 
suite requires 'ruby-ruby-prof' so I added that as a native-input.

You have done #$some-input in the 'install' phase -- package 
transformation don't know to adjust those, so I replaced it with a 
search-input-file equivalent.

Putting the fonts in the output of mruby-zest doesn't lookk quite right 
to me, so I instead moved things to a post-unpack phase and used 
'substitute*' instead of 'symlink'.

I noticed that pcre was bundled (deps/mruby-regexp-pcre/pcre'), so I 
removed that.

By looking at
<https://github.com/archlinux/svntogit-community/tree/packages/zynaddsubfx/trunk>,
I noticed it looks for libzest.so in /opt/zyn-fusion/libzest.so, which 
is incorrect in Guix (and other distros too).

I removed the wrapping because:

   * no reason was listed for wrapping
   * presumably the incorrect reference was the thing that
     LD_LIBRARY_PATH was a work-around for, but that reference has now
     been corrected.
   * LD_LIBRARY_PATH is leaky and hence to be avoided
     (if zest spawns a subprocess, then that subprocess would get
     LD_LIBRARY_PATH too)

As a bonus, this allowed removing 'bash-minimal' from the inputs.

 From 
<https://github.com/archlinux/svntogit-community/blob/packages/zynaddsubfx/trunk/zynaddsubfx-mruby-zest-build-3.0.6-system_wide_location.patch>,
I noticed that apparently some references to schema/test.json and 
MainWindow.qml were incorrect.  These are now patched.

I simplified the unbundling substitutions a little.
For example, it turned out that build_config.rb did not need any 
substitutions somehow.

The makefile uses pkg-config instead of TARGET-pkg-config, which is 
incorrect when cross-compiling, so I patched that.

On the fonts: I noticed that the fonts don't become part of the closure
(with "guix gc --references").  As such, maybe the source files that
use the fonts are actually unused.  I propose to give installing the
examples that use the fonts a try, or alternatively explicitly choose
to not install the (font-using) examples.

---

I changed the version of zynaddsubfx from (package-version mruby-zest)
to "3.0.6", otherwise the package would break after a mruby-zest update.

I re-added the comment of 'remove-sse-flags-from-generic-target' that 
was removed.

I changed #$mruby-zest to a search-input-file equivalent, for the same
reason as with previous patches.

You forgot to mention the home page, so I adjusted the commit message.

You are fixing a file name, not a path, so I adjusted the new phase name.

By removing the 'ntk' input, zynaddsubfx now has one less interface,
so I re-added it, with a comment.

It appeared that doxygen documentation wasn't actually build
(there is no .html in the output, and the configuraiton script
complained about some missing component), so I removed the new doxygen
(native-)input.

bash completions weren't installed in the new version even though they
were in the previous version, so I made some changes to support that.

I adjusted the substitution of libzest.so to substitute all cases,
in case somehow the first dlopen fails.

I compared the diff between old and new source code, there doesn't
appear to be anything 'suspicious' though its just a cursory look
and it would be easy to hide things:

guix shell diffoscope -- diffoscope 
/gnu/store/9j09zj472211nrrs5jmyxdsq2lpfd36q-zynaddsubfx-3.0.5.tar.bz2 
/gnu/store/xyjiq3nmk372ap4vq7sl7n7f9rc5xshs-zynaddsubfx-3.0.6.tar.bz2

Comments

Maxim Cournoyer Oct. 27, 2022, 1:18 p.m. UTC | #1
Hi Maxime,

Impressive work.  Are there outstanding things worthy of being fixed in
your opinion, or do you think this version is ready to be merged and we
can iterate improvements on top in time?  To my cursory look, I'd think
it looks in a good enough shape, but I thought I'd ask before merging.
M Oct. 27, 2022, 3:11 p.m. UTC | #2
On 27-10-2022 15:18, Maxim Cournoyer wrote:
> Hi Maxime,
> 
> Impressive work.  Are there outstanding things worthy of being fixed in
> your opinion, or do you think this version is ready to be merged and we
> can iterate improvements on top in time?  To my cursory look, I'd think
> it looks in a good enough shape, but I thought I'd ask before merging.

My primary concern is that while it builds, I haven't tested whether 
zynaddsubfx still works after my modifications.  There are two secondary 
concerns:

(*) The bundling.  Both zynaddsubfx and the new mruby-zest bundle rtosc.
Having multiple versions of the same library in the same program is 
known to potentially cause trouble, see e.g. 
<https://issues.guix.gnu.org/47115>.

The other bundling could be considered as 'can iterate improvements on 
top over time' I suppose.

(*) The fonts don't appear to be actually used (at least in the current 
configuration), so as you appear to prefer no font inputs, that seems 
feasible to me:

> On the fonts: I noticed that the fonts don't become part of the closure
> (with "guix gc --references").  As such, maybe the source files that
> use the fonts are actually unused.  I propose to give installing the
> examples that use the fonts a try, or alternatively explicitly choose
> to not install the (font-using) examples. 

(Alternatively, maybe I messed up the substitutions)

Greetings,
Maxime.
Sughosha May 3, 2023, 11:15 p.m. UTC | #3
I opened a new issue #63254 with the new patches to add mruby-zest,
unbundle nanovg and rtosc and then patch the source files:
https://issues.guix.gnu.org/63254

Sughosha
diff mbox series

Patch

From 3feccc0f627a2bb44c5f03bd54fa9d08198c1702 Mon Sep 17 00:00:00 2001
From: Sughosha <sughosha@proton.me>
Date: Sat, 15 Oct 2022 15:59:55 +0200
Subject: [PATCH 4/4] gnu: zynaddsubfx: Update to 3.0.6.

Upstream has switched from ntk to mruby-zest.

* gnu/packages/music.scm (zynaddsubfx): Update to 3.0.6.
Update package style.
[arguments]{#:phases}[fix-zyn-fusion-location]: New phase for new dependency.
[inputs]: Add mruby-zest.
[home-page]: Update home page.

Modified-by: Maxime Devos <maximedevos@telenet.be>
---
 gnu/packages/music.scm | 74 +++++++++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 29 deletions(-)

diff --git a/gnu/packages/music.scm b/gnu/packages/music.scm
index 63c66b17d5..0f82c85bcb 100644
--- a/gnu/packages/music.scm
+++ b/gnu/packages/music.scm
@@ -3167,42 +3167,58 @@  (define-public mruby-zest
 (define-public zynaddsubfx
   (package
     (name "zynaddsubfx")
-    (version "3.0.5")
+    (version "3.0.6")
     (source (origin
               (method url-fetch)
               (uri (string-append
-                    "mirror://sourceforge/zynaddsubfx/zynaddsubfx/"
-                    version "/zynaddsubfx-" version ".tar.bz2"))
+                    "mirror://sourceforge/zynaddsubfx/zynaddsubfx/" version
+                    "/zynaddsubfx-" version ".tar.bz2"))
               (sha256
                (base32
-                "0qwzg14h043rmyf9jqdylxhyfy4sl0vsr0gjql51wjhid0i34ivl"))))
+                "1bkirvcg0lz1i7ypnz3dyh218yhrqpnijxs8n3wlgwbcixvn1lfb"))))
     (build-system cmake-build-system)
     (arguments
-     `(#:phases
-       (modify-phases %standard-phases
-         ;; Move SSE compiler optimization flags from generic target to
-         ;; athlon64 and core2 targets, because otherwise the build would fail
-         ;; on non-Intel machines.
-         (add-after 'unpack 'remove-sse-flags-from-generic-target
-          (lambda _
-            (substitute* "src/CMakeLists.txt"
-              (("-msse -msse2 -mfpmath=sse") "")
-              (("-march=(athlon64|core2)" flag)
-               (string-append flag " -msse -msse2 -mfpmath=sse")))
-            #t)))))
-    (inputs
-     (list liblo
-           ntk
-           mesa
-           alsa-lib
-           jack-1
-           fftw
-           minixml
-           libxpm
-           zlib))
-    (native-inputs
-     (list pkg-config))
-    (home-page "http://zynaddsubfx.sf.net/")
+     (list #:configure-flags #~(list "-DGuiModule=zest"
+                                     (string-append "-DZYN_DATADIR="
+                                                    #$output
+                                                    "/share/zynaddsubfx")
+                                     (string-append "-DBASHCOMP_PKG_PATH="
+                                                    #$output
+                                                    "/share/bash-completion/completions"))
+           #:phases #~(modify-phases %standard-phases
+                        ;; Move SSE compiler optimization flags from generic
+                        ;; target to athlon64 and core2 targets, because
+                        ;; otherwise the build would fail on non-Intel machines.
+                        (add-after 'unpack 'remove-sse-flags-from-generic-target
+                          (lambda _
+                            (substitute* "src/CMakeLists.txt"
+                              (("-msse -msse2 -mfpmath=sse")
+                               "")
+                              (("-march=(athlon64|core2)" flag)
+                               (string-append flag
+                                              " -msse -msse2 -mfpmath=sse")))))
+                        (add-after 'unpack 'fix-zyn-fusion-location
+                          (lambda* (#:key inputs #:allow-other-keys)
+                            (substitute* "src/main.cpp"
+                              (("\\./zyn-fusion")
+                               (search-input-file inputs "bin/zyn-fusion")))
+                            (substitute*
+                             "src/Plugin/ZynAddSubFX/ZynAddSubFX-UI-Zest.cpp"
+                              (("(\\./|/opt/zyn-fusion/|)libzest.so")
+                               (search-input-file inputs
+                                                  "lib/zyn-fusion/libzest.so"))))))))
+    (inputs (list alsa-lib
+                  fftwf
+                  jack-1
+                  liblo
+                  libxpm
+                  mesa
+                  minixml
+                  mruby-zest
+                  ntk ; optional, alternative interface
+                  zlib))
+    (native-inputs (list pkg-config ruby))
+    (home-page "https://zynaddsubfx.sourceforge.io/")
     (synopsis "Software synthesizer")
     (description
      "ZynAddSubFX is a feature heavy realtime software synthesizer.  It offers
-- 
2.38.0