dumb: Package dumbplay & split libaldmb, take ownership #28182
Conversation
|
Edited comment, didn't realise there was a new PR format. |
There was a problem hiding this comment.
Is this package really something you'd use in an environment where allegro4's dependencies aren't already installed? Maybe push all the extras (aldumb and dumbplay) into a single subpackage with all extra deps?
I'm not sure this will receive any updates any more, so it's not going to make maintenance much harder, I'd just prefer to be sure it's not adding to many unnecessary subpackages.
Also, please include the rationale you exposed in the PR message in the commit message itself.
srcpkgs/dumb/template
Outdated
| aldumb-devel_package() { | ||
| depends="aldumb>=${version}_${revision} ${sourcepkg}-devel>=${version}_${revision}" | ||
| short_desc+=", Allegro4 integration - development files" | ||
| pkg_install() { | ||
| vmove usr/lib/libaldmb.so | ||
| vmove usr/include/aldumb.h | ||
| } | ||
| } |
There was a problem hiding this comment.
IMO this doesn't need to be split. devel packages can be a little bit bloated, it isn't a big issue.
There was a problem hiding this comment.
Makes sense, especially if the only people who are going to be using the Allegro stuff would be building against it.
There was a problem hiding this comment.
I figured giving aldmb a devel package would help make it more orthogonal w/ dumb-devel but figures aldmb is already such a specific use case that eliminating the superfluous subpackage is a better idea, I'll do that.
srcpkgs/dumb/template
Outdated
| aldumb_package() { | ||
| depends="${sourcepkg}>=${version}_${revision}" | ||
| short_desc+=", Allegro4 integration" | ||
| pkg_install() { | ||
| vmove "usr/lib/libaldmb.so.*" | ||
| } | ||
| } |
There was a problem hiding this comment.
There is of course the issue that this removes libaldmb from users who rely on it being provided by dumb.
There was a problem hiding this comment.
Fair point, may affect users who build forks of old games like https://github.com/carstene1ns/alex4
I'm guessing there's no way to deal with a split on existing installations other than hoping there are no users who actively use aldmb.so from dumb? (I severely doubt there are, but still)
Allegro 4 is ancient and none of the packages in Void's repos that depend on I'll concede that the proposed split is more than a little philosophical rather than purely technical, but I can see an (admittedly extremely rare) case for running dumb and dumbout on a headless server where the chains of X11 & desktop sound server dependencies allegro4 would pull in only serve to add unnecessary bloat. IMO that justifies splitting a seldom used glue library.
Will do on next push. |
ScrelliCopter
force-pushed
the
dumbutils2
branch
from
35356ee
to
491c035
Compare
This is a second go at void-linux#18472 with (in my opinion) a better approach using subpackages instead of build time options. libaldmb is a separate library that isn't used by the rest of the package and splitting it avoids a bunch of unnecessary X11 & other desktop dependencies on `dumb`. For the reference player I created a `dumbplay` subpackage which keeps the SDL2 dependency out of the main library package, the tiny dumbout util has minimal dependencies and thus probably belongs in the main package. I also updated the homepage which still pointed to the old pre-fork page.
ScrelliCopter
force-pushed
the
dumbutils2
branch
from
491c035
to
61668df
Compare
General
Have the results of the proposed changes been tested?
Description
This is a second go at #18472 with (in my opinion) a better approach using subpackages instead of build time options.
libaldmb is a separate library that isn't used by the rest of the package and splitting it avoids a bunch of unnecessary X11 & other desktop dependencies on
dumb.For the reference player I created a
dumbplaysubpackage which keeps the SDL2 dependency out of the main library package,the dumbout util technically doesn't need it but I figured it's not important enough to create yet another subpackage for.the tiny dumbout util has minimal dependencies and thus probably belongs in the main package.I also updated the homepage which still pointed to the old pre-fork page.
Checked and couldn't see any other packages using the Allegro 4 integration (or libdumb at all for that matter) so I'm certain this change shouldn't break any existing package.