media-sound/cadence: general overhaul #341
Conversation
|
Hi @gerion0 thanks for the PR!
I/we missed that Cadence has been added to the gentoo tree. We generally don't keep any packages in the overlay that exist in the gentoo tree. Is there anything missing/broken with the ebuild in the gentoo tree that you're using the one from this overlay? |
|
The main point is missing support for Claudia/Ladish (also the blocker for me personally since I use mainly Claudia ). I think, this is due to the fact that ladish is not packaged in Gentoo. The last four points, that I mentioned earlier, are also missing in the upstream ebuild:
All of these points can also be added upstream (except the claudia one). Maybe I'll make a separate pull request in the Gentoo repo. Nevertheless, I have made the pull request here, because I need ladish support. |
Detailed Changes: - reduce the diff to upstream ebuild - add jack_capture - add python3_7 - update to EAPI 7 - undo catarina removal - drop gnome icon fix - add python3_8 - add shebang fix - add better desktop entries for claudia - enable installation on Gentoo prefix (not tested) - drop revision for live ebuild Signed-off-by: Gerion Entrup <[email protected]>
|
Have made an upstream pull request. |
There was a problem hiding this comment.
Figured I should review this because the package in the gentoo tree probably won't get LADISH support, so for now you probably want to keep using the version from this overlay.
First of all: Thanks for the PR! And sorry for the delay.
I've reviewed it from our/my perspective, haven't checked what the ebuild in the gentoo tree looks like. There might be some items that are in there that I have a different opinion on.
If you feel there's too much to change I can also cherry-pick some of the changes you made and make a new PR myself. Let me know what you prefer.
drop revision for live ebuild
I'd prefer to keep this. AFAIK there are no exceptions regarding revbumps for live ebuilds, they should be revbumped based on the same criteria as regular ebuilds. I use a lot of binary packages and lack of revbumps is generally the cause of most of my issues when I'm installing them on a machine because the ebuilds are different but no rebuild was triggered because there was no revbump.
If you want to ensure you're building the same commit you can set EGIT_COMMIT or EGIT_BRANCH depending on what you want to pin exactly.
| LICENSE="GPL-2" | ||
| SLOT="0" | ||
|
|
||
| IUSE="a2jmidid ladish -pulseaudio opengl" |
There was a problem hiding this comment.
Since these are already shuffled vs the previous rev can you put them in alphabetical order?
|
|
||
| REQUIRED_USE="${PYTHON_REQUIRED_USE}" | ||
|
|
||
| CDEPEND=" |
There was a problem hiding this comment.
I don't think any of these are CDEPEND (at least not my understanding of CDEPEND from https://wiki.gentoo.org/wiki/Future_EAPI/New_dependency_types, AFAIK it's not an official thing) they are all required at runtime so should be RDEPEND, right?
There was a problem hiding this comment.
I took this directly from the upstream ebuild. That said, I have looked into the real dependencies. For me, it looks like there are some build deps (for the c++ sources):
- QtGui
- QtCore
- QtWidgets
- QtSvg
Of course, there is the dependency to PyQt5 already, but I think, this is not enough, since Qt is used from C++ directly.
The rest should be RDEPENDs, yes.
|
|
||
| CDEPEND=" | ||
| ${PYTHON_DEPS} | ||
| $(python_gen_cond_dep ' |
There was a problem hiding this comment.
Does putting multiple packages in a single python_gen_cond_dep work?
I believe the normal/official way is a python_gen_cond_dep per dep, have to say I prefer that as well over this.
There was a problem hiding this comment.
According to mgorny, who (afaik) introduces this whole thing, multiple packages in a single function are the official way, see:
- https://blogs.gentoo.org/mgorny/2020/02/10/no-more-python_targets-in-single-r1/
- https://dev.gentoo.org/~mgorny/python-guide/any.html (similar pattern only)
- https://dev.gentoo.org/~mgorny/python-guide/single.html (function explanation but without multi package example)
Moreover, the upstream ebuild also uses this.
| dev-python/PyQt5[dbus,gui,opengl?,svg,widgets,${PYTHON_MULTI_USEDEP}] | ||
| ') | ||
| media-sound/jack2[dbus] | ||
| media-sound/jack_capture |
There was a problem hiding this comment.
I'd prefer to put this behind a USE flag, render is probably the best option for that.
| } | ||
|
|
||
| src_install() { | ||
| emake PREFIX="${EPREFIX}/usr" DESTDIR="${ED}" install |
There was a problem hiding this comment.
I guess we need ${ED} here instead of ${D} because there's no configure for cadence because it's Makefile/install only?
| make_desktop_entry claudia Claudia claudia "AudioVideo;AudioVideoEditing;Qt" | ||
| make_desktop_entry claudia-launcher "Claudia Launcher" claudia-launcher "AudioVideo;AudioVideoEditing;Qt" | ||
| fi | ||
| make_desktop_entry catarina Catarina catarina "AudioVideo;AudioVideoEditing;Qt" |
There was a problem hiding this comment.
I'd prefer to leave catarina out or alternatively put it behind a USE flag. Most users won't need it and it's just noise in the application list if it's present.
There was a problem hiding this comment.
Hmm, I think, we should keep it. Yes, of course, this is a test tool. However, the upstream Makefile also builds/installs it by default, therefore there might be users that want to use it and it should be the choice of the user whether to use or ignore it.
I'm okay with putting it behind a useflag.
There was a problem hiding this comment.
OK, USE flag is fine, guess we can just use one called catarina.
| rm -rf "${ED}"/usr/share/applications/*.desktop | ||
|
|
||
| if use !pulseaudio; then | ||
| rm -rf "${ED}"/usr/bin/cadence-pulse2{jack,loopback} |
There was a problem hiding this comment.
I vastly prefer just listing things separately. That way grepping ebuilds for filenames always works and tbh I also don't really see the gain, it takes me more brain-time to process this than just the straight direct lines.
There was a problem hiding this comment.
I took this from the in-tree ebuild. It reduces the code size a little bit and of course makes it easier to get the differences between in-tree ebuild and audio-overlay ebuild. The latter was exactly the reason why I chose to reduce the diff: To be able to see with one glance what the differences are and to keep both ebuils up-to-date simultainously.
However, I don't have a really strong feeling about this. If you wish, I can revert it.
There was a problem hiding this comment.
If you wish, I can revert it.
Yeah, I'd prefer to keep it as it was.
With regards to the diff to upstream: I'd prefer to focus on trying to make it so the ebuild in this overlay is no longer necessary and you (and anyone else) can just use the ebuild in the gentoo tree.
But I don't expect LADISH to be added to the gentoo tree, both because it's pretty much unmaintained and because it currently requires Python 2.7 (there have been three recent commits, but there's more to do to make it work with Python 3).
Would your use case also be served with NSM (https://github.com/linuxaudio/new-session-manager, it's a fork of Non Session Manager) instead of LADISH? It's the session manager the majority of the Linux audio community has decided to standardize on. It might take a bit of time before it offers the same features and UX as LADISH, especially with regards to the GUIs but it's at least a sustainable way forward.
| rm -rf "${ED}"/usr/share/cadence/pulse2{jack, loopback} | ||
| fi | ||
| if use !ladish; then | ||
| rm -rf "${ED}"/usr/bin/claudia{,-launcher} |
| rm -rf "${ED}"/usr/share/cadence/icons/claudia-hicolor/ | ||
| fi | ||
|
|
||
| # Replace desktop entries with QA issues with these |
There was a problem hiding this comment.
👍
We should create an issue or PR upstream about this.
The same issue for Carla was eventually fixed, Falk just didn't apply the same change to Cadence (yet).
There was a problem hiding this comment.
Of course, the whole src_install phase is a mess. The rm section should be part of the upstrem Makefile, too.
Are these binary packages live packages? Normally the revision should ensure that the user can switch back if something fails and get an update for changes that are worthy of a complete rebuild. For live ebuilds the first point normally is meaningless, since the upstream version normally changed anyway, so you can't switch back if you install the ebuild with a minor revision. The second point also is critical because the newer version maybe does not build at all even if the ebuild is better, because upstream has commited something weird. Let's take the last revision of cadence from this overlay. The difference between the revisions where the updated Python versions. Making a revision triggers an update for the user. Let's say that the user has Cadence installed with Python 3.6, then the revision triggers an update that is not useful. Let's say that the newer Cadence source does not build at all (because of upstream issues), then the user get's presented an update that she does not need and also fails. I (as a user) don't want (forced) updates for live ebuilds in general since there is a high probability that the source code won't build anymore and if I want an update, I can trigger this myself (or via a regulary Regarding the policy: You are correct. I have not found any policy for that. However, in the Gentoo source tree are only two ebuilds that have live ebuild revisions: git and gnulib. Git seems to "resolve" the rebuild problem by keeping all old revisions in the tree as files but update them to the newest one (there is no difference between
|
|
@gerion0 Thanks for the info, those are all valid points, my/our intentions are obviously not to cause breakage for users, but I've also had issues myself when not revbumping ebuilds. P.S. That Python version change is actually one of the most annoying issues with the current setup in gentoo. I don't like how Python versions for packages are handled. I have to rebuild all packages that have any kind of change in Python versions (even if it's just adding 3.9 support without it being enabled) otherwise my binary packages are useless. There has to be a better solution, although I don't really have one at the moment either (apart from storing stuff like this not in USE flags but in some other property). |
Maybe runtime useflags would be the solution. Unfortunately, noone has implement them yet. |
|
@gerion0 I did some testing and I think I can get my desired behavior by using I'll create a PR to remove the revision of the other 9999 ebuilds in this repo to clean it up, I'll leave cadence out of that PR so there are no merge conflicts. |
|
Is this PR still needed? If so, please rebase it onto the current master branch to fix the CI pipeline. |
|
Sorry, I'm using the Gentoo upstream Cadence with Pipewire at the meantime, so I have no interest in updating this PR (and cannot really test it). |
Detailed Changes:
(I can revert this if you want. However I don't find revisions on live ebuilds meaningful. Revisions should lead to a new merge of the ebuild, if the ebuild changes notably. However, live ebuilds should be merged regulary anyway and a new merge not only brings the new ebuild features but also new code, which may not compile at all or has other problems. Therefore, a new merge could be unwanted.)
Signed-off-by: Gerion Entrup [email protected]