-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add zyn-fusion #362
base: master
Are you sure you want to change the base?
Add zyn-fusion #362
Conversation
Repoman QA results:
|
This is based on the ebuild from https://bugs.gentoo.org/700326, but changed to build 3.0.5 with properly fetched sources. Package-Manager: Portage-3.0.1, Repoman-2.3.23
This enables zyn-fusion support Package-Manager: Portage-3.0.1, Repoman-2.3.23
Repoman QA results:
|
Repoman QA results:
|
6385d42
to
35e7b41
Compare
Repoman QA results:
|
Repoman QA results:
|
Repoman QA results:
|
Repoman QA results:
|
Do we need to do something with this?
And with zyn-fusion added, do you think there's any reason to also have/keep the old zynaddsubfx? AFAIK it's no longer developed and everyone is encouraged to update to zyn-fusion. |
zyn-fusion is just the GUI. It's a dependency of zynaddsubfx[zest]; alone it does nothing. zynaddsubfx is still developed, as it's still the core of zyn-fusion. I don't know about the old GUI, but if that's deprecated/unmaintained then we could just drop those USE flags for zynaddsubfx and make zyn-fusion a hard dep? Not sure there's much benefit in that though, as long as upstream still has it and it works. I think we can ignore that QA warning. The code is definitely broken there, but it's an upstream issue, inherited from the version in portage anyway, and I don't know if there's actually a way a user could hit that codepath. Upstream rtosc still has the same code. |
Filed the QA warning up-upstream anyway. |
We've checked this with fundamental (the Zyn dev) and the old UI is still supported and supposed to work. Only caveat is that for the plugin which UI to use is chosen at build time. To prevent confusion it's probably best to allow either using the old/ftlk UI or the new/fusion UI. Arch has two packages, one called zynaddsubfx and one called zyn-fusion, where one blocks the other. I know this is technically not correct, but it does seem a bit simpler to me, especially for users. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's quiet a beast :P
I've added some comments/questions inline.
} | ||
|
||
-const struct mrb_data_type mrb_nvg_context_type; | ||
+extern const struct mrb_data_type mrb_nvg_context_type; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know if this has already been PRed upstream? Arch is carrying a similar patch https://aur.archlinux.org/cgit/aur.git/tree/gcc10.patch?h=zyn-fusion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure; this is a common theme for gcc10 compat issues and I wouldn't be surprised if upstream hasn't heard of it yet. I can throw a PR over at them :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. Would be nice to do that. Don't need to wait for it with this PR though :)
$(cmake_use_find_package doc Doxygen) | ||
$(cmake_use_find_package fltk FLTK) | ||
) | ||
if use zest ; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we have a exactly one of REQUIRED_USE
for fltk
and zest
?
Now you can have both set but you'll still always get one of the two for the plugin UI, I think that's a bit confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering that myself when I was writing the ebuild, but then I looked at the REQUIRED_USE documentation and it says it should be used sparingly:
https://devmanual.gentoo.org/general-concepts/use-flags/index.html#conflicting-use-flags
In particular it seems exclusions like that are more useful for dependencies, while for leaf level ebuilds it makes more sense to favour user intent (which, if they ask for zest, probably means they don't want fltk).
An einfo message to this effect might be in order, though.
I think it doesn't make sense to embed zynaddsubfx into zyn-fusion itself. It would be duplicating work into two ebuilds, and complicate dependencies if e.g. an app somewhere decides it needs to depend on zynaddsubfx (the core plugin) for some functionality, regardless of GUI. We could add a message to zyn-fusion that it is only the GUI, and that if users want to use it with zynaddsubfx they need to build zynaddsubfx[zest]. |
Yeah, agreed
That would probably be a good idea. I'm not sure if there's a better solution. I guess we could let zyn-fusion depend on zynaddsubfx but that does mean that one would get zynaddsubfx as well even if you only want to run the UI locally, not sure how relevant that usecase is though. Also I guess that would mean a circular dependency with |
Finally had some time to try this out locally. I get the following error trying to emerge zynaddsubfx
Any idea why this is happening? |
Huh. CMake version or some missing module? Can you put a full build log somewhere and CMake version, and I'll debug when I get home? |
Sure
This is all there is in the log
There is CmakeError.log file but the FAILED messages in there don't seem to be related :? |
FYI, I have a busy week, so I'll be a bit late updating this, but I'm not forgetting :) |
I took care of the comments, but I'm at a loss as to the CMake issue since it doesn't happen here. Also, the zynaddsubfx ebuild is basically unchanged other than adding the dep and changing the UI mode, which seems unrelated to that Nio issue. Can you check if you can build upstream zynaddsubfx from portage? |
Repoman QA results:
|
Yes, I'll give this a try this week and report back, will probably be Wednesday if that's OK. |
I get the same error when building zyn from the gentoo tree. Which USE flags are you using?
You might not run into this when JACK is not enabled, see https://github.com/zynaddsubfx/zynaddsubfx/blob/3.0.5/src/Nio/CMakeLists.txt#L23 I think that
I'll create a PR for the ebuild in gentoo and for zynaddsubfx itself with this fix |
FYI The upstream PR with this fixed has been merged into Zyn zynaddsubfx/zynaddsubfx#82 |
Notice to new upstream: if you have some problems with this ebuild you may ping me on irc. I sent those ebuilds to marcan a while ago, and he fixed some points that were in those. |
Can this at least be forwarded to GURU? Doesn't seem like it's getting accepted here anytime soon. |
Le Fri, 08 Nov 2024 03:59:41 -0800,
SpomJ ***@***.***> a écrit :
Can this at least be forwarded to GURU? Doesn't seem like
it's getting accepted here anytime soon.
I think I can close this issue as last version of zynaddsubfx in portage
tree uses the new fusion interface.
I liked much the idea of being able to have both interfaces (gentoo is
about choice), but it was not really possible to have only fusion the
way those ebuild were thought, (only fltk, or both fltk+fusion was
supported)
|
zyn-fusion uses a whole pile of vendored git submodules, and some mruby gems stuff. This is the best I could do to eliminate all the compile-time clones and have everything fetched properly in the ebuild.
Note that some of the hashes aren't even pinned by the root version of zyn-fusion (the gems stuff), so for those it's just "whatever was master right now".
I did my best to fix the install layout to fit LSB standards, and fixed the QA warnings.
The zynaddsubfx ebuild is just from upstream portage, with the USE flag and optional dep added. When built that way, it requires zyn-fusion to show its UI, but does not otherwise depend on the package at build time (it dynamically loads it).